[libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member variable to member function

[Laurent Pinchart]

Hi Jean-Michel,

On Fri, Oct 15, 2021 at 09:03:42AM +0200, Jean-Michel Hautbois wrote:
> On 15/10/2021 03:58, Laurent Pinchart wrote:
> > The Awb::generateZones() member function fills the zones vector passed
> > as an argument, which is actually a member variable. Use it directly in
> > the function.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/ipa/ipu3/algorithms/awb.cpp | 12 ++++++++----
> >  src/ipa/ipu3/algorithms/awb.h   |  2 +-
> >  2 files changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> > index e2b18336d582..809de66aa7fa 100644
> > --- a/src/ipa/ipu3/algorithms/awb.cpp
> > +++ b/src/ipa/ipu3/algorithms/awb.cpp
> > @@ -196,8 +196,10 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)
> >  }
> >  
> >  /* Generate an RGB vector with the average values for each zone */
> > -void Awb::generateZones(std::vector<RGB> &zones)
> > +void Awb::generateZones()
> >  {
> > +	zones_.clear();
> > +
> >  	for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {
> >  		RGB zone;
> >  		double counted = awbStats_[i].counted;
> > @@ -206,7 +208,7 @@ void Awb::generateZones(std::vector<RGB> &zones)
> >  			if (zone.G >= kMinGreenLevelInZone) {
> >  				zone.R = awbStats_[i].sum.red / counted;
> >  				zone.B = awbStats_[i].sum.blue / counted;
> > -				zones.push_back(zone);
> > +				zones_.push_back(zone);
> >  			}
> >  		}
> >  	}
> > @@ -298,11 +300,13 @@ void Awb::awbGreyWorld()
> >  void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> >  {
> >  	ASSERT(stats->stats_3a_status.awb_en);
> > -	zones_.clear();
> > +
> >  	clearAwbStats();
> >  	generateAwbStats(stats);
> > -	generateZones(zones_);
> > +	generateZones();
> > +
> >  	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
> > +
> >  	if (zones_.size() > 10) {
> >  		awbGreyWorld();
> >  		LOG(IPU3Awb, Debug) << "Gain found for red: " << asyncResults_.redGain
> > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> > index 677384eda54a..3b81f600aa02 100644
> > --- a/src/ipa/ipu3/algorithms/awb.h
> > +++ b/src/ipa/ipu3/algorithms/awb.h
> > @@ -65,7 +65,7 @@ public:
> >  
> >  private:
> >  	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> > -	void generateZones(std::vector<RGB> &zones);
> > +	void generateZones();
> >  	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
> >  	void clearAwbStats();
> >  	void awbGreyWorld();
> > 
> > base-commit: ccec150589a177654b9039d21163c36ccff85cff
> 
> Should we always include this in our future patches ? I can see you make
> it for your series systematically :-) ?

I've enabled the base=auto option for git format-patch by default. It's
nice to have, but for libcamera, probably won't be that useful yet.

> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

-- 
Regards,

Laurent Pinchart