| Message ID | 20211103130445.64458-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jean-Michel Hautbois (2021-11-03 13:04:45) > 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. Does it now cause saturated cells to be included in the calculations in an adverse way? I.e. do we now go back to over saturated lights/windows causing us to under-expose? or is this handled separately? > This case is reported in bug #84. 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 | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index b5d736c1..7ac8dfb2 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -124,16 +124,14 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats, > &stats->awb_raw_buffer.meta_data[cellPosition] > ); > > - if (cell->sat_ratio == 0) { > - 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]++; > } > } > > -- > 2.32.0 >
Hi Kieran, On 03/11/2021 16:06, Kieran Bingham wrote: > Quoting Jean-Michel Hautbois (2021-11-03 13:04:45) >> 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. > > Does it now cause saturated cells to be included in the calculations in > an adverse way? > > I.e. do we now go back to over saturated lights/windows causing us to > under-expose? or is this handled separately? Yes, so I will send a v2 (but with a different subject) to use only a certain amount of saturated cells. This way the histogram can't be empty and we still know we are saturated so we correct the exposure. > > >> This case is reported in bug #84. > > 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 | 18 ++++++++---------- >> 1 file changed, 8 insertions(+), 10 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index b5d736c1..7ac8dfb2 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -124,16 +124,14 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats, >> &stats->awb_raw_buffer.meta_data[cellPosition] >> ); >> >> - if (cell->sat_ratio == 0) { >> - 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]++; >> } >> } >> >> -- >> 2.32.0 >>
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index b5d736c1..7ac8dfb2 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -124,16 +124,14 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats, &stats->awb_raw_buffer.meta_data[cellPosition] ); - if (cell->sat_ratio == 0) { - 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]++; } }
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. This case is reported in bug #84. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)