[libcamera-devel] [RFC PATCH 5/5] ipa: ipu3: Move IPU3 agc into algorithms
Umang Jain
umang.jain at ideasonboard.com
Mon Aug 9 12:33:47 CEST 2021
Hi JM,
Thanks for the patch.
On 8/9/21 2:50 PM, Jean-Michel Hautbois wrote:
> Use the Context class and algorithm interface to properly call the AGC
> algorithm from IPAIPU3.
> We need to pass shutter speed and analogue gain through Context. Add a
> dedicated AGC structure for that.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
> .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 56 ++++++++++---------
> src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 31 +++++-----
> src/ipa/ipu3/algorithms/meson.build | 1 +
> src/ipa/ipu3/ipa_context.h | 11 ++++
> src/ipa/ipu3/ipu3.cpp | 25 +++++----
> src/ipa/ipu3/meson.build | 1 -
> 6 files changed, 70 insertions(+), 55 deletions(-)
> rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (81%)
> rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (61%)
>
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> similarity index 81%
> rename from src/ipa/ipu3/ipu3_agc.cpp
> rename to src/ipa/ipu3/algorithms/agc.cpp
> index 733814dd..1146a34a 100644
> --- a/src/ipa/ipu3/ipu3_agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -2,26 +2,25 @@
> /*
> * Copyright (C) 2021, Ideas On Board
SPDX license missing, I think it's missing in 4/5 too
> *
> - * ipu3_agc.cpp - AGC/AEC control algorithm
> + * agc.cpp - AGC/AEC control algorithm
> */
>
> -#include "ipu3_agc.h"
> +#include "agc.h"
>
> -#include <algorithm>
> #include <cmath>
> +
> +#include <algorithm>
I guess this a transient change that creeped in? :)
> #include <numeric>
>
> #include <libcamera/base/log.h>
>
> -#include <libcamera/ipa/core_ipa_interface.h>
> -
> #include "libipa/histogram.h"
>
> namespace libcamera {
>
> using namespace std::literals::chrono_literals;
>
> -namespace ipa::ipu3 {
> +namespace ipa::ipu3::algorithms {
>
> LOG_DEFINE_CATEGORY(IPU3Agc)
>
> @@ -36,8 +35,8 @@ static constexpr uint32_t kMaxISO = 1500;
>
> /* Maximum analogue gain value
> * \todo grab it from a camera helper */
> -static constexpr uint32_t kMinGain = kMinISO / 100;
> -static constexpr uint32_t kMaxGain = kMaxISO / 100;
> +static constexpr double kMinGain = kMinISO / 100;
> +static constexpr double kMaxGain = kMaxISO / 100;
Probably "/ 100.0" , if you want converted to double?
>
> /* \todo use calculated value based on sensor */
> static constexpr uint32_t kMinExposure = 1;
> @@ -50,24 +49,24 @@ static constexpr double kEvGainTarget = 0.5;
> /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */
> static constexpr uint8_t kCellSize = 8;
>
> -IPU3Agc::IPU3Agc()
> +Agc::Agc()
> : frameCount_(0), lastFrame_(0), converged_(false),
> - updateControls_(false), iqMean_(0.0),
> - lineDuration_(0s), maxExposureTime_(0s),
> + updateControls_(false), iqMean_(0.0), maxExposureTime_(0s),
> prevExposure_(0s), prevExposureNoDg_(0s),
> currentExposure_(0s), currentExposureNoDg_(0s)
> {
> }
>
> -void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo)
> +int Agc::configure(IPAContext &context)
> {
> - aeGrid_ = bdsGrid;
> + /* The AGC algorithm uses the AWB statistics */
> + aeGrid_ = context.awb.grid.bdsGrid;
> + maxExposureTime_ = kMaxExposure * context.agc.lineDuration;
>
> - lineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;
> - maxExposureTime_ = kMaxExposure * lineDuration_;
> + return 0;
> }
>
> -void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
> +void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
> {
> const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;
> Rectangle aeRegion = { statsAeGrid.x_start,
> @@ -108,7 +107,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
> iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> }
>
> -void IPU3Agc::filterExposure()
> +void Agc::filterExposure()
> {
> double speed = 0.2;
> if (prevExposure_ == 0s) {
> @@ -128,7 +127,7 @@ void IPU3Agc::filterExposure()
> prevExposure_ = speed * currentExposure_ +
> prevExposure_ * (1.0 - speed);
> prevExposureNoDg_ = speed * currentExposureNoDg_ +
> - prevExposureNoDg_ * (1.0 - speed);
> + prevExposureNoDg_ * (1.0 - speed);
> }
> /*
> * We can't let the no_dg exposure deviate too far below the
> @@ -142,7 +141,7 @@ void IPU3Agc::filterExposure()
> LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << prevExposure_;
> }
>
> -void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
> +void Agc::lockExposureGain(IPAContext &context)
> {
> updateControls_ = false;
>
> @@ -160,7 +159,9 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
> double newGain = kEvGainTarget * knumHistogramBins / iqMean_;
>
> /* extracted from Rpi::Agc::computeTargetExposure */
> - libcamera::utils::Duration currentShutter = exposure * lineDuration_;
> + libcamera::utils::Duration currentShutter = context.agc.shutterTime;
> + uint32_t exposure = currentShutter / context.agc.lineDuration;
> + double &gain = context.agc.analogueGain;
> currentExposureNoDg_ = currentShutter * gain;
> LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
> << " Shutter speed " << currentShutter
> @@ -177,26 +178,27 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
> if (currentShutter < maxExposureTime_) {
> exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);
> newExposure = currentExposure_ / exposure;
> - gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);
> + gain = std::clamp(static_cast<double>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);
> updateControls_ = true;
> } else if (currentShutter >= maxExposureTime_) {
> - gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);
> + gain = std::clamp(static_cast<double>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);
> newExposure = currentExposure_ / gain;
> exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure);
> updateControls_ = true;
> }
> - LOG(IPU3Agc, Debug) << "Adjust exposure " << exposure * lineDuration_ << " and gain " << gain;
> + context.agc.shutterTime = exposure * context.agc.lineDuration;
> + LOG(IPU3Agc, Debug) << "Adjust exposure " << exposure * context.agc.lineDuration << " and gain " << gain;
> }
> lastFrame_ = frameCount_;
> }
>
> -void IPU3Agc::process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain)
> +void Agc::process(IPAContext &context)
> {
> - processBrightness(stats);
> - lockExposureGain(exposure, gain);
> + processBrightness(context.stats);
> + lockExposureGain(context);
> frameCount_++;
> }
>
> -} /* namespace ipa::ipu3 */
> +} /* namespace ipa::ipu3::algorithms */
>
> } /* namespace libcamera */
> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h
> similarity index 61%
> rename from src/ipa/ipu3/ipu3_agc.h
> rename to src/ipa/ipu3/algorithms/agc.h
> index 3b7f781e..e87a06f3 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -4,44 +4,44 @@
> *
> * ipu3_agc.h - IPU3 AGC/AEC control algorithm
> */
> -#ifndef __LIBCAMERA_IPU3_AGC_H__
> -#define __LIBCAMERA_IPU3_AGC_H__
> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
> +#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
> +
> +#include <linux/intel-ipu3.h>
>
> #include <array>
> #include <unordered_map>
>
> -#include <linux/intel-ipu3.h>
> -
> #include <libcamera/base/utils.h>
>
> #include <libcamera/geometry.h>
>
> -#include "algorithms/algorithm.h"
> -#include "libipa/algorithm.h"
> +#include "algorithm.h"
>
> namespace libcamera {
>
> struct IPACameraSensorInfo;
>
> -namespace ipa::ipu3 {
> +namespace ipa::ipu3::algorithms {
>
> using utils::Duration;
>
> -class IPU3Agc : public ipa::Algorithm
> +class Agc : public Algorithm
> {
> public:
> - IPU3Agc();
> - ~IPU3Agc() = default;
> + Agc();
> + ~Agc() = default;
> +
> + int configure(IPAContext &context) override;
> + void process(IPAContext &context) override;
>
> - void initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo);
> - void process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain);
> bool converged() { return converged_; }
> bool updateControls() { return updateControls_; }
>
> private:
> void processBrightness(const ipu3_uapi_stats_3a *stats);
> void filterExposure();
> - void lockExposureGain(uint32_t &exposure, double &gain);
> + void lockExposureGain(IPAContext &context);
>
> struct ipu3_uapi_grid_config aeGrid_;
>
> @@ -53,7 +53,6 @@ private:
>
> double iqMean_;
>
> - Duration lineDuration_;
> Duration maxExposureTime_;
>
> Duration prevExposure_;
> @@ -62,8 +61,8 @@ private:
> Duration currentExposureNoDg_;
> };
>
> -} /* namespace ipa::ipu3 */
> +} /* namespace ipa::ipu3::algorithms */
>
> } /* namespace libcamera */
>
> -#endif /* __LIBCAMERA_IPU3_AGC_H__ */
> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index df36d719..3faf913e 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: CC0-1.0
>
> ipu3_ipa_algorithms = files([
> + 'agc.cpp',
> 'awb.cpp',
> 'contrast.cpp',
> ])
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index c43b275b..1d7a1984 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -11,10 +11,14 @@
>
> #include <linux/intel-ipu3.h>
>
> +#include <libcamera/base/utils.h>
> +
> #include <libcamera/geometry.h>
>
> namespace libcamera {
>
> +using libcamera::utils::Duration;
> +
I think you can drop "libcamera::" here, just utils::Duration
> namespace ipa::ipu3 {
>
> struct IPAContext {
> @@ -33,6 +37,13 @@ struct IPAContext {
> Size bdsOutputSize;
> } grid;
> } awb;
> +
> + /* AGC specific parameters to share */
> + struct Agc {
> + Duration lineDuration;
> + Duration shutterTime;
> + double analogueGain;
> + } agc;
> };
>
> } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 82506461..07b1d11c 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -22,12 +22,12 @@
>
> #include "libcamera/internal/framebuffer.h"
>
> +#include "algorithms/agc.h"
> #include "algorithms/algorithm.h"
> #include "algorithms/awb.h"
> #include "algorithms/contrast.h"
> #include "ipa_context.h"
>
> -#include "ipu3_agc.h"
> #include "libipa/camera_sensor_helper.h"
>
> static constexpr uint32_t kMaxCellWidthPerSet = 160;
> @@ -37,6 +37,8 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(IPAIPU3)
>
> +using namespace std::literals::chrono_literals;
> +
> namespace ipa::ipu3 {
>
> class IPAIPU3 : public IPAIPU3Interface
> @@ -81,8 +83,6 @@ private:
> uint32_t minGain_;
> uint32_t maxGain_;
>
> - /* Interface to the AEC/AGC algorithm */
> - std::unique_ptr<IPU3Agc> agcAlgo_;
> /* Interface to the Camera Helper */
> std::unique_ptr<CameraSensorHelper> camHelper_;
>
> @@ -103,6 +103,7 @@ int IPAIPU3::init(const IPASettings &settings)
> }
>
> /* Construct our Algorithms */
> + algorithms_.emplace_back(new algorithms::Agc());
> algorithms_.emplace_back(new algorithms::Awb());
> algorithms_.emplace_back(new algorithms::Contrast());
>
> @@ -214,11 +215,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> calculateBdsGrid(configInfo.bdsOutputSize);
> context_.awb.grid.bdsOutputSize = configInfo.bdsOutputSize;
>
> + /* Prepare AGC parameters */
> + context_.agc.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
> + context_.agc.shutterTime = exposure_ * context_.agc.lineDuration;
> + context_.agc.analogueGain = camHelper_->gain(gain_);
> +
> configureAlgorithms();
>
> bdsGrid_ = context_.awb.grid.bdsGrid;
> - agcAlgo_ = std::make_unique<IPU3Agc>();
> - agcAlgo_->initialise(bdsGrid_, sensorInfo_);
>
> return 0;
> }
> @@ -341,12 +345,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> /* Run the process for each algorithm on the stats */
> processAlgorithms(stats);
>
> - double gain = camHelper_->gain(gain_);
> - agcAlgo_->process(stats, exposure_, gain);
> - gain_ = camHelper_->gainCode(gain);
> -
> - if (agcAlgo_->updateControls())
> - setControls(frame);
> + setControls(frame);
>
> /* \todo Use VBlank value calculated from each frame exposure. */
> int64_t frameDuration = sensorInfo_.lineLength * (defVBlank_ + sensorInfo_.outputSize.height) /
> @@ -365,6 +364,10 @@ void IPAIPU3::setControls(unsigned int frame)
> IPU3Action op;
> op.op = ActionSetSensorControls;
>
> + /* Convert gain and exposure */
> + gain_ = camHelper_->gainCode(context_.agc.analogueGain);
> + exposure_ = context_.agc.shutterTime / context_.agc.lineDuration;
> +
> ControlList ctrls(ctrls_);
> ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
> ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
> index d1126947..39320980 100644
> --- a/src/ipa/ipu3/meson.build
> +++ b/src/ipa/ipu3/meson.build
> @@ -6,7 +6,6 @@ ipa_name = 'ipa_ipu3'
>
> ipu3_ipa_sources = files([
> 'ipu3.cpp',
> - 'ipu3_agc.cpp',
> ])
>
> ipu3_ipa_sources += ipu3_ipa_algorithms
More information about the libcamera-devel
mailing list