[PATCH v2 2/7] pipeline: rpi: Remove CameraData::scalerCrop_

[Jacopo Mondi]

Hi Naush

On Mon, Sep 30, 2024 at 03:14:10PM GMT, Naushir Patuck wrote:
> Do not cache the scalerCrop_ parameter. The cached value is used to
> update the request metadata, but since this is not an expensive
> operation (and can only occur once per frame), caching it is of limited
> value.
>
> This will simplify logic in a future commit where we can specify a
> crop per-output stream.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j
> ---
>  .../pipeline/rpi/common/pipeline_base.cpp      | 18 +++---------------
>  .../pipeline/rpi/common/pipeline_base.h        |  1 -
>  2 files changed, 3 insertions(+), 16 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 3041fd1ed9fd..11f1bfd4a5da 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -544,12 +544,6 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
>  		return ret;
>  	}
>
> -	/*
> -	 * Set the scaler crop to the value we are using (scaled to native sensor
> -	 * coordinates).
> -	 */
> -	data->scalerCrop_ = data->scaleIspCrop(data->ispCrop_);
> -
>  	/*
>  	 * Update the ScalerCropMaximum to the correct value for this camera mode.
>  	 * For us, it's the same as the "analogue crop".
> @@ -569,7 +563,8 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
>
>  	/* Add the ScalerCrop control limits based on the current mode. */
>  	Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->ispMinCropSize_));
> -	ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop, data->scalerCrop_);
> +	ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,
> +						     data->scaleIspCrop(data->ispCrop_));
>
>  	data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
>
> @@ -1320,13 +1315,6 @@ void CameraData::applyScalerCrop(const ControlList &controls)
>  		if (ispCrop != ispCrop_) {
>  			ispCrop_ = ispCrop;
>  			platformSetIspCrop();
> -
> -			/*
> -			 * Also update the ScalerCrop in the metadata with what we actually
> -			 * used. But we must first rescale that from ISP (camera mode) pixels
> -			 * back into sensor native pixels.
> -			 */
> -			scalerCrop_ = scaleIspCrop(ispCrop_);
>  		}
>  	}
>  }
> @@ -1483,7 +1471,7 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request
>  	request->metadata().set(controls::SensorTimestamp,
>  				bufferControls.get(controls::SensorTimestamp).value_or(0));
>
> -	request->metadata().set(controls::ScalerCrop, scalerCrop_);
> +	request->metadata().set(controls::ScalerCrop, scaleIspCrop(ispCrop_));
>  }
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> index f9cecf70f179..5161c16e518f 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> @@ -134,7 +134,6 @@ public:
>  	/* For handling digital zoom. */
>  	IPACameraSensorInfo sensorInfo_;
>  	Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */
> -	Rectangle scalerCrop_; /* crop in sensor native pixels */
>  	Size ispMinCropSize_;
>
>  	unsigned int dropFrameCount_;
> --
> 2.34.1
>