[libcamera-devel] [PATCH 3/4] ipa: ipu3: Replace Accumulator::uncounted to Accumulator::total

[Jean-Michel Hautbois]

On 02/09/2021 14:33, Kieran Bingham wrote:
> On 02/09/2021 09:32, Jean-Michel Hautbois wrote:
>> Replace uncounted by total as it is clearer on what it contains. It is
>> always possible to know how many pixels are not relevant for the
>> algorithm by calculating total-counted.
> 
> Was uncounted not set before? I expected to see code removing references
> to setting the uncounted value.
> 
> If it was previously unused (other than initialisation) add that
> information to the commit message.
> 

It was not used and won't be used in a near future either :-).

> 
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>>  src/ipa/ipu3/algorithms/awb.cpp | 10 +++++++---
>>  src/ipa/ipu3/algorithms/awb.h   |  2 +-
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
>> index 8cfbb23d..9b2cbba7 100644
>> --- a/src/ipa/ipu3/algorithms/awb.cpp
>> +++ b/src/ipa/ipu3/algorithms/awb.cpp
>> @@ -38,8 +38,8 @@ static constexpr uint32_t kMinGreenLevelInZone = 32;
>>   * \var Accumulator::counted
>>   * \brief Number of relevant regions used to calculate the sums
>>   *
>> - * \var Accumulator::uncounted
>> - * \brief Remaining number of regions in the zone
>> + * \var Accumulator::total
>> + * \brief Total number of regions in the zone
>>   *
>>   * \var Accumulator::rSum
>>   * \brief Sum of the red values in the zone
>> @@ -189,6 +189,10 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats,
>>  	uint32_t regionWidth = round(grid.width / static_cast<double>(kAwbStatsSizeX));
>>  	uint32_t regionHeight = round(grid.height / static_cast<double>(kAwbStatsSizeY));
>>  
>> +	for (unsigned int i = 0; i < kAwbStatsSizeX; i++)
>> +		for (unsigned int j = 0; j < kAwbStatsSizeY; j++)
>> +			awbStats_[j * kAwbStatsSizeX + i].total = regionWidth * regionHeight;
> 
> If all (zones?) have the same total number of regions, do we even need
> to set it in the structure?
> 
> Perhaps if uncounted/total are unused, it could simply be dropped?
> 
>> +
>>  	/*
>>  	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
>>  	 * (grid.width x grid.height).
>> @@ -223,7 +227,7 @@ void Awb::clearAwbStats()
>>  		awbStats_[i].rSum = 0;
>>  		awbStats_[i].gSum = 0;
>>  		awbStats_[i].counted = 0;
>> -		awbStats_[i].uncounted = 0;
>> +		awbStats_[i].total = 0;
>>  	}
>>  }
>>  
>> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
>> index 6ae934fc..13490b07 100644
>> --- a/src/ipa/ipu3/algorithms/awb.h
>> +++ b/src/ipa/ipu3/algorithms/awb.h
>> @@ -35,7 +35,7 @@ struct Ipu3AwbCell {
>>  
>>  struct Accumulator {
>>  	unsigned int counted;
>> -	unsigned int uncounted;
>> +	unsigned int total;
>>  	unsigned long long rSum;
>>  	unsigned long long gSum;
>>  	unsigned long long bSum;
>>