[libcamera-devel] [PATCH v4] libcamera: Fix V4L2 pixel format mapping for XRGB32 and RGBX32

[Laurent Pinchart]

Hi Vedant,

Thank you for the patch.

On Fri, Jun 25, 2021 at 04:32:24PM +0530, Vedant Paranjape wrote:
> On running libcamera gstreamer element on Raspberry Pi 4B+, there was a
> warning as follows:
> 
> [3:23:59.717513107] [3577] WARN V4L2 v4l2_pixelformat.cpp:180 Unsupported V4L2 pixel format XB24
> 
> This patch fixes the warning, by adding BGRX8888 pixel format
> in libcamera, and updating the existing mapping of V4L2_PIX_FMT_XRGB32
> to its corresponding libcamera pixel format, which is formats::BGRX8888.
> Additionally, add a new mapping of V4L2_PIX_FMT_RBGX32 to formats::XBGR8888.

I'd still like to split the fix (which should have a Fixes: tag) and the
addition of the new format.

> Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/libcamera/formats.cpp          | 12 +++++++++++-
>  src/libcamera/v4l2_pixelformat.cpp |  3 ++-
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 55822f4c..e41bfc05 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -198,13 +198,23 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
>  	{ formats::XBGR8888, {
>  		.name = "XBGR8888",
>  		.format = formats::XBGR8888,
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),

This goes in the fix.

>  		.bitsPerPixel = 32,
>  		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
>  		.packed = false,
>  		.pixelsPerGroup = 1,
>  		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
>  	} },
> +	{ formats::BGRX8888, {
> +		.name = "BGRX8888",
> +		.format = formats::BGRX8888,
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),
> +		.bitsPerPixel = 32,
> +		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
> +		.packed = false,
> +		.pixelsPerGroup = 1,
> +		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
> +	} },	

And this doesn't (and there are extra white spaces at the end of the
line, 'git show' should have shown you).

>  	{ formats::ABGR8888, {
>  		.name = "ABGR8888",
>  		.format = formats::ABGR8888,
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index 166d93cf..78a08303 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -51,7 +51,8 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_RGB24), formats::BGR888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_BGR24), formats::RGB888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_XBGR32), formats::XRGB8888 },
> -	{ V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::XBGR8888 },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::BGRX8888 },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_RGBX32), formats::XBGR8888 },

Here, the fix would be

> -	{ V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::XBGR8888 },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_RGBX32), formats::XBGR8888 },

The commit message should be updated accordingly, by explaining that the
commit fixes the V4L2 format for formats::XBGR8888.

>  	{ V4L2PixelFormat(V4L2_PIX_FMT_RGBA32), formats::ABGR8888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_ABGR32), formats::ARGB8888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_ARGB32), formats::BGRA8888 },

-- 
Regards,

Laurent Pinchart