[libcamera-devel] [PATCH v3 6/9] ipa: ipu3: convert AWB to the new algorithm interface
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Thu Aug 19 13:43:46 CEST 2021
Hi Kieran,
On 18/08/2021 23:39, Kieran Bingham wrote:
> On 18/08/2021 16:54, Jean-Michel Hautbois wrote:
>> When the stats are received, pass it with the context to the existing
>> AWB algorithm. IPAFrameContext now has a new structure to store the
>> gains calculated by the AWB algorithm.
>>
>> When an EventFillParams event is received, call prepare() and set the new
>> gains accordingly in the params structure.
>> There is no more a need for the IPU3Awb::initialise() function, as the
>> params are always set in prepare().
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>> src/ipa/ipu3/ipa_context.h | 8 +++
>> src/ipa/ipu3/ipu3.cpp | 5 +-
>> src/ipa/ipu3/ipu3_awb.cpp | 107 ++++++++++++++++++-------------------
>> src/ipa/ipu3/ipu3_awb.h | 6 +--
>> 4 files changed, 65 insertions(+), 61 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index 5964ba6d..beae3ac6 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -29,6 +29,14 @@ struct IPAFrameContext {
>> double gain;
>> } agc;
>>
>> + struct Awb {
>> + struct Gains {
>> + double red;
>> + double green;
>> + double blue;
>> + } gains;
>> + } awb;
>> +
>> struct Contrast {
>> struct ipu3_uapi_gamma_corr_lut gammaCorrection;
>> } contrast;
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index ee0dd9fe..2b4a4c8c 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -339,7 +339,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
>> }
>>
>> awbAlgo_ = std::make_unique<IPU3Awb>();
>> - awbAlgo_->initialise(params_, context_.configuration.grid.bdsOutputSize, context_.configuration.grid.bdsGrid);
>>
>> agcAlgo_ = std::make_unique<IPU3Agc>();
>> agcAlgo_->initialise(context_.configuration.grid.bdsGrid, sensorInfo_);
>> @@ -420,7 +419,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
>> algo->prepare(context_, params_);
>>
>> if (agcAlgo_->updateControls())
>> - awbAlgo_->updateWbParameters(params_);
>> + awbAlgo_->prepare(context_, params_);
>>
>> *params = params_;
>>
>> @@ -446,7 +445,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>> exposure_ = context_.frameContext.agc.exposure;
>> gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
>>
>> - awbAlgo_->calculateWBGains(stats);
>> + awbAlgo_->process(context_, stats);
>>
>> if (agcAlgo_->updateControls())
>> setControls(frame);
>> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
>> index c2d9e0c1..f90aa492 100644
>> --- a/src/ipa/ipu3/ipu3_awb.cpp
>> +++ b/src/ipa/ipu3/ipu3_awb.cpp
>> @@ -107,25 +107,6 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>> .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,
>> @@ -140,39 +121,12 @@ IPU3Awb::IPU3Awb()
>> asyncResults_.greenGain = 1.0;
>> asyncResults_.redGain = 1.0;
>> asyncResults_.temperatureK = 4500;
>> -}
>>
>> -IPU3Awb::~IPU3Awb()
>> -{
>> + zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
>> }
>>
>> -void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid)
>> +IPU3Awb::~IPU3Awb()
>> {
>> - params.use.acc_awb = 1;
>> - params.acc_param.awb.config = imguCssAwbDefaults;
>> -
>> - awbGrid_ = bdsGrid;
>> - params.acc_param.awb.config.grid = awbGrid_;
>
> I don't think I see acc_param.awb.config.grid
>
>> -
>> - params.use.acc_bnr = 1;
>> - params.acc_param.bnr = imguCssBnrDefaults;
>> - /**
>> - * Optical center is column (respectively row) startminus X (respectively Y) center.
>> - * 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;
>> -
>> - zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
>> }
>>
>> /**
>> @@ -321,22 +275,65 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
>> }
>> }
>>
>> -void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms)
>> +void IPU3Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>> {
>> + calculateWBGains(stats);
>> +
>> + /*
>> + * Gains are only recalculated if enough zones were detected.
>
> Check the spacing on this comment indents.
>
>> + * The results are cached, so if no results were calculated, we set the
>> + * cached values from asyncResults_ here.
>> + */
>> + context.frameContext.awb.gains.blue = asyncResults_.blueGain;
>> + context.frameContext.awb.gains.green = asyncResults_.greenGain;
>> + context.frameContext.awb.gains.red = asyncResults_.redGain;
>> +}
>> +
>> +void IPU3Awb::prepare(IPAContext &context, ipu3_uapi_params ¶ms)
>> +{
>> + params.acc_param.awb.config.rgbs_thr_gr = 8191;
>> + params.acc_param.awb.config.rgbs_thr_r = 8191;
>> + params.acc_param.awb.config.rgbs_thr_gb = 8191;
>> + params.acc_param.awb.config.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT;
>> +> + awbGrid_ = context.configuration.grid.bdsGrid;
>
> It looks like we can drop awbGrid_ now and just take a local reference
> here if you want to shorten lines?
>
Well, the grid is used in generateAwbStats() which is called at
process() call. I could pass it a grid using a reference to the context
too...
>
>> +
>> + params.acc_param.awb.config.grid = context.configuration.grid.bdsGrid;
>
> Separate each component (awb/bnr/ccm), so at least a new line here.
>
>
>> + params.acc_param.bnr = imguCssBnrDefaults;
>> +
>> + /*
>> + * Optical center is column start (respectively row start) of the
>> + * region of interest minus its X center (respectively Y center).
>> + *
>> + * For the moment use BDS as a first approximation, but it should
>> + * be calculated based on Shading (SHD) parameters.
>> + */
>> + Size &bdsOutputSize = context.configuration.grid.bdsOutputSize;
>> + 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.acc_param.ccm = imguCssCcmDefault;
>
> Can we avoid mixing CCM in the middle of the BNR?
>
> Can it just be set last? Is the comment that got removed below no longer
> relevant? or should it be updated as a todo at all?
>
>
>
>> +
>> /*
>> * 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;
>> + params.acc_param.bnr.wb_gains.gr = 16 * context.frameContext.awb.gains.green;
>> + params.acc_param.bnr.wb_gains.r = 4096 * context.frameContext.awb.gains.red;
>> + params.acc_param.bnr.wb_gains.b = 4096 * context.frameContext.awb.gains.blue;
>> + params.acc_param.bnr.wb_gains.gb = 16 * context.frameContext.awb.gains.green;
>>
>> LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK;
>>
>> - /* The CCM matrix may change when color temperature will be used */
>> - params.acc_param.ccm = imguCssCcmDefault;
>> + params.use.acc_awb = 1;
>> + params.use.acc_bnr = 1;
>> + params.use.acc_ccm = 1;
>> }
>>
>> } /* namespace ipa::ipu3 */
>> diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/ipu3_awb.h
>> index eeb2920b..4de3fae2 100644
>> --- a/src/ipa/ipu3/ipu3_awb.h
>> +++ b/src/ipa/ipu3/ipu3_awb.h
>> @@ -29,9 +29,8 @@ 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);
>> + void prepare(IPAContext &context, ipu3_uapi_params ¶ms) override;
>> + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
>>
>> struct Ipu3AwbCell {
>> unsigned char greenRedAvg;
>> @@ -72,6 +71,7 @@ public:
>> };
>>
>> private:
>> + void calculateWBGains(const ipu3_uapi_stats_3a *stats);
>> void generateZones(std::vector<RGB> &zones);
>> void generateAwbStats(const ipu3_uapi_stats_3a *stats);
>> void clearAwbStats();
>>
More information about the libcamera-devel
mailing list