[libcamera-devel,14/20] pipeline: rpi: Move flip handling validation code
diff mbox series

Message ID 20231006132000.23504-15-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi: Preliminary PiSP support
Related show

Commit Message

Naushir Patuck Oct. 6, 2023, 1:19 p.m. UTC
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.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp          | 14 --------------
 src/libcamera/pipeline/rpi/vc4/vc4.cpp             | 12 ++++++++++--
 2 files changed, 10 insertions(+), 16 deletions(-)

Comments

Jacopo Mondi Oct. 12, 2023, 10:05 a.m. UTC | #1
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 ?

>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@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
>
Naushir Patuck Oct. 12, 2023, 12:07 p.m. UTC | #2
On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi
<jacopo.mondi@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@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@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
> >
Naushir Patuck Oct. 12, 2023, 12:55 p.m. UTC | #3
On Thu, 12 Oct 2023 at 13:07, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi
> <jacopo.mondi@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.
>

And now after spending a bit more time looking at it, I think the
solution is actually simpler than I thought!  I have a candidate fix
that can be used to replace this patch.  Should I send it in a v2
together with all the other minor fixes you noted?

> Regards,
> Naush
>
> >
> > >
> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > Reviewed-by: David Plowman <david.plowman@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
> > >
Jacopo Mondi Oct. 12, 2023, 2:40 p.m. UTC | #4
Hi Naush

On Thu, Oct 12, 2023 at 01:55:00PM +0100, Naushir Patuck wrote:
> On Thu, 12 Oct 2023 at 13:07, Naushir Patuck <naush@raspberrypi.com> wrote:
> >
> > On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi
> > <jacopo.mondi@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.
> >
>
> And now after spending a bit more time looking at it, I think the
> solution is actually simpler than I thought!  I have a candidate fix
> that can be used to replace this patch.  Should I send it in a v2
> together with all the other minor fixes you noted?
>

If you want, I certainly won't oppose. My proposal to fix minors when
applying still holds, but if you consider worth sending a v2 I'm all
for it!

Thanks
  j

> > Regards,
> > Naush
> >
> > >
> > > >
> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > > Reviewed-by: David Plowman <david.plowman@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
> > > >
Naushir Patuck Oct. 12, 2023, 2:48 p.m. UTC | #5
On Thu, 12 Oct 2023 at 15:40, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Naush
>
> On Thu, Oct 12, 2023 at 01:55:00PM +0100, Naushir Patuck wrote:
> > On Thu, 12 Oct 2023 at 13:07, Naushir Patuck <naush@raspberrypi.com> wrote:
> > >
> > > On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi
> > > <jacopo.mondi@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.
> > >
> >
> > And now after spending a bit more time looking at it, I think the
> > solution is actually simpler than I thought!  I have a candidate fix
> > that can be used to replace this patch.  Should I send it in a v2
> > together with all the other minor fixes you noted?
> >
>
> If you want, I certainly won't oppose. My proposal to fix minors when
> applying still holds, but if you consider worth sending a v2 I'm all
> for it!

I've got a v2 ready to go, so it's no problem.  I'll wait for further
comments from other folks and post this tomorrow (Friday) morning if
that's ok?

Regards,
Naush


>
> Thanks
>   j
>
> > > Regards,
> > > Naush
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > > > Reviewed-by: David Plowman <david.plowman@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
> > > > >

Patch
diff mbox series

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;