[v4,7/7] pipeline: rpi: Handler controls::rpi::ScalerCrops

21788 diff mbox series

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 latter takes
priority over the former. If only controls::ScalerCrop is provided, the
pipeline handler 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@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp     | 77 +++++++++++++++----
 1 file changed, 60 insertions(+), 17 deletions(-)

Hi all,

One last tag needed on this patch and the series can then be merged.

Regards,
Naush

On Thu, 31 Oct 2024 at 09:52, Naushir Patuck <naush@raspberrypi.com> 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 latter takes
> priority over the former. If only controls::ScalerCrop is provided, the
> pipeline handler 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@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  .../pipeline/rpi/common/pipeline_base.cpp     | 77 +++++++++++++++----
>  1 file changed, 60 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..917c45b3f052 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_.at(0).ispCrop),
> +                                           data->scaleIspCrop(data->cropParams_.at(1).ispCrop),
> +                                           ctrlMap[&controls::ScalerCrop].def());
> +               }
>         }
>
>         data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
> @@ -1295,11 +1311,30 @@ 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::rpi::ScalerCrops is preset, 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. Otherwise
> +        * if controls::ScalerCrop is present, apply the same crop to all ISP
> +        * output streams.
> +        */
> +       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);
> +       }
> +
> +       for (auto const &[i, scalerCrop] : utils::enumerate(scalerCrops)) {
> +               Rectangle nativeCrop = scalerCrop;
>
>                 if (!nativeCrop.width || !nativeCrop.height)
>                         nativeCrop = { 0, 0, 1, 1 };
> @@ -1315,13 +1350,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_.at(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_.at(i).ispCrop) {
> +                       cropParams_.at(i).ispCrop = ispCrop;
> +                       platformSetIspCrop(cropParams_.at(i).ispIndex, ispCrop);
>                 }
>         }
>  }
> @@ -1478,10 +1513,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
>

Hi Naush,

Thank you for the patch.

On Thu, Oct 31, 2024 at 09:49:57AM +0000, 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 latter takes
> priority over the former. If only controls::ScalerCrop is provided, the
> pipeline handler 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@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../pipeline/rpi/common/pipeline_base.cpp     | 77 +++++++++++++++----
>  1 file changed, 60 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..917c45b3f052 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_.at(0).ispCrop),
> +					    data->scaleIspCrop(data->cropParams_.at(1).ispCrop),
> +					    ctrlMap[&controls::ScalerCrop].def());
> +		}
>  	}
>  
>  	data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
> @@ -1295,11 +1311,30 @@ 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::rpi::ScalerCrops is preset, 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. Otherwise
> +	 * if controls::ScalerCrop is present, apply the same crop to all ISP
> +	 * output streams.
> +	 */
> +	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);
> +	}
> +
> +	for (auto const &[i, scalerCrop] : utils::enumerate(scalerCrops)) {
> +		Rectangle nativeCrop = scalerCrop;
>  
>  		if (!nativeCrop.width || !nativeCrop.height)
>  			nativeCrop = { 0, 0, 1, 1 };
> @@ -1315,13 +1350,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_.at(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_.at(i).ispCrop) {
> +			cropParams_.at(i).ispCrop = ispCrop;
> +			platformSetIspCrop(cropParams_.at(i).ispIndex, ispCrop);
>  		}
>  	}
>  }
> @@ -1478,10 +1513,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 */

On Mon, Nov 04, 2024 at 09:05:04AM +0000, Naushir Patuck wrote:
> Hi all,
> 
> One last tag needed on this patch and the series can then be merged.

Reviewed and pushed :-)

> On Thu, 31 Oct 2024 at 09:52, Naushir Patuck <naush@raspberrypi.com> 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 latter takes
> > priority over the former. If only controls::ScalerCrop is provided, the
> > pipeline handler 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@raspberrypi.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  .../pipeline/rpi/common/pipeline_base.cpp     | 77 +++++++++++++++----
> >  1 file changed, 60 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..917c45b3f052 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_.at(0).ispCrop),
> > +                                           data->scaleIspCrop(data->cropParams_.at(1).ispCrop),
> > +                                           ctrlMap[&controls::ScalerCrop].def());
> > +               }
> >         }
> >
> >         data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
> > @@ -1295,11 +1311,30 @@ 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::rpi::ScalerCrops is preset, 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. Otherwise
> > +        * if controls::ScalerCrop is present, apply the same crop to all ISP
> > +        * output streams.
> > +        */
> > +       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);
> > +       }
> > +
> > +       for (auto const &[i, scalerCrop] : utils::enumerate(scalerCrops)) {
> > +               Rectangle nativeCrop = scalerCrop;
> >
> >                 if (!nativeCrop.width || !nativeCrop.height)
> >                         nativeCrop = { 0, 0, 1, 1 };
> > @@ -1315,13 +1350,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_.at(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_.at(i).ispCrop) {
> > +                       cropParams_.at(i).ispCrop = ispCrop;
> > +                       platformSetIspCrop(cropParams_.at(i).ispIndex, ispCrop);
> >                 }
> >         }
> >  }
> > @@ -1478,10 +1513,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 */

On Mon, 4 Nov 2024 at 15:25, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Nov 04, 2024 at 09:05:04AM +0000, Naushir Patuck wrote:
> > Hi all,
> >
> > One last tag needed on this patch and the series can then be merged.
>
> Reviewed and pushed :-)

Thanks Laurent!

>
> > On Thu, 31 Oct 2024 at 09:52, Naushir Patuck <naush@raspberrypi.com> 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 latter takes
> > > priority over the former. If only controls::ScalerCrop is provided, the
> > > pipeline handler 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@raspberrypi.com>
> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > >  .../pipeline/rpi/common/pipeline_base.cpp     | 77 +++++++++++++++----
> > >  1 file changed, 60 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..917c45b3f052 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_.at(0).ispCrop),
> > > +                                           data->scaleIspCrop(data->cropParams_.at(1).ispCrop),
> > > +                                           ctrlMap[&controls::ScalerCrop].def());
> > > +               }
> > >         }
> > >
> > >         data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
> > > @@ -1295,11 +1311,30 @@ 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::rpi::ScalerCrops is preset, 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. Otherwise
> > > +        * if controls::ScalerCrop is present, apply the same crop to all ISP
> > > +        * output streams.
> > > +        */
> > > +       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);
> > > +       }
> > > +
> > > +       for (auto const &[i, scalerCrop] : utils::enumerate(scalerCrops)) {
> > > +               Rectangle nativeCrop = scalerCrop;
> > >
> > >                 if (!nativeCrop.width || !nativeCrop.height)
> > >                         nativeCrop = { 0, 0, 1, 1 };
> > > @@ -1315,13 +1350,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_.at(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_.at(i).ispCrop) {
> > > +                       cropParams_.at(i).ispCrop = ispCrop;
> > > +                       platformSetIspCrop(cropParams_.at(i).ispIndex, ispCrop);
> > >                 }
> > >         }
> > >  }
> > > @@ -1478,10 +1513,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 */
>
> --
> Regards,
>
> Laurent Pinchart