[libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip handling validation code

[Naushir Patuck]

On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi Naush
>
> On Fri, Oct 06, 2023 at 02:19:54PM +0100, Naushir Patuck via libcamera-devel wrote:
> > Move the handling of Bayer order changes due to flips entirely into
> > platformValidate(). This removes the need for this code to be split
> > between platformValidate() and validate() as it is right now.
>
> Doesn't this mean the same code has to be repeated in both platform's
> pipelines ?

Sadly yes :(

I haven't figured out a clean/tidy way to do this in pipeline_base and
remove the duplication.  Things get a bit more complicated when user
packing requests interfere with the raw output as well, and packing
constraints are platform specific (e.g. no CSI2 packing on Pi5).  I'll
continue working on getting a cleaner solution, but depending on the
timeframe, it might have to be put on top of this work.

Regards,
Naush

>
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  .../pipeline/rpi/common/pipeline_base.cpp          | 14 --------------
> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp             | 12 ++++++++++--
> >  2 files changed, 10 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 825eae80d014..7437d38dc9ba 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -242,20 +242,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >               if (ret)
> >                       return Invalid;
> >
> > -             /*
> > -              * Some sensors change their Bayer order when they are h-flipped
> > -              * or v-flipped, according to the transform. If this one does, we
> > -              * must advertise the transformed Bayer order in the raw stream.
> > -              * Note how we must fetch the "native" (i.e. untransformed) Bayer
> > -              * order, because the sensor may currently be flipped!
> > -              */
> > -             BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);
> > -             if (data_->flipsAlterBayerOrder_) {
> > -                     bayer.order = data_->nativeBayerOrder_;
> > -                     bayer = bayer.transform(combinedTransform_);
> > -             }
> > -             raw.cfg->pixelFormat = bayer.toPixelFormat();
> > -
> >               if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))
> >                       status = Adjusted;
> >       }
> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > index 233473e2fe2b..4b42ddc71115 100644
> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > @@ -411,9 +411,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig
> >               StreamConfiguration *rawStream = rawStreams[0].cfg;
> >               BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);
> >
> > -             /* Handle flips to make sure to match the RAW stream format. */
> > -             if (flipsAlterBayerOrder_)
> > +             /*
> > +              * Some sensors change their Bayer order when they are h-flipped
> > +              * or v-flipped, according to the transform. If this one does, we
> > +              * must advertise the transformed Bayer order in the raw stream.
> > +              * Note how we must fetch the "native" (i.e. untransformed) Bayer
> > +              * order, because the sensor may currently be flipped!
> > +              */
> > +             if (flipsAlterBayerOrder_) {
> > +                     rawBayer.order = nativeBayerOrder_;
> >                       rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);
> > +             }
> >
> >               /* Apply the user requested packing. */
> >               rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;
> > --
> > 2.34.1
> >