[PATCH 1/3] ipa: ipu3: Refer to integration time instead of shutter speed
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed May 8 15:33:09 CEST 2024
Hi Dan,
Thank you for the patch.
On Thu, Apr 18, 2024 at 01:46:30PM +0100, Daniel Scally wrote:
> 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 like this (I may be tempted to reply with U+2661), and I'd like the
change to be applied globally through libcamera. "Integration time" even
deserves a place in a glossary in my opinion.
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart 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);
> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list