[libcamera-devel] [PATCH v2] ipa: ipu3: agc: Remove the threshold for the histogram calculation

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Tue, Nov 16, 2021 at 04:24:44PM +0100, Jean-Michel Hautbois wrote:
> Until commit f8f07f9468c6 (ipa: ipu3: agc: Improve gain calculation)
> the gain to apply on the exposure value was only using the histogram.
> Now that the global brightness of the frame is estimated too, we don't
> need to remove part of the saturated pixels from the equation anymore.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

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

> ---
> v2: - remove the empty histogram test
>     - don't take Kieran's R-B tag as it needs testing again
> 
> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 32 +++++++++-----------------------
>  1 file changed, 9 insertions(+), 23 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 4e424857..bd02c474 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -58,12 +58,6 @@ static constexpr uint32_t knumHistogramBins = 256;
>  /* Target value to reach for the top 2% of the histogram */
>  static constexpr double kEvGainTarget = 0.5;
>  
> -/*
> - * Maximum ratio of saturated pixels in a cell for the cell to be considered
> - * non-saturated and counted by the AGC algorithm.
> - */
> -static constexpr uint32_t kMinCellsPerZoneRatio = 255 * 20 / 100;
> -
>  /* Number of frames to wait before calculating stats on minimum exposure */
>  static constexpr uint32_t kNumStartupFrames = 10;
>  
> @@ -133,27 +127,19 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>  					&stats->awb_raw_buffer.meta_data[cellPosition]
>  				);
>  
> -			if (cell->sat_ratio <= kMinCellsPerZoneRatio) {
> -				uint8_t gr = cell->Gr_avg;
> -				uint8_t gb = cell->Gb_avg;
> -				/*
> -				 * Store the average green value to estimate the
> -				 * brightness. Even the overexposed pixels are
> -				 * taken into account.
> -				 */
> -				hist[(gr + gb) / 2]++;
> -			}
> +			uint8_t gr = cell->Gr_avg;
> +			uint8_t gb = cell->Gb_avg;
> +			/*
> +			 * Store the average green value to estimate the
> +			 * brightness. Even the overexposed pixels are
> +			 * taken into account.
> +			 */
> +			hist[(gr + gb) / 2]++;
>  		}
>  	}
>  
> -	Histogram cumulativeHist = Histogram(Span<uint32_t>(hist));
>  	/* Estimate the quantile mean of the top 2% of the histogram */
> -	if (cumulativeHist.total() == 0) {
> -		/* Force the value as histogram is empty */
> -		iqMean_ = knumHistogramBins - 0.5;
> -	} else {
> -		iqMean_ = cumulativeHist.interQuantileMean(0.98, 1.0);
> -	}
> +	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart