[libcamera-devel] [PATCH 09/13] ipa: ipu3: agc: Rename gains properly

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Wed, Oct 13, 2021 at 05:41:21PM +0200, Jean-Michel Hautbois wrote:
> We have mixed terms between gain, analogue gain and the exposure value
> gain.
> 
> Makes it clear when we are using the analogue gain from the sensor, and
> when we are using the calculated gain to be applied to the exposure
> value to reach the target.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

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

> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 3ec1c60c..bd28998a 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -123,7 +123,7 @@ void Agc::filterExposure()
>  	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << filteredExposure_;
>  }
>  
> -void Agc::lockExposureGain(uint32_t &exposure, double &gain)
> +void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>  {
>  	/* Algorithm initialization should wait for first valid frames */
>  	/* \todo - have a number of frames given by DelayedControls ?
> @@ -135,16 +135,17 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>  	if (std::abs(iqMean_ - kEvGainTarget * knumHistogramBins) <= 1) {
>  		LOG(IPU3Agc, Debug) << "!!! Good exposure with iqMean = " << iqMean_;
>  	} else {
> -		double newGain = kEvGainTarget * knumHistogramBins / iqMean_;
> +		double evGain = kEvGainTarget * knumHistogramBins / iqMean_;
>  
>  		/* extracted from Rpi::Agc::computeTargetExposure */
>  		Duration currentShutter = exposure * lineDuration_;
> -		currentExposureNoDg_ = currentShutter * gain;
> +		currentExposureNoDg_ = currentShutter * analogueGain;
>  		LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
>  				    << " Shutter speed " << currentShutter
> -				    << " Gain " << gain;
> +				    << " Gain " << analogueGain
> +				    << " Needed ev gain " << evGain;
>  
> -		currentExposure_ = currentExposureNoDg_ * newGain;
> +		currentExposure_ = currentExposureNoDg_ * evGain;
>  		Duration maxTotalExposure = kMaxShutterSpeed * kMaxGain;
>  		currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>  		LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
> @@ -180,7 +181,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>  				<< stepGain;
>  
>  		exposure = shutterTime / lineDuration_;
> -		gain = stepGain;
> +		analogueGain = stepGain;
>  	}
>  	lastFrame_ = frameCount_;
>  }
> @@ -188,9 +189,9 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>  void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>  {
>  	uint32_t &exposure = context.frameContext.agc.exposure;
> -	double &gain = context.frameContext.agc.gain;
> +	double &analogueGain = context.frameContext.agc.gain;
>  	processBrightness(stats, context.configuration.grid.bdsGrid);
> -	lockExposureGain(exposure, gain);
> +	lockExposureGain(exposure, analogueGain);
>  	frameCount_++;
>  }
>  

-- 
Regards,

Laurent Pinchart