[libcamera-devel] [PATCH v2 06/10] ipa: ipu3: convert AWB to the new algorithm interface
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Aug 13 11:49:56 CEST 2021
On 12/08/2021 17:52, 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 byt the AWB algorithm.
s/byt/by/
>
> When a EventFillParams event is received, call prepare() and set the new
/When a/When an/
> 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>
> ---
> src/ipa/ipu3/ipa_context.h | 8 ++++
> src/ipa/ipu3/ipu3.cpp | 5 +--
> src/ipa/ipu3/ipu3_awb.cpp | 76 ++++++++++++++++++++------------------
> src/ipa/ipu3/ipu3_awb.h | 6 +--
> 4 files changed, 54 insertions(+), 41 deletions(-)
>
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index e995c663..7bb4f598 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -38,6 +38,14 @@ struct IPAFrameContext {
> double gain;
> } agc;
>
> + struct Awb {
> + struct Gains {
> + double redGain;
> + double greenGain;
> + double blueGain;
They're all in a structure called gains, can they be named simpler as
just 'red', 'green', 'blue' ?
> + } gains;
> + } awb;
> +
> struct Contrast {
> uint16_t lut[IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES];
> } contrast;
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index e09adfc3..dba5554d 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -230,7 +230,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_);
> @@ -311,7 +310,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_;
>
> @@ -337,7 +336,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> gain = context_.frameContext.agc.gain;
> gain_ = camHelper_->gainCode(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 0f3bcdc9..b422b095 100644
> --- a/src/ipa/ipu3/ipu3_awb.cpp
> +++ b/src/ipa/ipu3/ipu3_awb.cpp
> @@ -121,42 +121,14 @@ IPU3Awb::IPU3Awb()
> asyncResults_.greenGain = 1.0;
> asyncResults_.redGain = 1.0;
> asyncResults_.temperatureK = 4500;
> +
> + zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
> }
>
> 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.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_ = bdsGrid;
> -
> - 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);
> -}
> -
> /**
> * The function estimates the correlated color temperature using
> * from RGB color space input.
> @@ -303,17 +275,51 @@ 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);
I'd put a new line here.
Perhaps a comment here saying "
/*
* Gains are only recalculated if enough zones were detected.
* The results are cached, so if no results were calculated, we set the
* cached values from asyncResults_ here.
*/
> + context.frameContext.awb.gains.blueGain = asyncResults_.blueGain;
> + context.frameContext.awb.gains.greenGain = asyncResults_.greenGain;
> + context.frameContext.awb.gains.redGain = asyncResults_.redGain;> +}
> +
> +void IPU3Awb::prepare(IPAContext &context, ipu3_uapi_params ¶ms)
> {
> + params.use.acc_awb = 1;
Set enable/use flags last... - or in this instance, after setting the
awb parameters.
> + 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;
> +
> + params.use.acc_bnr = 1;
And this shows that there are modules to further break out of this...
Anything that has it's own 'module' on the IPU3 should have it's own
corresponding module in the code base I think.
> + params.acc_param.bnr = imguCssBnrDefaults;
new line?
> + /**
s#/**#/*#
> + * 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.
> + */
> + 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.use.acc_ccm = 1;
Same, but keep both BNR and CCM here in this patch, and they can move
out to their own modules separately.
Fairly trivial comments in this patch.
With those handled as best as you see fit,
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> + params.acc_param.ccm = imguCssCcmDefault;
> +
> /*
> * 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.greenGain;
> + params.acc_param.bnr.wb_gains.r = 4096 * context.frameContext.awb.gains.redGain;
> + params.acc_param.bnr.wb_gains.b = 4096 * context.frameContext.awb.gains.blueGain;
> + params.acc_param.bnr.wb_gains.gb = 16 * context.frameContext.awb.gains.greenGain;
>
> LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK;
>
> 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