[libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Add JPEG_1X8 and BGR888_1X24 mbus formats to formatInfoMap

[Umang Jain]

Hello

On 10/10/22 3:07 PM, Jacopo Mondi via libcamera-devel wrote:
> Hi Xavier
>
> On Mon, Oct 10, 2022 at 11:22:56AM +0200, Xavier Roumegue (OSS) via libcamera-devel wrote:
>> From: Xavier Roumegue <xavier.roumegue at oss.nxp.com>
>>
>> The warnings "Unknown subdev format 0x4001, defaulting to RGB encoding" and
>> "Unknown subdev format 0x1013, defaulting to RGB encoding" are thrown while using
>> simple pipeline handler with NXP ISI hardware.
>> The JPEG_1X8 and BGR888_1X24 media bus formats, supported by the ISI driver, are
>> missing in the V4L2SubdeviceFormatInfo structure storing the correspondence
>> between a media bus format and a colour encoding. So populate the structure with
>> the missing media bus formats.
>>
>> Signed-off-by: Xavier Roumegue <xavier.roumegue at oss.nxp.com>
> Just to point out that for JPEG, the entries we have in the main
> formats map, report YUV as color encoding

Aren't we comparing two different things here. I remember asking Laurent 
that we do colorEncoding / PixelFormatInfo like structures for media-bus 
formats as well and we ended up with  [1]

It's mentioned in the documentation [2] that V4L2 Pixel formats are 
different from media bus formats. So I guess the formats mentioned in 
formats.cpp do not apply / convey any kind of reference here

"The media bus pixel codes describe image formats as flowing over 
physical buses (both between separate physical components and inside SoC 
devices).
  This should not be confused with the V4L2 pixel formats that describe, 
using four character codes, image formats as stored in memory."

[1] https://git.libcamera.org/libcamera/libcamera.git/commit/?id=d5ad19b
[2] 
https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html#media-bus-pixel-codes
>
> src/libcamera/formats.cpp-      /* Compressed formats. */
> src/libcamera/formats.cpp-      { formats::MJPEG, {
> src/libcamera/formats.cpp-              .name = "MJPEG",
> src/libcamera/formats.cpp-              .format = formats::MJPEG,
> src/libcamera/formats.cpp-              .v4l2Formats = {
> src/libcamera/formats.cpp-                      V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> src/libcamera/formats.cpp:                      V4L2PixelFormat(V4L2_PIX_FMT_JPEG),
> src/libcamera/formats.cpp-              },
> src/libcamera/formats.cpp-              .bitsPerPixel = 0,
> src/libcamera/formats.cpp-              .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> src/libcamera/formats.cpp-              .packed = false,
> src/libcamera/formats.cpp-              .pixelsPerGroup = 1,
> src/libcamera/formats.cpp-              .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},
> src/libcamera/formats.cpp-      } },
>
> Unfortunately the kernel documentation for the format doesn't help
> much:
>
> https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html?highlight=media_bus_fmt_jpeg_1x8
>> ---
>>   src/libcamera/v4l2_subdevice.cpp | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
>> index 9ef95963..f34eea24 100644
>> --- a/src/libcamera/v4l2_subdevice.cpp
>> +++ b/src/libcamera/v4l2_subdevice.cpp
>> @@ -68,6 +68,7 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = {
>>   	{ MEDIA_BUS_FMT_RGB565_2X8_LE, { 16, "RGB565_2X8_LE", PixelFormatInfo::ColourEncodingRGB } },
>>   	{ MEDIA_BUS_FMT_RGB666_1X18, { 18, "RGB666_1X18", PixelFormatInfo::ColourEncodingRGB } },
>>   	{ MEDIA_BUS_FMT_RGB888_1X24, { 24, "RGB888_1X24", PixelFormatInfo::ColourEncodingRGB } },
>> +	{ MEDIA_BUS_FMT_BGR888_1X24, { 24, "BGR888_1X24", PixelFormatInfo::ColourEncodingRGB } },

This looks good to me
>>   	{ MEDIA_BUS_FMT_RGB888_2X12_BE, { 24, "RGB888_2X12_BE", PixelFormatInfo::ColourEncodingRGB } },
>>   	{ MEDIA_BUS_FMT_RGB888_2X12_LE, { 24, "RGB888_2X12_LE", PixelFormatInfo::ColourEncodingRGB } },
>>   	{ MEDIA_BUS_FMT_ARGB8888_1X32, { 32, "ARGB8888_1X32", PixelFormatInfo::ColourEncodingRGB } },
>> @@ -133,6 +134,7 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = {
>>   	{ MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } },
>>   	/* \todo Clarify colour encoding for HSV formats */
>>   	{ MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } },
>> +	{ MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingRGB } },

This needs some investigation to be sure...
>>   };
>>
>>   } /* namespace */
>> --
>> 2.37.3
>>