[libcamera-devel] [PATCH 05/13] ipa: ipu3: agc: Rename exposure values properly

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Wed, Oct 13, 2021 at 05:41:17PM +0200, Jean-Michel Hautbois wrote:
> The exposure value is filtered in filterExposure() using the
> currentExposure_ and setting a prevExposure_ variable. This is misnamed
> as it is not the previous exposure, but a filtered value.
> 
> Rename it accordingly.
> 
> 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 | 28 ++++++++++++++--------------
>  src/ipa/ipu3/algorithms/agc.h   |  4 ++--
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 5eafe82c..7c2d4201 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -49,7 +49,7 @@ static constexpr double kEvGainTarget = 0.5;
>  
>  Agc::Agc()
>  	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
> -	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),
> +	  maxExposureTime_(0s), filteredExposure_(0s), filteredExposureNoDg_(0s),
>  	  currentExposure_(0s), currentExposureNoDg_(0s)
>  {
>  }
> @@ -94,24 +94,24 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,
>  void Agc::filterExposure()
>  {
>  	double speed = 0.2;
> -	if (prevExposure_ == 0s) {
> +	if (filteredExposure_ == 0s) {
>  		/* DG stands for digital gain.*/
> -		prevExposure_ = currentExposure_;
> -		prevExposureNoDg_ = currentExposureNoDg_;
> +		filteredExposure_ = currentExposure_;
> +		filteredExposureNoDg_ = currentExposureNoDg_;
>  	} else {
>  		/*
>  		 * If we are close to the desired result, go faster to avoid making
>  		 * multiple micro-adjustments.
>  		 * \ todo: Make this customisable?
>  		 */
> -		if (prevExposure_ < 1.2 * currentExposure_ &&
> -		    prevExposure_ > 0.8 * currentExposure_)
> +		if (filteredExposure_ < 1.2 * currentExposure_ &&
> +		    filteredExposure_ > 0.8 * currentExposure_)
>  			speed = sqrt(speed);
>  
> -		prevExposure_ = speed * currentExposure_ +
> -				prevExposure_ * (1.0 - speed);
> -		prevExposureNoDg_ = speed * currentExposureNoDg_ +
> -				prevExposureNoDg_ * (1.0 - speed);
> +		filteredExposure_ = speed * currentExposure_ +
> +				filteredExposure_ * (1.0 - speed);
> +		filteredExposureNoDg_ = speed * currentExposureNoDg_ +
> +				filteredExposureNoDg_ * (1.0 - speed);
>  	}
>  	/*
>  	 * We can't let the no_dg exposure deviate too far below the
> @@ -119,10 +119,10 @@ void Agc::filterExposure()
>  	 * in the ISP to hide it (which will cause nasty oscillation).
>  	 */
>  	double fastReduceThreshold = 0.4;
> -	if (prevExposureNoDg_ <
> -	    prevExposure_ * fastReduceThreshold)
> -		prevExposureNoDg_ = prevExposure_ * fastReduceThreshold;
> -	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << prevExposure_;
> +	if (filteredExposureNoDg_ <
> +	    filteredExposure_ * fastReduceThreshold)
> +		filteredExposureNoDg_ = filteredExposure_ * fastReduceThreshold;
> +	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << filteredExposure_;
>  }
>  
>  void Agc::lockExposureGain(uint32_t &exposure, double &gain)
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 64b71c65..cd4d4855 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -46,8 +46,8 @@ private:
>  	Duration lineDuration_;
>  	Duration maxExposureTime_;
>  
> -	Duration prevExposure_;
> -	Duration prevExposureNoDg_;
> +	Duration filteredExposure_;
> +	Duration filteredExposureNoDg_;
>  	Duration currentExposure_;
>  	Duration currentExposureNoDg_;
>  

-- 
Regards,

Laurent Pinchart