[libcamera-devel] [PATCH v3 9/9] ipa: ipu3: Move IPU3 agc into algorithms
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Aug 19 00:05:52 CEST 2021
Hi Jean-Michel,
Thank you for the patch.
On Wed, Aug 18, 2021 at 05:54:03PM +0200, Jean-Michel Hautbois wrote:
> Now that the interface is properly used by the AGC class, move it into
> ipa::ipu3::algorithms and let the loops do the calls.
> As we need to exchange the exposure_ and gain_ by passing them through the
> FrameContext, use the calculated values in setControls() function to
> ease the reading.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------
> src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------
> src/ipa/ipu3/algorithms/meson.build | 1 +
> src/ipa/ipu3/ipu3.cpp | 15 +++++--------
> src/ipa/ipu3/meson.build | 1 -
> 5 files changed, 26 insertions(+), 32 deletions(-)
> rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)
> rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)
>
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> similarity index 93%
> rename from src/ipa/ipu3/ipu3_agc.cpp
> rename to src/ipa/ipu3/algorithms/agc.cpp
> index 1c1f5fb5..bb119cb4 100644
> --- a/src/ipa/ipu3/ipu3_agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -5,7 +5,7 @@
> * ipu3_agc.cpp - AGC/AEC control algorithm
> */
>
> -#include "ipu3_agc.h"
> +#include "agc.h"
>
> #include <algorithm>
> #include <chrono>
> @@ -21,7 +21,7 @@ namespace libcamera {
>
> using namespace std::literals::chrono_literals;
>
> -namespace ipa::ipu3 {
> +namespace ipa::ipu3::algorithms {
>
> LOG_DEFINE_CATEGORY(IPU3Agc)
>
> @@ -50,14 +50,13 @@ 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), iqMean_(0.0), lineDuration_(0s),
> - maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),
> - currentExposure_(0s), currentExposureNoDg_(0s)
Why can those two fields not be initialized anymore ?
> + maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)
> {
> }
>
> -int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> +int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> {
> aeGrid_ = context.configuration.grid.bdsGrid;
>
> @@ -67,7 +66,7 @@ int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> 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) {
> @@ -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(uint32_t &exposure, double &gain)
> {
> /* Algorithm initialization should wait for first valid frames */
> /* \todo - have a number of frames given by DelayedControls ?
> @@ -185,7 +184,7 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
> lastFrame_ = frameCount_;
> }
>
> -void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> +void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> {
> uint32_t &exposure = context.frameContext.agc.exposure;
> double &gain = context.frameContext.agc.gain;
> @@ -194,6 +193,6 @@ void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> 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 73%
> rename from src/ipa/ipu3/ipu3_agc.h
> rename to src/ipa/ipu3/algorithms/agc.h
> index 0e922664..1739d2fd 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -2,10 +2,10 @@
> /*
> * Copyright (C) 2021, Ideas On Board
> *
> - * ipu3_agc.h - IPU3 AGC/AEC control algorithm
> + * 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>
>
> @@ -13,21 +13,21 @@
>
> #include <libcamera/geometry.h>
>
> -#include "algorithms/algorithm.h"
> +#include "algorithm.h"
>
> namespace libcamera {
>
> struct IPACameraSensorInfo;
>
> -namespace ipa::ipu3 {
> +namespace ipa::ipu3::algorithms {
>
> using utils::Duration;
>
> -class IPU3Agc : public Algorithm
> +class Agc : public Algorithm
> {
> public:
> - IPU3Agc();
> - ~IPU3Agc() = default;
> + Agc();
> + ~Agc() = default;
>
> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> @@ -53,8 +53,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 87377f4e..acaadd90 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -2,6 +2,7 @@
>
> ipu3_ipa_algorithms = files([
> 'algorithm.cpp',
> + 'agc.cpp',
Wrong alphabetical order.
> 'awb.cpp',
> 'contrast.cpp',
> ])
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index f4f49025..d73ec79d 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -30,9 +30,9 @@
> #include "libcamera/internal/mapped_framebuffer.h"
>
> #include "algorithms/algorithm.h"
> +#include "algorithms/agc.h"
Here too.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> #include "algorithms/awb.h"
> #include "algorithms/contrast.h"
> -#include "ipu3_agc.h"
> #include "libipa/camera_sensor_helper.h"
>
> static constexpr uint32_t kMaxCellWidthPerSet = 160;
> @@ -136,8 +136,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_;
>
> @@ -218,6 +216,7 @@ int IPAIPU3::init(const IPASettings &settings,
> *ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>
> /* Construct our Algorithms */
> + algorithms_.emplace_back(new algorithms::Agc());
> algorithms_.emplace_back(new algorithms::Awb());
> algorithms_.emplace_back(new algorithms::Contrast());
>
> @@ -337,10 +336,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> return ret;
> }
>
> -
> - agcAlgo_ = std::make_unique<IPU3Agc>();
> - agcAlgo_->configure(context_, configInfo);
> -
> return 0;
> }
>
> @@ -436,9 +431,6 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> for (auto const &algo : algorithms_)
> algo->process(context_, stats);
>
> - agcAlgo_->process(context_, stats);
> - exposure_ = context_.frameContext.agc.exposure;
> - gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
> setControls(frame);
>
> /* \todo Use VBlank value calculated from each frame exposure. */
> @@ -458,6 +450,9 @@ void IPAIPU3::setControls(unsigned int frame)
> IPU3Action op;
> op.op = ActionSetSensorControls;
>
> + 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_));
> 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
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list