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

[Kieran Bingham]

Quoting Laurent Pinchart (2021-10-15 02:58:03)
> 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();
> +

https://www.cplusplus.com/reference/vector/vector/clear/ states a
reallocation is not guaranteed to happen and the vector capacity is not
guaranteed to change due to calling this function.

That's a shame, it would be nice to be able to guarantee it won't
reallocate to keep the memory reserved for the next usage ;-)

Anyway, that pre-dates this patch and is likely as correct as it can be
anyway.

Seems a reasonable clean up.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


>         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
> -- 
> Regards,
> 
> Laurent Pinchart
>