[{"id":20235,"web_url":"https://patchwork.libcamera.org/comment/20235/","msgid":"","date":"2021-10-15T07:03:42","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nThanks for the patch !\n\nOn 15/10/2021 03:58, Laurent Pinchart wrote:\n> The Awb::generateZones() member function fills the zones vector passed\n> as an argument, which is actually a member variable. Use it directly in\n> the function.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/ipu3/algorithms/awb.cpp | 12 ++++++++----\n> src/ipa/ipu3/algorithms/awb.h | 2 +-\n> 2 files changed, 9 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index e2b18336d582..809de66aa7fa 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -196,8 +196,10 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> }\n> \n> /* Generate an RGB vector with the average values for each zone */\n> -void Awb::generateZones(std::vector &zones)\n> +void Awb::generateZones()\n> {\n> +\tzones_.clear();\n> +\n> \tfor (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {\n> \t\tRGB zone;\n> \t\tdouble counted = awbStats_[i].counted;\n> @@ -206,7 +208,7 @@ void Awb::generateZones(std::vector &zones)\n> \t\t\tif (zone.G >= kMinGreenLevelInZone) {\n> \t\t\t\tzone.R = awbStats_[i].sum.red / counted;\n> \t\t\t\tzone.B = awbStats_[i].sum.blue / counted;\n> -\t\t\t\tzones.push_back(zone);\n> +\t\t\t\tzones_.push_back(zone);\n> \t\t\t}\n> \t\t}\n> \t}\n> @@ -298,11 +300,13 @@ void Awb::awbGreyWorld()\n> void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n> {\n> \tASSERT(stats->stats_3a_status.awb_en);\n> -\tzones_.clear();\n> +\n> \tclearAwbStats();\n> \tgenerateAwbStats(stats);\n> -\tgenerateZones(zones_);\n> +\tgenerateZones();\n> +\n> \tLOG(IPU3Awb, Debug) << \"Valid zones: \" << zones_.size();\n> +\n> \tif (zones_.size() > 10) {\n> \t\tawbGreyWorld();\n> \t\tLOG(IPU3Awb, Debug) << \"Gain found for red: \" << asyncResults_.redGain\n> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> index 677384eda54a..3b81f600aa02 100644\n> --- a/src/ipa/ipu3/algorithms/awb.h\n> +++ b/src/ipa/ipu3/algorithms/awb.h\n> @@ -65,7 +65,7 @@ public:\n> \n> private:\n> \tvoid calculateWBGains(const ipu3_uapi_stats_3a *stats);\n> -\tvoid generateZones(std::vector &zones);\n> +\tvoid generateZones();\n> \tvoid generateAwbStats(const ipu3_uapi_stats_3a *stats);\n> \tvoid clearAwbStats();\n> \tvoid awbGreyWorld();\n> \n> base-commit: ccec150589a177654b9039d21163c36ccff85cff\n> \n\nShould we always include this in our future patches ? I can see you make\nit for your series systematically :-) ?\n\nReviewed-by: Jean-Michel Hautbois ","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3312BC324C\n\tfor ;\n\tFri, 15 Oct 2021 07:03:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9168E68F50;\n\tFri, 15 Oct 2021 09:03:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B43866023B\n\tfor ;\n\tFri, 15 Oct 2021 09:03:44 +0200 (CEST)","from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:ae2a:a11b:484b:c825])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 568F62E3;\n\tFri, 15 Oct 2021 09:03:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ri4MUvRs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634281424;\n\tbh=PbjJqZaSPWpJszticghfUiztZ+1P8qa22eMTL1hu32A=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ri4MUvRsRDON02WNos201uQXF+CJSkPdyMmoA1ANyzuDZSHph4J/tF+HAqAe1Xmhk\n\t9C8nwwDsWBn/GyGG93gl0dClZBJf0T+HFOCH7GqUf0TpbpkEwn1vd6S+HIxYFXUDz6\n\tO1rFAwvt9tHoSOc/tnxgLKghTatEyOfba8hXy5mQ=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Fri, 15 Oct 2021 09:03:42 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.13.0","MIME-Version":"1.0","In-Reply-To":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20245,"web_url":"https://patchwork.libcamera.org/comment/20245/","msgid":"<163433227752.456562.1410130812543660604@Monstersaurus>","date":"2021-10-15T21:11:17","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-10-15 02:58:03)\n> The Awb::generateZones() member function fills the zones vector passed\n> as an argument, which is actually a member variable. Use it directly in\n> the function.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/ipu3/algorithms/awb.cpp | 12 ++++++++----\n> src/ipa/ipu3/algorithms/awb.h | 2 +-\n> 2 files changed, 9 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index e2b18336d582..809de66aa7fa 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -196,8 +196,10 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> }\n> \n> /* Generate an RGB vector with the average values for each zone */\n> -void Awb::generateZones(std::vector &zones)\n> +void Awb::generateZones()\n> {\n> + zones_.clear();\n> +\n\nhttps://www.cplusplus.com/reference/vector/vector/clear/ states a\nreallocation is not guaranteed to happen and the vector capacity is not\nguaranteed to change due to calling this function.\n\nThat's a shame, it would be nice to be able to guarantee it won't\nreallocate to keep the memory reserved for the next usage ;-)\n\nAnyway, that pre-dates this patch and is likely as correct as it can be\nanyway.\n\nSeems a reasonable clean up.\n\nReviewed-by: Kieran Bingham \n\n\n> for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {\n> RGB zone;\n> double counted = awbStats_[i].counted;\n> @@ -206,7 +208,7 @@ void Awb::generateZones(std::vector &zones)\n> if (zone.G >= kMinGreenLevelInZone) {\n> zone.R = awbStats_[i].sum.red / counted;\n> zone.B = awbStats_[i].sum.blue / counted;\n> - zones.push_back(zone);\n> + zones_.push_back(zone);\n> }\n> }\n> }\n> @@ -298,11 +300,13 @@ void Awb::awbGreyWorld()\n> void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n> {\n> ASSERT(stats->stats_3a_status.awb_en);\n> - zones_.clear();\n> +\n> clearAwbStats();\n> generateAwbStats(stats);\n> - generateZones(zones_);\n> + generateZones();\n> +\n> LOG(IPU3Awb, Debug) << \"Valid zones: \" << zones_.size();\n> +\n> if (zones_.size() > 10) {\n> awbGreyWorld();\n> LOG(IPU3Awb, Debug) << \"Gain found for red: \" << asyncResults_.redGain\n> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> index 677384eda54a..3b81f600aa02 100644\n> --- a/src/ipa/ipu3/algorithms/awb.h\n> +++ b/src/ipa/ipu3/algorithms/awb.h\n> @@ -65,7 +65,7 @@ public:\n> \n> private:\n> void calculateWBGains(const ipu3_uapi_stats_3a *stats);\n> - void generateZones(std::vector &zones);\n> + void generateZones();\n> void generateAwbStats(const ipu3_uapi_stats_3a *stats);\n> void clearAwbStats();\n> void awbGreyWorld();\n> \n> base-commit: ccec150589a177654b9039d21163c36ccff85cff\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DE2FDC323E\n\tfor ;\n\tFri, 15 Oct 2021 21:11:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5450A68F4A;\n\tFri, 15 Oct 2021 23:11:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7AE2568546\n\tfor ;\n\tFri, 15 Oct 2021 23:11:20 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F24422E3;\n\tFri, 15 Oct 2021 23:11:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sxGGEAbQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634332280;\n\tbh=kz8DvzgJw6hgUBDD2rNdipytL0OXuQqXhfB1VCvQe7Q=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=sxGGEAbQUpRa5932HWERBBMoeLtW4meUmxshc/J0r5OqND+2ad271qsIiC63pP9Os\n\tiROrt5dk3imp0Jq4hwv9eZc+gZkiRbf94pSYMk+4WgHT7gR53ghs+UVdb4pGAdzdMK\n\tsdENnTHlBRzq6p+tpq6XfwhgI5X3t3osINtXH8e8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 15 Oct 2021 22:11:17 +0100","Message-ID":"<163433227752.456562.1410130812543660604@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20246,"web_url":"https://patchwork.libcamera.org/comment/20246/","msgid":"","date":"2021-10-15T21:17:38","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Oct 15, 2021 at 10:11:17PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2021-10-15 02:58:03)\n> > The Awb::generateZones() member function fills the zones vector passed\n> > as an argument, which is actually a member variable. Use it directly in\n> > the function.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/algorithms/awb.cpp | 12 ++++++++----\n> > src/ipa/ipu3/algorithms/awb.h | 2 +-\n> > 2 files changed, 9 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> > index e2b18336d582..809de66aa7fa 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > @@ -196,8 +196,10 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> > }\n> > \n> > /* Generate an RGB vector with the average values for each zone */\n> > -void Awb::generateZones(std::vector &zones)\n> > +void Awb::generateZones()\n> > {\n> > + zones_.clear();\n> > +\n> \n> https://www.cplusplus.com/reference/vector/vector/clear/ states a\n> reallocation is not guaranteed to happen and the vector capacity is not\n> guaranteed to change due to calling this function.\n> \n> That's a shame, it would be nice to be able to guarantee it won't\n> reallocate to keep the memory reserved for the next usage ;-)\n\nAccording to https://en.cppreference.com/w/cpp/container/vector/clear,\n\n\"Leaves the capacity() of the vector unchanged (note: the standard's\nrestriction on the changes to capacity is in the specification of\nvector::reserve, see [1])\"\n\n[1] http://stackoverflow.com/a/18467916\n\n> Anyway, that pre-dates this patch and is likely as correct as it can be\n> anyway.\n> \n> Seems a reasonable clean up.\n> \n> Reviewed-by: Kieran Bingham \n> \n> > for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {\n> > RGB zone;\n> > double counted = awbStats_[i].counted;\n> > @@ -206,7 +208,7 @@ void Awb::generateZones(std::vector &zones)\n> > if (zone.G >= kMinGreenLevelInZone) {\n> > zone.R = awbStats_[i].sum.red / counted;\n> > zone.B = awbStats_[i].sum.blue / counted;\n> > - zones.push_back(zone);\n> > + zones_.push_back(zone);\n> > }\n> > }\n> > }\n> > @@ -298,11 +300,13 @@ void Awb::awbGreyWorld()\n> > void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n> > {\n> > ASSERT(stats->stats_3a_status.awb_en);\n> > - zones_.clear();\n> > +\n> > clearAwbStats();\n> > generateAwbStats(stats);\n> > - generateZones(zones_);\n> > + generateZones();\n> > +\n> > LOG(IPU3Awb, Debug) << \"Valid zones: \" << zones_.size();\n> > +\n> > if (zones_.size() > 10) {\n> > awbGreyWorld();\n> > LOG(IPU3Awb, Debug) << \"Gain found for red: \" << asyncResults_.redGain\n> > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> > index 677384eda54a..3b81f600aa02 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.h\n> > +++ b/src/ipa/ipu3/algorithms/awb.h\n> > @@ -65,7 +65,7 @@ public:\n> > \n> > private:\n> > void calculateWBGains(const ipu3_uapi_stats_3a *stats);\n> > - void generateZones(std::vector &zones);\n> > + void generateZones();\n> > void generateAwbStats(const ipu3_uapi_stats_3a *stats);\n> > void clearAwbStats();\n> > void awbGreyWorld();\n> > \n> > base-commit: ccec150589a177654b9039d21163c36ccff85cff","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D8BC4C324C\n\tfor ;\n\tFri, 15 Oct 2021 21:17:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D27768F51;\n\tFri, 15 Oct 2021 23:17:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C12DD68546\n\tfor ;\n\tFri, 15 Oct 2021 23:17:54 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 27A392E3;\n\tFri, 15 Oct 2021 23:17:54 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bFSLyDiz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634332674;\n\tbh=flSvpd3ju4NWnA7WvdB058abGctKLtOsIcgKqFpShiY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bFSLyDiz5P9F1JkQIlRaTnD1RsWZa6hgWcYPA7iyPT8DOtWAtSEvR4I2AvrCzL9LQ\n\twf9kgDNgcdBsVd2MQw8n89V3QUbgJu0LG3/xR+cYfEIeCc0VWmp5d6ZKOfkbTxfKhx\n\t9o4MI/AEIvePCo4384uA4i1fp/DMWM38LoHX9a+s=","Date":"Sat, 16 Oct 2021 00:17:38 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>\n\t<163433227752.456562.1410130812543660604@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163433227752.456562.1410130812543660604@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20247,"web_url":"https://patchwork.libcamera.org/comment/20247/","msgid":"","date":"2021-10-15T21:25:49","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Fri, Oct 15, 2021 at 09:03:42AM +0200, Jean-Michel Hautbois wrote:\n> On 15/10/2021 03:58, Laurent Pinchart wrote:\n> > The Awb::generateZones() member function fills the zones vector passed\n> > as an argument, which is actually a member variable. Use it directly in\n> > the function.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/algorithms/awb.cpp | 12 ++++++++----\n> > src/ipa/ipu3/algorithms/awb.h | 2 +-\n> > 2 files changed, 9 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> > index e2b18336d582..809de66aa7fa 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > @@ -196,8 +196,10 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> > }\n> > \n> > /* Generate an RGB vector with the average values for each zone */\n> > -void Awb::generateZones(std::vector &zones)\n> > +void Awb::generateZones()\n> > {\n> > +\tzones_.clear();\n> > +\n> > \tfor (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {\n> > \t\tRGB zone;\n> > \t\tdouble counted = awbStats_[i].counted;\n> > @@ -206,7 +208,7 @@ void Awb::generateZones(std::vector &zones)\n> > \t\t\tif (zone.G >= kMinGreenLevelInZone) {\n> > \t\t\t\tzone.R = awbStats_[i].sum.red / counted;\n> > \t\t\t\tzone.B = awbStats_[i].sum.blue / counted;\n> > -\t\t\t\tzones.push_back(zone);\n> > +\t\t\t\tzones_.push_back(zone);\n> > \t\t\t}\n> > \t\t}\n> > \t}\n> > @@ -298,11 +300,13 @@ void Awb::awbGreyWorld()\n> > void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n> > {\n> > \tASSERT(stats->stats_3a_status.awb_en);\n> > -\tzones_.clear();\n> > +\n> > \tclearAwbStats();\n> > \tgenerateAwbStats(stats);\n> > -\tgenerateZones(zones_);\n> > +\tgenerateZones();\n> > +\n> > \tLOG(IPU3Awb, Debug) << \"Valid zones: \" << zones_.size();\n> > +\n> > \tif (zones_.size() > 10) {\n> > \t\tawbGreyWorld();\n> > \t\tLOG(IPU3Awb, Debug) << \"Gain found for red: \" << asyncResults_.redGain\n> > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> > index 677384eda54a..3b81f600aa02 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.h\n> > +++ b/src/ipa/ipu3/algorithms/awb.h\n> > @@ -65,7 +65,7 @@ public:\n> > \n> > private:\n> > \tvoid calculateWBGains(const ipu3_uapi_stats_3a *stats);\n> > -\tvoid generateZones(std::vector &zones);\n> > +\tvoid generateZones();\n> > \tvoid generateAwbStats(const ipu3_uapi_stats_3a *stats);\n> > \tvoid clearAwbStats();\n> > \tvoid awbGreyWorld();\n> > \n> > base-commit: ccec150589a177654b9039d21163c36ccff85cff\n> \n> Should we always include this in our future patches ? I can see you make\n> it for your series systematically :-) ?\n\nI've enabled the base=auto option for git format-patch by default. It's\nnice to have, but for libcamera, probably won't be that useful yet.\n\n> Reviewed-by: Jean-Michel Hautbois ","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 422A0C323E\n\tfor ;\n\tFri, 15 Oct 2021 21:26:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A863068F4F;\n\tFri, 15 Oct 2021 23:26:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F010A68546\n\tfor ;\n\tFri, 15 Oct 2021 23:26:05 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 709212E3;\n\tFri, 15 Oct 2021 23:26:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SDnDR3r+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634333165;\n\tbh=Cv+rbUotjGLHUu34Q5ZaTIF5bmjlYybsFe+wV1/w+ag=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SDnDR3r+PsTiEE8tTLJtPpXLRURQ+NqyR4YguKPLReYeDIuQqfQdECHbBWJ9/GmkK\n\tDh2roYD3K2evkiHhlZFbT/RPUavnwsnAJp2vhU6wNqGyg3hUdKqzdOA6JM/comPz2N\n\tIfU8ugravQsIYUb3bwkV+hPAOs/Aijkm4M8w91y0=","Date":"Sat, 16 Oct 2021 00:25:49 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20258,"web_url":"https://patchwork.libcamera.org/comment/20258/","msgid":"<20211018042247.GE2066071@pyrite.rasen.tech>","date":"2021-10-18T04:22:47","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Fri, Oct 15, 2021 at 04:58:03AM +0300, Laurent Pinchart wrote:\n> The Awb::generateZones() member function fills the zones vector passed\n> as an argument, which is actually a member variable. Use it directly in\n> the function.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/ipu3/algorithms/awb.cpp | 12 ++++++++----\n> src/ipa/ipu3/algorithms/awb.h | 2 +-\n> 2 files changed, 9 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index e2b18336d582..809de66aa7fa 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -196,8 +196,10 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> }\n> \n> /* Generate an RGB vector with the average values for each zone */\n> -void Awb::generateZones(std::vector &zones)\n> +void Awb::generateZones()\n> {\n> +\tzones_.clear();\n> +\n> \tfor (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {\n> \t\tRGB zone;\n> \t\tdouble counted = awbStats_[i].counted;\n> @@ -206,7 +208,7 @@ void Awb::generateZones(std::vector &zones)\n> \t\t\tif (zone.G >= kMinGreenLevelInZone) {\n> \t\t\t\tzone.R = awbStats_[i].sum.red / counted;\n> \t\t\t\tzone.B = awbStats_[i].sum.blue / counted;\n> -\t\t\t\tzones.push_back(zone);\n> +\t\t\t\tzones_.push_back(zone);\n> \t\t\t}\n> \t\t}\n> \t}\n> @@ -298,11 +300,13 @@ void Awb::awbGreyWorld()\n> void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n> {\n> \tASSERT(stats->stats_3a_status.awb_en);\n> -\tzones_.clear();\n> +\n> \tclearAwbStats();\n> \tgenerateAwbStats(stats);\n> -\tgenerateZones(zones_);\n> +\tgenerateZones();\n> +\n> \tLOG(IPU3Awb, Debug) << \"Valid zones: \" << zones_.size();\n> +\n> \tif (zones_.size() > 10) {\n> \t\tawbGreyWorld();\n> \t\tLOG(IPU3Awb, Debug) << \"Gain found for red: \" << asyncResults_.redGain\n> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> index 677384eda54a..3b81f600aa02 100644\n> --- a/src/ipa/ipu3/algorithms/awb.h\n> +++ b/src/ipa/ipu3/algorithms/awb.h\n> @@ -65,7 +65,7 @@ public:\n> \n> private:\n> \tvoid calculateWBGains(const ipu3_uapi_stats_3a *stats);\n> -\tvoid generateZones(std::vector &zones);\n> +\tvoid generateZones();\n> \tvoid generateAwbStats(const ipu3_uapi_stats_3a *stats);\n> \tvoid clearAwbStats();\n> \tvoid awbGreyWorld();\n> \n> base-commit: ccec150589a177654b9039d21163c36ccff85cff\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0E5C9C323E\n\tfor ;\n\tMon, 18 Oct 2021 04:22:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6AEC468F5B;\n\tMon, 18 Oct 2021 06:22:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96AB160128\n\tfor ;\n\tMon, 18 Oct 2021 06:22:54 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0FDD312A;\n\tMon, 18 Oct 2021 06:22:52 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Om6TJfkI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634530974;\n\tbh=dqkhHNGDRMNr7cx9ax7oZg7oTMp5ojY0Zkg5Vik/sXg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Om6TJfkI2vVyiES0Qauqlp4jcfrbdOzAzdQVCpF+8plDw5y85Uz5rtf2uFLc7mvCr\n\t4P5fUcG5ykwGd4V8iAPTwwNPUEpaMzcWH82D+sKpKh7Gs9Uc4pgQWWPS2zRkh3dApP\n\tNwInkMe2bpj7FjIsfpz7RX6jc6A7KQB4tPKfZh2c=","Date":"Mon, 18 Oct 2021 13:22:47 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20211018042247.GE2066071@pyrite.rasen.tech>","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: ipu3: awb: Don't pass member\n\tvariable to member function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]