[libcamera-devel] [PATCH v4 5/9] ipa: ipu3: Introduce a modular tone mapping algorithm

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Aug 20 03:04:56 CEST 2021


Hi Jean-Michel,

Thank you for the patch.

On Thu, Aug 19, 2021 at 04:57:55PM +0200, Jean-Michel Hautbois wrote:
> Introduce a new algorithm to manage the tone mapping handling of the
> IPU3.
> 
> The initial algorithm is chosen to configure the gamma contrast curve
> which moves the implementation out of AWB for simplicity. As it is
> initialised with a default gamma value of 1.1, there is no need to use
> the default table at initialisation anymore.
> 
> This demonstrates the way to use process() call when the EventStatReady
> comes in. The function calculates the LUT in the context of a frame, and
> when prepare() is called, the parameters are filled with the updated
> values.
> 
> AGC is modified to take the new process interface into account.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/meson.build      |  1 +
>  src/ipa/ipu3/algorithms/tone_mapping.cpp | 58 ++++++++++++++++++++++++
>  src/ipa/ipu3/algorithms/tone_mapping.h   | 32 +++++++++++++
>  src/ipa/ipu3/ipa_context.h               |  3 ++
>  src/ipa/ipu3/ipu3.cpp                    | 17 ++++++-
>  src/ipa/ipu3/ipu3_agc.cpp                |  5 +-
>  src/ipa/ipu3/ipu3_agc.h                  |  3 --
>  src/ipa/ipu3/ipu3_awb.cpp                | 41 +----------------
>  src/ipa/ipu3/ipu3_awb.h                  |  2 +-
>  9 files changed, 114 insertions(+), 48 deletions(-)
>  create mode 100644 src/ipa/ipu3/algorithms/tone_mapping.cpp
>  create mode 100644 src/ipa/ipu3/algorithms/tone_mapping.h
> 
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index dc538b79..71eedfa4 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -2,4 +2,5 @@
>  
>  ipu3_ipa_algorithms = files([
>      'algorithm.cpp',
> +    'tone_mapping.cpp',
>  ])
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> new file mode 100644
> index 00000000..0dd6755e
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google inc.
> + *
> + * tone_mapping.cpp - IPU3 ToneMapping and Gamma control
> + */
> +
> +#include "tone_mapping.h"
> +
> +#include <cmath>
> +#include <string.h>
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3::algorithms {
> +
> +ToneMapping::ToneMapping()
> +	: gamma_(1.0)
> +{
> +}
> +
> +void ToneMapping::prepare([[maybe_unused]] IPAContext &context, ipu3_uapi_params &params)
> +{
> +	/* Copy the calculated LUT into the parameters buffer. */
> +	memcpy(params.acc_param.gamma.gc_lut.lut,
> +	       context.frameContext.toneMapping.gammaCorrection.lut,
> +	       IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *
> +	       sizeof(params.acc_param.gamma.gc_lut.lut[0]));
> +
> +	/* Enable the custom gamma table. */
> +	params.use.acc_gamma = 1;
> +	params.acc_param.gamma.gc_ctrl.enable = 1;
> +}
> +
> +void ToneMapping::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const ipu3_uapi_stats_3a *&stats)

Line wrap.

> +{
> +	/*
> +	 * Hardcode gamma to 1.1 as a default for now.
> +	 *
> +	 * \todo Expose gamma control setting through the libcamera control API
> +	 */
> +	gamma_ = 1.1;
> +
> +	struct ipu3_uapi_gamma_corr_lut &lut =
> +		context.frameContext.toneMapping.gammaCorrection;
> +
> +	for (uint32_t i = 0; i < std::size(lut.lut); i++) {
> +		double j = static_cast<double>(i) / (std::size(lut.lut) - 1);
> +		double gamma = std::pow(j, 1.0 / gamma_);
> +
> +		/* The output value is expressed on 13 bits. */
> +		lut.lut[i] = gamma * 8191;
> +	}
> +}
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h
> new file mode 100644
> index 00000000..24176728
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google inc.
> + *
> + * tone_mapping.h - IPU3 ToneMapping and Gamma control
> + */
> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_TONE_MAPPING_H__
> +#define __LIBCAMERA_IPU3_ALGORITHMS_TONE_MAPPING_H__
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3::algorithms {
> +
> +class ToneMapping : public Algorithm
> +{
> +public:
> +	ToneMapping();
> +
> +	void prepare(IPAContext &context, ipu3_uapi_params &params) override;
> +	void process(IPAContext &context, const ipu3_uapi_stats_3a *&stats) override;

As commented by Kieran on the previous patch, the second argument should
be either a const pointer or a const reference, not a reference to a
pointer.

> +
> +private:
> +	double gamma_;
> +};
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_TONE_MAPPING_H__ */
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index a031ab83..4b12f129 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -24,6 +24,9 @@ struct IPASessionConfiguration {
>  };
>  
>  struct IPAFrameContext {
> +	struct {
> +		struct ipu3_uapi_gamma_corr_lut gammaCorrection;
> +	} toneMapping;
>  };
>  
>  struct IPAContext {
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index fa09d9f2..edc2d911 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -30,6 +30,7 @@
>  #include "libcamera/internal/mapped_framebuffer.h"
>  
>  #include "algorithms/algorithm.h"
> +#include "algorithms/tone_mapping.h"
>  #include "ipu3_agc.h"
>  #include "ipu3_awb.h"
>  #include "libipa/camera_sensor_helper.h"
> @@ -94,6 +95,17 @@ static constexpr uint32_t kMaxCellHeightPerSet = 56;
>   * \brief BDS output size configured by the pipeline handler
>   */
>  
> +/**
> + * \struct IPAFrameContext::toneMapping
> + * \brief Context for ToneMapping and Gamma control
> + *
> + * \var IPAFrameContext::toneMapping::gammaCorrection
> + * \brief Per-pixel tone mapping implemented as a LUT
> + *
> + * The LUT structure is defined by the IPU3 kernel interface. See
> + * <linux/intel-ipu3.h> struct ipu3_uapi_gamma_corr_lut for further details.

I'm not sure this paragraph brings much, given that the type of the
field is ipu3_uapi_gamma_corr_lut.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + */
> +
>  namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAIPU3)
> @@ -225,6 +237,9 @@ int IPAIPU3::init(const IPASettings &settings,
>  
>  	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>  
> +	/* Construct our Algorithms */
> +	algorithms_.push_back(std::make_unique<algorithms::ToneMapping>());
> +
>  	return 0;
>  }
>  
> @@ -422,7 +437,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
>  		algo->prepare(context_, params_);
>  
>  	if (agcAlgo_->updateControls())
> -		awbAlgo_->updateWbParameters(params_, agcAlgo_->gamma());
> +		awbAlgo_->updateWbParameters(params_);
>  
>  	*params = params_;
>  
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> index 408eb849..20c2d3b4 100644
> --- a/src/ipa/ipu3/ipu3_agc.cpp
> +++ b/src/ipa/ipu3/ipu3_agc.cpp
> @@ -52,7 +52,7 @@ static constexpr uint8_t kCellSize = 8;
>  
>  IPU3Agc::IPU3Agc()
>  	: frameCount_(0), lastFrame_(0), converged_(false),
> -	  updateControls_(false), iqMean_(0.0), gamma_(1.0),
> +	  updateControls_(false), iqMean_(0.0),
>  	  lineDuration_(0s), maxExposureTime_(0s),
>  	  prevExposure_(0s), prevExposureNoDg_(0s),
>  	  currentExposure_(0s), currentExposureNoDg_(0s)
> @@ -104,9 +104,6 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
>  		}
>  	}
>  
> -	/* 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);
>  }
> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
> index f00b98d6..2d86127d 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/ipu3_agc.h
> @@ -33,8 +33,6 @@ public:
>  	void process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &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);
> @@ -50,7 +48,6 @@ private:
>  	bool updateControls_;
>  
>  	double iqMean_;
> -	double gamma_;
>  
>  	Duration lineDuration_;
>  	Duration maxExposureTime_;
> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
> index 4bb321b3..c2d9e0c1 100644
> --- a/src/ipa/ipu3/ipu3_awb.cpp
> +++ b/src/ipa/ipu3/ipu3_awb.cpp
> @@ -133,31 +133,6 @@ static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = {
>  	0, 0, 8191, 0
>  };
>  
> -/* Default settings for Gamma correction */
> -const struct ipu3_uapi_gamma_corr_lut imguCssGammaLut = { {
> -	63, 79, 95, 111, 127, 143, 159, 175, 191, 207, 223, 239, 255, 271, 287,
> -	303, 319, 335, 351, 367, 383, 399, 415, 431, 447, 463, 479, 495, 511,
> -	527, 543, 559, 575, 591, 607, 623, 639, 655, 671, 687, 703, 719, 735,
> -	751, 767, 783, 799, 815, 831, 847, 863, 879, 895, 911, 927, 943, 959,
> -	975, 991, 1007, 1023, 1039, 1055, 1071, 1087, 1103, 1119, 1135, 1151,
> -	1167, 1183, 1199, 1215, 1231, 1247, 1263, 1279, 1295, 1311, 1327, 1343,
> -	1359, 1375, 1391, 1407, 1423, 1439, 1455, 1471, 1487, 1503, 1519, 1535,
> -	1551, 1567, 1583, 1599, 1615, 1631, 1647, 1663, 1679, 1695, 1711, 1727,
> -	1743, 1759, 1775, 1791, 1807, 1823, 1839, 1855, 1871, 1887, 1903, 1919,
> -	1935, 1951, 1967, 1983, 1999, 2015, 2031, 2047, 2063, 2079, 2095, 2111,
> -	2143, 2175, 2207, 2239, 2271, 2303, 2335, 2367, 2399, 2431, 2463, 2495,
> -	2527, 2559, 2591, 2623, 2655, 2687, 2719, 2751, 2783, 2815, 2847, 2879,
> -	2911, 2943, 2975, 3007, 3039, 3071, 3103, 3135, 3167, 3199, 3231, 3263,
> -	3295, 3327, 3359, 3391, 3423, 3455, 3487, 3519, 3551, 3583, 3615, 3647,
> -	3679, 3711, 3743, 3775, 3807, 3839, 3871, 3903, 3935, 3967, 3999, 4031,
> -	4063, 4095, 4127, 4159, 4223, 4287, 4351, 4415, 4479, 4543, 4607, 4671,
> -	4735, 4799, 4863, 4927, 4991, 5055, 5119, 5183, 5247, 5311, 5375, 5439,
> -	5503, 5567, 5631, 5695, 5759, 5823, 5887, 5951, 6015, 6079, 6143, 6207,
> -	6271, 6335, 6399, 6463, 6527, 6591, 6655, 6719, 6783, 6847, 6911, 6975,
> -	7039, 7103, 7167, 7231, 7295, 7359, 7423, 7487, 7551, 7615, 7679, 7743,
> -	7807, 7871, 7935, 7999, 8063, 8127, 8191
> -} };
> -
>  IPU3Awb::IPU3Awb()
>  	: Algorithm()
>  {
> @@ -197,10 +172,6 @@ void IPU3Awb::initialise(ipu3_uapi_params &params, const Size &bdsOutputSize, st
>  	params.use.acc_ccm = 1;
>  	params.acc_param.ccm = imguCssCcmDefault;
>  
> -	params.use.acc_gamma = 1;
> -	params.acc_param.gamma.gc_lut = imguCssGammaLut;
> -	params.acc_param.gamma.gc_ctrl.enable = 1;
> -
>  	zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
>  }
>  
> @@ -350,7 +321,7 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
>  	}
>  }
>  
> -void IPU3Awb::updateWbParameters(ipu3_uapi_params &params, double agcGamma)
> +void IPU3Awb::updateWbParameters(ipu3_uapi_params &params)
>  {
>  	/*
>  	 * Green gains should not be touched and considered 1.
> @@ -362,18 +333,10 @@ void IPU3Awb::updateWbParameters(ipu3_uapi_params &params, double agcGamma)
>  	params.acc_param.bnr.wb_gains.b = 4096 * asyncResults_.blueGain;
>  	params.acc_param.bnr.wb_gains.gb = 16;
>  
> -	LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK
> -			    << " and gamma calculated: " << agcGamma;
> +	LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK;
>  
>  	/* The CCM matrix may change when color temperature will be used */
>  	params.acc_param.ccm = imguCssCcmDefault;
> -
> -	for (uint32_t i = 0; i < 256; i++) {
> -		double j = i / 255.0;
> -		double gamma = std::pow(j, 1.0 / agcGamma);
> -		/* The maximum value 255 is represented on 13 bits in the IPU3 */
> -		params.acc_param.gamma.gc_lut.lut[i] = gamma * 8191;
> -	}
>  }
>  
>  } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/ipu3_awb.h
> index ea2d4320..eeb2920b 100644
> --- a/src/ipa/ipu3/ipu3_awb.h
> +++ b/src/ipa/ipu3/ipu3_awb.h
> @@ -31,7 +31,7 @@ public:
>  
>  	void initialise(ipu3_uapi_params &params, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid);
>  	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> -	void updateWbParameters(ipu3_uapi_params &params, double agcGamma);
> +	void updateWbParameters(ipu3_uapi_params &params);
>  
>  	struct Ipu3AwbCell {
>  		unsigned char greenRedAvg;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list