[libcamera-devel] [PATCH 09/12] ipa: ipu3: awb: Use the line stride for the stats

[Laurent Pinchart]

Hello,

On Tue, Sep 28, 2021 at 05:02:42PM +0100, Kieran Bingham wrote:
> On Thu, Sep 23, 2021 at 10:16:22AM +0200, Jean-Michel Hautbois wrote:
> > The statistics buffer 'ipu3_uapi_awb_raw_buffer' stores the ImgU
> > calculation results in a buffer aligned to a multiple of 4. The AWB loop

s/aligned to a multiple of 4/aligned horizontally to a multiple of 4 cells/

> > should take care of it to add the proper offset between lines and avoid
> > any staircase effect.
> > 
> > It is now no more required to pass the grid configuration context to the
> 
> "It is no longer required to ..."
> 
> That's a nice perk indeed.
> 
> > private functions called from process() which simplifies the code flow.
> > 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > ---
> >  src/ipa/ipu3/algorithms/awb.cpp | 16 ++++++++--------
> >  src/ipa/ipu3/algorithms/awb.h   |  7 +++----
> >  2 files changed, 11 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> > index 8a926691..a5391653 100644
> > --- a/src/ipa/ipu3/algorithms/awb.cpp
> > +++ b/src/ipa/ipu3/algorithms/awb.cpp
> > @@ -172,8 +172,10 @@ int Awb::configure(IPAContext &context,
> >  		   [[maybe_unused]] const IPAConfigInfo &configInfo)
> >  {
> >  	const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
> > +	/* The grid is aligned to the next multiple of 4 */

s/grid/grid width/

> > +	stride_ = (grid.width + 3) / 4 * 4;
> 
> Feels like something that could have been handled by an alignedUp macro.
> We have that in geometry.h but for Size ...
> 
> So perhaps we can just open code it (unless anyone knows of a
> std:: macro that does this?)

utils::alignUp()

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > -	cellsPerZoneX_ = round(grid.width / static_cast<double>(kAwbStatsSizeX));
> > +	cellsPerZoneX_ = round(stride_ / static_cast<double>(kAwbStatsSizeX));

This isn't right, you will end up using the padding cells in your
calculations.

> >  	cellsPerZoneY_ = round(grid.height / static_cast<double>(kAwbStatsSizeY));
> >  
> >  	/*
> > @@ -234,8 +236,7 @@ void Awb::generateZones(std::vector<RGB> &zones)
> >  }
> >  
> >  /* Translate the IPU3 statistics into the default statistics zone array */
> > -void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats,
> > -			   const ipu3_uapi_grid_config &grid)
> > +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
> >  {
> >  	/*
> >  	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
> > @@ -243,7 +244,7 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats,
> >  	 */
> >  	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)
> > +			uint32_t cellPosition = (cellY * stride_ + cellX)
> >  					      * sizeof(Ipu3AwbCell);
> >  			uint32_t zoneX = cellX / cellsPerZoneX_;
> >  			uint32_t zoneY = cellY / cellsPerZoneY_;
> > @@ -318,13 +319,12 @@ void Awb::awbGreyWorld()
> >  	asyncResults_.blueGain = blueGain;
> >  }
> >  
> > -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
> > -			   const ipu3_uapi_grid_config &grid)
> > +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> >  {
> >  	ASSERT(stats->stats_3a_status.awb_en);
> >  	zones_.clear();
> >  	clearAwbStats();
> > -	generateAwbStats(stats, grid);
> > +	generateAwbStats(stats);
> >  	generateZones(zones_);
> >  	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
> >  	if (zones_.size() > 10) {
> > @@ -336,7 +336,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
> >  
> >  void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >  {
> > -	calculateWBGains(stats, context.configuration.grid.bdsGrid);
> > +	calculateWBGains(stats);
> >  
> >  	/*
> >  	 * Gains are only recalculated if enough zones were detected.
> > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> > index 681d8c2b..b3e0ad82 100644
> > --- a/src/ipa/ipu3/algorithms/awb.h
> > +++ b/src/ipa/ipu3/algorithms/awb.h
> > @@ -74,11 +74,9 @@ public:
> >  	};
> >  
> >  private:
> > -	void calculateWBGains(const ipu3_uapi_stats_3a *stats,
> > -			      const ipu3_uapi_grid_config &grid);
> > +	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> >  	void generateZones(std::vector<RGB> &zones);
> > -	void generateAwbStats(const ipu3_uapi_stats_3a *stats,
> > -			      const ipu3_uapi_grid_config &grid);
> > +	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
> >  	void clearAwbStats();
> >  	void awbGreyWorld();
> >  	uint32_t estimateCCT(double red, double green, double blue);
> > @@ -87,6 +85,7 @@ private:
> >  	Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
> >  	AwbStatus asyncResults_;
> >  
> > +	uint32_t stride_;
> >  	uint32_t cellsPerZoneX_;
> >  	uint32_t cellsPerZoneY_;
> >  	uint32_t cellsPerZoneThreshold_;

-- 
Regards,

Laurent Pinchart