[libcamera-devel,v2,03/13] ipa: ipu3: awb: Use saturation under 90%

14210 diff mbox series

The AWB grey world algorithm tries to find a grey value and it can't do
it on over-exposed images. To exclude those, the saturation ratio is
used for each cell, and the cell is included only if this ratio is 0.

Now that we have changed the threshold, more cells may be considered as
partially saturated and excluded, preventing the algorithm from running
efficiently.

Change that behaviour, and consider 90% as a good enough ratio.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/awb.cpp | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Hi Jean-Michel,

Thank you for the patch.

On Wed, Oct 20, 2021 at 05:45:57PM +0200, Jean-Michel Hautbois wrote:
> The AWB grey world algorithm tries to find a grey value and it can't do
> it on over-exposed images. To exclude those, the saturation ratio is
> used for each cell, and the cell is included only if this ratio is 0.
> 
> Now that we have changed the threshold, more cells may be considered as
> partially saturated and excluded, preventing the algorithm from running
> efficiently.
> 
> Change that behaviour, and consider 90% as a good enough ratio.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/awb.cpp | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> index e44bbd6c..62c72cce 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -19,6 +19,12 @@ LOG_DEFINE_CATEGORY(IPU3Awb)
>  
>  static constexpr uint32_t kMinGreenLevelInZone = 32;
>  
> +/* Minimum proportion of cells counted within a zone for it to be relevant */
> +static constexpr double kMinRelevantCellsRatio = 0.8;
> +
> +/* Number of cells below the saturation ratio */
> +static constexpr uint32_t kSaturationThreshold = 255 * 90 / 100;

Isn't this the ratio of saturated pixels in the cell, not the number of
cells ?

/*
 * Maximum ratio of saturated pixels in a cell for the cell to be considered
 * non-saturated and counted by the AWB algorithm.
 */
static constexpr uint32_t kMaxCellSaturationRatio = 255 * 90 / 100;

Maybe the first constant could then become

/*
 * Minimum proportion of non-saturated cells in a zone for the zone to be used
 * by the AWB algorithm.
 */
static constexpr double kMinCellsPerZoneRatio = 0.8;

(this matches the "cells per zone" from the cellsPerZoneThreshold_
variable below)

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

> +
>  /**
>   * \struct Accumulator
>   * \brief RGB statistics for a given zone
> @@ -160,7 +166,8 @@ int Awb::configure(IPAContext &context,
>  	 * for it to be relevant for the grey world algorithm.
>  	 * \todo This proportion could be configured.
>  	 */
> -	cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100;
> +	cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * kMinRelevantCellsRatio;
> +	LOG(IPU3Awb, Debug) << "Threshold for AWB is set to " << cellsPerZoneThreshold_;
>  
>  	return 0;
>  }
> @@ -234,7 +241,18 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
>  				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
>  					&stats->awb_raw_buffer.meta_data[cellPosition]
>  				);
> -			if (currentCell->sat_ratio == 0) {
> +
> +			/*
> +			 * Use cells which have less than 90%
> +			 * saturation as an initial means to include
> +			 * otherwise bright cells which are not fully
> +			 * saturated.
> +			 *
> +			 * \todo The 90% saturation rate may require
> +			 * further empirical measurements and
> +			 * optimisation during camera tuning phases.
> +			 */
> +			if (currentCell->sat_ratio <= kSaturationThreshold) {
>  				/* The cell is not saturated, use the current cell */
>  				awbStats_[awbZonePosition].counted++;
>  				uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;