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

[Jacopo Mondi]

Hi Xavier

On Mon, Oct 10, 2022 at 12:40:36PM +0200, Xavier Roumegue (OSS) wrote:
> Hi Jacopo,
>
> On 10/10/22 11:37, Jacopo Mondi 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
> >
> > 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
> My poor colorspace knowledge does not give me a valuable opinion on that. It
> seems that jpeg is not really bound to a colorspace.
> I posted a v2 version with YUV as color encoding for JPEG so that the
> encoding remains consistant in libcamera.

Same for me, I cannot confirm which one is "right", but given that
colorspaces have been investigated in detail recently, I would use the
main formats table as a reference.

Thanks
  j

>
> Xavier
>
> > > ---
> > >   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 } },
> > >   	{ 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 } },
> > >   };
> > >
> > >   } /* namespace */
> > > --
> > > 2.37.3
> > >