[libcamera-devel] [PATCH v5 11/13] libcamera: rpi: Change default stream formats

[Naushir Patuck]

Hi Laurent,

On Wed, 27 Sept 2023 at 08:48, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> On Wed, Sep 27, 2023 at 08:30:05AM +0100, Naushir Patuck wrote:
> > On Tue, 26 Sept 2023 at 23:51, Laurent Pinchart via libcamera-devel wrote:
> > > On Thu, Sep 21, 2023 at 06:55:48PM +0200, Jacopo Mondi via libcamera-devel wrote:
> > > > From: Naushir Patuck <naush at raspberrypi.com>
> > > >
> > > > Switch to XRGB8888 as a default Viewfinder role output format, and
> > > > YUV420 as a default output format for everything else.
> > >
> > > The commit message should explain *why* those formats are better.
> >
> > RGB24/32 formats are typically better supported by display pipelines
> > than YUV420.
>
> This patch switches from YUV 4:2:0 semi-planar to planar, and from ARGB
> to XRGB, so I'm not sure how that's related :-)

Sorry, I phrased that completely wrong!  Here's a (hopefully) clearer
commit message:

Switch to XRGB8888 as a default Viewfinder role output format, this is
a more correct description of the ISP hardware output, and what is
accepted by the Raspberry Pi hardware.

Switch to YUV420 as a default output format for everything else, as
this format is best supported by encoding (e.g. H.264, JPEG) sinks on
the Raspberry Pi platform.

>
> > Happy to add this line to the commit message, or can
> > this be done while applying?
>
> If you reply with an updated commit message, I can handle it when
> applying.
>
> > > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > > Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > > > ---
> > > >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > > > index c560e48c12fb..f8e8e13dc837 100644
> > > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > > > @@ -268,7 +268,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > > >
> > > >               if (fmts.find(out.dev->toV4L2PixelFormat(cfgPixFmt)) == fmts.end()) {
> > > >                       /* If we cannot find a native format, use a default one. */
> > > > -                     cfgPixFmt = formats::NV12;
> > > > +                     cfgPixFmt = formats::YUV420;
> > > >                       status = Adjusted;
> > > >               }
> > > >
> > > > @@ -431,7 +431,7 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span<const StreamRole
> > > >
> > > >               case StreamRole::StillCapture:
> > > >                       fmts = data->ispFormats();
> > > > -                     pixelFormat = formats::NV12;
> > > > +                     pixelFormat = formats::YUV420;
> > > >                       /*
> > > >                        * Still image codecs usually expect the sYCC color space.
> > > >                        * Even RGB codecs will be fine as the RGB we get with the
> > > > @@ -465,7 +465,7 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span<const StreamRole
> > > >
> > > >               case StreamRole::Viewfinder:
> > > >                       fmts = data->ispFormats();
> > > > -                     pixelFormat = formats::ARGB8888;
> > > > +                     pixelFormat = formats::XRGB8888;
> > > >                       colorSpace = ColorSpace::Sycc;
> > > >                       size = { 800, 600 };
> > > >                       bufferCount = 4;
>
> --
> Regards,
>
> Laurent Pinchart