[libcamera-devel] [PATCH 03/13] ipa: ipu3: awb: Use saturation under 90%

[Jean-Michel Hautbois]

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 at 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;
>