[libcamera-devel,04/22] ipa: ipu3: agc: Limit the number of saturated cells

14474 diff mbox series

When the histogram is calculated, we check if a cell is saturated or not
before cumulating its green value. This is wrong, and it can lead to an
empty histogram in case of a fully saturated frame.

Use a constant to limit the amount of pixels within a cell before
considering it saturated. If at the end of the loop we still have an
empty histogram, then make it a fully saturated one.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=84
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Quoting Jean-Michel Hautbois (2021-11-08 13:13:32)
> When the histogram is calculated, we check if a cell is saturated or not
> before cumulating its green value. This is wrong, and it can lead to an
> empty histogram in case of a fully saturated frame.
> 
> Use a constant to limit the amount of pixels within a cell before
> considering it saturated. If at the end of the loop we still have an
> empty histogram, then make it a fully saturated one.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=84
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index b5d736c1..2bf68e04 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -63,6 +63,12 @@ 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;
> +
>  Agc::Agc()
>         : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
>           minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s),
> @@ -124,7 +130,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>                                         &stats->awb_raw_buffer.meta_data[cellPosition]
>                                 );
>  
> -                       if (cell->sat_ratio == 0) {
> +                       if (cell->sat_ratio <= kMinCellsPerZoneRatio) {
>                                 uint8_t gr = cell->Gr_avg;
>                                 uint8_t gb = cell->Gb_avg;
>                                 /*
> @@ -137,8 +143,14 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>                 }
>         }
>  
> +       Histogram cumulativeHist = Histogram(Span<uint32_t>(hist));
>         /* Estimate the quantile mean of the top 2% of the histogram */
> -       iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> +       if (cumulativeHist.total() == 0) {
> +               /* Force the value as histogram is empty */
> +               iqMean_ = knumHistogramBins - 0.5;
> +       } else {
> +               iqMean_ = cumulativeHist.interQuantileMean(0.98, 1.0);
> +       }

I wondered if this logic should go in the Histogram itself, but I think
it might be use case specific whether to fill artificially at the top or
bottom, or otherwise - so this is probably the best way to handle this.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  }
>  
>  /**
> -- 
> 2.32.0
>