[PATCH v2 7/7] pipeline: rpi: Handler controls::rpi::ScalerCrops

[Jacopo Mondi]

Hi Naush

On Mon, Sep 30, 2024 at 03:14:15PM GMT, Naushir Patuck wrote:
> Handle multiple scaler crops being set through the rpi::ScalerCrops
> control. We now populate the cropParams_ map in the loop where we handle
> the output stream configuration items. The key of this map is the index
> of the stream configuration structure set by the application. This will
> also be the same index used to specify the crop rectangles through the
> ScalerCrops control.
>
> CameraData::applyScalerCrop() has been adapted to look at either
> controls::ScalerCrop or controls::rpi::ScalerCrops. The former takes
> priority over the latter, and if present, will apply the same scaler
> crop to all output streams.
>
> Finally return all crops through the same ScalerCrops control via
> request metadata. The first configure stream's crop rectangle is also
> returned via the ScalerCrop control in the request metadata.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/rpi/common/pipeline_base.cpp     | 76 ++++++++++++++-----
>  1 file changed, 59 insertions(+), 17 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 267e6bd9cd70..9393e79ae4c7 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -181,12 +181,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>
>  	rawStreams_.clear();
>  	outStreams_.clear();
> +	unsigned int rawStreamIndex = 0;
> +	unsigned int outStreamIndex = 0;
>
> -	for (const auto &[index, cfg] : utils::enumerate(config_)) {
> +	for (auto &cfg : config_) {
>  		if (PipelineHandlerBase::isRaw(cfg.pixelFormat))
> -			rawStreams_.emplace_back(index, &cfg);
> +			rawStreams_.emplace_back(rawStreamIndex++, &cfg);
>  		else
> -			outStreams_.emplace_back(index, &cfg);
> +			outStreams_.emplace_back(outStreamIndex++, &cfg);
>  	}
>
>  	/* Sort the streams so the highest resolution is first. */
> @@ -565,10 +567,24 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
>  	const auto cropParamsIt = data->cropParams_.find(0);
>  	if (cropParamsIt != data->cropParams_.end()) {
>  		const CameraData::CropParams &cropParams = cropParamsIt->second;
> -		/* Add the ScalerCrop control limits based on the current mode. */
> +		/*
> +		 * Add the ScalerCrop control limits based on the current mode and
> +		 * the first configured stream.
> +		 */
>  		Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(cropParams.ispMinCropSize));
>  		ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,
>  							     data->scaleIspCrop(cropParams.ispCrop));
> +		if (data->cropParams_.size() == 2) {
> +			/*
> +			 * The control map for rpi::ScalerCrops has the min value
> +			 * as the default crop for stream 0, max value as the default
> +			 * value for stream 1.
> +			 */
> +			ctrlMap[&controls::rpi::ScalerCrops] =
> +				ControlInfo(data->scaleIspCrop(data->cropParams_[0].ispCrop),
> +					    data->scaleIspCrop(data->cropParams_[1].ispCrop),
> +					    ctrlMap[&controls::ScalerCrop].def());
> +		}
>  	}
>
>  	data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
> @@ -1295,11 +1311,29 @@ Rectangle CameraData::scaleIspCrop(const Rectangle &ispCrop) const
>
>  void CameraData::applyScalerCrop(const ControlList &controls)
>  {
> -	const auto &scalerCrop = controls.get<Rectangle>(controls::ScalerCrop);
> -	const auto cropParamsIt = cropParams_.find(0);
> -	if (scalerCrop && cropParamsIt != cropParams_.end()) {
> -		Rectangle nativeCrop = *scalerCrop;
> -		CropParams &cropParams = cropParamsIt->second;
> +	const auto &scalerCropRPi = controls.get<Span<const Rectangle>>(controls::rpi::ScalerCrops);
> +	const auto &scalerCropCore = controls.get<Rectangle>(controls::ScalerCrop);
> +	std::vector<Rectangle> scalerCrops;
> +
> +	/*
> +	 * First thing to do is create a vector of crops to apply to each ISP output
> +	 * based on either controls::ScalerCrop or controls::rpi::ScalerCrops if
> +	 * present.
> +	 *
> +	 * If controls::ScalerCrop is present, apply the same crop to all ISP output
> +	 * streams. Otherwise if controls::rpi::ScalerCrops, apply the given crops
> +	 * to the ISP output streams, indexed by the same order in which they had
> +	 * been configured. This is not the same as the ISP output index.
> +	 */
> +	for (unsigned int i = 0; i < cropParams_.size(); i++) {
> +		if (scalerCropRPi && i < scalerCropRPi->size())
> +			scalerCrops.push_back(scalerCropRPi->data()[i]);
> +		else if (scalerCropCore)
> +			scalerCrops.push_back(*scalerCropCore);
> +	}

I think you want to be stricter here and better defines what happens
if
1) ScalerCrops has less entries than the number of configured streams

        Your ScalerCrops documentation reports

        The order of rectangles passed into the control must match the order of
        streams configured by the application. The pipeline handler will only
        configure crop retangles up-to the number of output streams configured.
        All subsequent rectangles passed into this control are ignored by the
        pipeline handler.

        which addresses what happens if you have more ScalerCrops entries than
        Streams but not the other way around.

2) Both ScalerCrop and ScalerCrops have been set by the application

The above code doesn't really handle the two above conditions if I got
it right.

Also, but that's up to you to decide, on vc4 you always crop at the
ISP input, while on pisp you always crop on the outputs both for
ScalerCrop and ScalerCrops. Not sure if you need (or can) differentiate,
in example, ScalerCrop goes to the ISP input crop and ScalerCrops goes
to the ISP outputs. Up to you!

> +
> +	for (auto const &[i, scalerCrop] : utils::enumerate(scalerCrops)) {
> +		Rectangle nativeCrop = scalerCrop;
>
>  		if (!nativeCrop.width || !nativeCrop.height)
>  			nativeCrop = { 0, 0, 1, 1 };
> @@ -1315,13 +1349,13 @@ void CameraData::applyScalerCrop(const ControlList &controls)
>  		 * 2. With the same mid-point, if possible.
>  		 * 3. But it can't go outside the sensor area.
>  		 */
> -		Size minSize = cropParams.ispMinCropSize.expandedToAspectRatio(nativeCrop.size());
> +		Size minSize = cropParams_[i].ispMinCropSize.expandedToAspectRatio(nativeCrop.size());
>  		Size size = ispCrop.size().expandedTo(minSize);
>  		ispCrop = size.centeredTo(ispCrop.center()).enclosedIn(Rectangle(sensorInfo_.outputSize));
>
> -		if (ispCrop != cropParams.ispCrop) {
> -			cropParams.ispCrop = ispCrop;
> -			platformSetIspCrop(cropParams.ispIndex, ispCrop);
> +		if (ispCrop != cropParams_[i].ispCrop) {
> +			cropParams_[i].ispCrop = ispCrop;
> +			platformSetIspCrop(cropParams_[i].ispIndex, ispCrop);
>  		}
>  	}>  }
> @@ -1478,10 +1512,18 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request
>  	request->metadata().set(controls::SensorTimestamp,
>  				bufferControls.get(controls::SensorTimestamp).value_or(0));
>
> -	const auto cropParamsIt = cropParams_.find(0);
> -	if (cropParamsIt != cropParams_.end())
> -		request->metadata().set(controls::ScalerCrop,
> -					scaleIspCrop(cropParamsIt->second.ispCrop));
> +	if (cropParams_.size()) {
> +		std::vector<Rectangle> crops;
> +
> +		for (auto const &[k, v] : cropParams_)
> +			crops.push_back(scaleIspCrop(v.ispCrop));
> +
> +		request->metadata().set(controls::ScalerCrop, crops[0]);
> +		if (crops.size() > 1) {
> +			request->metadata().set(controls::rpi::ScalerCrops,
> +						Span<const Rectangle>(crops.data(), crops.size()));
> +		}
> +	}
>  }
>
>  } /* namespace libcamera */
> --
> 2.34.1
>