[libcamera-devel] [PATCH v2 10/13] ipa: ipu3: agc: Introduce previous exposure value

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Wed, Oct 20, 2021 at 05:46:04PM +0200, Jean-Michel Hautbois wrote:
> We need to calculate the gain on the previous exposure value calculated.
> Now that we initialise the exposure and gain values in configure(), we
> know the initial exposure value, and we can set it before any loop is
> running.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 13 +++++++++++--
>  src/ipa/ipu3/algorithms/agc.h   |  1 +
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index eec77378..19f3a420 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -42,7 +42,8 @@ static constexpr double kEvGainTarget = 0.5;
>  Agc::Agc()
>  	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
>  	  minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s),
> -	  filteredExposureNoDg_(0s), currentExposure_(0s), currentExposureNoDg_(0s)
> +	  filteredExposureNoDg_(0s), currentExposure_(0s),
> +	  currentExposureNoDg_(0s), prevExposureValue_(0s)
>  {
>  }
>  
> @@ -58,9 +59,14 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>  		context.configuration.agc.minAnalogueGain;
>  	context.frameContext.agc.exposure = minExposureLines_;
>  
> +	/* \todo replace the exposure in lines storage with time based ones */

This could have gone to an earlier patch.

>  	minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_;
>  	maxExposureLines_ = context.configuration.agc.maxShutterSpeed / lineDuration_;
>  
> +	prevExposureValue_ = context.frameContext.agc.gain
> +			   * context.frameContext.agc.exposure
> +			   * lineDuration_;
> +
>  	return 0;
>  }
>  
> @@ -146,7 +152,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>  				    << " Gain " << analogueGain
>  				    << " Needed ev gain " << evGain;
>  
> -		currentExposure_ = currentExposureNoDg_ * evGain;
> +		currentExposure_ = prevExposureValue_ * evGain;
>  		utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;
>  		utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;
>  
> @@ -175,6 +181,9 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>  
>  		exposure = shutterTime / lineDuration_;
>  		analogueGain = stepGain;
> +
> +		/* Update the exposure value for the next process call */

		/*
		 * Update the exposure value for the next process call.
		 *
		 * \todo Obtain the values of the exposure time and analog gain
		 * that were actually used by the sensor, either from embedded
		 * data when available, or from the delayed controls
		 * infrastructure in case a slow down caused a mismatch.
		 */

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

> +		prevExposureValue_ = shutterTime * analogueGain;
>  	}
>  	lastFrame_ = frameCount_;
>  }
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index cd26d08c..2ae88e9f 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -49,6 +49,7 @@ private:
>  	utils::Duration filteredExposureNoDg_;
>  	utils::Duration currentExposure_;
>  	utils::Duration currentExposureNoDg_;
> +	utils::Duration prevExposureValue_;
>  
>  	uint32_t stride_;
>  };

-- 
Regards,

Laurent Pinchart