[libcamera-devel] [PATCH v3 07/19] ipa: ipu3: agc: Document AGC mean-based algorithm
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Oct 26 11:11:18 CEST 2021
Quoting Jean-Michel Hautbois (2021-10-26 07:23:45)
>
>
> On 25/10/2021 23:26, Kieran Bingham wrote:
> > 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?
>
> It can be tuned later if we want to have low and high bounds. As we work
> with cumulative histograms, 0.5 is... half of the counted pixels.
>
> Quoting "Raspberry Pi Camera Algorithm and Tuning Guide"
> (https://datasheets.raspberrypi.com/camera/raspberry-pi-camera-guide.pdf):
>
> "I(0.98, 1) = 0.5, lower bound. Here we are saying that the top 2% of
> the histogram must lie at or above 0.5 in the pixel range (metering all
> happens before gamma, so this is a moderately bright value). Or in
> short, we are requiring “some of the pixels to be reasonably bright”.
> This is a good strategy for snowy or beach scenes, also for images of
> documents. It actually raises the exposure of the whole scene; without
> it, the entire image would look dull grey which is undesirable in such
> circumstances. On the other hand when, as is usually the case, there is
> an abundance of bright and dark pixels, this constraint has no effect,
> so it is often reasonable to apply this constraint all the time."
That sounds like important information to convey along with this
variable, but should probably be sumarised a bit.
I don't mind if this is handled later though on top of this series as
it's already quite large, and nearing integration.
Could you make a note to revisit this later please?
> >> 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...
>
> It might be calculate or compute then ;-).
>
> >
> >
> >> 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.
>
> In this case, it is more an estimation, I think, as it will apply new
> values, which will take some frames to have an effect, and will
> re-estimate the exposure/gain...
>
> When we will have the IPAFrameContext implemented with a queue and the
> "real" exposure and gain applied on the frame we are looking at, then it
> will be more a calculation, I guess :-).
Ok - I think it sounds like we're shooting ourselves in the foot by not
keeping a queue of FrameContexts that we've sent out. That should not be
difficult to implement, so lets tackle that after integrating this
series.
> > 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