[libcamera-devel] [RFC PATCH 3/5] ipa: ipu3: Introduce a modular contrast algorithm
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Aug 9 11:52:42 CEST 2021
On 09/08/2021 10:20, Jean-Michel Hautbois wrote:
> Implement a new modular framework for algorithms with a common context
> structure that is passed to each algorithm through a common API.
This doesn't implement the framework. (This text is likely leftover from
a previous split).
This patch:
"""
Introduce a new algorithm to manage the contrast handling of the IPU3.
"""
>
> The initial algorithm is chosen to configure the gamma contrast curve
> which replicates the implementation from AWB for simplicity. As it is
You don't replicate it anymore, you move it.
> initialised with a default gamma value of 1.0, there is no need to use
> the default table at init time anymore.>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/contrast.cpp | 52 ++++++++++++++++++++++++++++
> src/ipa/ipu3/algorithms/contrast.h | 32 +++++++++++++++++
> src/ipa/ipu3/algorithms/meson.build | 1 +
> src/ipa/ipu3/ipu3.cpp | 6 +++-
> 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 +-
> 8 files changed, 94 insertions(+), 48 deletions(-)
> create mode 100644 src/ipa/ipu3/algorithms/contrast.cpp
> create mode 100644 src/ipa/ipu3/algorithms/contrast.h
>
> diff --git a/src/ipa/ipu3/algorithms/contrast.cpp b/src/ipa/ipu3/algorithms/contrast.cpp
> new file mode 100644
> index 00000000..832dbf35
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/contrast.cpp
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * constrast.cpp - IPU3 Contrast and Gamma control
> + */
> +
> +#include "contrast.h"
> +
> +#include <cmath>
> +
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3::algorithms {
> +
> +LOG_DEFINE_CATEGORY(IPU3Contrast)
> +
> +Contrast::Contrast()
> + : gamma_(1.0)
> +{
> + LOG(IPU3Contrast, Info) << "Instantiate Gamma";
> +}
> +
> +int Contrast::initialise(IPAContext &context)
> +{
> + ipu3_uapi_params ¶ms = context.params;
> +
> + params.use.acc_gamma = 1;
> + params.acc_param.gamma.gc_ctrl.enable = 1;
> +
> + /* Limit the gamma effect for now */
> + gamma_ = 1.1;
The commit message stated we initialise to 1.0 (which we do on
construction) but here we actually initialise to 1.1...
> +
> + /* Plot the gamma curve into the look up table */
> + for (uint32_t i = 0; i < 256; i++) {
> + double j = i / 255.0;
> + double gamma = std::pow(j, 1.0 / gamma_);
> +
> + /* The maximum value 255 is represented on 13 bits in the IPU3 */
> + params.acc_param.gamma.gc_lut.lut[i] = gamma * 8191;
> + }
> +
> + LOG(IPU3Contrast, Info) << "Processed Gamma Curve";
I think we can drop Info line now or move it to Debug if you think it's
helpful to keep it... But if we keep it as Debug, we should print
something more useful, like what gamma value we actually set.
I think it's probably not helpful though.
> +
> + return 0;
> +}
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/ipu3/algorithms/contrast.h b/src/ipa/ipu3/algorithms/contrast.h
> new file mode 100644
> index 00000000..958e8bb8
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/contrast.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google
> + *
> + * constrast.h - IPU3 Contrast and Gamma control
> + */
> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_CONTRAST_H__
> +#define __LIBCAMERA_IPU3_ALGORITHMS_CONTRAST_H__
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3::algorithms {
> +
> +class Contrast : public Algorithm
> +{
> +public:
> + Contrast();
> + ~Contrast() = default;
> +
> + int initialise(IPAContext &context) override;
> +
> +private:
> + double gamma_;
> +};
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_CONTRAST_H__ */
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index 67148333..f71d1e61 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: CC0-1.0
>
> ipu3_ipa_algorithms = files([
> + 'contrast.cpp',
> ])
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 55c4da72..7035802f 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -23,6 +23,7 @@
> #include "libcamera/internal/framebuffer.h"
>
> #include "algorithms/algorithm.h"
> +#include "algorithms/contrast.h"
> #include "ipa_context.h"
>
> #include "ipu3_agc.h"
> @@ -103,6 +104,9 @@ int IPAIPU3::init(const IPASettings &settings)
> return -ENODEV;
> }
>
> + /* Construct our Algorithms */
> + algorithms_.emplace_back(new algorithms::Contrast());
> +
> /* Reset all the hardware settings */
> context_.params = {};
>
> @@ -290,7 +294,7 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> {
> if (agcAlgo_->updateControls())
> - awbAlgo_->updateWbParameters(context_.params, agcAlgo_->gamma());
> + awbAlgo_->updateWbParameters(context_.params);
>
> *params = context_.params;
>
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> index 6253ab94..733814dd 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 e3e1d28b..3b7f781e 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/ipu3_agc.h
> @@ -37,8 +37,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);
> @@ -54,7 +52,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 9b409c8f..043c3838 100644
> --- a/src/ipa/ipu3/ipu3_awb.cpp
> +++ b/src/ipa/ipu3/ipu3_awb.cpp
> @@ -134,31 +134,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()
> {
> @@ -198,10 +173,6 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, 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);
> }
>
> @@ -351,7 +322,7 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> }
> }
>
> -void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms, double agcGamma)
> +void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms)
> {
> /*
> * Green gains should not be touched and considered 1.
> @@ -363,18 +334,10 @@ void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms, 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 284e3844..8b05ac03 100644
> --- a/src/ipa/ipu3/ipu3_awb.h
> +++ b/src/ipa/ipu3/ipu3_awb.h
> @@ -32,7 +32,7 @@ public:
>
> void initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid);
> void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> - void updateWbParameters(ipu3_uapi_params ¶ms, double agcGamma);
> + void updateWbParameters(ipu3_uapi_params ¶ms);
>
> struct Ipu3AwbCell {
> unsigned char greenRedAvg;
>
More information about the libcamera-devel
mailing list