| Message ID | 20211116152443.56512-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | d9a2a1f703273a28b001dee40fddd378bba7a1b6 |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@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); > } > > /**
Quoting Jean-Michel Hautbois (2021-11-16 15:24:44) > 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@ideasonboard.com> > --- > v2: - remove the empty histogram test > - don't take Kieran's R-B tag as it needs testing again Retested. No issues/blackouts on covered lens. Copes with backlights. I can certainly see the small subtle changes when the brightness is changed by covering a backlight - but I don't think there's anything inappropriate there. Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > 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); > } > > /** > -- > 2.32.0 >
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); } /**
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@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(-)