[PATCH v3 1/7] controls: rpi: Add a vendor rpi::ScalerCrops control

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Thu, Oct 03, 2024 at 08:47:14AM +0100, Naushir Patuck wrote:
> Add a vendor control rpi::ScalerCrops that is analogous to the current
> core::ScalerCrop, but can apply a different crop to each configured
> stream.
> 
> This control takes a span of Rectangle structures - the order of
> rectangles must match the order of streams configured by the application.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
>  src/ipa/rpi/common/ipa_base.cpp    | 14 ++++++++++++++
>  src/libcamera/control_ids_rpi.yaml | 25 +++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index ee3848b54f21..d408cdb74255 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -96,6 +96,15 @@ const ControlInfoMap::Map ipaAfControls{
>  	{ &controls::LensPosition, ControlInfo(0.0f, 32.0f, 1.0f) }
>  };
>  
> +/* Platform specific controls */

s/Platform specific/Platform-specific/

> +const std::map<const std::string, ControlInfoMap::Map> platformControls {
> +	{ "pisp",
> +	  {
> +		{ &controls::rpi::ScalerCrops, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) }
> +	  }

Two spaces is a weird indentation. You can write

	{ "pisp", {
		{ &controls::rpi::ScalerCrops, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) }
	} },

I'll make those modifications when applying.

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

> +	},
> +};
> +
>  } /* namespace */
>  
>  LOG_DEFINE_CATEGORY(IPARPI)
> @@ -159,6 +168,10 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams &params, Ini
>  	if (lensPresent_)
>  		ctrlMap.merge(ControlInfoMap::Map(ipaAfControls));
>  
> +	auto platformCtrlsIt = platformControls.find(controller_.getTarget());
> +	if (platformCtrlsIt != platformControls.end())
> +		ctrlMap.merge(ControlInfoMap::Map(platformCtrlsIt->second));
> +
>  	monoSensor_ = params.sensorInfo.cfaPattern == properties::draft::ColorFilterArrangementEnum::MONO;
>  	if (!monoSensor_)
>  		ctrlMap.merge(ControlInfoMap::Map(ipaColourControls));
> @@ -1070,6 +1083,7 @@ void IpaBase::applyControls(const ControlList &controls)
>  			break;
>  		}
>  
> +		case controls::rpi::SCALER_CROPS:
>  		case controls::SCALER_CROP: {
>  			/* We do nothing with this, but should avoid the warning below. */
>  			break;
> diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml
> index 42c4bf5d408f..34bbdfc863c5 100644
> --- a/src/libcamera/control_ids_rpi.yaml
> +++ b/src/libcamera/control_ids_rpi.yaml
> @@ -30,4 +30,29 @@ controls:
>  
>          \sa StatsOutputEnable
>  
> +  - ScalerCrops:
> +      type: Rectangle
> +      size: [n]
> +      description: |
> +        An array of rectangles, where each singular value has identical
> +        functionality to the ScalerCrop control. This control allows the
> +        Raspberry Pi pipeline handler to control individual scaler crops per
> +        output stream.
> +
> +        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.
> +
> +        If both rpi::ScalerCrops and ScalerCrop controls are present in a
> +        ControlList, the latter is discarded, and crops are obtained from this
> +        control.
> +
> +        Note that using different crop rectangles for each output stream with
> +        this control is only applicable on the Pi5/PiSP platform. This control
> +        should also be considered temporary/draft and will be replaced with
> +        official libcamera API support for per-stream controls in the future.
> +
> +        \sa ScalerCrop
>  ...

-- 
Regards,

Laurent Pinchart