[libcamera-devel] [PATCH] ipa: ipu3: agc: Remove the saturation ratio test

[Jean-Michel Hautbois]

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