[libcamera-devel] [PATCH v2 2/4] ipa: raspberrypi: Use the new sensor limits fields in CameraMode

[Jacopo Mondi]

Hi Naush

On Mon, Mar 27, 2023 at 10:34:37AM +0100, Naushir Patuck via libcamera-devel wrote:
> Use the new analogue gain and shutter speed limit fields in the ipa
> code when reporting back the control value limits and calculating the
> analogue gain code to use. This also replaces the now unused (and
> removed) maxSensorGainCode_ field.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---

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

Thanks
  j

>  src/ipa/raspberrypi/raspberrypi.cpp | 37 +++++++++--------------------
>  1 file changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 3f1afb846420..c10e57e07ab0 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -224,9 +224,6 @@ private:
>  	Duration minFrameDuration_;
>  	Duration maxFrameDuration_;
>
> -	/* Maximum gain code for the sensor. */
> -	uint32_t maxSensorGainCode_;
> -
>  	/* Track the frame length times over FrameLengthsQueueSize frames. */
>  	std::deque<Duration> frameLengths_;
>  	Duration lastTimeout_;
> @@ -455,8 +452,6 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip
>  		}
>  	}
>
> -	maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();
> -
>  	/* Setup a metadata ControlList to output metadata. */
>  	libcameraMetadata_ = ControlList(controls::controls);
>
> @@ -517,26 +512,17 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip
>  	 * based on the current sensor mode.
>  	 */
>  	ControlInfoMap::Map ctrlMap = ipaControls;
> -	const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength;
> -	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;
>  	ctrlMap[&controls::FrameDurationLimits] =
> -		ControlInfo(static_cast<int64_t>(minSensorFrameDuration.get<std::micro>()),
> -			    static_cast<int64_t>(maxSensorFrameDuration.get<std::micro>()));
> +		ControlInfo(static_cast<int64_t>(mode_.minFrameDuration.get<std::micro>()),
> +			    static_cast<int64_t>(mode_.maxFrameDuration.get<std::micro>()));
>
>  	ctrlMap[&controls::AnalogueGain] =
> -		ControlInfo(1.0f, static_cast<float>(helper_->gain(maxSensorGainCode_)));
> -
> -	/*
> -	 * Calculate the max exposure limit from the frame duration limit as V4L2
> -	 * will limit the maximum control value based on the current VBLANK value.
> -	 */
> -	Duration maxShutter = Duration::max();
> -	helper_->getBlanking(maxShutter, minSensorFrameDuration, maxSensorFrameDuration);
> -	const uint32_t exposureMin = sensorCtrls_.at(V4L2_CID_EXPOSURE).min().get<int32_t>();
> +		ControlInfo(static_cast<float>(mode_.minAnalogueGain),
> +			    static_cast<float>(mode_.maxAnalogueGain));
>
>  	ctrlMap[&controls::ExposureTime] =
> -		ControlInfo(static_cast<int32_t>(helper_->exposure(exposureMin, mode_.minLineLength).get<std::micro>()),
> -			    static_cast<int32_t>(maxShutter.get<std::micro>()));
> +		ControlInfo(static_cast<int32_t>(mode_.minShutter.get<std::micro>()),
> +			    static_cast<int32_t>(mode_.maxShutter.get<std::micro>()));
>
>  	/* Declare Autofocus controls, only if we have a controllable lens */
>  	if (lensPresent_)
> @@ -1496,9 +1482,6 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
>
>  void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration)
>  {
> -	const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength;
> -	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;
> -
>  	/*
>  	 * This will only be applied once AGC recalculations occur.
>  	 * The values may be clamped based on the sensor mode capabilities as well.
> @@ -1506,9 +1489,9 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>  	minFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration;
>  	maxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration;
>  	minFrameDuration_ = std::clamp(minFrameDuration_,
> -				       minSensorFrameDuration, maxSensorFrameDuration);
> +				       mode_.minFrameDuration, mode_.maxFrameDuration);
>  	maxFrameDuration_ = std::clamp(maxFrameDuration_,
> -				       minSensorFrameDuration, maxSensorFrameDuration);
> +				       mode_.minFrameDuration, mode_.maxFrameDuration);
>  	maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);
>
>  	/* Return the validated limits via metadata. */
> @@ -1531,6 +1514,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>
>  void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>  {
> +	const int32_t minGainCode = helper_->gainCode(mode_.minAnalogueGain);
> +	const int32_t maxGainCode = helper_->gainCode(mode_.maxAnalogueGain);
>  	int32_t gainCode = helper_->gainCode(agcStatus->analogueGain);
>
>  	/*
> @@ -1538,7 +1523,7 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>  	 * DelayedControls. The AGC will correctly handle a lower gain returned
>  	 * by the sensor, provided it knows the actual gain used.
>  	 */
> -	gainCode = std::min<int32_t>(gainCode, maxSensorGainCode_);
> +	gainCode = std::clamp<int32_t>(gainCode, minGainCode, maxGainCode);
>
>  	/* getBlanking might clip exposure time to the fps limits. */
>  	Duration exposure = agcStatus->shutterTime;
> --
> 2.34.1
>