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

[Jacopo Mondi]

Hi Naush

On Fri, Oct 13, 2023 at 10:10:03AM +0100, Naushir Patuck via libcamera-devel wrote:
> Move the handling of Bayer order changes due to flips to run before
> platformValidate(). This removes the need for this code to be split
> between platformValidate() and validate() as it is right now.
>
> Also add some validation to ensure the vc4 pipeline handler only
> supports CSI2 packing or no packing.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  .../pipeline/rpi/common/pipeline_base.cpp     | 43 +++++++++++++------
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        | 15 ++++---
>  2 files changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 825eae80d014..7c88b87e0608 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -231,16 +231,12 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>  		}
>  	}
>
> -	/* Do any platform specific fixups. */
> -	status = data_->platformValidate(this);
> -	if (status == Invalid)
> -		return Invalid;
> -
> -	/* Further fixups on the RAW streams. */
> +	/* Start with some initial generic RAW stream adjustments. */
>  	for (auto &raw : rawStreams_) {
> -		int ret = raw.dev->tryFormat(&raw.format);
> -		if (ret)
> -			return Invalid;
> +		StreamConfiguration *rawStream = raw.cfg;
> +
> +		/* Adjust the RAW stream to match the computed sensor format. */
> +		BayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.mbus_code);
>
>  		/*
>  		 * Some sensors change their Bayer order when they are h-flipped
> @@ -249,12 +245,33 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>  		 * 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_);
> +			sensorBayer.order = data_->nativeBayerOrder_;
> +			sensorBayer = sensorBayer.transform(combinedTransform_);
> +		}
> +
> +		/* Apply the sensor adjusted Bayer order to the user request. */
> +		BayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);
> +		cfgBayer.order = sensorBayer.order;
> +
> +		if (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {
> +			rawStream->pixelFormat = cfgBayer.toPixelFormat();
> +			status = Adjusted;
>  		}
> -		raw.cfg->pixelFormat = bayer.toPixelFormat();
> +	}
> +
> +	/* Do any platform specific fixups. */
> +	Status st = data_->platformValidate(this);
> +	if (st == Invalid)
> +		return Invalid;
> +	else if (st == Adjusted)
> +		status = Adjusted;
> +
> +	/* Further fixups on the RAW streams. */
> +	for (auto &raw : rawStreams_) {
> +		int ret = raw.dev->tryFormat(&raw.format);
> +		if (ret)
> +			return Invalid;
>
>  		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..425ab9ae6ea5 100644
> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> @@ -409,16 +409,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig
>
>  		/* Adjust the RAW stream to match the computed sensor format. */
>  		StreamConfiguration *rawStream = rawStreams[0].cfg;
> -		BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);
> +		BayerFormat rawBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);
>
> -		/* Handle flips to make sure to match the RAW stream format. */
> -		if (flipsAlterBayerOrder_)
> -			rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);
> +		/* Apply the sensor bitdepth. */
> +		rawBayer.bitDepth = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code).bitDepth;
>
> -		/* Apply the user requested packing. */
> -		rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;
> -		PixelFormat rawFormat = rawBayer.toPixelFormat();
> +		/* Default to CSI2 packing if the user request is unsupported. */
> +		if (rawBayer.packing != BayerFormat::Packing::CSI2 &&
> +		    rawBayer.packing != BayerFormat::Packing::None)
> +			rawBayer.packing = BayerFormat::Packing::CSI2;

Do you need to return Adjusted in this case ?

Thanks
  j

>
> +		PixelFormat rawFormat = rawBayer.toPixelFormat();
>  		if (rawStream->pixelFormat != rawFormat ||
>  		    rawStream->size != rpiConfig->sensorFormat_.size) {
>  			rawStream->pixelFormat = rawFormat;
> --
> 2.34.1
>