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

[David Plowman]

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.

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!

Thanks again and best regards
David

On Thu, 18 Jun 2020 at 01:46, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> 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