[libcamera-devel] [PATCH v5 3/4] ipa: ipu3: Add support for IPU3 AWB algorithm
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Mon Apr 19 15:52:26 CEST 2021
Hi Kieran,
On 19/04/2021 13:01, Kieran Bingham wrote:
> Hi JM,
>
> On 16/04/2021 08:49, Jean-Michel Hautbois wrote:
>> The IPA will locally modify the parameters before they are passed down
>> to the ImgU. Use a local parameter object to give a reference to those
>> algorithms.
>>
>> Inherit from the Algorithm class to implement basic AWB functions.
>>
>> The configure() call will set exposure and gain to their minimum value,
>> so while AGC is not there, the frames will be dark.
>>
>> Once AWB is done, a color temperature is estimated and a default CCM matrix
>> will be used (yet to be tuned).
>> Implement a basic "grey-world" AWB algorithm just for demonstration purpose.
>>
>> The BDS output size is passed by the pipeline handler to the IPA.
>> The best grid is then calculated to maximize the number of pixels taken
>> into account in each cells.
>>
>> As commented in the source code, it can be improved, as it has (at least)
>> one limitation: if a cell is big (say 128 pixels wide) and indicated as
>> saturated, it won't be taken into account at all.
>> Maybe is it possible to have a smaller one, at the cost of a few pixels
>> to lose, in which case we can center the grid using the x_start and
>> y_start parameters.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>> src/ipa/ipu3/ipu3.cpp | 86 +++++++++-
>> src/ipa/ipu3/ipu3_awb.cpp | 351 ++++++++++++++++++++++++++++++++++++++
>> src/ipa/ipu3/ipu3_awb.h | 91 ++++++++++
>> src/ipa/ipu3/meson.build | 7 +-
>> 4 files changed, 526 insertions(+), 9 deletions(-)
>> create mode 100644 src/ipa/ipu3/ipu3_awb.cpp
>> create mode 100644 src/ipa/ipu3/ipu3_awb.h
>>
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index 34a907f2..ab052a8a 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -21,6 +21,11 @@
>> #include "libcamera/internal/buffer.h"
>> #include "libcamera/internal/log.h"
>>
>> +#include "ipu3_awb.h"
>> +
>> +static constexpr uint32_t kMaxCellWidthPerSet = 160;
>> +static constexpr uint32_t kMaxCellHeightPerSet = 80;
>> +
>> namespace libcamera {
>>
>> LOG_DEFINE_CATEGORY(IPAIPU3)
>> @@ -49,6 +54,7 @@ private:
>> const ipu3_uapi_stats_3a *stats);
>>
>> void setControls(unsigned int frame);
>> + void calculateBdsGrid(const Size &bdsOutputSize);
>>
>> std::map<unsigned int, MappedFrameBuffer> buffers_;
>>
>> @@ -61,6 +67,14 @@ private:
>> uint32_t gain_;
>> uint32_t minGain_;
>> uint32_t maxGain_;
>> +
>> + /* Interface to the AWB algorithm */
>> + std::unique_ptr<ipa::IPU3Awb> awbAlgo_;
>> +
>> + /* Local parameter storage */
>> + struct ipu3_uapi_params params_;
>> +
>> + struct ipu3_uapi_grid_config bdsGrid_;
>> };
>>
>> int IPAIPU3::start()
>> @@ -70,8 +84,58 @@ int IPAIPU3::start()
>> return 0;
>> }
>>
>> +/**
>> + * This method calculates a grid for the AWB algorithm in the IPU3 firmware.
>> + * Its input is the BDS output size calculated in the ImgU.
>> + * It is limited for now to the simplest method: find the lesser error
>> + * with the width/height and respective log2 width/height of the cells.
>> + *
>> + * \todo The frame is divided into cells which can be 8x8 => 128x128.
>> + * As a smaller cell improves the algorithm precision, adapting the
>> + * x_start and y_start parameters of the grid would provoke a loss of
>> + * some pixels but would also result in more accurate algorithms.
>> + */
>> +void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)
>> +{
>> + uint32_t minError = std::numeric_limits<uint32_t>::max();
>> + Size best;
>> + Size bestLog2;
>> + bdsGrid_ = {};
>> +
>> + for (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {
>> + uint32_t width = std::min(kMaxCellWidthPerSet,
>> + bdsOutputSize.width >> widthShift);
>> + width = width << widthShift;
>> + for (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) {
>> + int32_t height = std::min(kMaxCellHeightPerSet,
>> + bdsOutputSize.height >> heightShift);
>> + height = height << heightShift;
>> + uint32_t error = std::abs(static_cast<int>(width - bdsOutputSize.width))
>> + + std::abs(static_cast<int>(height - bdsOutputSize.height));
>> +
>> + if (error > minError)
>> + continue;
>> +
>> + minError = error;
>> + best.width = width;
>> + best.height = height;
>> + bestLog2.width = widthShift;
>> + bestLog2.height = heightShift;
>> + }
>> + }
>> +
>> + bdsGrid_.width = best.width >> bestLog2.width;
>> + bdsGrid_.block_width_log2 = bestLog2.width;
>> + bdsGrid_.height = best.height >> bestLog2.height;
>> + bdsGrid_.block_height_log2 = bestLog2.height;
>> +
>> + LOG(IPAIPU3, Debug) << "Best grid found is: ("
>> + << (int)bdsGrid_.width << " << " << (int)bdsGrid_.block_width_log2 << ") x ("
>> + << (int)bdsGrid_.height << " <<" << (int)bdsGrid_.block_height_log2 << ")";
>
> Somehow there's still an odd spacing around those << ;-)
>
>> +}
>> +
>> void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,
>> - [[maybe_unused]] const Size &bdsOutputSize)
>> + const Size &bdsOutputSize)
>> {
>> if (entityControls.empty())
>> return;
>> @@ -92,11 +156,18 @@ void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls
>>
>> minExposure_ = std::max(itExp->second.min().get<int32_t>(), 1);
>> maxExposure_ = itExp->second.max().get<int32_t>();
>> - exposure_ = maxExposure_;
>> + exposure_ = minExposure_;
>>
>> minGain_ = std::max(itGain->second.min().get<int32_t>(), 1);
>> maxGain_ = itGain->second.max().get<int32_t>();
>> - gain_ = maxGain_;
>> + gain_ = minGain_;
>> +
>> + params_ = {};
>> +
>> + calculateBdsGrid(bdsOutputSize);
>> +
>> + awbAlgo_ = std::make_unique<ipa::IPU3Awb>();
>> + awbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);
>> }
>>
>> void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)
>> @@ -168,10 +239,10 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
>>
>> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
>> {
>> - /* Prepare parameters buffer. */
>> - memset(params, 0, sizeof(*params));
>> + /* Pass a default gamma of 1.0 (default linear correction) */
>> + awbAlgo_->updateWbParameters(params_, 1.0);
>>
>> - /* \todo Fill in parameters buffer. */
>> + *params = params_;
>>
>> ipa::ipu3::IPU3Action op;
>> op.op = ipa::ipu3::ActionParamFilled;
>> @@ -184,8 +255,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>> {
>> ControlList ctrls(controls::controls);
>>
>> - /* \todo React to statistics and update internal state machine. */
>> - /* \todo Add meta-data information to ctrls. */
>> + awbAlgo_->calculateWBGains(stats);
>>
>> ipa::ipu3::IPU3Action op;
>> op.op = ipa::ipu3::ActionMetadataReady;
>> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
>> new file mode 100644
>> index 00000000..b4920e4b
>> --- /dev/null
>> +++ b/src/ipa/ipu3/ipu3_awb.cpp
>> @@ -0,0 +1,351 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2021, Ideas On Board
>> + *
>> + * ipu3_awb.cpp - AWB control algorithm
>> + */
>> +#include "ipu3_awb.h"
>> +
>> +#include <cmath>
>> +#include <numeric>
>> +#include <unordered_map>
>> +
>> +#include "libcamera/internal/log.h"
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa {
>> +
>> +LOG_DEFINE_CATEGORY(IPU3Awb)
>> +
>> +/**
>> + * \struct IspStatsRegion
>> + * \brief RGB statistics for a given region
>> + *
>> + * The IspStatsRegion structure is intended to abstract the ISP specific
>> + * statistics and use an agnostic algorithm to compute AWB.
>> + *
>> + * \var IspStatsRegion::counted
>> + * \brief Number of pixels used to calculate the sums
>> + *
>> + * \var IspStatsRegion::notcounted
>
> should this be notCounted ? (or uncounted to be a single word ?)
OK, I voted for uncounted ;-).
>> + * \brief Remaining number of pixels in the region
>> + *
>> + * \var IspStatsRegion::rSum
>> + * \brief Sum of the red values in the region
>> + *
>> + * \var IspStatsRegion::gSum
>> + * \brief Sum of the green values in the region
>> + *
>> + * \var IspStatsRegion::bSum
>> + * \brief Sum of the blue values in the region
>> + */
>> +
>> +/**
>> + * \struct AwbStatus
>> + * \brief AWB parameters calculated
>> + *
>> + * The AwbStatus structure is intended to store the AWB
>> + * parameters calculated by the algorithm
>> + *
>> + * \var AwbStatus::temperatureK
>> + * \brief Color temperature calculated
>> + *
>> + * \var AwbStatus::redGain
>> + * \brief Gain calculated for the red channel
>> + *
>> + * \var AwbStatus::greenGain
>> + * \brief Gain calculated for the green channel
>> + *
>> + * \var AwbStatus::blueGain
>> + * \brief Gain calculated for the blue channel
>> + */
>> +
>> +/**
>> + * \struct Ipu3AwbCell
>> + * \brief Memory layout for each cell in AWB metadata
>> + *
>> + * The Ipu3AwbCell structure is used to get individual values
>> + * such as red average or saturation ratio in a particular cell.
>> + *
>> + * \var Ipu3AwbCell::greenRedAvg
>> + * \brief Green average for red lines in the cell
>> + *
>> + * \var Ipu3AwbCell::redAvg
>> + * \brief Red average in the cell
>> + *
>> + * \var Ipu3AwbCell::blueAvg
>> + * \brief blue average in the cell
>> + *
>> + * \var Ipu3AwbCell::greenBlueAvg
>> + * \brief Green average for blue lines
>> + *
>> + * \var Ipu3AwbCell::satRatio
>> + * \brief Saturation ratio in the cell
>> + *
>> + * \var Ipu3AwbCell::padding
>> + * \brief array of unused bytes for padding
>> + */
>> +
>> +/* Default settings for Bayer noise reduction replicated from the Kernel */
>> +static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>> + .wb_gains = { 16, 16, 16, 16 },
>> + .wb_gains_thr = { 255, 255, 255, 255 },
>> + .thr_coeffs = { 1700, 0, 31, 31, 0, 16 },
>> + .thr_ctrl_shd = { 26, 26, 26, 26 },
>> + .opt_center{ -648, 0, -366, 0 },
>> + .lut = {
>> + { 17, 23, 28, 32, 36, 39, 42, 45,
>> + 48, 51, 53, 55, 58, 60, 62, 64,
>> + 66, 68, 70, 72, 73, 75, 77, 78,
>> + 80, 82, 83, 85, 86, 88, 89, 90 } },
>> + .bp_ctrl = { 20, 0, 1, 40, 0, 6, 0, 6, 0 },
>> + .dn_detect_ctrl{ 9, 3, 4, 0, 8, 0, 1, 1, 1, 1, 0 },
>> + .column_size = 1296,
>> + .opt_center_sqr = { 419904, 133956 },
>> +};
>> +
>> +/* Default settings for Auto White Balance replicated from the Kernel*/
>> +static const struct ipu3_uapi_awb_config_s imguCssAwbDefaults = {
>> + .rgbs_thr_gr = 8191,
>> + .rgbs_thr_r = 8191,
>> + .rgbs_thr_gb = 8191,
>> + .rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT,
>> + .grid = {
>> + .width = 160,
>> + .height = 36,
>> + .block_width_log2 = 3,
>> + .block_height_log2 = 4,
>> + .height_per_slice = 1, /* Overridden by kernel. */
>> + .x_start = 0,
>> + .y_start = 0,
>> + .x_end = 0,
>> + .y_end = 0,
>> + },
>> +};
>> +
>> +/* Default color correction matrix defined as an identity matrix */
>> +static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = {
>> + 8191, 0, 0, 0,
>> + 0, 8191, 0, 0,
>> + 0, 0, 8191, 0
>> +};
>> +
>> +IPU3Awb::IPU3Awb()
>> + : Algorithm()
>> +{
>> + asyncResults_.blueGain = 1.0;
>> + asyncResults_.greenGain = 1.0;
>> + asyncResults_.redGain = 1.0;
>> + asyncResults_.temperatureK = 4500;
>> +}
>> +
>> +IPU3Awb::~IPU3Awb()
>> +{
>> +}
>> +
>> +void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid)
>> +{
>> + params.use.acc_awb = 1;
>> + params.acc_param.awb.config = imguCssAwbDefaults;
>> +
>> + awbGrid_ = bdsGrid;
>> + params.acc_param.awb.config.grid = awbGrid_;
>> +
>> + params.use.acc_bnr = 1;
>> + params.acc_param.bnr = imguCssBnrDefaults;
>> + /**
>> + * Optical center is colum (resp row) start - X (resp Y) center.
>
> s/colum/column/
>
> What is resp?
respectively ?
>
>> + * For the moment use BDS as a first approximation, but it should
>> + * be calculated based on Shading (SHD) parameters.
>> + */
>> + params.acc_param.bnr.column_size = bdsOutputSize.width;
>> + params.acc_param.bnr.opt_center.x_reset = awbGrid_.x_start - (bdsOutputSize.width / 2);
>> + params.acc_param.bnr.opt_center.y_reset = awbGrid_.y_start - (bdsOutputSize.height / 2);
>> + params.acc_param.bnr.opt_center_sqr.x_sqr_reset = params.acc_param.bnr.opt_center.x_reset
>> + * params.acc_param.bnr.opt_center.x_reset;
>> + params.acc_param.bnr.opt_center_sqr.y_sqr_reset = params.acc_param.bnr.opt_center.y_reset
>> + * params.acc_param.bnr.opt_center.y_reset;
>> +
>> + params.use.acc_ccm = 1;
>> + params.acc_param.ccm = imguCssCcmDefault;
>> +
>> + params.use.acc_gamma = 1;
>> + params.acc_param.gamma.gc_ctrl.enable = 1;
>> +
>> + zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
>> +}
>> +
>> +/**
>> + * The function estimates the correlated color temperature using
>> + * from RGB color space input.
>> + * In physics and color science, the Planckian locus or black body locus is
>> + * the path or locus that the color of an incandescent black body would take
>> + * in a particular chromaticity space as the blackbody temperature changes.
>> + *
>> + * If a narrow range of color temperatures is considered (those encapsulating
>> + * daylight being the most practical case) one can approximate the Planckian
>> + * locus in order to calculate the CCT in terms of chromaticity coordinates.
>> + *
>> + * More detailed information can be found in:
>> + * https://en.wikipedia.org/wiki/Color_temperature#Approximation
>> + */
>> +uint32_t IPU3Awb::estimateCCT(double red, double green, double blue)
>> +{
>> + /* Convert the RGB values to CIE tristimulus values (XYZ) */
>> + double X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);
>> + double Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);
>> + double Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);
>> +
>> + /* Calculate the normalized chromaticity values */
>> + double x = X / (X + Y + Z);
>> + double y = Y / (X + Y + Z);
>> +
>> + /* Calculate CCT */
>> + double n = (x - 0.3320) / (0.1858 - y);
>> + return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;
>> +}
>> +
>> +/* Generate a RGB vector with the average values for each region */
>
> s/a/an/
>
>
>> +void IPU3Awb::generateZones(std::vector<RGB> &zones)
>> +{
>> + for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {
>> + RGB zone;
>> + double counted = awbStats_[i].counted;
>> + if (counted >= 16) {
>> + zone.G = awbStats_[i].gSum / counted;
>> + if (zone.G >= 32) {
>
> What is the selection criteria or meaning behind 16 and 32 here?
>
> Perhaps we should define them as documented constexpr's at the top of
> the function?
Indeed the values are taken from RPi code, but it should be calculated
based on the original grid size.
>
>> + zone.R = awbStats_[i].rSum / counted;
>> + zone.B = awbStats_[i].bSum / counted;
>> + zones.push_back(zone);
>> + }
>> + }
>> + }
>> +}
>> +
>> +/* Translate the IPU3 statistics into the default statistics region array */
>> +void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
>> +{
>> + uint32_t regionWidth = round(awbGrid_.width / static_cast<double>(kAwbStatsSizeX));
>> + uint32_t regionHeight = round(awbGrid_.height / static_cast<double>(kAwbStatsSizeY));
>> +
>> + /**
>
> only /* needed here.
>
> /** is only for doxygen comments
>
>
>> + * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
>> + * (awbGrid_.width x awbGrid_.height).
>> + */
>> + for (unsigned int j = 0; j < kAwbStatsSizeY * regionHeight; j++) {
>> + for (unsigned int i = 0; i < kAwbStatsSizeX * regionWidth; i++) {
>> + uint32_t cellPosition = j * awbGrid_.width + i;
>> + uint32_t cellX = (cellPosition / regionWidth) % kAwbStatsSizeX;
>> + uint32_t cellY = ((cellPosition / awbGrid_.width) / regionHeight) % kAwbStatsSizeY;
>> +
>> + uint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX;
>> + cellPosition *= 8;
>> +
>> + /* Cast the initial IPU3 structure to simplify the reading */
>> + Ipu3AwbCell *currentCell = reinterpret_cast<Ipu3AwbCell *>(const_cast<uint8_t *>(&stats->awb_raw_buffer.meta_data[cellPosition]));
>> + if (currentCell->satRatio == 0) {
>> + /* The cell is not saturated, use the current cell */
>> + awbStats_[awbRegionPosition].counted++;
>> + uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg;
>> + awbStats_[awbRegionPosition].gSum += greenValue / 2;
>> + awbStats_[awbRegionPosition].rSum += currentCell->redAvg;
>> + awbStats_[awbRegionPosition].bSum += currentCell->blueAvg;
>> + }
>> + }
>> + }
>> +}
>> +
>> +void IPU3Awb::clearAwbStats()
>> +{
>> + for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {
>> + awbStats_[i].bSum = 0;
>> + awbStats_[i].rSum = 0;
>> + awbStats_[i].gSum = 0;
>> + awbStats_[i].counted = 0;
>> + awbStats_[i].notcounted = 0;
>> + }
>> +}
>> +
>> +void IPU3Awb::awbGrey()
>
> awbGreyWorld?
>
>> +{
>> + LOG(IPU3Awb, Debug) << "Grey world AWB";
>> + /**
>
> /*
>
>> + * Make a separate list of the derivatives for each of red and blue, so
>> + * that we can sort them to exclude the extreme gains. We could
>
> s/ / /
> (double space to single before We)
>
>> + * consider some variations, such as normalising all the zones first, or
>> + * doing an L2 average etc.
>> + */
>> + std::vector<RGB> &redDerivative(zones_);
>
> Is the & intentional here?
>
> I'm a little confused as to what that's doing. Creating a local reference?
>
>> + std::vector<RGB> blueDerivative(redDerivative);
>
> Is this equivalent to blueDerivative(zones_) ? or is there a specific
> reason to initiliase it to the redDerivative?
>
> I don't expect there's much difference between either, but initialising
> both from zones_ would look more obvious that they are initialised from
> the same dataset.
Again, this is from RPi and I think it is optimizing an allocation (one
new/delete less) per call at lease.
You have a reference for the red values, and blue is using the reference
too.
I don't think there is a difference in the implementation by doing:
std::vector<RGB> redDerivative(zones_);
std::vector<RGB> blueDerivative(zones_);
But it may be lighter.
>
>> + std::sort(redDerivative.begin(), redDerivative.end(),
>> + [](RGB const &a, RGB const &b) {
>> + return a.G * b.R < b.G * a.R;
>> + });
>> + std::sort(blueDerivative.begin(), blueDerivative.end(),
>> + [](RGB const &a, RGB const &b) {
>> + return a.G * b.B < b.G * a.B;
>> + });
>> +
>> + /* Average the middle half of the values. */
>> + int discard = redDerivative.size() / 4;
>> + RGB sumRed(0, 0, 0), sumBlue(0, 0, 0);
>
> Hrm ... Perhaps those are better on two lines. ... Not sure, it doesn't
> hurt but I haven't seen us use single line multiple constructs much I
> don't think.
>
>
>> + for (auto ri = redDerivative.begin() + discard,
>> + bi = blueDerivative.begin() + discard;
>> + ri != redDerivative.end() - discard; ri++, bi++)
>> + sumRed += *ri, sumBlue += *bi;
>> +
>> + double redGain = sumRed.G / (sumRed.R + 1),
>> + blueGain = sumBlue.G / (sumBlue.B + 1);
>> +
>> + /* Color temperature is not relevant in Gray world but still useful to estimate it :-) */
>
> Gray or Grey?
>
>
>> + asyncResults_.temperatureK = estimateCCT(sumRed.R, sumRed.G, sumBlue.B);
>> + asyncResults_.redGain = redGain;
>> + asyncResults_.greenGain = 1.0;
>> + asyncResults_.blueGain = blueGain;
>
> Are these results really calculated asynchronously already?
> Or is that a future improvement?
Good catch, they are calculated in the same thread, so not really async
no...
>
>> +}
>> +
>> +void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
>> +{
>> + ASSERT(stats->stats_3a_status.awb_en);
>> + zones_.clear();
>> + clearAwbStats();
>> + generateAwbStats(stats);
>> + generateZones(zones_);
>> + LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
>> + if (zones_.size() > 10)
>> + awbGrey();
>> +
>> + LOG(IPU3Awb, Debug) << "Gain found for red: " << asyncResults_.redGain
>> + << " and for blue: " << asyncResults_.blueGain;
>
> Should the debug print be in the context scope of the if (zones_... ) ?
>
> It won't change otherwise will it?
>
>
>> +}
>> +
>> +void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms, double agcGamma)
>> +{
>> + /**
>
> /*
>
>> + * Green gains should not be touched and considered 1.
>> + * Default is 16, so do not change it at all.
>> + * 4096 is the value for a gain of 1.0
>> + */
>> + params.acc_param.bnr.wb_gains.gr = 16;
>> + params.acc_param.bnr.wb_gains.r = 4096 * asyncResults_.redGain;
>> + 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;
>> +
>> + /* 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 */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/ipu3_awb.h
>> new file mode 100644
>> index 00000000..0994d18d
>> --- /dev/null
>> +++ b/src/ipa/ipu3/ipu3_awb.h
>> @@ -0,0 +1,91 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2021, Ideas On Board
>> + *
>> + * ipu3_awb.h - IPU3 AWB control algorithm
>> + */
>> +#ifndef __LIBCAMERA_IPU3_AWB_H__
>> +#define __LIBCAMERA_IPU3_AWB_H__
>> +
>> +#include <vector>
>> +
>> +#include <linux/intel-ipu3.h>
>> +
>> +#include <libcamera/geometry.h>
>> +
>> +#include "libipa/algorithm.h"
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa {
>> +
>> +/* Region size for the statistics generation algorithm */
>> +static constexpr uint32_t kAwbStatsSizeX = 16;
>> +static constexpr uint32_t kAwbStatsSizeY = 12;
>> +
>> +class IPU3Awb : public Algorithm
>> +{
>> +public:
>> + IPU3Awb();
>> + ~IPU3Awb();
>> +
>> + 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);
>> +
>> + struct Ipu3AwbCell {
>> + unsigned char greenRedAvg;
>> + unsigned char redAvg;
>> + unsigned char blueAvg;
>> + unsigned char greenBlueAvg;
>> + unsigned char satRatio;
>> + unsigned char padding[3];
>> + } __attribute__((packed));
>> +
>> + /* \todo Make these three structs available to all the ISPs ? */
>
> Interesting, we might indeed want some colour helpers.
> But this can be later.
>
>> + struct RGB {
>> + RGB(double _R = 0, double _G = 0, double _B = 0)
>> + : R(_R), G(_G), B(_B)
>> + {
>> + }
>> + double R, G, B;
>> + RGB &operator+=(RGB const &other)
>> + {
>> + R += other.R, G += other.G, B += other.B;
>> + return *this;
>> + }
>> + };
>> +
>> + struct IspStatsRegion {
>> + unsigned int counted;
>> + unsigned int notcounted;
>
> mentioned above, this might be 'uncounted'
>
> All my comments here seem fairly trivial.
>
> with those:
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Thanks !
>
>> + unsigned long long rSum;
>> + unsigned long long gSum;
>> + unsigned long long bSum;
>> + };
>> +
>> + struct AwbStatus {
>> + double temperatureK;
>> + double redGain;
>> + double greenGain;
>> + double blueGain;
>> + };
>> +
>> +private:
>> + void generateZones(std::vector<RGB> &zones);
>> + void generateAwbStats(const ipu3_uapi_stats_3a *stats);
>> + void clearAwbStats();
>> + void awbGrey();
>> + uint32_t estimateCCT(double red, double green, double blue);
>> +
>> + struct ipu3_uapi_grid_config awbGrid_;
>> +
>> + std::vector<RGB> zones_;
>> + IspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
>> + AwbStatus asyncResults_;
>> +};
>> +
>> +} /* namespace ipa */
>> +
>> +} /* namespace libcamera*/
>> +#endif /* __LIBCAMERA_IPU3_AWB_H__ */
>> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
>> index a241f617..1040698e 100644
>> --- a/src/ipa/ipu3/meson.build
>> +++ b/src/ipa/ipu3/meson.build
>> @@ -2,8 +2,13 @@
>>
>> ipa_name = 'ipa_ipu3'
>>
>> +ipu3_ipa_sources = files([
>> + 'ipu3.cpp',
>> + 'ipu3_awb.cpp',
>> +])
>> +
>> mod = shared_module(ipa_name,
>> - ['ipu3.cpp', libcamera_generated_ipa_headers],
>> + [ipu3_ipa_sources, libcamera_generated_ipa_headers],
>> name_prefix : '',
>> include_directories : [ipa_includes, libipa_includes],
>> dependencies : libcamera_dep,
>>
>
More information about the libcamera-devel
mailing list