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

[Laurent Pinchart]

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 :-)

> 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