[libcamera-devel,v2,01/12] android: camera_device: Refuse unsupported formats
diff mbox series

Message ID 20200902152236.69770-2-jacopo@jmondi.org
State Accepted
Headers show
Series
  • android: camera_device: Fix JPEG/RAW sizes
Related show

Commit Message

Jacopo Mondi Sept. 2, 2020, 3:22 p.m. UTC 
The current implementation of CameraDevice::initializeStreamConfigurations()
fails if an image format marked as mandatory is not supported by the
libcamera::Camera device, but erroneously accepts non-mandatory
non-supported formats in the list of accepted ones.

Fix this by ignoring non supported image formats which are not marked as
mandatory.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart Sept. 5, 2020, 5:38 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Wed, Sep 02, 2020 at 05:22:25PM +0200, Jacopo Mondi wrote:
> The current implementation of CameraDevice::initializeStreamConfigurations()
> fails if an image format marked as mandatory is not supported by the
> libcamera::Camera device, but erroneously accepts non-mandatory
> non-supported formats in the list of accepted ones.
> 
> Fix this by ignoring non supported image formats which are not marked as
> mandatory.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/android/camera_device.cpp | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 9605b9e9069e..ad0d7fd15d90 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -392,10 +392,16 @@ int CameraDevice::initializeStreamConfigurations()
>  				break;
>  			}
>  		}
> -		if (camera3Format.mandatory && !mappedFormat.isValid()) {
> -			LOG(HAL, Error) << "Failed to map Android format "
> -					<< camera3Format.name << " ("
> -					<< utils::hex(androidFormat) << ")";
> +
> +		if (!mappedFormat.isValid()) {
> +			/* If the format is not mandatory, skip it. */
> +			if (!camera3Format.mandatory)
> +				continue;
> +
> +			LOG(HAL, Error)
> +				<< "Failed to map mandatory Android format "
> +				<< camera3Format.name << " ("
> +				<< utils::hex(androidFormat) << "): aborting";
>  			return -EINVAL;
>  		}
>
Hirokazu Honda Sept. 7, 2020, 7:48 a.m. UTC | #2 
LGTM

On Sun, Sep 6, 2020 at 2:38 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Sep 02, 2020 at 05:22:25PM +0200, Jacopo Mondi wrote:
> > The current implementation of CameraDevice::initializeStreamConfigurations()
> > fails if an image format marked as mandatory is not supported by the
> > libcamera::Camera device, but erroneously accepts non-mandatory
> > non-supported formats in the list of accepted ones.
> >
> > Fix this by ignoring non supported image formats which are not marked as
> > mandatory.
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> > ---
> >  src/android/camera_device.cpp | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 9605b9e9069e..ad0d7fd15d90 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -392,10 +392,16 @@ int CameraDevice::initializeStreamConfigurations()
> >                               break;
> >                       }
> >               }
> > -             if (camera3Format.mandatory && !mappedFormat.isValid()) {
> > -                     LOG(HAL, Error) << "Failed to map Android format "
> > -                                     << camera3Format.name << " ("
> > -                                     << utils::hex(androidFormat) << ")";
> > +
> > +             if (!mappedFormat.isValid()) {
> > +                     /* If the format is not mandatory, skip it. */
> > +                     if (!camera3Format.mandatory)
> > +                             continue;
> > +
> > +                     LOG(HAL, Error)
> > +                             << "Failed to map mandatory Android format "
> > +                             << camera3Format.name << " ("
> > +                             << utils::hex(androidFormat) << "): aborting";
> >                       return -EINVAL;
> >               }
> >
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 9605b9e9069e..ad0d7fd15d90 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -392,10 +392,16 @@  int CameraDevice::initializeStreamConfigurations()
 				break;
 			}
 		}
-		if (camera3Format.mandatory && !mappedFormat.isValid()) {
-			LOG(HAL, Error) << "Failed to map Android format "
-					<< camera3Format.name << " ("
-					<< utils::hex(androidFormat) << ")";
+
+		if (!mappedFormat.isValid()) {
+			/* If the format is not mandatory, skip it. */
+			if (!camera3Format.mandatory)
+				continue;
+
+			LOG(HAL, Error)
+				<< "Failed to map mandatory Android format "
+				<< camera3Format.name << " ("
+				<< utils::hex(androidFormat) << "): aborting";
 			return -EINVAL;
 		}