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

11961 diff mbox series

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@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

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@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@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',
>  ])
>  
>