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

[Kieran Bingham]

Quoting Jean-Michel Hautbois (2021-10-13 16:41:22)
> We need to calculate the gain on the previous exposure value calculated.
> This value needs to be updated after each process call, initialised to
> 0s.
> 

Seems resonable.

Checking lockExposureGain() looks like it could be refactored to indent
that whole exposure calculation one level, but lets deal with that after
this series.


> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 8 ++++++--
>  src/ipa/ipu3/algorithms/agc.h   | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index bd28998a..7efe0907 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -46,7 +46,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)
>  {
>  }
>  
> @@ -145,7 +146,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>                                     << " Gain " << analogueGain
>                                     << " Needed ev gain " << evGain;
>  
> -               currentExposure_ = currentExposureNoDg_ * evGain;
> +               currentExposure_ = prevExposureValue * evGain;
>                 Duration maxTotalExposure = kMaxShutterSpeed * kMaxGain;
>                 currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>                 LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
> @@ -182,6 +183,9 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>  
>                 exposure = shutterTime / lineDuration_;
>                 analogueGain = stepGain;
> +
> +               /* Update the exposure value for the next process call */
> +               prevExposureValue = shutterTime * analogueGain;
>         }
>         lastFrame_ = frameCount_;
>  }
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 7605ab39..32817c4f 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -51,6 +51,7 @@ private:
>         Duration filteredExposureNoDg_;
>         Duration currentExposure_;
>         Duration currentExposureNoDg_;
> +       Duration prevExposureValue;

It looks like this should have an underscore at the end.

With that,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>  
>         uint32_t stride_;
>  };
> -- 
> 2.30.2
>