[{"id":36856,"web_url":"https://patchwork.libcamera.org/comment/36856/","msgid":"<7c23a25e-21d5-4d09-922d-bf059b520211@ideasonboard.com>","date":"2025-11-17T12:08:37","subject":"Re: [PATCH v4 17/21] ipa: mali-c55: Convert AWB to UQ4_8 usage","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 14. 1:54 keltezéssel, Kieran Bingham írta:\n> Utilise the new FixedPoint type to explicitly calculate gains for AWB in Q4.8\n> format. This ensures that reporting of gains in metadata reflect the true\n> AWB gains applied.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/mali-c55/algorithms/awb.cpp | 44 ++++++++++++++---------------\n> src/ipa/mali-c55/ipa_context.h | 10 ++++---\n> 2 files changed, 28 insertions(+), 26 deletions(-)\n> \n> diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp\n> index 3d546e5a854b..6e213f43e762 100644\n> --- a/src/ipa/mali-c55/algorithms/awb.cpp\n> +++ b/src/ipa/mali-c55/algorithms/awb.cpp\n> @@ -37,8 +37,8 @@ int Awb::configure([[maybe_unused]] IPAContext &context,\n> \t * for the first frame we will make no assumptions and leave the R/B\n> \t * channels unmodified.\n> \t */\n> -\tcontext.activeState.awb.rGain = 1.0;\n> -\tcontext.activeState.awb.bGain = 1.0;\n> +\tcontext.activeState.awb.rGain = 1.0f;\n> +\tcontext.activeState.awb.bGain = 1.0f;\n> \n> \treturn 0;\n> }\n> @@ -50,8 +50,8 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context\n> \tblock.header->flags = MALI_C55_PARAM_BLOCK_FL_NONE;\n> \tblock.header->size = sizeof(struct mali_c55_params_awb_gains);\n> \n> -\tdouble rGain = context.activeState.awb.rGain;\n> -\tdouble bGain = context.activeState.awb.bGain;\n> +\tUQ4_8 rGain = context.activeState.awb.rGain;\n> +\tUQ4_8 bGain = context.activeState.awb.bGain;\n> \n> \t/*\n> \t * The gains here map as follows:\n> @@ -63,10 +63,10 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context\n> \t * This holds true regardless of the bayer order of the input data, as\n> \t * the mapping is done internally in the ISP.\n> \t */\n> -\tblock.awb_gains->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain);\n> -\tblock.awb_gains->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);\n> -\tblock.awb_gains->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);\n> -\tblock.awb_gains->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain);\n> +\tblock.awb_gains->gain00 = rGain.quantized();\n> +\tblock.awb_gains->gain01 = UQ4_8(1.0f).quantized();\n> +\tblock.awb_gains->gain10 = UQ4_8(1.0f).quantized();\n> +\tblock.awb_gains->gain11 = bGain.quantized();\n> \n> \tframeContext.awb.rGain = rGain;\n> \tframeContext.awb.bGain = bGain;\n> @@ -162,8 +162,8 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> \t\t * The statistics are in Q4.8 format, so we convert to double\n> \t\t * here.\n> \t\t */\n> -\t\trgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_rg_gr);\n> -\t\tbgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_bg_br);\n> +\t\trgSum += UQ4_8(awb_ratios[i].avg_rg_gr).value();\n> +\t\tbgSum += UQ4_8(awb_ratios[i].avg_bg_br).value();\n> \t\tcounted_zones++;\n> \t}\n> \n> @@ -186,8 +186,8 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> \t * figure by the gains that were applied when the statistics for this\n> \t * frame were generated.\n> \t */\n> -\tdouble rRatio = rgAvg / frameContext.awb.rGain;\n> -\tdouble bRatio = bgAvg / frameContext.awb.bGain;\n> +\tdouble rRatio = rgAvg / frameContext.awb.rGain.value();\n> +\tdouble bRatio = bgAvg / frameContext.awb.bGain.value();\n> \n> \t/*\n> \t * And then we can simply invert the ratio to find the gain we should\n> @@ -203,24 +203,24 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> \t * want to fix the miscolouring as quickly as possible.\n> \t */\n> \tdouble speed = frame < kNumStartupFrames ? 1.0 : 0.2;\n> -\trGain = speed * rGain + context.activeState.awb.rGain * (1.0 - speed);\n> -\tbGain = speed * bGain + context.activeState.awb.bGain * (1.0 - speed);\n> +\trGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0 - speed);\n> +\tbGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0 - speed);\n> \n> -\tcontext.activeState.awb.rGain = rGain;\n> -\tcontext.activeState.awb.bGain = bGain;\n> +\tcontext.activeState.awb.rGain = static_cast(rGain);\n> +\tcontext.activeState.awb.bGain = static_cast(bGain);\n\nI'm wondering if it might make sense to replace the `double`s with `float`s.\nBut otherwise the transformation looks ok.\n\nReviewed-by: Barnabás Pőcze \n\n\n\n> \n> \tmetadata.set(controls::ColourGains, {\n> -\t\tstatic_cast(frameContext.awb.rGain),\n> -\t\tstatic_cast(frameContext.awb.bGain),\n> +\t\tframeContext.awb.rGain.value(),\n> +\t\tframeContext.awb.bGain.value(),\n> \t});\n> \n> \tLOG(MaliC55Awb, Debug) << \"For frame number \" << frame << \": \"\n> \t\t<< \"Average R/G Ratio: \" << rgAvg\n> \t\t<< \", Average B/G Ratio: \" << bgAvg\n> -\t\t<< \"\\nrGain applied to this frame: \" << frameContext.awb.rGain\n> -\t\t<< \", bGain applied to this frame: \" << frameContext.awb.bGain\n> -\t\t<< \"\\nrGain to apply: \" << context.activeState.awb.rGain\n> -\t\t<< \", bGain to apply: \" << context.activeState.awb.bGain;\n> +\t\t<< \"\\nrGain applied to this frame: \" << frameContext.awb.rGain.value()\n> +\t\t<< \", bGain applied to this frame: \" << frameContext.awb.bGain.value()\n> +\t\t<< \"\\nrGain to apply: \" << context.activeState.awb.rGain.value()\n> +\t\t<< \", bGain to apply: \" << context.activeState.awb.bGain.value();\n> }\n> \n> REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> index 13885eb83b5c..86d060a731cb 100644\n> --- a/src/ipa/mali-c55/ipa_context.h\n> +++ b/src/ipa/mali-c55/ipa_context.h\n> @@ -14,6 +14,8 @@\n> \n> #include \n> \n> +#include \"libipa/fixedpoint.h\"\n> +\n> namespace libcamera {\n> \n> namespace ipa::mali_c55 {\n> @@ -53,8 +55,8 @@ struct IPAActiveState {\n> \t} agc;\n> \n> \tstruct {\n> -\t\tdouble rGain;\n> -\t\tdouble bGain;\n> +\t\tUQ4_8 rGain;\n> +\t\tUQ4_8 bGain;\n> \t} awb;\n> };\n> \n> @@ -66,8 +68,8 @@ struct IPAFrameContext : public FrameContext {\n> \t} agc;\n> \n> \tstruct {\n> -\t\tdouble rGain;\n> -\t\tdouble bGain;\n> +\t\tUQ4_8 rGain;\n> +\t\tUQ4_8 bGain;\n> \t} awb;\n> };\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 BA57FBD80A\n\tfor ;\n\tMon, 17 Nov 2025 12:08:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F193D60856;\n\tMon, 17 Nov 2025 13:08:42 +0100 (CET)","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 1DD3460856\n\tfor ;\n\tMon, 17 Nov 2025 13:08:41 +0100 (CET)","from [192.168.33.31] (185.221.143.100.nat.pool.zt.hu\n\t[185.221.143.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F2EECFE;\n\tMon, 17 Nov 2025 13:06:37 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"C3JLyDbP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763381198;\n\tbh=Ub4w6B122PLBhBHafij9UT5UfelqF8myAi7IJezciWY=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=C3JLyDbPwBoMQz/az/WMn2o9Gc3Ar0VAa3HUW64HmGdcPAwBFEGYvDNCnWMQBdo9a\n\tcJ2UPQky5Gln5XjDJcTYLqy73AW4NS0nnwpD5uMUL+XakNo6oPopG79R3axyHsUbcw\n\t7+ujSTyViQFVr6GvXI7VvO9avtb9S26e0cg30HVQ=","Message-ID":"<7c23a25e-21d5-4d09-922d-bf059b520211@ideasonboard.com>","Date":"Mon, 17 Nov 2025 13:08:37 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 17/21] ipa: mali-c55: Convert AWB to UQ4_8 usage","To":"Kieran Bingham ,\n\tlibcamera devel ","References":"<20251114005428.90024-1-kieran.bingham@ideasonboard.com>\n\t<20251114005428.90024-18-kieran.bingham@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251114005428.90024-18-kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36858,"web_url":"https://patchwork.libcamera.org/comment/36858/","msgid":"<176338153807.1979555.16777242168849441090@ping.linuxembedded.co.uk>","date":"2025-11-17T12:12:18","subject":"Re: [PATCH v4 17/21] ipa: mali-c55: Convert AWB to UQ4_8 usage","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-11-17 12:08:37)\n> Hi\n> \n> 2025. 11. 14. 1:54 keltezéssel, Kieran Bingham írta:\n> > Utilise the new FixedPoint type to explicitly calculate gains for AWB in Q4.8\n> > format. This ensures that reporting of gains in metadata reflect the true\n> > AWB gains applied.\n> > \n> > Signed-off-by: Kieran Bingham \n> > ---\n> > src/ipa/mali-c55/algorithms/awb.cpp | 44 ++++++++++++++---------------\n> > src/ipa/mali-c55/ipa_context.h | 10 ++++---\n> > 2 files changed, 28 insertions(+), 26 deletions(-)\n> > \n> > diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp\n> > index 3d546e5a854b..6e213f43e762 100644\n> > --- a/src/ipa/mali-c55/algorithms/awb.cpp\n> > +++ b/src/ipa/mali-c55/algorithms/awb.cpp\n> > @@ -37,8 +37,8 @@ int Awb::configure([[maybe_unused]] IPAContext &context,\n> > * for the first frame we will make no assumptions and leave the R/B\n> > * channels unmodified.\n> > */\n> > - context.activeState.awb.rGain = 1.0;\n> > - context.activeState.awb.bGain = 1.0;\n> > + context.activeState.awb.rGain = 1.0f;\n> > + context.activeState.awb.bGain = 1.0f;\n> > \n> > return 0;\n> > }\n> > @@ -50,8 +50,8 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context\n> > block.header->flags = MALI_C55_PARAM_BLOCK_FL_NONE;\n> > block.header->size = sizeof(struct mali_c55_params_awb_gains);\n> > \n> > - double rGain = context.activeState.awb.rGain;\n> > - double bGain = context.activeState.awb.bGain;\n> > + UQ4_8 rGain = context.activeState.awb.rGain;\n> > + UQ4_8 bGain = context.activeState.awb.bGain;\n> > \n> > /*\n> > * The gains here map as follows:\n> > @@ -63,10 +63,10 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context\n> > * This holds true regardless of the bayer order of the input data, as\n> > * the mapping is done internally in the ISP.\n> > */\n> > - block.awb_gains->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain);\n> > - block.awb_gains->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);\n> > - block.awb_gains->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);\n> > - block.awb_gains->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain);\n> > + block.awb_gains->gain00 = rGain.quantized();\n> > + block.awb_gains->gain01 = UQ4_8(1.0f).quantized();\n> > + block.awb_gains->gain10 = UQ4_8(1.0f).quantized();\n> > + block.awb_gains->gain11 = bGain.quantized();\n> > \n> > frameContext.awb.rGain = rGain;\n> > frameContext.awb.bGain = bGain;\n> > @@ -162,8 +162,8 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> > * The statistics are in Q4.8 format, so we convert to double\n> > * here.\n> > */\n> > - rgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_rg_gr);\n> > - bgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_bg_br);\n> > + rgSum += UQ4_8(awb_ratios[i].avg_rg_gr).value();\n> > + bgSum += UQ4_8(awb_ratios[i].avg_bg_br).value();\n> > counted_zones++;\n> > }\n> > \n> > @@ -186,8 +186,8 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> > * figure by the gains that were applied when the statistics for this\n> > * frame were generated.\n> > */\n> > - double rRatio = rgAvg / frameContext.awb.rGain;\n> > - double bRatio = bgAvg / frameContext.awb.bGain;\n> > + double rRatio = rgAvg / frameContext.awb.rGain.value();\n> > + double bRatio = bgAvg / frameContext.awb.bGain.value();\n> > \n> > /*\n> > * And then we can simply invert the ratio to find the gain we should\n> > @@ -203,24 +203,24 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> > * want to fix the miscolouring as quickly as possible.\n> > */\n> > double speed = frame < kNumStartupFrames ? 1.0 : 0.2;\n> > - rGain = speed * rGain + context.activeState.awb.rGain * (1.0 - speed);\n> > - bGain = speed * bGain + context.activeState.awb.bGain * (1.0 - speed);\n> > + rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0 - speed);\n> > + bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0 - speed);\n> > \n> > - context.activeState.awb.rGain = rGain;\n> > - context.activeState.awb.bGain = bGain;\n> > + context.activeState.awb.rGain = static_cast(rGain);\n> > + context.activeState.awb.bGain = static_cast(bGain);\n> \n> I'm wondering if it might make sense to replace the `double`s with `float`s.\n> But otherwise the transformation looks ok.\n\nYes, these gains here don't need to be doubles. I'll add a preceeding\npatch.\n\n> \n> Reviewed-by: Barnabás Pőcze \n> \n> \n> \n> > \n> > metadata.set(controls::ColourGains, {\n> > - static_cast(frameContext.awb.rGain),\n> > - static_cast(frameContext.awb.bGain),\n> > + frameContext.awb.rGain.value(),\n> > + frameContext.awb.bGain.value(),\n> > });\n> > \n> > LOG(MaliC55Awb, Debug) << \"For frame number \" << frame << \": \"\n> > << \"Average R/G Ratio: \" << rgAvg\n> > << \", Average B/G Ratio: \" << bgAvg\n> > - << \"\\nrGain applied to this frame: \" << frameContext.awb.rGain\n> > - << \", bGain applied to this frame: \" << frameContext.awb.bGain\n> > - << \"\\nrGain to apply: \" << context.activeState.awb.rGain\n> > - << \", bGain to apply: \" << context.activeState.awb.bGain;\n> > + << \"\\nrGain applied to this frame: \" << frameContext.awb.rGain.value()\n> > + << \", bGain applied to this frame: \" << frameContext.awb.bGain.value()\n> > + << \"\\nrGain to apply: \" << context.activeState.awb.rGain.value()\n> > + << \", bGain to apply: \" << context.activeState.awb.bGain.value();\n> > }\n> > \n> > REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> > diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> > index 13885eb83b5c..86d060a731cb 100644\n> > --- a/src/ipa/mali-c55/ipa_context.h\n> > +++ b/src/ipa/mali-c55/ipa_context.h\n> > @@ -14,6 +14,8 @@\n> > \n> > #include \n> > \n> > +#include \"libipa/fixedpoint.h\"\n> > +\n> > namespace libcamera {\n> > \n> > namespace ipa::mali_c55 {\n> > @@ -53,8 +55,8 @@ struct IPAActiveState {\n> > } agc;\n> > \n> > struct {\n> > - double rGain;\n> > - double bGain;\n> > + UQ4_8 rGain;\n> > + UQ4_8 bGain;\n> > } awb;\n> > };\n> > \n> > @@ -66,8 +68,8 @@ struct IPAFrameContext : public FrameContext {\n> > } agc;\n> > \n> > struct {\n> > - double rGain;\n> > - double bGain;\n> > + UQ4_8 rGain;\n> > + UQ4_8 bGain;\n> > } awb;\n> > };\n> > \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 E5C69C0F1B\n\tfor ;\n\tMon, 17 Nov 2025 12:12:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9F1A9609DE;\n\tMon, 17 Nov 2025 13:12:22 +0100 (CET)","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 9D8DD60856\n\tfor ;\n\tMon, 17 Nov 2025 13:12:20 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A66FF142;\n\tMon, 17 Nov 2025 13:10:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UVH0QpeQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763381417;\n\tbh=V5Gp8vsfPKZogEfDt9mX2GW7zhmklyrNH5IsiDXpl4g=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=UVH0QpeQNdj2S6O2c+A4AUXbAXVvQdPUMl5phkTM0HdVcXLm9cXqscTnmDm/PR1OH\n\tB+6jKWR1vrHPj+OVJH6Fl0JCkockRVQVN5XFFUKdbsWTfSNZynEzTuuZGFeKovYxoG\n\taf4KPLsmpDaNjVtyk+9aZXB7CgratbQdhPHhOPTs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<7c23a25e-21d5-4d09-922d-bf059b520211@ideasonboard.com>","References":"<20251114005428.90024-1-kieran.bingham@ideasonboard.com>\n\t<20251114005428.90024-18-kieran.bingham@ideasonboard.com>\n\t<7c23a25e-21d5-4d09-922d-bf059b520211@ideasonboard.com>","Subject":"Re: [PATCH v4 17/21] ipa: mali-c55: Convert AWB to UQ4_8 usage","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera devel ","Date":"Mon, 17 Nov 2025 12:12:18 +0000","Message-ID":"<176338153807.1979555.16777242168849441090@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":36883,"web_url":"https://patchwork.libcamera.org/comment/36883/","msgid":"<176346969785.880260.14950234564777415851@isaac-ThinkPad-T16-Gen-2>","date":"2025-11-18T12:41:37","subject":"Re: [PATCH v4 17/21] ipa: mali-c55: Convert AWB to UQ4_8 usage","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch!\n\nReviewed-by: Isaac Scott \n\nQuoting Kieran Bingham (2025-11-14 00:54:21)\n> Utilise the new FixedPoint type to explicitly calculate gains for AWB in Q4.8\n> format. This ensures that reporting of gains in metadata reflect the true\n> AWB gains applied.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/mali-c55/algorithms/awb.cpp | 44 ++++++++++++++---------------\n> src/ipa/mali-c55/ipa_context.h | 10 ++++---\n> 2 files changed, 28 insertions(+), 26 deletions(-)\n> \n> diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp\n> index 3d546e5a854b..6e213f43e762 100644\n> --- a/src/ipa/mali-c55/algorithms/awb.cpp\n> +++ b/src/ipa/mali-c55/algorithms/awb.cpp\n> @@ -37,8 +37,8 @@ int Awb::configure([[maybe_unused]] IPAContext &context,\n> * for the first frame we will make no assumptions and leave the R/B\n> * channels unmodified.\n> */\n> - context.activeState.awb.rGain = 1.0;\n> - context.activeState.awb.bGain = 1.0;\n> + context.activeState.awb.rGain = 1.0f;\n> + context.activeState.awb.bGain = 1.0f;\n> \n> return 0;\n> }\n> @@ -50,8 +50,8 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context\n> block.header->flags = MALI_C55_PARAM_BLOCK_FL_NONE;\n> block.header->size = sizeof(struct mali_c55_params_awb_gains);\n> \n> - double rGain = context.activeState.awb.rGain;\n> - double bGain = context.activeState.awb.bGain;\n> + UQ4_8 rGain = context.activeState.awb.rGain;\n> + UQ4_8 bGain = context.activeState.awb.bGain;\n> \n> /*\n> * The gains here map as follows:\n> @@ -63,10 +63,10 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context\n> * This holds true regardless of the bayer order of the input data, as\n> * the mapping is done internally in the ISP.\n> */\n> - block.awb_gains->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain);\n> - block.awb_gains->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);\n> - block.awb_gains->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);\n> - block.awb_gains->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain);\n> + block.awb_gains->gain00 = rGain.quantized();\n> + block.awb_gains->gain01 = UQ4_8(1.0f).quantized();\n> + block.awb_gains->gain10 = UQ4_8(1.0f).quantized();\n> + block.awb_gains->gain11 = bGain.quantized();\n> \n> frameContext.awb.rGain = rGain;\n> frameContext.awb.bGain = bGain;\n> @@ -162,8 +162,8 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> * The statistics are in Q4.8 format, so we convert to double\n> * here.\n> */\n> - rgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_rg_gr);\n> - bgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_bg_br);\n> + rgSum += UQ4_8(awb_ratios[i].avg_rg_gr).value();\n> + bgSum += UQ4_8(awb_ratios[i].avg_bg_br).value();\n> counted_zones++;\n> }\n> \n> @@ -186,8 +186,8 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> * figure by the gains that were applied when the statistics for this\n> * frame were generated.\n> */\n> - double rRatio = rgAvg / frameContext.awb.rGain;\n> - double bRatio = bgAvg / frameContext.awb.bGain;\n> + double rRatio = rgAvg / frameContext.awb.rGain.value();\n> + double bRatio = bgAvg / frameContext.awb.bGain.value();\n> \n> /*\n> * And then we can simply invert the ratio to find the gain we should\n> @@ -203,24 +203,24 @@ void Awb::process(IPAContext &context, const uint32_t frame,\n> * want to fix the miscolouring as quickly as possible.\n> */\n> double speed = frame < kNumStartupFrames ? 1.0 : 0.2;\n> - rGain = speed * rGain + context.activeState.awb.rGain * (1.0 - speed);\n> - bGain = speed * bGain + context.activeState.awb.bGain * (1.0 - speed);\n> + rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0 - speed);\n> + bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0 - speed);\n> \n> - context.activeState.awb.rGain = rGain;\n> - context.activeState.awb.bGain = bGain;\n> + context.activeState.awb.rGain = static_cast(rGain);\n> + context.activeState.awb.bGain = static_cast(bGain);\n> \n> metadata.set(controls::ColourGains, {\n> - static_cast(frameContext.awb.rGain),\n> - static_cast(frameContext.awb.bGain),\n> + frameContext.awb.rGain.value(),\n> + frameContext.awb.bGain.value(),\n> });\n> \n> LOG(MaliC55Awb, Debug) << \"For frame number \" << frame << \": \"\n> << \"Average R/G Ratio: \" << rgAvg\n> << \", Average B/G Ratio: \" << bgAvg\n> - << \"\\nrGain applied to this frame: \" << frameContext.awb.rGain\n> - << \", bGain applied to this frame: \" << frameContext.awb.bGain\n> - << \"\\nrGain to apply: \" << context.activeState.awb.rGain\n> - << \", bGain to apply: \" << context.activeState.awb.bGain;\n> + << \"\\nrGain applied to this frame: \" << frameContext.awb.rGain.value()\n> + << \", bGain applied to this frame: \" << frameContext.awb.bGain.value()\n> + << \"\\nrGain to apply: \" << context.activeState.awb.rGain.value()\n> + << \", bGain to apply: \" << context.activeState.awb.bGain.value();\n> }\n> \n> REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> index 13885eb83b5c..86d060a731cb 100644\n> --- a/src/ipa/mali-c55/ipa_context.h\n> +++ b/src/ipa/mali-c55/ipa_context.h\n> @@ -14,6 +14,8 @@\n> \n> #include \n> \n> +#include \"libipa/fixedpoint.h\"\n> +\n> namespace libcamera {\n> \n> namespace ipa::mali_c55 {\n> @@ -53,8 +55,8 @@ struct IPAActiveState {\n> } agc;\n> \n> struct {\n> - double rGain;\n> - double bGain;\n> + UQ4_8 rGain;\n> + UQ4_8 bGain;\n> } awb;\n> };\n> \n> @@ -66,8 +68,8 @@ struct IPAFrameContext : public FrameContext {\n> } agc;\n> \n> struct {\n> - double rGain;\n> - double bGain;\n> + UQ4_8 rGain;\n> + UQ4_8 bGain;\n> } awb;\n> };\n> \n> -- \n> 2.51.1\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 91416BD80A\n\tfor ;\n\tTue, 18 Nov 2025 12:41:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 372D260AB1;\n\tTue, 18 Nov 2025 13:41:43 +0100 (CET)","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 EBCE660A85\n\tfor ;\n\tTue, 18 Nov 2025 13:41:40 +0100 (CET)","from thinkpad.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3E239D52;\n\tTue, 18 Nov 2025 13:39:37 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FmiRyouV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763469577;\n\tbh=qIxXcan8W46ZeBSF4NIrWX8sMgmqIPy7P6ZPVMgJ2Ys=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=FmiRyouVbayR07wU1RCsg+Ckfx9/JHfDHvyKHIQe7NQznuETXx33QtXy5qv3c9hzP\n\tOnkmATbDppLWs7kaPq+nEJg6eX9gnO3ahArxytKSI323MpOM0bRvFJmOIpXdju8hQz\n\tw7y9xQaFnME6E8bUKfPEY/z2I7i8W7O5LuBjKtqg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251114005428.90024-18-kieran.bingham@ideasonboard.com>","References":"<20251114005428.90024-1-kieran.bingham@ideasonboard.com>\n\t<20251114005428.90024-18-kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v4 17/21] ipa: mali-c55: Convert AWB to UQ4_8 usage","From":"Isaac Scott ","Cc":"Kieran Bingham ","To":"Kieran Bingham ,\n\tlibcamera devel ","Date":"Tue, 18 Nov 2025 12:41:37 +0000","Message-ID":"<176346969785.880260.14950234564777415851@isaac-ThinkPad-T16-Gen-2>","User-Agent":"alot/0.10","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\" "}}]