[libcamera-devel] [PATCH v3 09/11] ipa: rkisp1: Introduce AGC

Jean-Michel Hautbois jeanmichel.hautbois at ideasonboard.com
Tue Nov 23 18:38:42 CET 2021


Hi Kieran,

On 23/11/2021 17:08, Kieran Bingham wrote:
> Hi JM,
> 
> Some question inline ...
> 
> Quoting Jean-Michel Hautbois (2021-11-23 15:04:21)
>> 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 at ideasonboard.com>
>> ---
>>   src/ipa/rkisp1/algorithms/agc.cpp     | 265 ++++++++++++++++++++++++++
>>   src/ipa/rkisp1/algorithms/agc.h       |  55 ++++++
>>   src/ipa/rkisp1/algorithms/meson.build |   1 +
>>   src/ipa/rkisp1/ipa_context.cpp        |  44 +++++
>>   src/ipa/rkisp1/ipa_context.h          |  17 ++
>>   src/ipa/rkisp1/rkisp1.cpp             |  72 +++----
>>   6 files changed, 419 insertions(+), 35 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..9c6d312e
>> --- /dev/null
>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
>> @@ -0,0 +1,265 @@
>> +/* 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), lineDuration_(0s), minShutterSpeed_(0s),
>> +         maxShutterSpeed_(0s), filteredExposure_(0s), 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, const IPACameraSensorInfo &configInfo)
>> +{
>> +       /* \todo use the IPAContext to provide the limits */
>> +       lineDuration_ = configInfo.lineLength * 1.0s / configInfo.pixelRate;
>> +
>> +       minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
>> +       maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
>> +                                   kMaxShutterSpeed);
>> +
>> +       minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
>> +       maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>> +
>> +       /* Configure the default exposure and gain. */
>> +       context.frameContext.agc.gain = minAnalogueGain_;
>> +       context.frameContext.agc.exposure = 10ms / lineDuration_;
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * \brief Apply a filter on the exposure value to limit the speed of changes
>> + */
>> +void Agc::filterExposure()
>> +{
>> +       double speed = 0.2;
>> +
>> +       /* Adapt instantly if we are in startup phase */
>> +       if (frameCount_ < kNumStartupFrames)
>> +               speed = 1.0;
>> +
>> +       if (filteredExposure_ == 0s) {
>> +               /* DG stands for digital gain.*/
> 
> I think that comment is now stale.
> If it's stale in the IPU3 as well, can you remove it there too please?

Sure, in a dedicated patch for IPU3 ? In the same series ?

> 
>> +               filteredExposure_ = currentExposure_;
>> +       } else {
>> +               /*
>> +                * If we are close to the desired result, go faster to avoid making
> 
> faster ? or slower?
> 
> Oh - ok this checks out ;-) I almost got caught out forgetting that the
> square root of a number less than one - increases towards one ;=)

You auto-replied, that's an easy answer for me :-).

> 
> 
>> +                * multiple micro-adjustments.
>> +                * \todo Make this customisable?
>> +                */
>> +               if (filteredExposure_ < 1.2 * currentExposure_ &&
>> +                   filteredExposure_ > 0.8 * currentExposure_)
>> +                       speed = sqrt(speed);
>> +
>> +               filteredExposure_ = speed * currentExposure_ +
>> +                                   filteredExposure_ * (1.0 - speed);
>> +       }
>> +
>> +       LOG(RkISP1Agc, Debug) << "After filtering, total_exposure " << 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(IPAFrameContext &frameContext, double yGain)
>> +{
>> +       /* Get the effective exposure and gain applied on the sensor. */
>> +       uint32_t exposure = frameContext.sensor.exposure;
>> +       double analogueGain = frameContext.sensor.gain;
>> +
>> +       /* 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 * 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;
>> +
>> +       /* \todo: estimate if we need to desaturate */
>> +       filterExposure();
> 
> Would returning the filtedExposure value from the function make it
> clearer or more explicit than it gets used ?

Why not, it is also true for IPU3 then.

> 
>> +
>> +       /* Divide the exposure value as new exposure and gain values */
>> +       utils::Duration exposureValue = filteredExposure_;
> 
> i.e.
> 	  utils::Duration exposureValue = filteredExposure();
> ?
> 
>> +       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 / 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.frameContext, yGain);
> 
> Does the algorithm need to track the frameCount internally? Can it get
> it from the frame / index of the request / stats?
> 

I have the introduction of frameId in the IPAFrameContext later, do you 
want it now too ? Same for IPU3 then (it makes this series deviate a bit 
of the initial subject probably but why not).

>> +       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..83159743
>> --- /dev/null
>> +++ b/src/ipa/rkisp1/algorithms/agc.h
>> @@ -0,0 +1,55 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2021, Ideas On Board
>> + *
>> + * agc.h - RkISP1 AGC/AEC mean-based control algorithm
>> + */
>> +#ifndef __LIBCAMERA_RKISP1_ALGORITHMS_AGC_H__
>> +#define __LIBCAMERA_RKISP1_ALGORITHMS_AGC_H__
>> +
>> +#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 filterExposure();
>> +       void computeExposure(IPAFrameContext &frameContext, double yGain);
>> +       double estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);
>> +
>> +       uint64_t frameCount_;
>> +
>> +       utils::Duration lineDuration_;
>> +       utils::Duration minShutterSpeed_;
>> +       utils::Duration maxShutterSpeed_;
>> +
>> +       double minAnalogueGain_;
>> +       double maxAnalogueGain_;
> 
> Do all these need to be duplicated from the context? Can't we use the
> values from the IPASessionConfiguration?

We can. It means we need to pass the IPASessionConfiguration to the 
computeExposure() call, and not only the IPAFrameContext.
Is there a real issue with having those cached (in C++ or memory 
footprint maybe) ?

> 
>> +
>> +       utils::Duration filteredExposure_;
>> +       utils::Duration currentExposure_;
>> +};
>> +
>> +} /* namespace ipa::rkisp1::algorithms */
>> +
>> +} /* namespace libcamera */
>> +
>> +#endif /* __LIBCAMERA_RKISP1_ALGORITHMS_AGC_H__ */
>> 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 819b2c73..16fc248f 100644
>> --- a/src/ipa/rkisp1/ipa_context.cpp
>> +++ b/src/ipa/rkisp1/ipa_context.cpp
>> @@ -55,4 +55,48 @@ namespace libcamera::ipa::rkisp1 {
>>    * are run. This needs to be turned into real per-frame data storage.
>>    */
>>   
>> +/**
>> + * \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 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 ff40efe3..8e756fb1 100644
>> --- a/src/ipa/rkisp1/ipa_context.h
>> +++ b/src/ipa/rkisp1/ipa_context.h
>> @@ -8,14 +8,31 @@
>>   #ifndef __LIBCAMERA_RKISP1_IPA_CONTEXT_H__
>>   #define __LIBCAMERA_RKISP1_IPA_CONTEXT_H__
>>   
>> +#include <libcamera/base/utils.h>
>> +
>>   namespace libcamera {
>>   
>>   namespace ipa::rkisp1 {
>>   
>>   struct IPASessionConfiguration {
>> +       struct {
>> +               utils::Duration minShutterSpeed;
>> +               utils::Duration maxShutterSpeed;
>> +               double minAnalogueGain;
>> +               double maxAnalogueGain;
>> +       } agc;
>>   };
>>   
>>   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 b5aa93f8..89d98b6c 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"
>>   
>> @@ -32,6 +33,8 @@ namespace libcamera {
>>   
>>   LOG_DEFINE_CATEGORY(IPARkISP1)
>>   
>> +using namespace std::literals::chrono_literals;
>> +
>>   namespace ipa::rkisp1 {
>>   
>>   class IPARkISP1 : public IPARkISP1Interface
>> @@ -77,6 +80,8 @@ private:
>>          unsigned int hwGammaOutMaxSamples_;
>>          unsigned int hwHistogramWeightGridsSize_;
>>   
>> +       utils::Duration lineDuration_;
>> +
> 
> Does this need to be stored privately? or can it be stored in the
> Context?
> 
> Or rather, does this get used by the algorithms directly? If not - then
> it's fine here.

I have, in IPU3, cached it because it was used at configure time and 
when updating controls after the algorithm have run.

Are you trying to remove all the cached values by any chance :-) ?

> 
> 
>>          /* Interface to the Camera Helper */
>>          std::unique_ptr<CameraSensorHelper> camHelper_;
>>   
>> @@ -120,6 +125,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;
>>   }
>>   
>> @@ -171,9 +179,29 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>>                  << "Exposure: " << minExposure_ << "-" << maxExposure_
>>                  << " Gain: " << minGain_ << "-" << maxGain_;
>>   
>> +       lineDuration_ = info.lineLength * 1.0s / info.pixelRate;
>> +
>>          /* Clean context at configuration */
>>          context_ = {};
>>   
>> +       /*
>> +        * 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_ * lineDuration_;
>> +       context_.configuration.agc.maxShutterSpeed = maxExposure_ * 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;
>>   }
>>   
>> @@ -218,6 +246,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;
>>          }
>> @@ -262,44 +293,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;
>> +       for (auto const &algo : algorithms_)
>> +               algo->process(context_, stats);
>>   
>> -               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;
>> -
>> -               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 + 1);
> 
> Is + 1 the correct target frame?
> Is that always true even if there are multiple frames queued?
> 

I have to be honest here: I reused the existing code. Calling 
setControls(frame) gives the same output, so I can't really say if it is 
correct or not, I did not dig it.

> 
> 
>>   
>>          metadataReady(frame, aeState);
>>   }
>> @@ -309,6 +308,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
>>


More information about the libcamera-devel mailing list