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

[Naushir Patuck]

Hi Laurent,

On Tue, 26 Sept 2023 at 23:51, Laurent Pinchart via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Hi Naush,
>
> Thank you for the patch.
>
> 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.  Happy to add this line to the commit message, or can
this be done while applying?

Regards,
Naush

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