[{"id":27061,"web_url":"https://patchwork.libcamera.org/comment/27061/","msgid":"<20230504175922.GR4551@pendragon.ideasonboard.com>","date":"2023-05-04T17:59:22","subject":"Re: [libcamera-devel] [PATCH 10/13] ipa: raspberrypi: agc: Move\n\tweights out of AGC","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, May 03, 2023 at 01:20:32PM +0100, Naushir Patuck via libcamera-devel wrote:\n> From: David Plowman \n> \n> The region weights for the the AGC zones are handled by the AGC\n> algorithm. Apply them directly in the IPA (vc4.cpp) to the statistics\n> that we pass to the AGC.\n> \n> Signed-off-by: David Plowman \n\nThere's a .com missing. I'll add it.\n\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Jacopo Mondi \n> ---\n> src/ipa/rpi/controller/agc_algorithm.h | 3 +++\n> src/ipa/rpi/controller/rpi/agc.cpp | 27 +++++++++++++++++---------\n> src/ipa/rpi/controller/rpi/agc.h | 1 +\n> src/ipa/rpi/vc4/vc4.cpp | 26 ++++++++++++++++++-------\n> 4 files changed, 41 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h\n> index 36e6c11058ee..b6949daa7135 100644\n> --- a/src/ipa/rpi/controller/agc_algorithm.h\n> +++ b/src/ipa/rpi/controller/agc_algorithm.h\n> @@ -6,6 +6,8 @@\n> */\n> #pragma once\n> \n> +#include \n> +\n> #include \n> \n> #include \"algorithm.h\"\n> @@ -18,6 +20,7 @@ public:\n> \tAgcAlgorithm(Controller *controller) : Algorithm(controller) {}\n> \t/* An AGC algorithm must provide the following: */\n> \tvirtual unsigned int getConvergenceFrames() const = 0;\n> +\tvirtual std::vector const &getWeights() const = 0;\n> \tvirtual void setEv(double ev) = 0;\n> \tvirtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0;\n> \tvirtual void setFixedShutter(libcamera::utils::Duration fixedShutter) = 0;\n> diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> index e6fb7b8dbeb3..e79c82e2e65b 100644\n> --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> @@ -292,6 +292,18 @@ unsigned int Agc::getConvergenceFrames() const\n> \t\treturn config_.convergenceFrames;\n> }\n> \n> +std::vector const &Agc::getWeights() const\n> +{\n> +\t/*\n> +\t * In case someone calls setMeteringMode and then this before the\n> +\t * algorithm has run and updated the meteringMode_ pointer.\n> +\t */\n> +\tauto it = config_.meteringModes.find(meteringModeName_);\n> +\tif (it == config_.meteringModes.end())\n> +\t\treturn meteringMode_->weights;\n> +\treturn it->second.weights;\n> +}\n> +\n> void Agc::setEv(double ev)\n> {\n> \tev_ = ev;\n> @@ -595,19 +607,16 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n> \tASSERT(weights.size() == stats->agcRegions.numRegions());\n> \n> \t/*\n> -\t * Note how the calculation below means that equal weights give you\n> -\t * \"average\" metering (i.e. all pixels equally important).\n> +\t * Note that the weights are applied by the IPA to the statistics directly,\n> +\t * before they are given to us here.\n> \t */\n> \tdouble rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> \tfor (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> \t\tauto ®ion = stats->agcRegions.get(i);\n> -\t\tdouble rAcc = std::min(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> -\t\tdouble gAcc = std::min(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> -\t\tdouble bAcc = std::min(region.val.bSum * gain, (maxVal - 1) * region.counted);\n> -\t\trSum += rAcc * weights[i];\n> -\t\tgSum += gAcc * weights[i];\n> -\t\tbSum += bAcc * weights[i];\n> -\t\tpixelSum += region.counted * weights[i];\n> +\t\trSum += std::min(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> +\t\tgSum += std::min(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> +\t\tbSum += std::min(region.val.bSum * gain, (maxVal - 1) * region.counted);\n> +\t\tpixelSum += region.counted;\n> \t}\n> \tif (pixelSum == 0.0) {\n> \t\tLOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h\n> index 4e5f272fac78..939f97295a02 100644\n> --- a/src/ipa/rpi/controller/rpi/agc.h\n> +++ b/src/ipa/rpi/controller/rpi/agc.h\n> @@ -69,6 +69,7 @@ public:\n> \tchar const *name() const override;\n> \tint read(const libcamera::YamlObject ¶ms) override;\n> \tunsigned int getConvergenceFrames() const override;\n> +\tstd::vector const &getWeights() const override;\n> \tvoid setEv(double ev) override;\n> \tvoid setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;\n> \tvoid setMaxShutter(libcamera::utils::Duration maxShutter) override;\n> diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp\n> index a32db9bcaf9a..e38024ac5418 100644\n> --- a/src/ipa/rpi/vc4/vc4.cpp\n> +++ b/src/ipa/rpi/vc4/vc4.cpp\n> @@ -211,13 +211,25 @@ RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span mem)\n> \t\t\t\t\t\tstats->awb_stats[i].counted,\n> \t\t\t\t\t\tstats->awb_stats[i].notcounted });\n> \n> -\tstatistics->agcRegions.init(hw.agcRegions);\n> -\tfor (i = 0; i < statistics->agcRegions.numRegions(); i++)\n> -\t\tstatistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum << scale,\n> -\t\t\t\t\t\t stats->agc_stats[i].g_sum << scale,\n> -\t\t\t\t\t\t stats->agc_stats[i].b_sum << scale },\n> -\t\t\t\t\t\tstats->agc_stats[i].counted,\n> -\t\t\t\t\t\tstats->awb_stats[i].notcounted });\n> +\tRPiController::AgcAlgorithm *agc = dynamic_cast(\n> +\t\tcontroller_.getAlgorithm(\"agc\"));\n> +\tif (!agc) {\n> +\t\tLOG(IPARPI, Debug) << \"No AGC algorithm - not copying statistics\";\n> +\t\tstatistics->agcRegions.init(0);\n> +\t} else {\n> +\t\tstatistics->agcRegions.init(hw.agcRegions);\n> +\t\tconst std::vector &weights = agc->getWeights();\n> +\t\tfor (i = 0; i < statistics->agcRegions.numRegions(); i++) {\n> +\t\t\tuint64_t rSum = (stats->agc_stats[i].r_sum << scale) * weights[i];\n> +\t\t\tuint64_t gSum = (stats->agc_stats[i].g_sum << scale) * weights[i];\n> +\t\t\tuint64_t bSum = (stats->agc_stats[i].b_sum << scale) * weights[i];\n> +\t\t\tuint32_t counted = stats->agc_stats[i].counted * weights[i];\n> +\t\t\tuint32_t notcounted = stats->agc_stats[i].notcounted * weights[i];\n> +\t\t\tstatistics->agcRegions.set(i, { { rSum, gSum, bSum },\n> +\t\t\t\t\t\t\tcounted,\n> +\t\t\t\t\t\t\tnotcounted });\n> +\t\t}\n> +\t}\n> \n> \tstatistics->focusRegions.init(hw.focusRegions);\n> \tfor (i = 0; i < statistics->focusRegions.numRegions(); i++)","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 C52D2BD16B\n\tfor ;\n\tThu, 4 May 2023 17:59:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 361F2633B5;\n\tThu, 4 May 2023 19:59:14 +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 1CAE5627DE\n\tfor ;\n\tThu, 4 May 2023 19:59:13 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(133-32-181-51.west.xps.vectant.ne.jp [133.32.181.51])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D2CA6E0;\n\tThu, 4 May 2023 19:59:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683223154;\n\tbh=Ea++gTuHNUx1m3GRRmrAJ+KlCW4txYiM1YJK/kTxKYs=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=uB5Cc9l5E0GqYDdaeFfvsXNiJBsnDMA6HANpsNZNY4QsxH8ssIxQGhCvMzdVA5Tr9\n\tZLmP4glWR/ebqG312F6fdsCVvXblCj8YgZm3ir9FwjeVuZEdvqa0q+NgqKj/4gQIn0\n\tqGkN12cB40GEXVhJVFrlLL4UFD7BiZsi+0iMf2olGo1cgGLTY/+L2b71/jpfbZoV3Q\n\t2gWcWpsj/3+sRnxyg+yqVcjoQPkUim6VPgVb/9/Fb7Rn0z0s1GeFWcrk0UeQJe+/SL\n\tfJZo4tt8PlA/H5iOOX8es5e1gxbxSWjAT1TStiRl+jDvWNhfkUENErtNiBjcWjhE6N\n\tNzXDsldlJugVQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683223149;\n\tbh=Ea++gTuHNUx1m3GRRmrAJ+KlCW4txYiM1YJK/kTxKYs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lZqCQZPZOXcsD6TtrazWqH9xrc+Cv65EA74b5kfjfmP0ttZdQetNo6QSzfEMsIs2w\n\tTFGoR2OtIxBwBXCxmYf5KS+11B9CQScbNdPXtnfA9RWam8AloVzC3t0Tv0Eb6ZKqK5\n\tmCQXaajQUwuogwEmJM2Ck20hsoj/5/0nFp7F20Yk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"lZqCQZPZ\"; dkim-atps=neutral","Date":"Thu, 4 May 2023 20:59:22 +0300","To":"Naushir Patuck ","Message-ID":"<20230504175922.GR4551@pendragon.ideasonboard.com>","References":"<20230503122035.32026-1-naush@raspberrypi.com>\n\t<20230503122035.32026-11-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230503122035.32026-11-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 10/13] ipa: raspberrypi: agc: Move\n\tweights out of AGC","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]