[libcamera-devel,09/22] ipa: ipu3: agc: Use exposure in time for storage

14477 diff mbox series

The minimum and maximum exposure are stored in lines. Replace it by
values in time, which removes a bit of code.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp | 21 +++++++++------------
 src/ipa/ipu3/algorithms/agc.h   |  4 ++--
 2 files changed, 11 insertions(+), 14 deletions(-)

Quoting Jean-Michel Hautbois (2021-11-08 13:13:37)
> The minimum and maximum exposure are stored in lines. Replace it by
> values in time, which removes a bit of code.

s/, which removes a bit of code/ to simplify the calculations./ ?


> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 21 +++++++++------------
>  src/ipa/ipu3/algorithms/agc.h   |  4 ++--
>  2 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 475e715f..3a15f5d9 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -68,8 +68,8 @@ static constexpr uint32_t kMinCellsPerZoneRatio = 255 * 20 / 100;
>  static constexpr uint32_t kNumStartupFrames = 10;
>  
>  Agc::Agc()
> -       : frameCount_(0), iqMean_(0.0), lineDuration_(0s), minExposureLines_(0),
> -         maxExposureLines_(0), filteredExposure_(0s), currentExposure_(0s),
> +       : frameCount_(0), iqMean_(0.0), lineDuration_(0s), minShutterSpeed_(0s),
> +         maxShutterSpeed_(0s), filteredExposure_(0s), currentExposure_(0s),
>           prevExposureValue_(0s)
>  {
>  }
> @@ -89,17 +89,16 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>         lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
>                       / configInfo.sensorInfo.pixelRate;
>  
> -       /* \todo replace the exposure in lines storage with time based ones. */
> -       minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_;
> -       maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_,
> -                                    kMaxShutterSpeed / lineDuration_);
> +       minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
> +       maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
> +                                   kMaxShutterSpeed);
>  
>         minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
>         maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>  
>         /* Configure the default exposure and gain. */
>         context.frameContext.agc.gain = minAnalogueGain_;
> -       context.frameContext.agc.exposure = minExposureLines_;
> +       context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
>  
>         prevExposureValue_ = context.frameContext.agc.gain
>                            * context.frameContext.agc.exposure
> @@ -217,11 +216,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain, double curre
>          * exposure value applied multiplied by the new estimated gain.
>          */
>         currentExposure_ = prevExposureValue_ * evGain;
> -       utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;
> -       utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;
>  
>         /* Clamp the exposure value to the min and max authorized */
> -       utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;
> +       utils::Duration maxTotalExposure = maxShutterSpeed_ * maxAnalogueGain_;
>         currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>         LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
>                             << ", maximum is " << maxTotalExposure;
> @@ -231,14 +228,14 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain, double curre
>  
>         /* Divide the exposure value as new exposure and gain values */
>         utils::Duration exposureValue = filteredExposure_;
> -       utils::Duration shutterTime = minShutterSpeed;
> +       utils::Duration shutterTime = minShutterSpeed_;

I think this gets set immediately from the line below, so we don't need
to initialise it to a specific minimum?


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

>  
>         /*
>         * Push the shutter time up to the maximum first, and only then
>         * increase the gain.
>         */
>         shutterTime = std::clamp<utils::Duration>(exposureValue / minAnalogueGain_,
> -                                                 minShutterSpeed, maxShutterSpeed);
> +                                                 minShutterSpeed_, maxShutterSpeed_);
>         double stepGain = std::clamp(exposureValue / shutterTime,
>                                      minAnalogueGain_, maxAnalogueGain_);
>         LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 0a9152a9..6f5d71e0 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -46,8 +46,8 @@ private:
>         double iqMean_;
>  
>         utils::Duration lineDuration_;
> -       uint32_t minExposureLines_;
> -       uint32_t maxExposureLines_;
> +       utils::Duration minShutterSpeed_;
> +       utils::Duration maxShutterSpeed_;
>  
>         double minAnalogueGain_;
>         double maxAnalogueGain_;
> -- 
> 2.32.0
>