[libcamera-devel] [PATCH v5 4/4] ipa: ipu3: Add support for IPU3 AEC/AGC algorithm

Kieran Bingham kieran.bingham at ideasonboard.com
Mon Apr 19 14:49:05 CEST 2021


Hi JM,

On 16/04/2021 08:49, Jean-Michel Hautbois wrote:
> Implement basic auto-exposure (AE) and auto-gain (AG) correction functions.
> The functions computeTargetExposure() and computeGain() are adapted from
> the Raspberry Pi AGC implementation to suit the IPU3 structures, and
> filtering is added to reduce visible stepsize when there are large
> exposure changes.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/ipa/ipu3/ipu3.cpp     |  16 ++-
>  src/ipa/ipu3/ipu3_agc.cpp | 206 ++++++++++++++++++++++++++++++++++++++
>  src/ipa/ipu3/ipu3_agc.h   |  62 ++++++++++++
>  src/ipa/ipu3/meson.build  |   1 +
>  4 files changed, 282 insertions(+), 3 deletions(-)
>  create mode 100644 src/ipa/ipu3/ipu3_agc.cpp
>  create mode 100644 src/ipa/ipu3/ipu3_agc.h
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index ab052a8a..48dc28c1 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -21,10 +21,11 @@
>  #include "libcamera/internal/buffer.h"
>  #include "libcamera/internal/log.h"
>  
> +#include "ipu3_agc.h"
>  #include "ipu3_awb.h"
>  
>  static constexpr uint32_t kMaxCellWidthPerSet = 160;
> -static constexpr uint32_t kMaxCellHeightPerSet = 80;
> +static constexpr uint32_t kMaxCellHeightPerSet = 56;

Is this reduced to solve an issue? or to improve something in particular?

Or does it represent some newly understood hardware limitation?

>  namespace libcamera {
>  
> @@ -70,6 +71,8 @@ private:
>  
>  	/* Interface to the AWB algorithm */
>  	std::unique_ptr<ipa::IPU3Awb> awbAlgo_;
> +	/* Interface to the AEC/AGC algorithm */
> +	std::unique_ptr<ipa::IPU3Agc> agcAlgo_;
>  
>  	/* Local parameter storage */
>  	struct ipu3_uapi_params params_;
> @@ -168,6 +171,9 @@ void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls
>  
>  	awbAlgo_ = std::make_unique<ipa::IPU3Awb>();
>  	awbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);
> +
> +	agcAlgo_ = std::make_unique<ipa::IPU3Agc>();
> +	agcAlgo_->initialise(bdsGrid_);
>  }
>  
>  void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)
> @@ -239,8 +245,8 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
>  
>  void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
>  {
> -	/* Pass a default gamma of 1.0 (default linear correction) */
> -	awbAlgo_->updateWbParameters(params_, 1.0);
> +	if (agcAlgo_->updateControls())
> +		awbAlgo_->updateWbParameters(params_, agcAlgo_->gamma());
>  
>  	*params = params_;
>  
> @@ -255,8 +261,12 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>  {
>  	ControlList ctrls(controls::controls);
>  
> +	agcAlgo_->process(stats, exposure_, gain_);
>  	awbAlgo_->calculateWBGains(stats);
>  
> +	if (agcAlgo_->updateControls())
> +		setControls(frame);
> +
>  	ipa::ipu3::IPU3Action op;
>  	op.op = ipa::ipu3::ActionMetadataReady;
>  	op.controls = ctrls;
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> new file mode 100644
> index 00000000..04ab55a1
> --- /dev/null
> +++ b/src/ipa/ipu3/ipu3_agc.cpp
> @@ -0,0 +1,206 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * ipu3_agc.cpp - AGC/AEC control algorithm
> + */
> +
> +#include "ipu3_agc.h"
> +
> +#include <algorithm>
> +#include <cmath>
> +#include <numeric>
> +
> +#include "libcamera/internal/log.h"
> +
> +#include "libipa/histogram.h"
> +
> +namespace libcamera {
> +
> +namespace ipa {

This probably applies to the others in this series, but now that the
IPAIPU3 class is in libcamera::ipa::ipu3, this should be too.

> +
> +LOG_DEFINE_CATEGORY(IPU3Agc)
> +
> +/* Number of frames to wait before calculating stats on minimum exposure */
> +static constexpr uint32_t kInitialFrameMinAECount = 4;
> +/* Number of frames to wait between new gain/exposure estimations */
> +static constexpr uint32_t kFrameSkipCount = 6;
> +
> +/* Maximum ISO value for analogue gain */
> +static constexpr uint32_t kMinISO = 100;
> +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;
> +
> +/* \todo use calculated value based on sensor */
> +static constexpr uint32_t kMinExposure = 1;
> +static constexpr uint32_t kMaxExposure = 1976;
> +
> +/* \todo those should be get from pipeline handler ! */

Pipelinehandler or CameraSensorInfo?

> +/* line duration in microseconds */
> +static constexpr double kLineDuration = 16.8;
> +static constexpr double kMaxExposureTime = kMaxExposure * kLineDuration;
> +
> +/* Histogram constants */
> +static constexpr uint32_t knumHistogramBins = 256;
> +static constexpr double kEvGainTarget = 0.5;
> +
> +IPU3Agc::IPU3Agc()
> +	: frameCount_(0), lastFrame_(0), converged_(false),
> +	  updateControls_(false), iqMean_(0.0), gamma_(1.0),
> +	  prevExposure_(0.0), prevExposureNoDg_(0.0),
> +	  currentExposure_(0.0), currentExposureNoDg_(0.0)
> +{
> +}
> +
> +void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid)
> +{
> +	aeGrid_ = bdsGrid;
> +}
> +
> +void IPU3Agc::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,
> +			       statsAeGrid.y_start,
> +			       static_cast<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1,
> +			       static_cast<unsigned int>(statsAeGrid.y_end - statsAeGrid.y_start) + 1 };
> +	Point topleft = aeRegion.topLeft();
> +	int topleftX = topleft.x >> aeGrid_.block_width_log2;
> +	int topleftY = topleft.y >> aeGrid_.block_height_log2;
> +
> +	uint32_t startY = topleftY * aeGrid_.width << aeGrid_.block_width_log2;
> +	uint32_t startX = topleftX << aeGrid_.block_width_log2;


is startY and startX a Point?

Can we order them alphabetically .. x .. then y?

Is all the shifting simply to make sure they are aligned to the block
width and height?

If so - do we have any better helpers in geometry.h to help with that ?
It looks like we don't - so perhaps we should add
Point->alignedDownTo(xAlignment, yAlignment) sometime....

(Not for this series).


> +	uint32_t endX = (startX + (aeRegion.size().width >> aeGrid_.block_width_log2)) << aeGrid_.block_width_log2;
> +	uint32_t i, j;
> +	uint32_t count = 0;
> +
> +	uint32_t hist[knumHistogramBins] = { 0 };
> +	for (j = topleftY;
> +	     j < topleftY + (aeRegion.size().height >> aeGrid_.block_height_log2);
> +	     j++) {
> +		for (i = startX + startY; i < endX + startY; i += 8) {

I still don't know what 8 is here?


> +			/**

/*

> +			 * The grid width (and maybe height) is not reliable.
> +			 * We observed a bit shift which makes the value 160 to be 32 in the stats grid.
> +			 * Use the one passed at init time.
> +			 */
> +			if (stats->awb_raw_buffer.meta_data[i + 4 + j * aeGrid_.width] == 0) {
> +				uint8_t Gr = stats->awb_raw_buffer.meta_data[i + j * aeGrid_.width];

i + 0 + to match the others?

> +				uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * aeGrid_.width];
> +				hist[(Gr + Gb) / 2]++;
> +				count++;
> +			}
> +		}
> +	}
> +
> +	/* Limit the gamma effect for now */
> +	gamma_ = 1.1;
> +
> +	/* Estimate the quantile mean of the top 2% of the histogram */
> +	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> +}
> +
> +void IPU3Agc::filterExposure(bool desaturate)
> +{
> +	double speed = 0.2;
> +	if (prevExposure_ == 0.0) {
> +		/* DG stands for digital gain.*/
> +		prevExposure_ = currentExposure_;
> +		prevExposureNoDg_ = currentExposureNoDg_;
> +	} else {
> +		/**

/*

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

/*

> +		 * When desaturing, take a big jump down in exposure_no_dg,

desaturating ?

> +		 * which we'll hide with digital gain.
> +		 */
> +		if (desaturate)
> +			prevExposureNoDg_ =
> +				currentExposureNoDg_;
> +		else
> +			prevExposureNoDg_ =
> +				speed * currentExposureNoDg_ +
> +				prevExposureNoDg_ * (1.0 - speed);
> +	}
> +	/**

/*


With all those:

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> +	 * We can't let the no_dg exposure deviate too far below the
> +	 * total exposure, as there might not be enough digital gain available
> +	 * in the ISP to hide it (which will cause nasty oscillation).
> +	 */
> +	double fastReduceThreshold = 0.4;
> +	if (prevExposureNoDg_ <
> +	    prevExposure_ * fastReduceThreshold)
> +		prevExposureNoDg_ = prevExposure_ * fastReduceThreshold;
> +	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << prevExposure_;
> +}
> +
> +void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)
> +{
> +	updateControls_ = false;
> +
> +	/* Algorithm initialization should wait for first valid frames */
> +	/* \todo - have a number of frames given by DelayedControls ?
> +	 * - implement a function for IIR */
> +	if ((frameCount_ < kInitialFrameMinAECount) || (frameCount_ - lastFrame_ < kFrameSkipCount))
> +		return;
> +
> +	/* Are we correctly exposed ? */
> +	if (std::abs(iqMean_ - kEvGainTarget * knumHistogramBins) <= 1) {
> +		LOG(IPU3Agc, Debug) << "!!! Good exposure with iqMean = " << iqMean_;
> +		converged_ = true;
> +	} else {
> +		double newGain = kEvGainTarget * knumHistogramBins / iqMean_;
> +
> +		/* extracted from Rpi::Agc::computeTargetExposure */
> +		double currentShutter = exposure * kLineDuration;
> +		currentExposureNoDg_ = currentShutter * gain;
> +		LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
> +				    << " Shutter speed " << currentShutter
> +				    << " Gain " << gain;
> +		currentExposure_ = currentExposureNoDg_ * newGain;
> +		double maxTotalExposure = kMaxExposureTime * kMaxGain;
> +		currentExposure_ = std::min(currentExposure_, maxTotalExposure);
> +		LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_;
> +
> +		/* \todo: estimate if we need to desaturate */
> +		filterExposure(false);
> +
> +		double newExposure = 0.0;
> +		if (currentShutter < kMaxExposureTime) {
> +			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);
> +			updateControls_ = true;
> +		} else if (currentShutter >= kMaxExposureTime) {
> +			gain = std::clamp(static_cast<uint32_t>(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 * kLineDuration << " and gain " << gain;
> +	}
> +	lastFrame_ = frameCount_;
> +}
> +
> +void IPU3Agc::process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, uint32_t &gain)
> +{
> +	processBrightness(stats);
> +	lockExposureGain(exposure, gain);
> +	frameCount_++;
> +}
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
> new file mode 100644
> index 00000000..f59bc420
> --- /dev/null
> +++ b/src/ipa/ipu3/ipu3_agc.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * ipu3_agc.h - IPU3 AGC/AEC control algorithm
> + */
> +#ifndef __LIBCAMERA_IPU3_AGC_H__
> +#define __LIBCAMERA_IPU3_AGC_H__
> +
> +#include <array>
> +#include <unordered_map>
> +
> +#include <linux/intel-ipu3.h>
> +
> +#include <libcamera/geometry.h>
> +
> +#include "libipa/algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +class IPU3Agc : public Algorithm
> +{
> +public:
> +	IPU3Agc();
> +	~IPU3Agc() = default;
> +
> +	void initialise(struct ipu3_uapi_grid_config &bdsGrid);
> +	void process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, uint32_t &gain);
> +	bool converged() { return converged_; }
> +	bool updateControls() { return updateControls_; }
> +	/* \todo Use a metadata exchange between IPAs */
> +	double gamma() { return gamma_; }
> +
> +private:
> +	void processBrightness(const ipu3_uapi_stats_3a *stats);
> +	void filterExposure(bool desaturate);
> +	void lockExposureGain(uint32_t &exposure, uint32_t &gain);
> +
> +	struct ipu3_uapi_grid_config aeGrid_;
> +
> +	uint64_t frameCount_;
> +	uint64_t lastFrame_;
> +
> +	bool converged_;
> +	bool updateControls_;
> +
> +	double iqMean_;
> +	double gamma_;
> +
> +	double prevExposure_;
> +	double prevExposureNoDg_;
> +	double currentExposure_;
> +	double currentExposureNoDg_;
> +};
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPU3_AGC_H__ */
> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
> index 1040698e..0d843846 100644
> --- a/src/ipa/ipu3/meson.build
> +++ b/src/ipa/ipu3/meson.build
> @@ -4,6 +4,7 @@ ipa_name = 'ipa_ipu3'
>  
>  ipu3_ipa_sources = files([
>      'ipu3.cpp',
> +    'ipu3_agc.cpp',
>      'ipu3_awb.cpp',
>  ])
>  
> 

-- 
Regards
--
Kieran


More information about the libcamera-devel mailing list