| Message ID | 20211013154125.133419-4-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jean-Michel Hautbois (2021-10-13 16:41:15) > 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, making the algorithm to not run. ", preventing the algorithm from running efficiently." ? or s/efficiently/correctly/ ? > > 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 | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index 5574bd44..30693923 100644 > --- a/src/ipa/ipu3/algorithms/awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -161,6 +161,7 @@ int Awb::configure(IPAContext &context, > * \todo This proportion could be configured. > */ > cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100; > + LOG(IPU3Awb, Debug) << "Threshold for AWB is set to " << cellsPerZoneThreshold_; This is now a bit confusing. Your commit message mentions 90%, but the cellsPerZoneThreshold is 80%? I guess these are two different thresholds... > > return 0; > } > @@ -232,7 +233,7 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > reinterpret_cast<const ipu3_uapi_awb_set_item *>( > &stats->awb_raw_buffer.meta_data[cellPosition] > ); This probably now deserves a comment explaining why 90% is used. /* * 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 == 0) { > + if (currentCell->sat_ratio <= 255 * 90 / 100) { > /* The cell is not saturated, use the current cell */ > awbStats_[awbZonePosition].counted++; > uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg; > -- > 2.30.2 >
On Thu, Oct 14, 2021 at 11:32:37AM +0100, Kieran Bingham wrote: > Quoting Jean-Michel Hautbois (2021-10-13 16:41:15) > > 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, making the algorithm to not run. > > ", preventing the algorithm from running efficiently." ? > > or s/efficiently/correctly/ ? > > > 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 | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > > index 5574bd44..30693923 100644 > > --- a/src/ipa/ipu3/algorithms/awb.cpp > > +++ b/src/ipa/ipu3/algorithms/awb.cpp > > @@ -161,6 +161,7 @@ int Awb::configure(IPAContext &context, > > * \todo This proportion could be configured. > > */ > > cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100; > > + LOG(IPU3Awb, Debug) << "Threshold for AWB is set to " << cellsPerZoneThreshold_; > > This is now a bit confusing. Your commit message mentions 90%, but the > cellsPerZoneThreshold is 80%? > > I guess these are two different thresholds... To be considered, a zone needs 80% of the cells below a 90% saturation ratio. I'd add constexpr constants to these values. > > return 0; > > } > > @@ -232,7 +233,7 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > > reinterpret_cast<const ipu3_uapi_awb_set_item *>( > > &stats->awb_raw_buffer.meta_data[cellPosition] > > ); > > This probably now deserves a comment explaining why 90% is used. > > /* > * 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 == 0) { > > + if (currentCell->sat_ratio <= 255 * 90 / 100) { > > /* The cell is not saturated, use the current cell */ > > awbStats_[awbZonePosition].counted++; > > uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;
Hi, On 15/10/2021 00:12, Laurent Pinchart wrote: > On Thu, Oct 14, 2021 at 11:32:37AM +0100, Kieran Bingham wrote: >> Quoting Jean-Michel Hautbois (2021-10-13 16:41:15) >>> 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, making the algorithm to not run. >> >> ", preventing the algorithm from running efficiently." ? >> >> or s/efficiently/correctly/ ? >> >>> 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 | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp >>> index 5574bd44..30693923 100644 >>> --- a/src/ipa/ipu3/algorithms/awb.cpp >>> +++ b/src/ipa/ipu3/algorithms/awb.cpp >>> @@ -161,6 +161,7 @@ int Awb::configure(IPAContext &context, >>> * \todo This proportion could be configured. >>> */ >>> cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100; >>> + LOG(IPU3Awb, Debug) << "Threshold for AWB is set to " << cellsPerZoneThreshold_; >> >> This is now a bit confusing. Your commit message mentions 90%, but the >> cellsPerZoneThreshold is 80%? >> >> I guess these are two different thresholds... > > To be considered, a zone needs 80% of the cells below a 90% saturation > ratio. Precisely. > > I'd add constexpr constants to these values. Ack. > >>> return 0; >>> } >>> @@ -232,7 +233,7 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) >>> reinterpret_cast<const ipu3_uapi_awb_set_item *>( >>> &stats->awb_raw_buffer.meta_data[cellPosition] >>> ); >> >> This probably now deserves a comment explaining why 90% is used. >> >> /* >> * 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 == 0) { >>> + if (currentCell->sat_ratio <= 255 * 90 / 100) { >>> /* The cell is not saturated, use the current cell */ >>> awbStats_[awbZonePosition].counted++; >>> uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg; >
diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index 5574bd44..30693923 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -161,6 +161,7 @@ int Awb::configure(IPAContext &context, * \todo This proportion could be configured. */ cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100; + LOG(IPU3Awb, Debug) << "Threshold for AWB is set to " << cellsPerZoneThreshold_; return 0; } @@ -232,7 +233,7 @@ 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) { + if (currentCell->sat_ratio <= 255 * 90 / 100) { /* The cell is not saturated, use the current cell */ awbStats_[awbZonePosition].counted++; uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;
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, making the algorithm to not run. 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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)