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

[Jean-Michel Hautbois]

Hi Laurent,

Thanks for the patch !

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 :-) ?

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