[libcamera-devel] [PATCH] libcamera: add support for planar YUV422 and YUV420 formats

[Laurent Pinchart]

Hi David,

On Thu, Jun 18, 2020 at 09:19:55AM +0100, David Plowman wrote:
> Hi Laurent
> 
> Thanks for the review and the correction.
> 
> Just to add some background, I've found that libjpeg works _way_
> faster if you can feed
> it directly with planar YUV420 (what it calls "raw" data), hence the
> motivation for this
> patch. I added YUV422 because, although we don't support it yet, it's
> common for folks
> to prefer 422 rather than 420 JPEGs, so I expect we'll need to do it
> at some point.

Does your ISP hardware support YUV422 ?

> I'm happy to re-submit the patch if you like, though it sounds like
> the work you're doing
> means this isn't worth the trouble. Just let me know!

I've pushed the format rework, and will send the rebased version of your
patch shortly.

> On Thu, 18 Jun 2020 at 01:46, Laurent Pinchart wrote:
> > On Thu, Jun 18, 2020 at 01:18:21AM +0300, Laurent Pinchart wrote:
> > > On Wed, Jun 17, 2020 at 05:44:42PM +0100, David Plowman wrote:
> > > > These formats can be helpful when downstream applications or libraries
> > > > support them natively (avoiding a costly conversion).
> > >
> > > But only if cameras can provide them :-)
> > >
> > > Does the Raspberry Pi ISP support both V4L2_PIX_FMT_YUV420 and
> > > V4L2_PIX_FMT_YUV422P ? I see mentions of the former in the driver, but
> > > not of the latter.
> > >
> > > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > > > ---
> > > >  src/libcamera/formats.cpp          | 14 ++++++++++++++
> > > >  src/libcamera/v4l2_pixelformat.cpp |  2 ++
> > > >  src/v4l2/v4l2_camera_proxy.cpp     |  2 ++
> > > >  3 files changed, 18 insertions(+)
> > > >
> > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > > > index 2ac3b41..dcd1dcf 100644
> > > > --- a/src/libcamera/formats.cpp
> > > > +++ b/src/libcamera/formats.cpp
> > > > @@ -268,6 +268,20 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> > > >             .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> > > >             .packed = false,
> > > >     } },
> > > > +   { PixelFormat(DRM_FORMAT_YUV422), {
> > > > +           .format = PixelFormat(DRM_FORMAT_YUV422),
> > > > +           .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUV422P),
> > > > +           .bitsPerPixel = 16,
> > > > +           .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> > > > +           .packed = false,
> > > > +   } },
> > > > +   { PixelFormat(DRM_FORMAT_YUV420), {
> > > > +           .format = PixelFormat(DRM_FORMAT_YUV420),
> > > > +           .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUV420),
> > > > +           .bitsPerPixel = 12,
> > > > +           .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> > > > +           .packed = false,
> > > > +   } },
> >
> > I may merge in the morning a patch series that rework format handling
> > and would conflict with this patch. If that's the case, I'll rebase this
> > patch and send the rebased version to the list.
> >
> > > >
> > > >     /* Greyscale formats. */
> > > >     { PixelFormat(DRM_FORMAT_R8), {
> > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> > > > index 94fae47..01778c0 100644
> > > > --- a/src/libcamera/v4l2_pixelformat.cpp
> > > > +++ b/src/libcamera/v4l2_pixelformat.cpp
> > > > @@ -64,6 +64,8 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
> > > >     { V4L2PixelFormat(V4L2_PIX_FMT_NV61), PixelFormat(DRM_FORMAT_NV61) },
> > > >     { V4L2PixelFormat(V4L2_PIX_FMT_NV12), PixelFormat(DRM_FORMAT_NV12) },
> > > >     { V4L2PixelFormat(V4L2_PIX_FMT_NV21), PixelFormat(DRM_FORMAT_NV21) },
> > > > +   { V4L2PixelFormat(V4L2_PIX_FMT_YUV422P), PixelFormat(DRM_FORMAT_YUV422) },
> > > > +   { V4L2PixelFormat(V4L2_PIX_FMT_YUV420), PixelFormat(DRM_FORMAT_YUV420) },
> > > >
> > > >     /* Greyscale formats. */
> > > >     { V4L2PixelFormat(V4L2_PIX_FMT_GREY), PixelFormat(DRM_FORMAT_R8) },
> > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> > > > index d7f14e6..a54d47e 100644
> > > > --- a/src/v4l2/v4l2_camera_proxy.cpp
> > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp
> > > > @@ -576,6 +576,8 @@ static const std::array<PixelFormatInfo, 14> pixelFormatInfo = {{
> > > >     { PixelFormat(DRM_FORMAT_NV61),         V4L2_PIX_FMT_NV61,      2, {{ {  8, 1, 1 }, { 16, 2, 1 }, {  0, 0, 0 } }} },
> > > >     { PixelFormat(DRM_FORMAT_NV24),         V4L2_PIX_FMT_NV24,      2, {{ {  8, 1, 1 }, { 16, 1, 1 }, {  0, 0, 0 } }} },
> > > >     { PixelFormat(DRM_FORMAT_NV42),         V4L2_PIX_FMT_NV42,      2, {{ {  8, 1, 1 }, { 16, 1, 1 }, {  0, 0, 0 } }} },
> > > > +   { PixelFormat(DRM_FORMAT_YUV422),       V4L2_PIX_FMT_YUV422P,   2, {{ {  8, 1, 1 }, { 8, 2, 1 }, {  8, 2, 1 } }} },
> > > > +   { PixelFormat(DRM_FORMAT_YUV420),       V4L2_PIX_FMT_YUV420,    2, {{ {  8, 1, 1 }, { 8, 2, 2 }, {  8, 2, 2 } }} },
> >
> > Additionally, don't those formats have 3 planes, not two ?
> 
> You are quite right, my mistake. It seems to work both ways...!
> 
> > > >     /* Compressed formats. */
> > > >     /*
> > > >      * \todo Get a better image size estimate for MJPEG, via

-- 
Regards,

Laurent Pinchart