[libcamera-devel] [PATCH v6 5/5] ipa: ipu3: awb: Make the naming consistent

[Kieran Bingham]

On 09/09/2021 15:19, Kieran Bingham wrote:
> On 09/09/2021 14:54, Jean-Michel Hautbois wrote:
>> The variables mix the terms cell, region and zone. It can confuse the
>> reader, and make the algorithm more difficult to follow.
>>
>> Rename the local variables consistent with their definitions:
>> - Cells are defined in Pixels
>> - Zones are defined in Cells
>>
>> Their is no "region" as such, so replace it with the correct term.
> 
> s/Their/There/
> 
> Can be fixed before merging. No need for a resend only on that.
> 
> 
> 
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>>  src/ipa/ipu3/algorithms/awb.cpp | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
>> index cf97208f..fd68b359 100644
>> --- a/src/ipa/ipu3/algorithms/awb.cpp
>> +++ b/src/ipa/ipu3/algorithms/awb.cpp
>> @@ -222,30 +222,30 @@ void Awb::generateZones(std::vector<RGB> &zones)
> 
> Locally I have a comment here:
> 
> /* Translate the IPU3 statistics into the default statistics region array */
> 
> Does that need updating? (only because I searched for "region".
> 
> It still seems to make sense in that context, so I don't think it needs
> to change unless you think it's now inaccurate.

Never mind, I now see that's done in this series already.

--
Kieran


> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
>>  void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats,
>>  			   const ipu3_uapi_grid_config &grid)
>>  {
>> -	uint32_t regionWidth = round(grid.width / static_cast<double>(kAwbStatsSizeX));
>> -	uint32_t regionHeight = round(grid.height / static_cast<double>(kAwbStatsSizeY));
>> +	uint32_t cellWidth = round(grid.width / static_cast<double>(kAwbStatsSizeX));
>> +	uint32_t cellHeight = round(grid.height / static_cast<double>(kAwbStatsSizeY));
>>  
>>  	/*
>>  	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
>>  	 * (grid.width x grid.height).
>>  	 */
>> -	for (unsigned int j = 0; j < kAwbStatsSizeY * regionHeight; j++) {
>> -		for (unsigned int i = 0; i < kAwbStatsSizeX * regionWidth; i++) {
>> +	for (unsigned int j = 0; j < kAwbStatsSizeY * cellHeight; j++) {
>> +		for (unsigned int i = 0; i < kAwbStatsSizeX * cellWidth; i++) {
>>  			uint32_t cellPosition = j * grid.width + i;
>> -			uint32_t cellX = (cellPosition / regionWidth) % kAwbStatsSizeX;
>> -			uint32_t cellY = ((cellPosition / grid.width) / regionHeight) % kAwbStatsSizeY;
>> +			uint32_t cellX = (cellPosition / cellWidth) % kAwbStatsSizeX;
>> +			uint32_t cellY = ((cellPosition / grid.width) / cellHeight) % kAwbStatsSizeY;
>>  
>> -			uint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX;
>> +			uint32_t awbZonePosition = cellY * kAwbStatsSizeX + cellX;
>>  
>>  			/* Cast the initial IPU3 structure to simplify the reading */
>>  			const ipu3_uapi_awb_set_item *currentCell = &stats->awb_raw_buffer.meta_data[cellPosition];
>>  			if (currentCell->sat_ratio == 0) {
>>  				/* The cell is not saturated, use the current cell */
>> -				awbStats_[awbRegionPosition].counted++;
>> +				awbStats_[awbZonePosition].counted++;
>>  				uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;
>> -				awbStats_[awbRegionPosition].sum.green += greenValue / 2;
>> -				awbStats_[awbRegionPosition].sum.red += currentCell->R_avg;
>> -				awbStats_[awbRegionPosition].sum.blue += currentCell->B_avg;
>> +				awbStats_[awbZonePosition].sum.green += greenValue / 2;
>> +				awbStats_[awbZonePosition].sum.red += currentCell->R_avg;
>> +				awbStats_[awbZonePosition].sum.blue += currentCell->B_avg;
>>  			}
>>  		}
>>  	}
>>

-- 
Regards
--
Kieran