[libcamera-devel] [PATCH 04/12] ipa: ipu3: awb: Make the naming consistent

[Laurent Pinchart]

On Tue, Sep 28, 2021 at 03:10:20PM +0100, Kieran Bingham wrote:
> On Thu, Sep 23, 2021 at 10:16:17AM +0200, 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:
> 
> s/variables consistent/variables to be consistent/
> 
> > - Cells are defined in Pixels
> > - Zones are defined in Cells
> > There is no "region" as such, so replace it with the correct term.
> > 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > ---
> >  src/ipa/ipu3/algorithms/awb.cpp | 31 +++++++++++++++++--------------
> >  1 file changed, 17 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> > index d78ae4f4..51a38d05 100644
> > --- a/src/ipa/ipu3/algorithms/awb.cpp
> > +++ b/src/ipa/ipu3/algorithms/awb.cpp
> > @@ -220,31 +220,34 @@ void Awb::generateZones(std::vector<RGB> &zones)
> >  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 cellsPerZoneX = round(grid.width / static_cast<double>(kAwbStatsSizeX));
> > +	uint32_t cellsPerZoneY = 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++) {
> > -			uint32_t cellPosition = j * grid.width + i;
> > -			uint32_t cellX = (cellPosition / regionWidth) % kAwbStatsSizeX;
> > -			uint32_t cellY = ((cellPosition / grid.width) / regionHeight) % kAwbStatsSizeY;
> > +	for (unsigned int cellY = 0; cellY < kAwbStatsSizeY * cellsPerZoneY; cellY++) {
> > +		for (unsigned int cellX = 0; cellX < kAwbStatsSizeX * cellsPerZoneX; cellX++) {
> > +			uint32_t cellPosition = (cellY * grid.width + cellX)
> > +					      * sizeof(Ipu3AwbCell);
> > +			uint32_t zoneX = cellX / cellsPerZoneX;
> > +			uint32_t zoneY = cellY / cellsPerZoneY;
> >  
> > -			uint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX;
> > -			cellPosition *= 8;
> > +			uint32_t awbZonePosition = zoneY * kAwbStatsSizeX + zoneX;
> >  
> >  			/* Cast the initial IPU3 structure to simplify the reading */
> > -			Ipu3AwbCell *currentCell = reinterpret_cast<Ipu3AwbCell *>(const_cast<uint8_t *>(&stats->awb_raw_buffer.meta_data[cellPosition]));
> > +			const Ipu3AwbCell *currentCell =
> > +				reinterpret_cast<const Ipu3AwbCell *>(
> > +					&stats->awb_raw_buffer.meta_data[cellPosition]
> > +				);
> >  			if (currentCell->satRatio == 0) {
> >  				/* The cell is not saturated, use the current cell */
> > -				awbStats_[awbRegionPosition].counted++;
> > +				awbStats_[awbZonePosition].counted++;
> >  				uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg;
> > -				awbStats_[awbRegionPosition].sum.green += greenValue / 2;
> > -				awbStats_[awbRegionPosition].sum.red += currentCell->redAvg;
> > -				awbStats_[awbRegionPosition].sum.blue += currentCell->blueAvg;
> > +				awbStats_[awbZonePosition].sum.green += greenValue / 2;
> > +				awbStats_[awbZonePosition].sum.red += currentCell->redAvg;
> > +				awbStats_[awbZonePosition].sum.blue += currentCell->blueAvg;
> >  			}
> >  		}
> >  	}

-- 
Regards,

Laurent Pinchart