[PATCH 1/3] ipa: ipu3: Refer to integration time instead of shutter speed

[Kieran Bingham]

Quoting Daniel Scally (2024-04-18 13:46:30)
> Within the IPU3 IPA module there are multiple references to min and
> max shutter speeds. In calculation and usage however those variables
> reflect integration time rather than shutter speed - this difference
> in terminology is particularly problematic because the minimum
> shutter speed is equivalent to the maximum integration time.
> 
> Replace references to shutter speed with integration time.

I think this makes sense.


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

> 
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 12 ++++++------
>  src/ipa/ipu3/algorithms/agc.h   |  4 ++--
>  src/ipa/ipu3/ipa_context.cpp    |  8 ++++----
>  src/ipa/ipu3/ipa_context.h      |  4 ++--
>  src/ipa/ipu3/ipu3.cpp           |  8 ++++----
>  5 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 46fc3b33..a59b73fb 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -51,7 +51,7 @@ LOG_DEFINE_CATEGORY(IPU3Agc)
>  static constexpr double kMinAnalogueGain = 1.0;
>  
>  /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
> -static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> +static constexpr utils::Duration kMaxIntegrationTime = 60ms;
>  
>  /* Histogram constants */
>  static constexpr uint32_t knumHistogramBins = 256;
> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10;
>  static constexpr double kRelativeLuminanceTarget = 0.16;
>  
>  Agc::Agc()
> -       : minShutterSpeed_(0s), maxShutterSpeed_(0s)
> +       : minIntegrationTime_(0s), maxIntegrationTime_(0s)
>  {
>  }
>  
> @@ -114,9 +114,9 @@ int Agc::configure(IPAContext &context,
>         stride_ = configuration.grid.stride;
>         bdsGrid_ = configuration.grid.bdsGrid;
>  
> -       minShutterSpeed_ = configuration.agc.minShutterSpeed;
> -       maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
> -                                   kMaxShutterSpeed);
> +       minIntegrationTime_ = configuration.agc.minIntegrationTime;
> +       maxIntegrationTime_ = std::min(configuration.agc.maxIntegrationTime,
> +                                      kMaxIntegrationTime);
>  
>         minAnalogueGain_ = std::max(configuration.agc.minAnalogueGain, kMinAnalogueGain);
>         maxAnalogueGain_ = configuration.agc.maxAnalogueGain;
> @@ -129,7 +129,7 @@ int Agc::configure(IPAContext &context,
>         context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
>  
>         /* \todo Run this again when FrameDurationLimits is passed in */
> -       configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
> +       configureExposureModeHelpers(minIntegrationTime_, maxIntegrationTime_,
>                                      minAnalogueGain_, maxAnalogueGain_);
>  
>         resetFrameCount();
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 945d1846..631dea52 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -42,8 +42,8 @@ private:
>         Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
>                                   const ipu3_uapi_grid_config &grid);
>  
> -       utils::Duration minShutterSpeed_;
> -       utils::Duration maxShutterSpeed_;
> +       utils::Duration minIntegrationTime_;
> +       utils::Duration maxIntegrationTime_;
>  
>         double minAnalogueGain_;
>         double maxAnalogueGain_;
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index c4fb5642..72f7cec9 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -92,11 +92,11 @@ namespace libcamera::ipa::ipu3 {
>   * \var IPASessionConfiguration::agc
>   * \brief AGC parameters configuration of the IPA
>   *
> - * \var IPASessionConfiguration::agc.minShutterSpeed
> - * \brief Minimum shutter speed supported with the configured sensor
> + * \var IPASessionConfiguration::agc.minIntegrationTime
> + * \brief Minimum integration time supported with the configured sensor
>   *
> - * \var IPASessionConfiguration::agc.maxShutterSpeed
> - * \brief Maximum shutter speed supported with the configured sensor
> + * \var IPASessionConfiguration::agc.maxIntegrationTime
> + * \brief Maximum integration time supported with the configured sensor
>   *
>   * \var IPASessionConfiguration::agc.minAnalogueGain
>   * \brief Minimum analogue gain supported with the configured sensor
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index a92cb6ce..6b1ffdd1 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -33,8 +33,8 @@ struct IPASessionConfiguration {
>         } af;
>  
>         struct {
> -               utils::Duration minShutterSpeed;
> -               utils::Duration maxShutterSpeed;
> +               utils::Duration minIntegrationTime;
> +               utils::Duration maxIntegrationTime;
>                 double minAnalogueGain;
>                 double maxAnalogueGain;
>         } agc;
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 4809eb60..f13cc394 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -217,13 +217,13 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>  
>         /*
>          * When the AGC computes the new exposure values for a frame, it needs
> -        * to know the limits for shutter speed and analogue gain.
> +        * to know the limits for integration time and analogue gain.
>          * As it depends on the sensor, update it with the controls.
>          *
> -        * \todo take VBLANK into account for maximum shutter speed
> +        * \todo take VBLANK into account for maximum integration time
>          */
> -       context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
> -       context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
> +       context_.configuration.agc.minIntegrationTime = minExposure * context_.configuration.sensor.lineDuration;
> +       context_.configuration.agc.maxIntegrationTime = maxExposure * context_.configuration.sensor.lineDuration;
>         context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
>         context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
>  }
> -- 
> 2.34.1
>