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

[Kieran Bingham]

On 02/09/2021 13:52, Jean-Michel Hautbois wrote:
> 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 :-).

Ok - maybe lets remove it instead then.


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