[libcamera-devel] [PATCH v3 07/19] ipa: ipu3: agc: Document AGC mean-based algorithm
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Oct 25 23:26:12 CEST 2021
Quoting Jean-Michel Hautbois (2021-10-22 16:12:06)
> The AGC class was not documented while developing. Extend that to
> reference the origins of the implementation, and improve the
> descriptions on how the algorithm operates internally.
>
> While at it, rename the functions which have bad names.
Aha, these are just renames. It threw me off thinking there were bigger
changes in here, but a rename is fine.
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> ---
> v3:
> - rename processBrightness to measureBrightness
> - rename lockExposureGain to computeExposure
> - reword some comments
> ---
> src/ipa/ipu3/algorithms/agc.cpp | 79 ++++++++++++++++++++++++++++++---
> src/ipa/ipu3/algorithms/agc.h | 6 +--
> 2 files changed, 75 insertions(+), 10 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 6c151232..b145e522 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -2,7 +2,7 @@
> /*
> * Copyright (C) 2021, Ideas On Board
> *
> - * ipu3_agc.cpp - AGC/AEC control algorithm
> + * ipu3_agc.cpp - AGC/AEC mean-based control algorithm
> */
>
> #include "agc.h"
> @@ -17,17 +17,37 @@
>
> #include "libipa/histogram.h"
>
> +/**
> + * \file agc.h
> + */
> +
> namespace libcamera {
>
> using namespace std::literals::chrono_literals;
>
> namespace ipa::ipu3::algorithms {
>
> +/**
> + * \class Agc
> + * \brief A mean-based auto-exposure algorithm
> + *
> + * This algorithm calculates a shutter time and an analogue gain so that the
Is it an analogue gain? or just a gain? Does the algorithm care if it is
analogue?
Perhaps it does, as it's targetted to the Sensor controls.
> + * average value of the green channel of the brightest 2% of pixels approaches
> + * 0.5. The AWB gains are not used here, and all cells in the grid have the same
> + * weight, like an average-metering case. In this metering mode, the camera uses
> + * light information from the entire scene and creates an average for the final
> + * exposure setting, giving no weighting to any particular portion of the
> + * metered area.
> + *
> + * Reference: Battiato, Messina & Castorina. (2008). Exposure
> + * Correction for Imaging Devices: An Overview. 10.1201/9781420054538.ch12.
> + */
> +
> LOG_DEFINE_CATEGORY(IPU3Agc)
>
> /* Number of frames to wait before calculating stats on minimum exposure */
> static constexpr uint32_t kInitialFrameMinAECount = 4;
> -/* Number of frames to wait between new gain/exposure estimations */
> +/* Number of frames to wait between new gain/shutter time estimations */
> static constexpr uint32_t kFrameSkipCount = 6;
>
> /* Limits for analogue gain values */
> @@ -36,6 +56,7 @@ static constexpr double kMaxAnalogueGain = 8.0;
>
> /* Histogram constants */
> static constexpr uint32_t knumHistogramBins = 256;
I'd have a new line here.
> +/* Target value to reach for the top 2% of the histogram */
Is this something that is expected to be tuned? or is 0.5 used to mean
quite literally 'the middle' between 1 and 0?
> static constexpr double kEvGainTarget = 0.5;
>
> Agc::Agc()
> @@ -45,10 +66,18 @@ Agc::Agc()
> {
> }
>
> +/**
> + * \brief Configure the AGC given a configInfo
> + * \param[in] context The shared IPA context
> + * \param[in] configInfo The IPA configuration data
> + *
> + * \return 0
> + */
> int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> {
> stride_ = context.configuration.grid.stride;
>
> + /* \todo use the IPAContext to provide the limits */
> lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
> / configInfo.sensorInfo.pixelRate;
>
> @@ -70,9 +99,15 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> return 0;
> }
>
> -void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,
> +/**
> + * \brief Estimate the mean value of the top 2% of the histogram
> + * \param[in] stats The statistics computed by the ImgU
> + * \param[in] grid The grid used to store the statistics in the IPU3
> + */
> +void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
> const ipu3_uapi_grid_config &grid)
> {
> + /* Initialise the histogram array */
> uint32_t hist[knumHistogramBins] = { 0 };
>
> for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
> @@ -87,6 +122,11 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,
> if (cell->sat_ratio == 0) {
> uint8_t gr = cell->Gr_avg;
> uint8_t gb = cell->Gb_avg;
> + /*
> + * Store the average green value to estimate the
> + * brightness. Even the over exposed pixels are
> + * taken into account.
> + */
> hist[(gr + gb) / 2]++;
> }
> }
> @@ -96,6 +136,9 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,
> iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> }
>
> +/**
> + * \brief Apply a filter on the exposure value to limit the speed of changes
> + */
> void Agc::filterExposure()
> {
> double speed = 0.2;
> @@ -106,7 +149,7 @@ void Agc::filterExposure()
> /*
> * If we are close to the desired result, go faster to avoid making
> * multiple micro-adjustments.
> - * \ todo: Make this customisable?
> + * \todo: Make this customisable?
> */
> if (filteredExposure_ < 1.2 * currentExposure_ &&
> filteredExposure_ > 0.8 * currentExposure_)
> @@ -119,7 +162,12 @@ void Agc::filterExposure()
> LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << filteredExposure_;
> }
>
> -void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
> +/**
> + * \brief Estimate the new exposure and gain values
> + * \param[inout] exposure The exposure value reference as a number of lines
> + * \param[inout] gain The gain reference to be updated
> + */
> +void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
> {
> /* Algorithm initialization should wait for first valid frames */
> /* \todo - have a number of frames given by DelayedControls ?
> @@ -136,19 +184,26 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
> return;
> }
>
> + /* Estimate the gain needed to have the proportion wanted */
> double evGain = kEvGainTarget * knumHistogramBins / iqMean_;
>
> /* extracted from Rpi::Agc::computeTargetExposure */
> + /* Calculate the shutter time in seconds */
> utils::Duration currentShutter = exposure * lineDuration_;
> LOG(IPU3Agc, Debug) << "Actual total exposure " << currentShutter * analogueGain
> << " Shutter speed " << currentShutter
> << " Gain " << analogueGain
> << " Needed ev gain " << evGain;
>
> + /*
> + * Estimate the current exposure value for the scene as the latest
> + * exposure value applied multiplied by the new estimated gain
> + */
Does this estimate what the sensor is currently configured to expose?
Or does it 'calculate' what is desired for the next control to set?
Estimate here implies that the algorithm is guessing what the sensor
might be configured as (which I would hope we can 'know' or determine?)
Ok, the brief of this function clears up my question, as it says
'Estimate the new'.
Is 'estimate' intentional? I would interpret that we would 'calculate'
the new values. Or in fact, given the function name, we might be
'computing' the new vaules...
> 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_;
> currentExposure_ = std::min(currentExposure_, maxTotalExposure);
> LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
> @@ -157,6 +212,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
> /* \todo: estimate if we need to desaturate */
> filterExposure();
>
> + /* Divide the exposure value as new exposure and gain values */
> utils::Duration exposureValue = filteredExposure_;
> utils::Duration shutterTime = minShutterSpeed;
>
> @@ -186,12 +242,21 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
> prevExposureValue_ = shutterTime * analogueGain;
> }
>
> +/**
> + * \brief Process IPU3 statistics, and run AGC operations
> + * \param[in] context The shared IPA context
> + * \param[in] stats The IPU3 statistics and ISP results
> + *
> + * Identify the current image brightness, and use that to estimate the optimal
> + * new exposure and gain for the scene.
Estimate or calculate? Estimate might be correct (here and above) if we
don't know in a single iteration what values to choose, as I suspect
it's somewhat of an adaptive algorithm that takes a couple of iterations
to get to a desired goal.
If estimate is correct - then I think that covers my main concerns, or
with an update to 'calculate' if that's more correct:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> + */
> void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> {
> + /* Get the latest exposure and gain applied */
> uint32_t &exposure = context.frameContext.agc.exposure;
> double &analogueGain = context.frameContext.agc.gain;
> - processBrightness(stats, context.configuration.grid.bdsGrid);
> - lockExposureGain(exposure, analogueGain);
> + measureBrightness(stats, context.configuration.grid.bdsGrid);
> + computeExposure(exposure, analogueGain);
> frameCount_++;
> }
>
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 1840205b..69e0b831 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -2,7 +2,7 @@
> /*
> * Copyright (C) 2021, Ideas On Board
> *
> - * agc.h - IPU3 AGC/AEC control algorithm
> + * agc.h - IPU3 AGC/AEC mean-based control algorithm
> */
> #ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
> #define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
> @@ -31,10 +31,10 @@ public:
> void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
>
> private:
> - void processBrightness(const ipu3_uapi_stats_3a *stats,
> + void measureBrightness(const ipu3_uapi_stats_3a *stats,
> const ipu3_uapi_grid_config &grid);
> void filterExposure();
> - void lockExposureGain(uint32_t &exposure, double &gain);
> + void computeExposure(uint32_t &exposure, double &gain);
>
> uint64_t frameCount_;
> uint64_t lastFrame_;
> --
> 2.32.0
>
More information about the libcamera-devel
mailing list