[libcamera-devel] libcamera: raspberry: Fix format validation

16646 diff mbox series

If the pixel format gets adjusted to the default NV12 format during
validation, it gets currently ignored and the user-supplied format is
used nonetheless.

Fix it by using the adjusted pixel format.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.36.1

Hi Jacopo,

Thank you for your patch.

On Fri, 15 Jul 2022 at 14:56, Jacopo Mondi <jacopo@jmondi.org> wrote:

> If the pixel format gets adjusted to the default NV12 format during
> validation, it gets currently ignored and the user-supplied format is
> used nonetheless.
>
> Fix it by using the adjusted pixel format.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>

Looks right to me.

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index fdc24cd530c2..8c8813038483 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -523,7 +523,7 @@ CameraConfiguration::Status
> RPiCameraConfiguration::validate()
>                 }
>
>                 V4L2DeviceFormat format;
> -               format.fourcc =
> V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> +               format.fourcc =
> V4L2PixelFormat::fromPixelFormat(cfgPixFmt);
>                 format.size = cfg.size;
>                 format.colorSpace = cfg.colorSpace;
>
> --
> 2.36.1
>
>

Quoting Jacopo Mondi via libcamera-devel (2022-07-15 14:56:09)
> If the pixel format gets adjusted to the default NV12 format during
> validation, it gets currently ignored and the user-supplied format is
> used nonetheless.
> 
> Fix it by using the adjusted pixel format.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index fdc24cd530c2..8c8813038483 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -523,7 +523,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>                 }

At least 'status = Adjusted' would be set above, so it wouldn't have
been able to pass through an actual configuration - but indeed, the
intention is to give a supported configuration back to the application.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
>                 V4L2DeviceFormat format;
> -               format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> +               format.fourcc = V4L2PixelFormat::fromPixelFormat(cfgPixFmt);
>                 format.size = cfg.size;
>                 format.colorSpace = cfg.colorSpace;
> 
> --
> 2.36.1
>