[v3,7/7] pipeline: rpi: Handler controls::rpi::ScalerCrops
diff mbox series

Message ID 20241003074720.18882-8-naush@raspberrypi.com
State Accepted
Headers show
Series
  • Raspberry Pi: Add controls::rpi::ScalerCrops
Related show

Commit Message

Naushir Patuck Oct. 3, 2024, 7:47 a.m. UTC 
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@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp     | 76 ++++++++++++++-----
 1 file changed, 59 insertions(+), 17 deletions(-)

Comments

Laurent Pinchart Oct. 28, 2024, 10:35 p.m. UTC | #1 
Hi Naush,

Thank you for the patch.

On Thu, Oct 03, 2024 at 08:47:20AM +0100, 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.

That contradicts patch 1/7 that states

        If both rpi::ScalerCrops and ScalerCrop controls are present in a
        ControlList, the latter is discarded, and crops are obtained from this
        control.

and also contradicts the code below.

> 
> 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     | 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..eba4dad6d212 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);

This changes the indices, but as far as I can tell, they're currently
unused, so that should be fine. Do I assume correctly the will be used
in the Pi 5 platformConfigure() implementation to populate cropParams_
in the right order ?

>  	}
>  
>  	/* 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.

That sounds like a real hack. I'm fine with it for now, but should it be
documented in 1/7 ? Or would you rather not have anyone depending on
this ?

> +			 */
> +			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,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.

This also contradicts the code below. You can pick the begaviour you
want, but let's make documentation, comments, commit messages and code
consistent.

> +	 */
> +	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 +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_.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 +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 */
Naushir Patuck Oct. 29, 2024, 8:58 a.m. UTC | #2 
Hi Laurent,

On Mon, 28 Oct 2024 at 22:35, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Naush,
>
> Thank you for the patch.
>
> On Thu, Oct 03, 2024 at 08:47:20AM +0100, 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.
>
> That contradicts patch 1/7 that states
>
>         If both rpi::ScalerCrops and ScalerCrop controls are present in a
>         ControlList, the latter is discarded, and crops are obtained from this
>         control.
>
> and also contradicts the code below.

Yes, this is an error in the commit message.  I'll update this to
reflect that rpi::ScalerCrops take precedence.

>
> >
> > 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     | 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..eba4dad6d212 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);
>
> This changes the indices, but as far as I can tell, they're currently
> unused, so that should be fine. Do I assume correctly the will be used
> in the Pi 5 platformConfigure() implementation to populate cropParams_
> in the right order ?

That's correct, the VC4 pipeline handler doesn't use this index, it's
only for PISP where the control is used.

>
> >       }
> >
> >       /* 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.
>
> That sounds like a real hack. I'm fine with it for now, but should it be
> documented in 1/7 ? Or would you rather not have anyone depending on
> this ?

100% a hack.  I'd be happy to see us get rid of this asap when we have
real per-stream controls.  I intend on working on this next...

>
> > +                      */
> > +                     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,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.
>
> This also contradicts the code below. You can pick the begaviour you
> want, but let's make documentation, comments, commit messages and code
> consistent.

As above, I'll fix up the comment here as well.

Regards,
Naush

>
> > +      */
> > +     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 +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_.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 +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 */
>
> --
> Regards,
>
> Laurent Pinchart

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 267e6bd9cd70..eba4dad6d212 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,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);
+	}
+
+	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_.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 +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 */