[libcamera-devel] [PATCH v2 3/3] ipa: ipu3: agc: Introduce lineDuration in IPASessionConfiguration
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Feb 7 16:34:32 CET 2022
Hi JM,
Quoting Jean-Michel Hautbois (2022-02-07 13:59:37)
> Instead of having a local cached value for line duration, store it in
> the IPASessionConfiguration::agc structure.
Looking at my previous comments on this patch in
https://patchwork.libcamera.org/patch/14778/, I was suggesting that I
believed this line duration should be in the sensor specific structure,
not the AGC specific structure.
I'm sorry my comment then was not clear, perhaps I should have made it
more direct.
Is the lineDuration specific to the AGC algorithm in any way that I have
not seen or understood?
In rkisp1/ipa_context.h, you have:
struct IPASessionConfiguration {
...
struct {
utils::Duration lineDuration;
} sensor;
...
};
> While at it, configure the default analogue gain and shutter speed to
> controlled fixed values.
>
> The latter is set to be 10ms as it will in most cases be close to the
> one needed, making the AGC faster to converge.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/agc.cpp | 25 +++++++++++++++----------
> src/ipa/ipu3/algorithms/agc.h | 3 +--
> src/ipa/ipu3/ipa_context.cpp | 3 +++
> src/ipa/ipu3/ipa_context.h | 1 +
> 4 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index a929085c..137c36cf 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10;
> static constexpr double kRelativeLuminanceTarget = 0.16;
>
> Agc::Agc()
> - : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s),
> + : frameCount_(0), minShutterSpeed_(0s),
> maxShutterSpeed_(0s), filteredExposure_(0s)
> {
> }
> @@ -85,11 +85,14 @@ Agc::Agc()
> */
> int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> {
> - stride_ = context.configuration.grid.stride;
> + IPASessionConfiguration &configuration = context.configuration;
> + IPAFrameContext &frameContext = context.frameContext;
> +
> + stride_ = configuration.grid.stride;
>
> /* \todo use the IPAContext to provide the limits */
> - lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
> - / configInfo.sensorInfo.pixelRate;
> + configuration.agc.lineDuration = configInfo.sensorInfo.lineLength * 1.0s
> + / configInfo.sensorInfo.pixelRate;
And in RKISP1 this is calculated during the IPA configure, before it
calls configure on the algorithms. Should we do that here to align them?
>
> minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
> maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
> @@ -99,8 +102,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>
> /* Configure the default exposure and gain. */
> - context.frameContext.agc.gain = minAnalogueGain_;
> - context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
> + frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);
> + frameContext.agc.exposure = 10ms / configuration.agc.lineDuration;
>
> return 0;
> }
> @@ -182,9 +185,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
> * \param[in] yGain The gain calculated based on the relative luminance target
> * \param[in] iqMeanGain The gain calculated based on the relative luminance target
> */
> -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
> +void Agc::computeExposure(IPAContext &context, double yGain,
> double iqMeanGain)
> {
> + const IPASessionConfiguration &configuration = context.configuration;
> + IPAFrameContext &frameContext = context.frameContext;
> /* Get the effective exposure and gain applied on the sensor. */
> uint32_t exposure = frameContext.sensor.exposure;
> double analogueGain = frameContext.sensor.gain;
> @@ -200,7 +205,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
> /* extracted from Rpi::Agc::computeTargetExposure */
>
> /* Calculate the shutter time in seconds */
> - utils::Duration currentShutter = exposure * lineDuration_;
> + utils::Duration currentShutter = exposure * configuration.agc.lineDuration;
>
> /*
> * Update the exposure value for the next computation using the values
> @@ -247,7 +252,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
> << stepGain;
>
> /* Update the estimated exposure and gain. */
> - frameContext.agc.exposure = shutterTime / lineDuration_;
> + frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration;
> frameContext.agc.gain = stepGain;
> }
>
> @@ -354,7 +359,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> break;
> }
>
> - computeExposure(context.frameContext, yGain, iqMeanGain);
> + computeExposure(context, yGain, iqMeanGain);
> frameCount_++;
> }
>
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 84bfe045..ad705605 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -34,7 +34,7 @@ private:
> double measureBrightness(const ipu3_uapi_stats_3a *stats,
> const ipu3_uapi_grid_config &grid) const;
> utils::Duration filterExposure(utils::Duration currentExposure);
> - void computeExposure(IPAFrameContext &frameContext, double yGain,
> + void computeExposure(IPAContext &context, double yGain,
> double iqMeanGain);
> double estimateLuminance(IPAFrameContext &frameContext,
> const ipu3_uapi_grid_config &grid,
> @@ -43,7 +43,6 @@ private:
>
> uint64_t frameCount_;
>
> - utils::Duration lineDuration_;
> utils::Duration minShutterSpeed_;
> utils::Duration maxShutterSpeed_;
>
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 86794ac1..ace9c66f 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {
> *
> * \var IPASessionConfiguration::agc.maxAnalogueGain
> * \brief Maximum analogue gain supported with the configured sensor
> + *
> + * \var IPASessionConfiguration::agc.lineDuration
> + * \brief Line duration in microseconds
> */
>
> /**
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index c6dc0814..7696fd14 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -30,6 +30,7 @@ struct IPASessionConfiguration {
> utils::Duration maxShutterSpeed;
> double minAnalogueGain;
> double maxAnalogueGain;
> + utils::Duration lineDuration;
> } agc;
> };
>
> --
> 2.32.0
>
More information about the libcamera-devel
mailing list