[libcamera-devel] [PATCH v1 2/3] ipa: raspberrypi: Populate SensorLimits

[Jacopo Mondi]

Hi Naush

On Wed, Mar 22, 2023 at 04:13:16PM +0000, Naushir Patuck via libcamera-devel wrote:
> Populate all the fields of the SensorLimits structure in configure().
> This allows us to use the cached values instead of re-computing them
> on every frame.
>
> For the gain -> code convertion, ensure we clamp to the analogue gain
> limits set in the SensorLimits structure.

I wonder if this can't be done by expanding your CameraMode structure.
CameraMode already combines information from the SensorInfo and could
very well include information from sensorControls.

This would avoid introducing another type to move it around across
different layers of your IPA.

Have you considered that and decided you prefer introducing
SensorLimits ?

>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 67 ++++++++++++++++-------------
>  1 file changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index c3b2c375036f..e8d8023bcfe7 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -224,8 +224,8 @@ private:
>  	Duration minFrameDuration_;
>  	Duration maxFrameDuration_;
>
> -	/* Maximum gain code for the sensor. */
> -	uint32_t maxSensorGainCode_;
> +	/* Mode specific sensor gain/exposure limits. */
> +	RPiController::AgcAlgorithm::SensorLimits sensorLimits_;
>
>  	/* Track the frame length times over FrameLengthsQueueSize frames. */
>  	std::deque<Duration> frameLengths_;
> @@ -439,8 +439,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);
>
> @@ -473,6 +471,28 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip
>  	/* Pass the camera mode to the CamHelper to setup algorithms. */
>  	helper_->setCameraMode(mode_);
>
> +	/*
> +	 * Store the sensor gain and shutter limits for the mode.
> +	 *
> +	 * The maximum shutter value are calculated from the frame duration limit
> +	 * as V4L2 will restrict the maximum control value based on the current
> +	 * VBLANK value.
> +	 *
> +	 * These limits get set in the AGC algorithm through applyFrameDurations()
> +	 * below.
> +	 */
> +	const ControlInfo &gainCtrl = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN);
> +	const ControlInfo &shutterCtrl = sensorCtrls_.at(V4L2_CID_EXPOSURE);
> +
> +	sensorLimits_.minAnalogueGain = helper_->gain(gainCtrl.min().get<int32_t>());
> +	sensorLimits_.maxAnalogueGain = helper_->gain(gainCtrl.max().get<int32_t>());
> +	sensorLimits_.minFrameDuration = mode_.minFrameLength * mode_.minLineLength;
> +	sensorLimits_.maxFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;
> +	sensorLimits_.minShutter = helper_->exposure(shutterCtrl.min().get<int32_t>(), mode_.minLineLength);
> +	sensorLimits_.maxShutter = Duration::max();
> +	helper_->getBlanking(sensorLimits_.maxShutter,
> +			     sensorLimits_.minFrameDuration, sensorLimits_.maxFrameDuration);
> +
>  	/*
>  	 * Initialise this ControlList correctly, even if empty, in case the IPA is
>  	 * running is isolation mode (passing the ControlList through the IPC layer).
> @@ -501,26 +521,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>(sensorLimits_.minFrameDuration.get<std::micro>()),
> +			    static_cast<int64_t>(sensorLimits_.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>(sensorLimits_.minAnalogueGain),
> +			    static_cast<float>(sensorLimits_.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>(sensorLimits_.minShutter.get<std::micro>()),
> +			    static_cast<int32_t>(sensorLimits_.maxShutter.get<std::micro>()));
>
>  	/* Declare Autofocus controls, only if we have a controllable lens */
>  	if (lensPresent_)
> @@ -1480,9 +1491,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.
> @@ -1490,9 +1498,9 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>  	minFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration;
>  	maxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration;
>  	minFrameDuration_ = std::clamp(minFrameDuration_,
> -				       minSensorFrameDuration, maxSensorFrameDuration);
> +				       sensorLimits_.minFrameDuration, sensorLimits_.maxFrameDuration);
>  	maxFrameDuration_ = std::clamp(maxFrameDuration_,
> -				       minSensorFrameDuration, maxSensorFrameDuration);
> +				       sensorLimits_.minFrameDuration, sensorLimits_.maxFrameDuration);
>  	maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);
>
>  	/* Return the validated limits via metadata. */
> @@ -1505,17 +1513,18 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>  	 * getBlanking() will update maxShutter with the largest exposure
>  	 * value possible.
>  	 */
> -	RPiController::AgcAlgorithm::SensorLimits limits;
> -	limits.maxShutter = Duration::max();
> -	helper_->getBlanking(limits.maxShutter, minFrameDuration_, maxFrameDuration_);
> +	sensorLimits_.maxShutter = Duration::max();
> +	helper_->getBlanking(sensorLimits_.maxShutter, minFrameDuration_, maxFrameDuration_);
>
>  	RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
>  		controller_.getAlgorithm("agc"));
> -	agc->setSensorLimits(limits);
> +	agc->setSensorLimits(sensorLimits_);
>  }
>
>  void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>  {
> +	const int32_t minGainCode = helper_->gainCode(sensorLimits_.minAnalogueGain);
> +	const int32_t maxGainCode = helper_->gainCode(sensorLimits_.maxAnalogueGain);
>  	int32_t gainCode = helper_->gainCode(agcStatus->analogueGain);
>
>  	/*
> @@ -1523,7 +1532,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
>