[libcamera-devel,v4,10/11] ipa: rkisp1: Introduce AGC

14758 diff mbox series

Now that we have IPAContext and Algorithm, we can implement a simple AGC
based on the IPU3 one. It is very similar, except that there is no
histogram used for an inter quantile mean. The RkISP1 is returning a 5x5
array (for V10) of luminance means. Estimating the relative luminance is
thus a simple mean of all the blocks already calculated by the ISP.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>

---
v4:
	- use #pragma once
	- Return filtered value from the function
	- Store line duration in IPASessionConfiguration
	- Use the hw revision to configure the number of AE cells
---
 src/ipa/rkisp1/algorithms/agc.cpp     | 279 ++++++++++++++++++++++++++
 src/ipa/rkisp1/algorithms/agc.h       |  45 +++++
 src/ipa/rkisp1/algorithms/meson.build |   1 +
 src/ipa/rkisp1/ipa_context.cpp        |  45 +++++
 src/ipa/rkisp1/ipa_context.h          |  19 ++
 src/ipa/rkisp1/rkisp1.cpp             |  73 ++++---
 6 files changed, 424 insertions(+), 38 deletions(-)
 create mode 100644 src/ipa/rkisp1/algorithms/agc.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/agc.h

Quoting Jean-Michel Hautbois (2021-11-24 13:40:18)
> Now that we have IPAContext and Algorithm, we can implement a simple AGC
> based on the IPU3 one. It is very similar, except that there is no
> histogram used for an inter quantile mean. The RkISP1 is returning a 5x5
> array (for V10) of luminance means. Estimating the relative luminance is
> thus a simple mean of all the blocks already calculated by the ISP.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> 
> ---
> v4:
>         - use #pragma once
>         - Return filtered value from the function
>         - Store line duration in IPASessionConfiguration
>         - Use the hw revision to configure the number of AE cells
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp     | 279 ++++++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/agc.h       |  45 +++++
>  src/ipa/rkisp1/algorithms/meson.build |   1 +
>  src/ipa/rkisp1/ipa_context.cpp        |  45 +++++
>  src/ipa/rkisp1/ipa_context.h          |  19 ++
>  src/ipa/rkisp1/rkisp1.cpp             |  73 ++++---
>  6 files changed, 424 insertions(+), 38 deletions(-)
>  create mode 100644 src/ipa/rkisp1/algorithms/agc.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/agc.h
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> new file mode 100644
> index 00000000..84ff8c7a
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -0,0 +1,279 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * agc.cpp - AGC/AEC mean-based control algorithm
> + */
> +
> +#include "agc.h"
> +
> +#include <algorithm>
> +#include <chrono>
> +#include <cmath>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/ipa/core_ipa_interface.h>
> +
> +/**
> + * \file agc.h
> + */
> +
> +namespace libcamera {
> +
> +using namespace std::literals::chrono_literals;
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class Agc
> + * \brief A mean-based auto-exposure algorithm
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Agc)
> +
> +/* Limits for analogue gain values */
> +static constexpr double kMinAnalogueGain = 1.0;
> +static constexpr double kMaxAnalogueGain = 8.0;
> +
> +/* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
> +static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> +
> +/* Number of frames to wait before calculating stats on minimum exposure */
> +static constexpr uint32_t kNumStartupFrames = 10;
> +
> +/*
> + * Relative luminance target.
> + *
> + * It's a number that's chosen so that, when the camera points at a grey
> + * target, the resulting image brightness is considered right.
> + */
> +static constexpr double kRelativeLuminanceTarget = 0.4;
> +
> +Agc::Agc()
> +       : frameCount_(0), currentExposure_(0s)
> +{
> +}
> +
> +/**
> + * \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,
> +                  [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> +{
> +       /* Configure the default exposure and gain. */
> +       context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
> +       context.frameContext.agc.exposure = 10ms / context.configuration.agc.lineDuration;
> +
> +       /*
> +        * According to the RkISP1 documentation:
> +        * - versions <= V11 have RKISP1_CIF_ISP_AE_MEAN_MAX_V10 entries,
> +        * - versions >= V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V12 entries.
> +        */
> +       if (context.configuration.hw.revision < RKISP1_V12)
> +               numCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;
> +       else
> +               numCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;
> +
> +       return 0;
> +}
> +
> +/**
> + * \brief Apply a filter on the exposure value to limit the speed of changes
> + */
> +utils::Duration Agc::filterExposure()
> +{
> +       utils::Duration filteredExposure = currentExposure_;
> +       double speed = 0.2;
> +
> +       /* Adapt instantly if we are in startup phase */
> +       if (frameCount_ < kNumStartupFrames)
> +               speed = 1.0;
> +
> +       if (filteredExposure == 0s) {
> +               filteredExposure = currentExposure_;

Is this right?

I read this as:

{
	utils::Duration filteredExposure = currentExposure_;
	if (filteredExposure == 0s) {
		filteredExposure = currentExposure_; // already set.
	}
	return filteredExposure; // Return 0s.
}


> +       } else {
> +               /*
> +                * If we are close to the desired result, go faster to avoid making
> +                * multiple micro-adjustments.
> +                * \todo Make this customisable?
> +                */
> +               if (filteredExposure < 1.2 * currentExposure_ &&
> +                   filteredExposure > 0.8 * currentExposure_)
> +                       speed = sqrt(speed);

And this now reads as:
	fE = cE;

	if (fE < 1.2 * cE) && (fe > 0.8 * cE)	// Always true.
		speed = sqrt(speed);	

> +
> +               filteredExposure = speed * currentExposure_ +
> +                                  filteredExposure * (1.0 - speed);
> +       }
> +
> +       LOG(RkISP1Agc, Debug) << "After filtering, total_exposure " << filteredExposure;
> +
> +       return filteredExposure;
> +}
> +
> +/**
> + * \brief Estimate the new exposure and gain values
> + * \param[inout] frameContext The shared IPA frame Context
> + * \param[in] yGain The gain calculated on the current brightness level
> + */
> +void Agc::computeExposure(IPAContext &context, double yGain)
> +{
> +       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;
> +
> +       utils::Duration minShutterSpeed = configuration.agc.minShutterSpeed;
> +       utils::Duration maxShutterSpeed = std::min(configuration.agc.maxShutterSpeed,
> +                                                  kMaxShutterSpeed);
> +
> +       double minAnalogueGain = std::max(configuration.agc.minAnalogueGain,
> +                                         kMinAnalogueGain);
> +       double maxAnalogueGain = std::min(configuration.agc.maxAnalogueGain,
> +                                         kMaxAnalogueGain);
> +
> +       /* Consider within 1% of the target as correctly exposed */
> +       if (std::abs(yGain - 1.0) < 0.01)
> +               LOG(RkISP1Agc, Debug) << "We are well exposed (iqMean = "
> +                                     << yGain << ")";
> +
> +       /* extracted from Rpi::Agc::computeTargetExposure */
> +
> +       /* Calculate the shutter time in seconds */
> +       utils::Duration currentShutter = exposure * configuration.agc.lineDuration;
> +
> +       /*
> +        * Update the exposure value for the next computation using the values
> +        * of exposure and gain really used by the sensor.
> +        */
> +       utils::Duration effectiveExposureValue = currentShutter * analogueGain;
> +
> +       LOG(RkISP1Agc, Debug) << "Actual total exposure " << currentShutter * analogueGain
> +                             << " Shutter speed " << currentShutter
> +                             << " Gain " << analogueGain
> +                             << " Needed ev gain " << yGain;
> +
> +       /*
> +        * Calculate the current exposure value for the scene as the latest
> +        * exposure value applied multiplied by the new estimated gain.
> +        */
> +       currentExposure_ = effectiveExposureValue * yGain;
> +
> +       /* Clamp the exposure value to the min and max authorized */
> +       utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain;
> +       currentExposure_ = std::min(currentExposure_, maxTotalExposure);
> +       LOG(RkISP1Agc, Debug) << "Target total exposure " << currentExposure_
> +                             << ", maximum is " << maxTotalExposure;
> +
> +       /*
> +        * Divide the exposure value as new exposure and gain values
> +        * \todo: estimate if we need to desaturate
> +        */
> +       utils::Duration exposureValue = filterExposure();
> +       utils::Duration shutterTime;
> +
> +       /*
> +       * Push the shutter time up to the maximum first, and only then
> +       * increase the gain.
> +       */
> +       shutterTime = std::clamp<utils::Duration>(exposureValue / minAnalogueGain,
> +                                                 minShutterSpeed, maxShutterSpeed);
> +       double stepGain = std::clamp(exposureValue / shutterTime,
> +                                    minAnalogueGain, maxAnalogueGain);
> +       LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are "
> +                             << shutterTime << " and "
> +                             << stepGain;
> +
> +       /* Update the estimated exposure and gain. */
> +       frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration;
> +       frameContext.agc.gain = stepGain;
> +}
> +
> +/**
> + * \brief Estimate the relative luminance of the frame with a given gain
> + * \param[in] ae The RkISP1 statistics and ISP results
> + * \param[in] gain The gain calculated on the current brightness level
> + * \return The relative luminance
> + *
> + * This function estimates the average relative luminance of the frame that
> + * would be output by the sensor if an additional \a gain was applied.
> + *
> + * The estimation is based on the AE statistics for the current frame. Y
> + * averages for all cells are first multiplied by the gain, and then saturated
> + * to approximate the sensor behaviour at high brightness values. The
> + * approximation is quite rough, as it doesn't take into account non-linearities
> + * when approaching saturation.
> + *
> + * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a
> + * theoretical perfect reflector of 100% reference white.
> + *
> + * More detailed information can be found in:
> + * https://en.wikipedia.org/wiki/Relative_luminance
> + */
> +double Agc::estimateLuminance(const rkisp1_cif_isp_ae_stat *ae,
> +                             double gain)
> +{
> +       double ySum = 0.0;
> +       unsigned int num = 0;
> +
> +       /* Sum the averages, saturated to 255. */
> +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {
> +               ySum += std::min(ae->exp_mean[aeCell] * gain, 255.0);
> +               num++;
> +       }
> +
> +       /* \todo Weight with the AWB gains */
> +
> +       return ySum / num / 255;
> +}
> +
> +/**
> + * \brief Process RkISP1 statistics, and run AGC operations
> + * \param[in] context The shared IPA context
> + * \param[in] stats The RKIsp1 statistics and ISP results
> + *
> + * Identify the current image brightness, and use that to estimate the optimal
> + * new exposure and gain for the scene.
> + */
> +void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)
> +{
> +       const rkisp1_cif_isp_stat *params = &stats->params;
> +       ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);
> +
> +       const rkisp1_cif_isp_ae_stat *ae = &params->ae;
> +
> +       /*
> +        * Estimate the gain needed to achieve a relative luminance target. To
> +        * account for non-linearity caused by saturation, the value needs to be
> +        * estimated in an iterative process, as multiplying by a gain will not
> +        * increase the relative luminance by the same factor if some image
> +        * regions are saturated.
> +        */
> +       double yGain = 1.0;
> +       double yTarget = kRelativeLuminanceTarget;
> +
> +       for (unsigned int i = 0; i < 8; i++) {
> +               double yValue = estimateLuminance(ae, yGain);
> +               double extra_gain = std::min(10.0, yTarget / (yValue + .001));
> +
> +               yGain *= extra_gain;
> +               LOG(RkISP1Agc, Debug) << "Y value: " << yValue
> +                                     << ", Y target: " << yTarget
> +                                     << ", gives gain " << yGain;
> +               if (extra_gain < 1.01)
> +                       break;
> +       }
> +
> +       computeExposure(context, yGain);
> +       frameCount_++;
> +}
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> new file mode 100644
> index 00000000..a59ae9c8
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * agc.h - RkISP1 AGC/AEC mean-based control algorithm
> + */
> +#pragma once
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include <libcamera/base/utils.h>
> +
> +#include <libcamera/geometry.h>
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +struct IPACameraSensorInfo;
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class Agc : public Algorithm
> +{
> +public:
> +       Agc();
> +       ~Agc() = default;
> +
> +       int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> +       void process(IPAContext &context, const rkisp1_stat_buffer *stats) override;
> +
> +private:
> +       void computeExposure(IPAContext &Context, double yGain);
> +       utils::Duration filterExposure();
> +       double estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);
> +
> +       uint64_t frameCount_;
> +
> +       uint32_t numCells_;
> +
> +       utils::Duration currentExposure_;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index 1c6c59cf..a19c1a4f 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  rkisp1_ipa_algorithms = files([
> +    'agc.cpp',
>  ])
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 6b53dfdf..dff00362 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -56,6 +56,24 @@ namespace libcamera::ipa::rkisp1 {
>   */
>  
>  /**
> + * \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.maxShutterSpeed
> + * \brief Maximum shutter speed supported with the configured sensor
> + *
> + * \var IPASessionConfiguration::agc.minAnalogueGain
> + * \brief Minimum analogue gain supported with the configured sensor
> + *
> + * \var IPASessionConfiguration::agc.maxAnalogueGain
> + * \brief Maximum analogue gain supported with the configured sensor
> + *
> + * \var IPASessionConfiguration::agc.lineDuration
> + * \brief Line duration in microseconds
> + *
>   * \var IPASessionConfiguration::hw
>   * \brief RkISP1-specific hardware information
>   *
> @@ -63,4 +81,31 @@ namespace libcamera::ipa::rkisp1 {
>   * \brief Hardware revision of the ISP
>   */
>  
> +/**
> + * \var IPAFrameContext::agc
> + * \brief Context for the Automatic Gain Control algorithm
> + *
> + * The exposure and gain determined are expected to be applied to the sensor
> + * at the earliest opportunity.
> + *
> + * \var IPAFrameContext::agc.exposure
> + * \brief Exposure time expressed as a number of lines
> + *
> + * \var IPAFrameContext::agc.gain
> + * \brief Analogue gain multiplier
> + *
> + * The gain should be adapted to the sensor specific gain code before applying.
> + */
> +
> +/**
> + * \var IPAFrameContext::sensor
> + * \brief Effective sensor values
> + *
> + * \var IPAFrameContext::sensor.exposure
> + * \brief Exposure time expressed as a number of lines
> + *
> + * \var IPAFrameContext::sensor.gain
> + * \brief Analogue gain multiplier
> + */
> +
>  } /* namespace libcamera::ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index f9ac6833..653d7e65 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -9,17 +9,36 @@
>  
>  #include <linux/rkisp1-config.h>
>  
> +#include <libcamera/base/utils.h>
> +
>  namespace libcamera {
>  
>  namespace ipa::rkisp1 {
>  
>  struct IPASessionConfiguration {
> +       struct {
> +               utils::Duration minShutterSpeed;
> +               utils::Duration maxShutterSpeed;
> +               double minAnalogueGain;
> +               double maxAnalogueGain;
> +               utils::Duration lineDuration;
> +       } agc;
> +
>         struct {
>                 rkisp1_cif_isp_version revision;
>         } hw;
>  };
>  
>  struct IPAFrameContext {
> +       struct {
> +               uint32_t exposure;
> +               double gain;
> +       } agc;
> +
> +       struct {
> +               uint32_t exposure;
> +               double gain;
> +       } sensor;
>  };
>  
>  struct IPAContext {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 59676a70..5c0a0330 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -25,6 +25,7 @@
>  
>  #include <libcamera/internal/mapped_framebuffer.h>
>  
> +#include "algorithms/agc.h"
>  #include "algorithms/algorithm.h"
>  #include "libipa/camera_sensor_helper.h"
>  
> @@ -34,6 +35,8 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPARkISP1)
>  
> +using namespace std::literals::chrono_literals;
> +
>  namespace ipa::rkisp1 {
>  
>  class IPARkISP1 : public IPARkISP1Interface
> @@ -75,7 +78,6 @@ private:
>  
>         /* revision-specific data */
>         rkisp1_cif_isp_version hwRevision_;
> -       unsigned int hwAeMeanMax_;
>         unsigned int hwHistBinNMax_;
>         unsigned int hwGammaOutMaxSamples_;
>         unsigned int hwHistogramWeightGridsSize_;
> @@ -95,13 +97,11 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
>         /* \todo Add support for other revisions */
>         switch (hwRevision) {
>         case RKISP1_V10:
> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;
>                 hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;
>                 hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;
>                 hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;
>                 break;
>         case RKISP1_V12:
> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;
>                 hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;
>                 hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;
>                 hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;
> @@ -126,6 +126,9 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
>                 return -ENODEV;
>         }
>  
> +       /* Construct our Algorithms */
> +       algorithms_.push_back(std::make_unique<algorithms::Agc>());
> +
>         return 0;
>  }
>  
> @@ -183,6 +186,26 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>         /* Set the hardware revision for the algorithms. */
>         context_.configuration.hw.revision = hwRevision_;
>  
> +       context_.configuration.agc.lineDuration = info.lineLength * 1.0s / info.pixelRate;
> +
> +       /*
> +        * When the AGC computes the new exposure values for a frame, it needs
> +        * to know the limits for shutter speed and analogue gain.
> +        * As it depends on the sensor, update it with the controls.
> +        *
> +        * \todo take VBLANK into account for maximum shutter speed
> +        */
> +       context_.configuration.agc.minShutterSpeed = minExposure_ * context_.configuration.agc.lineDuration;
> +       context_.configuration.agc.maxShutterSpeed = maxExposure_ * context_.configuration.agc.lineDuration;
> +       context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);
> +       context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);
> +
> +       for (auto const &algo : algorithms_) {
> +               int ret = algo->configure(context_, info);
> +               if (ret)
> +                       return ret;
> +       }
> +
>         return 0;
>  }
>  
> @@ -227,6 +250,9 @@ void IPARkISP1::processEvent(const RkISP1Event &event)
>                         reinterpret_cast<rkisp1_stat_buffer *>(
>                                 mappedBuffers_.at(bufferId).planes()[0].data());
>  
> +               context_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> +               context_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +
>                 updateStatistics(frame, stats);
>                 break;
>         }
> @@ -271,44 +297,12 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
>  void IPARkISP1::updateStatistics(unsigned int frame,
>                                  const rkisp1_stat_buffer *stats)
>  {
> -       const rkisp1_cif_isp_stat *params = &stats->params;
>         unsigned int aeState = 0;
>  
> -       if (stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP) {
> -               const rkisp1_cif_isp_ae_stat *ae = &params->ae;
> -
> -               const unsigned int target = 60;
> -
> -               unsigned int value = 0;
> -               unsigned int num = 0;
> -               for (unsigned int i = 0; i < hwAeMeanMax_; i++) {
> -                       if (ae->exp_mean[i] <= 15)
> -                               continue;
> -
> -                       value += ae->exp_mean[i];
> -                       num++;
> -               }
> -               value /= num;
> +       for (auto const &algo : algorithms_)
> +               algo->process(context_, stats);
>  
> -               double factor = (double)target / value;
> -
> -               if (frame % 3 == 0) {
> -                       double exposure;
> -
> -                       exposure = factor * exposure_ * gain_ / minGain_;
> -                       exposure_ = std::clamp<uint64_t>((uint64_t)exposure,
> -                                                        minExposure_,
> -                                                        maxExposure_);
> -
> -                       exposure = exposure / exposure_ * minGain_;
> -                       gain_ = std::clamp<uint64_t>((uint64_t)exposure,
> -                                                    minGain_, maxGain_);
> -
> -                       setControls(frame + 1);
> -               }
> -
> -               aeState = fabs(factor - 1.0f) < 0.05f ? 2 : 1;
> -       }
> +       setControls(frame);
>  
>         metadataReady(frame, aeState);
>  }
> @@ -318,6 +312,9 @@ void IPARkISP1::setControls(unsigned int frame)
>         RkISP1Action op;
>         op.op = ActionV4L2Set;
>  
> +       exposure_ = context_.frameContext.agc.exposure;
> +       gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
> +
>         ControlList ctrls(ctrls_);
>         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
>         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> -- 
> 2.32.0
>

On Wed, Nov 24, 2021 at 02:30:23PM +0000, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-11-24 13:40:18)
> > Now that we have IPAContext and Algorithm, we can implement a simple AGC
> > based on the IPU3 one. It is very similar, except that there is no
> > histogram used for an inter quantile mean. The RkISP1 is returning a 5x5
> > array (for V10) of luminance means. Estimating the relative luminance is
> > thus a simple mean of all the blocks already calculated by the ISP.
> > 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > 
> > ---
> > v4:
> >         - use #pragma once
> >         - Return filtered value from the function
> >         - Store line duration in IPASessionConfiguration
> >         - Use the hw revision to configure the number of AE cells
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp     | 279 ++++++++++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/agc.h       |  45 +++++
> >  src/ipa/rkisp1/algorithms/meson.build |   1 +
> >  src/ipa/rkisp1/ipa_context.cpp        |  45 +++++
> >  src/ipa/rkisp1/ipa_context.h          |  19 ++
> >  src/ipa/rkisp1/rkisp1.cpp             |  73 ++++---
> >  6 files changed, 424 insertions(+), 38 deletions(-)
> >  create mode 100644 src/ipa/rkisp1/algorithms/agc.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/agc.h
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > new file mode 100644
> > index 00000000..84ff8c7a
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -0,0 +1,279 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Ideas On Board
> > + *
> > + * agc.cpp - AGC/AEC mean-based control algorithm
> > + */
> > +
> > +#include "agc.h"
> > +
> > +#include <algorithm>
> > +#include <chrono>
> > +#include <cmath>
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/ipa/core_ipa_interface.h>
> > +
> > +/**
> > + * \file agc.h
> > + */
> > +
> > +namespace libcamera {
> > +
> > +using namespace std::literals::chrono_literals;
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +/**
> > + * \class Agc
> > + * \brief A mean-based auto-exposure algorithm
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(RkISP1Agc)
> > +
> > +/* Limits for analogue gain values */
> > +static constexpr double kMinAnalogueGain = 1.0;
> > +static constexpr double kMaxAnalogueGain = 8.0;
> > +
> > +/* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
> > +static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> > +
> > +/* Number of frames to wait before calculating stats on minimum exposure */
> > +static constexpr uint32_t kNumStartupFrames = 10;
> > +
> > +/*
> > + * Relative luminance target.
> > + *
> > + * It's a number that's chosen so that, when the camera points at a grey
> > + * target, the resulting image brightness is considered right.
> > + */
> > +static constexpr double kRelativeLuminanceTarget = 0.4;
> > +
> > +Agc::Agc()
> > +       : frameCount_(0), currentExposure_(0s)
> > +{
> > +}
> > +
> > +/**
> > + * \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,
> > +                  [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> > +{
> > +       /* Configure the default exposure and gain. */
> > +       context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
> > +       context.frameContext.agc.exposure = 10ms / context.configuration.agc.lineDuration;
> > +
> > +       /*
> > +        * According to the RkISP1 documentation:
> > +        * - versions <= V11 have RKISP1_CIF_ISP_AE_MEAN_MAX_V10 entries,
> > +        * - versions >= V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V12 entries.
> > +        */
> > +       if (context.configuration.hw.revision < RKISP1_V12)
> > +               numCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;
> > +       else
> > +               numCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> > + * \brief Apply a filter on the exposure value to limit the speed of changes
> > + */
> > +utils::Duration Agc::filterExposure()
> > +{
> > +       utils::Duration filteredExposure = currentExposure_;
> > +       double speed = 0.2;
> > +
> > +       /* Adapt instantly if we are in startup phase */
> > +       if (frameCount_ < kNumStartupFrames)
> > +               speed = 1.0;
> > +
> > +       if (filteredExposure == 0s) {
> > +               filteredExposure = currentExposure_;
> 
> Is this right?
> 
> I read this as:
> 
> {
> 	utils::Duration filteredExposure = currentExposure_;
> 	if (filteredExposure == 0s) {
> 		filteredExposure = currentExposure_; // already set.
> 	}
> 	return filteredExposure; // Return 0s.
> }
> 
> 
> > +       } else {
> > +               /*
> > +                * If we are close to the desired result, go faster to avoid making
> > +                * multiple micro-adjustments.
> > +                * \todo Make this customisable?
> > +                */
> > +               if (filteredExposure < 1.2 * currentExposure_ &&
> > +                   filteredExposure > 0.8 * currentExposure_)
> > +                       speed = sqrt(speed);
> 
> And this now reads as:
> 	fE = cE;
> 
> 	if (fE < 1.2 * cE) && (fe > 0.8 * cE)	// Always true.
> 		speed = sqrt(speed);	

Doesn't look right indeed. As a side note, I'd pass the currentExposure_
to the function instead of storing it in a member variable. It's the
filtered exposure you need to store to implement an IIR filter (I would
still return the filter output from this function, even if stored as a
member variable, so that filteredExposure_ would only be used internally
here).

A similar fix for the IPU3 IPA would be nice.

> > +
> > +               filteredExposure = speed * currentExposure_ +
> > +                                  filteredExposure * (1.0 - speed);
> > +       }
> > +
> > +       LOG(RkISP1Agc, Debug) << "After filtering, total_exposure " << filteredExposure;
> > +
> > +       return filteredExposure;
> > +}
> > +
> > +/**
> > + * \brief Estimate the new exposure and gain values
> > + * \param[inout] frameContext The shared IPA frame Context
> > + * \param[in] yGain The gain calculated on the current brightness level
> > + */
> > +void Agc::computeExposure(IPAContext &context, double yGain)
> > +{
> > +       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;
> > +
> > +       utils::Duration minShutterSpeed = configuration.agc.minShutterSpeed;
> > +       utils::Duration maxShutterSpeed = std::min(configuration.agc.maxShutterSpeed,
> > +                                                  kMaxShutterSpeed);
> > +
> > +       double minAnalogueGain = std::max(configuration.agc.minAnalogueGain,
> > +                                         kMinAnalogueGain);
> > +       double maxAnalogueGain = std::min(configuration.agc.maxAnalogueGain,
> > +                                         kMaxAnalogueGain);
> > +
> > +       /* Consider within 1% of the target as correctly exposed */
> > +       if (std::abs(yGain - 1.0) < 0.01)
> > +               LOG(RkISP1Agc, Debug) << "We are well exposed (iqMean = "
> > +                                     << yGain << ")";
> > +
> > +       /* extracted from Rpi::Agc::computeTargetExposure */
> > +
> > +       /* Calculate the shutter time in seconds */
> > +       utils::Duration currentShutter = exposure * configuration.agc.lineDuration;
> > +
> > +       /*
> > +        * Update the exposure value for the next computation using the values
> > +        * of exposure and gain really used by the sensor.
> > +        */
> > +       utils::Duration effectiveExposureValue = currentShutter * analogueGain;
> > +
> > +       LOG(RkISP1Agc, Debug) << "Actual total exposure " << currentShutter * analogueGain
> > +                             << " Shutter speed " << currentShutter
> > +                             << " Gain " << analogueGain
> > +                             << " Needed ev gain " << yGain;
> > +
> > +       /*
> > +        * Calculate the current exposure value for the scene as the latest
> > +        * exposure value applied multiplied by the new estimated gain.
> > +        */
> > +       currentExposure_ = effectiveExposureValue * yGain;
> > +
> > +       /* Clamp the exposure value to the min and max authorized */
> > +       utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain;
> > +       currentExposure_ = std::min(currentExposure_, maxTotalExposure);
> > +       LOG(RkISP1Agc, Debug) << "Target total exposure " << currentExposure_
> > +                             << ", maximum is " << maxTotalExposure;
> > +
> > +       /*
> > +        * Divide the exposure value as new exposure and gain values
> > +        * \todo: estimate if we need to desaturate
> > +        */
> > +       utils::Duration exposureValue = filterExposure();
> > +       utils::Duration shutterTime;
> > +
> > +       /*
> > +       * Push the shutter time up to the maximum first, and only then
> > +       * increase the gain.
> > +       */
> > +       shutterTime = std::clamp<utils::Duration>(exposureValue / minAnalogueGain,
> > +                                                 minShutterSpeed, maxShutterSpeed);
> > +       double stepGain = std::clamp(exposureValue / shutterTime,
> > +                                    minAnalogueGain, maxAnalogueGain);
> > +       LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are "
> > +                             << shutterTime << " and "
> > +                             << stepGain;
> > +
> > +       /* Update the estimated exposure and gain. */
> > +       frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration;
> > +       frameContext.agc.gain = stepGain;
> > +}
> > +
> > +/**
> > + * \brief Estimate the relative luminance of the frame with a given gain
> > + * \param[in] ae The RkISP1 statistics and ISP results
> > + * \param[in] gain The gain calculated on the current brightness level
> > + * \return The relative luminance
> > + *
> > + * This function estimates the average relative luminance of the frame that
> > + * would be output by the sensor if an additional \a gain was applied.
> > + *
> > + * The estimation is based on the AE statistics for the current frame. Y
> > + * averages for all cells are first multiplied by the gain, and then saturated
> > + * to approximate the sensor behaviour at high brightness values. The
> > + * approximation is quite rough, as it doesn't take into account non-linearities
> > + * when approaching saturation.
> > + *
> > + * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a
> > + * theoretical perfect reflector of 100% reference white.
> > + *
> > + * More detailed information can be found in:
> > + * https://en.wikipedia.org/wiki/Relative_luminance
> > + */
> > +double Agc::estimateLuminance(const rkisp1_cif_isp_ae_stat *ae,
> > +                             double gain)
> > +{
> > +       double ySum = 0.0;
> > +       unsigned int num = 0;
> > +
> > +       /* Sum the averages, saturated to 255. */
> > +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {
> > +               ySum += std::min(ae->exp_mean[aeCell] * gain, 255.0);
> > +               num++;
> > +       }
> > +
> > +       /* \todo Weight with the AWB gains */
> > +
> > +       return ySum / num / 255;

Drop num and replace it with numCells_.

> > +}
> > +
> > +/**
> > + * \brief Process RkISP1 statistics, and run AGC operations
> > + * \param[in] context The shared IPA context
> > + * \param[in] stats The RKIsp1 statistics and ISP results
> > + *
> > + * Identify the current image brightness, and use that to estimate the optimal
> > + * new exposure and gain for the scene.
> > + */
> > +void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)
> > +{
> > +       const rkisp1_cif_isp_stat *params = &stats->params;
> > +       ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);
> > +
> > +       const rkisp1_cif_isp_ae_stat *ae = &params->ae;
> > +
> > +       /*
> > +        * Estimate the gain needed to achieve a relative luminance target. To
> > +        * account for non-linearity caused by saturation, the value needs to be
> > +        * estimated in an iterative process, as multiplying by a gain will not
> > +        * increase the relative luminance by the same factor if some image
> > +        * regions are saturated.
> > +        */
> > +       double yGain = 1.0;
> > +       double yTarget = kRelativeLuminanceTarget;
> > +
> > +       for (unsigned int i = 0; i < 8; i++) {
> > +               double yValue = estimateLuminance(ae, yGain);
> > +               double extra_gain = std::min(10.0, yTarget / (yValue + .001));
> > +
> > +               yGain *= extra_gain;
> > +               LOG(RkISP1Agc, Debug) << "Y value: " << yValue
> > +                                     << ", Y target: " << yTarget
> > +                                     << ", gives gain " << yGain;
> > +               if (extra_gain < 1.01)
> > +                       break;
> > +       }
> > +
> > +       computeExposure(context, yGain);
> > +       frameCount_++;
> > +}
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> > new file mode 100644
> > index 00000000..a59ae9c8
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/agc.h
> > @@ -0,0 +1,45 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Ideas On Board
> > + *
> > + * agc.h - RkISP1 AGC/AEC mean-based control algorithm
> > + */
> > +#pragma once
> > +
> > +#include <linux/rkisp1-config.h>
> > +
> > +#include <libcamera/base/utils.h>
> > +
> > +#include <libcamera/geometry.h>
> > +
> > +#include "algorithm.h"
> > +
> > +namespace libcamera {
> > +
> > +struct IPACameraSensorInfo;
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +class Agc : public Algorithm
> > +{
> > +public:
> > +       Agc();
> > +       ~Agc() = default;
> > +
> > +       int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> > +       void process(IPAContext &context, const rkisp1_stat_buffer *stats) override;
> > +
> > +private:
> > +       void computeExposure(IPAContext &Context, double yGain);
> > +       utils::Duration filterExposure();
> > +       double estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);
> > +
> > +       uint64_t frameCount_;
> > +
> > +       uint32_t numCells_;
> > +
> > +       utils::Duration currentExposure_;
> > +};
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> > index 1c6c59cf..a19c1a4f 100644
> > --- a/src/ipa/rkisp1/algorithms/meson.build
> > +++ b/src/ipa/rkisp1/algorithms/meson.build
> > @@ -1,4 +1,5 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >  
> >  rkisp1_ipa_algorithms = files([
> > +    'agc.cpp',
> >  ])
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 6b53dfdf..dff00362 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -56,6 +56,24 @@ namespace libcamera::ipa::rkisp1 {
> >   */
> >  
> >  /**
> > + * \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.maxShutterSpeed
> > + * \brief Maximum shutter speed supported with the configured sensor
> > + *
> > + * \var IPASessionConfiguration::agc.minAnalogueGain
> > + * \brief Minimum analogue gain supported with the configured sensor
> > + *
> > + * \var IPASessionConfiguration::agc.maxAnalogueGain
> > + * \brief Maximum analogue gain supported with the configured sensor
> > + *
> > + * \var IPASessionConfiguration::agc.lineDuration
> > + * \brief Line duration in microseconds
> > + *
> >   * \var IPASessionConfiguration::hw
> >   * \brief RkISP1-specific hardware information
> >   *
> > @@ -63,4 +81,31 @@ namespace libcamera::ipa::rkisp1 {
> >   * \brief Hardware revision of the ISP
> >   */
> >  
> > +/**
> > + * \var IPAFrameContext::agc
> > + * \brief Context for the Automatic Gain Control algorithm
> > + *
> > + * The exposure and gain determined are expected to be applied to the sensor
> > + * at the earliest opportunity.
> > + *
> > + * \var IPAFrameContext::agc.exposure
> > + * \brief Exposure time expressed as a number of lines
> > + *
> > + * \var IPAFrameContext::agc.gain
> > + * \brief Analogue gain multiplier
> > + *
> > + * The gain should be adapted to the sensor specific gain code before applying.
> > + */
> > +
> > +/**
> > + * \var IPAFrameContext::sensor
> > + * \brief Effective sensor values
> > + *
> > + * \var IPAFrameContext::sensor.exposure
> > + * \brief Exposure time expressed as a number of lines
> > + *
> > + * \var IPAFrameContext::sensor.gain
> > + * \brief Analogue gain multiplier
> > + */
> > +
> >  } /* namespace libcamera::ipa::rkisp1 */
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index f9ac6833..653d7e65 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -9,17 +9,36 @@
> >  
> >  #include <linux/rkisp1-config.h>
> >  
> > +#include <libcamera/base/utils.h>
> > +
> >  namespace libcamera {
> >  
> >  namespace ipa::rkisp1 {
> >  
> >  struct IPASessionConfiguration {
> > +       struct {
> > +               utils::Duration minShutterSpeed;
> > +               utils::Duration maxShutterSpeed;
> > +               double minAnalogueGain;
> > +               double maxAnalogueGain;
> > +               utils::Duration lineDuration;
> > +       } agc;
> > +
> >         struct {
> >                 rkisp1_cif_isp_version revision;
> >         } hw;
> >  };
> >  
> >  struct IPAFrameContext {
> > +       struct {
> > +               uint32_t exposure;
> > +               double gain;
> > +       } agc;
> > +
> > +       struct {
> > +               uint32_t exposure;
> > +               double gain;
> > +       } sensor;
> >  };
> >  
> >  struct IPAContext {
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 59676a70..5c0a0330 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -25,6 +25,7 @@
> >  
> >  #include <libcamera/internal/mapped_framebuffer.h>
> >  
> > +#include "algorithms/agc.h"
> >  #include "algorithms/algorithm.h"
> >  #include "libipa/camera_sensor_helper.h"
> >  
> > @@ -34,6 +35,8 @@ namespace libcamera {
> >  
> >  LOG_DEFINE_CATEGORY(IPARkISP1)
> >  
> > +using namespace std::literals::chrono_literals;
> > +
> >  namespace ipa::rkisp1 {
> >  
> >  class IPARkISP1 : public IPARkISP1Interface
> > @@ -75,7 +78,6 @@ private:
> >  
> >         /* revision-specific data */
> >         rkisp1_cif_isp_version hwRevision_;
> > -       unsigned int hwAeMeanMax_;
> >         unsigned int hwHistBinNMax_;
> >         unsigned int hwGammaOutMaxSamples_;
> >         unsigned int hwHistogramWeightGridsSize_;
> > @@ -95,13 +97,11 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
> >         /* \todo Add support for other revisions */
> >         switch (hwRevision) {
> >         case RKISP1_V10:
> > -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;
> >                 hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;
> >                 hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;
> >                 hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;
> >                 break;
> >         case RKISP1_V12:
> > -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;
> >                 hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;
> >                 hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;
> >                 hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;
> > @@ -126,6 +126,9 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
> >                 return -ENODEV;
> >         }
> >  
> > +       /* Construct our Algorithms */
> > +       algorithms_.push_back(std::make_unique<algorithms::Agc>());
> > +
> >         return 0;
> >  }
> >  
> > @@ -183,6 +186,26 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
> >         /* Set the hardware revision for the algorithms. */
> >         context_.configuration.hw.revision = hwRevision_;
> >  
> > +       context_.configuration.agc.lineDuration = info.lineLength * 1.0s / info.pixelRate;
> > +
> > +       /*
> > +        * When the AGC computes the new exposure values for a frame, it needs
> > +        * to know the limits for shutter speed and analogue gain.
> > +        * As it depends on the sensor, update it with the controls.
> > +        *
> > +        * \todo take VBLANK into account for maximum shutter speed
> > +        */
> > +       context_.configuration.agc.minShutterSpeed = minExposure_ * context_.configuration.agc.lineDuration;
> > +       context_.configuration.agc.maxShutterSpeed = maxExposure_ * context_.configuration.agc.lineDuration;
> > +       context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);
> > +       context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);
> > +
> > +       for (auto const &algo : algorithms_) {
> > +               int ret = algo->configure(context_, info);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> >         return 0;
> >  }
> >  
> > @@ -227,6 +250,9 @@ void IPARkISP1::processEvent(const RkISP1Event &event)
> >                         reinterpret_cast<rkisp1_stat_buffer *>(
> >                                 mappedBuffers_.at(bufferId).planes()[0].data());
> >  
> > +               context_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> > +               context_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());

Shorter lines would be good.

> > +
> >                 updateStatistics(frame, stats);
> >                 break;
> >         }
> > @@ -271,44 +297,12 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
> >  void IPARkISP1::updateStatistics(unsigned int frame,
> >                                  const rkisp1_stat_buffer *stats)
> >  {
> > -       const rkisp1_cif_isp_stat *params = &stats->params;
> >         unsigned int aeState = 0;
> >  
> > -       if (stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP) {
> > -               const rkisp1_cif_isp_ae_stat *ae = &params->ae;
> > -
> > -               const unsigned int target = 60;
> > -
> > -               unsigned int value = 0;
> > -               unsigned int num = 0;
> > -               for (unsigned int i = 0; i < hwAeMeanMax_; i++) {
> > -                       if (ae->exp_mean[i] <= 15)
> > -                               continue;
> > -
> > -                       value += ae->exp_mean[i];
> > -                       num++;
> > -               }
> > -               value /= num;
> > +       for (auto const &algo : algorithms_)
> > +               algo->process(context_, stats);
> >  
> > -               double factor = (double)target / value;
> > -
> > -               if (frame % 3 == 0) {
> > -                       double exposure;
> > -
> > -                       exposure = factor * exposure_ * gain_ / minGain_;
> > -                       exposure_ = std::clamp<uint64_t>((uint64_t)exposure,
> > -                                                        minExposure_,
> > -                                                        maxExposure_);
> > -
> > -                       exposure = exposure / exposure_ * minGain_;
> > -                       gain_ = std::clamp<uint64_t>((uint64_t)exposure,
> > -                                                    minGain_, maxGain_);
> > -
> > -                       setControls(frame + 1);
> > -               }
> > -
> > -               aeState = fabs(factor - 1.0f) < 0.05f ? 2 : 1;
> > -       }
> > +       setControls(frame);
> >  
> >         metadataReady(frame, aeState);
> >  }
> > @@ -318,6 +312,9 @@ void IPARkISP1::setControls(unsigned int frame)
> >         RkISP1Action op;
> >         op.op = ActionV4L2Set;
> >  
> > +       exposure_ = context_.frameContext.agc.exposure;
> > +       gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);

I think these can become local variables.

> > +
> >         ControlList ctrls(ctrls_);
> >         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
> >         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));