[libcamera-devel] [PATCH v1] libcamera: Update v4l2 pixel format to libcamera pixel format mapping

[Laurent Pinchart]

Hi Vedant,

On Thu, Jun 17, 2021 at 10:30:03AM +0530, Vedant Paranjape wrote:
> On Thu, 17 Jun, 2021, 10:24 Laurent Pinchart wrote:
> > On Thu, Jun 17, 2021 at 01:30:57PM +0900, paul.elder at ideasonboard.com wrote:
> > > Hi Vedant,
> > >
> > > For the subject, maybe "libcamera: Fix V4L2 pixel format mapping for
> > > XRGB32" ?
> > >
> > > On Thu, Jun 17, 2021 at 02:18:44AM +0530, Vedant Paranjape wrote:
> > > > According to DRM FourCCs and linux kernel docs V4L2_PIX_FMT_XRGB32
> > >
> > > I find it odd that the DRM fourccs would document v4l2 pixel formats...
> > >
> > > > is BX24, but it was mapped to formats::XBGR8888, which is XB24.
> >
> > The patch is correct, but the explanation isn't. There are 4CC values
> > that refer to different formats in V4L2 and DRM. We can't rely on 'XR24'
> > in DRM referring to the same format as 'XR24' in V4L2.
> >
> 
> Then what is the common denominator between DRM and V4L2 pix format we can
> use to find a mapping between these ?

Only the documentation I'm afraid.
Documentation/userspace-api/media/v4l/pixfmt-rgb.rst for V4L2 (I highly
encourage you to compile the kernel documentation and view the HTML
output), and include/uapi/drm/drm_fourcc.h for DRM. Note that the former
shows how the bytes are layed out in memory, while the latter shows the
format of a work that is then stored in little endian, which results in
bytes being swapped.

> Also can you please help me with a right commit message. It seems tricky in
> this case.

I'll let Paul help :-) It would be useful if you split the patch in two,
first a patch to fix the error, and then a patch to add the new format.

> > > This patch fixes the mismatch of V4L2_PIX_FMT_XRGB32, i.e, matches it to
> > > > formats::BGRX8888. Furthermore, this patch adds V4L2_PIX_FMT_RBGX32 pixel
> > >
> > > "Additionally, add a mapping from V4L2_PIX_FMT_RGBX32 to formats::XBGR8888" ?
> > >
> > > Paul
> > >
> > > > format which is represented by XB24, and maps V4L2_PIX_FMT_RGBX32
> > > > to formats::XBGR8888
> > > >
> > > > Linux kernel pixel format docs:
> > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/pixfmt-rgb.html#bits-per-component
> > > > DRM FourCC pixel format docs:
> > https://elixir.bootlin.com/linux/latest/source/include/uapi/drm/drm_fourcc.h#L150
> > > >
> > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > > > ---
> > > >  src/libcamera/v4l2_pixelformat.cpp | 3 ++-
> >
> > You also need to update formats.cpp.
> >
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> > > > index 166d93cf..e86788ed 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_RGBX32), formats::XBGR8888 },
> > > > +   { V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::BGRX8888 },
> > > >     { V4L2PixelFormat(V4L2_PIX_FMT_RGBA32), formats::ABGR8888 },
> > > >     { V4L2PixelFormat(V4L2_PIX_FMT_ABGR32), formats::ARGB8888 },
> > > >     { V4L2PixelFormat(V4L2_PIX_FMT_ARGB32), formats::BGRA8888 },

-- 
Regards,

Laurent Pinchart