[{"id":34146,"web_url":"https://patchwork.libcamera.org/comment/34146/","msgid":"","date":"2025-05-07T16:51:17","subject":"Re: [PATCH v3 09/16] ipa: rkisp1: Refactor automatic/manual\n\tstructure in IPAActiveState","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Apr 03, 2025 at 05:49:14PM +0200, Stefan Klug wrote:\n> Swap gains and automatic/manual in the IPAActiveState structure. This is\n> in preparation to adding another member, which is easier in the new\n> structure. The patch contains no functional changes.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> \n> Changes in v2:\n> - Use one named struct instead of two anonymous ones\n> \n> Changes in v3:\n> - Collected tags\n> - Updated documentation\n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------\n> src/ipa/rkisp1/ipa_context.cpp | 13 ++++++++-----\n> src/ipa/rkisp1/ipa_context.h | 10 ++++++----\n> 3 files changed, 26 insertions(+), 21 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index eafe93081bb1..a9759e53f593 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -124,8 +124,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> int Awb::configure(IPAContext &context,\n> \t\t const IPACameraSensorInfo &configInfo)\n> {\n> -\tcontext.activeState.awb.gains.manual = RGB{ 1.0 };\n> -\tcontext.activeState.awb.gains.automatic =\n> +\tcontext.activeState.awb.manual.gains = RGB{ 1.0 };\n> +\tcontext.activeState.awb.automatic.gains =\n> \t\tawbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> \tcontext.activeState.awb.autoEnabled = true;\n> \tcontext.activeState.awb.temperatureK = kDefaultColourTemperature;\n> @@ -173,8 +173,8 @@ void Awb::queueRequest(IPAContext &context,\n> \tconst auto &colourTemperature = controls.get(controls::ColourTemperature);\n> \tbool update = false;\n> \tif (colourGains) {\n> -\t\tawb.gains.manual.r() = (*colourGains)[0];\n> -\t\tawb.gains.manual.b() = (*colourGains)[1];\n> +\t\tawb.manual.gains.r() = (*colourGains)[0];\n> +\t\tawb.manual.gains.b() = (*colourGains)[1];\n> \t\t/*\n> \t\t * \\todo Colour temperature reported in metadata is now\n> \t\t * incorrect, as we can't deduce the temperature from the gains.\n> @@ -183,17 +183,17 @@ void Awb::queueRequest(IPAContext &context,\n> \t\tupdate = true;\n> \t} else if (colourTemperature) {\n> \t\tconst auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> -\t\tawb.gains.manual.r() = gains.r();\n> -\t\tawb.gains.manual.b() = gains.b();\n> +\t\tawb.manual.gains.r() = gains.r();\n> +\t\tawb.manual.gains.b() = gains.b();\n> \t\tawb.temperatureK = *colourTemperature;\n> \t\tupdate = true;\n> \t}\n> \n> \tif (update)\n> \t\tLOG(RkISP1Awb, Debug)\n> -\t\t\t<< \"Set colour gains to \" << awb.gains.manual;\n> +\t\t\t<< \"Set colour gains to \" << awb.manual.gains;\n> \n> -\tframeContext.awb.gains = awb.gains.manual;\n> +\tframeContext.awb.gains = awb.manual.gains;\n> \tframeContext.awb.temperatureK = awb.temperatureK;\n> }\n> \n> @@ -208,7 +208,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> \t * most up-to-date automatic values we can read.\n> \t */\n> \tif (frameContext.awb.autoEnabled) {\n> -\t\tframeContext.awb.gains = context.activeState.awb.gains.automatic;\n> +\t\tframeContext.awb.gains = context.activeState.awb.automatic.gains;\n> \t\tframeContext.awb.temperatureK = context.activeState.awb.temperatureK;\n> \t}\n> \n> @@ -325,14 +325,14 @@ void Awb::process(IPAContext &context,\n> \t/* Filter the values to avoid oscillations. */\n> \tdouble speed = 0.2;\n> \tawbResult.gains = awbResult.gains * speed +\n> -\t\t\t activeState.awb.gains.automatic * (1 - speed);\n> +\t\t\t activeState.awb.automatic.gains * (1 - speed);\n> \n> -\tactiveState.awb.gains.automatic = awbResult.gains;\n> +\tactiveState.awb.automatic.gains = awbResult.gains;\n> \n> \tLOG(RkISP1Awb, Debug)\n> \t\t<< std::showpoint\n> \t\t<< \"Means \" << rgbMeans << \", gains \"\n> -\t\t<< activeState.awb.gains.automatic << \", temp \"\n> +\t\t<< activeState.awb.automatic.gains << \", temp \"\n> \t\t<< activeState.awb.temperatureK << \"K\";\n> }\n> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 99611bd5b390..39b97d143e95 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -191,14 +191,17 @@ namespace libcamera::ipa::rkisp1 {\n> * \\var IPAActiveState::awb\n> * \\brief State for the Automatic White Balance algorithm\n> *\n> - * \\struct IPAActiveState::awb.gains\n> + * \\var IPAActiveState::awb::AwbState\n\nShouldn't this be \\struct ?\n\n> + * \\brief Struct for the AWB regulation state\n> + *\n> + * \\struct IPAActiveState::awb::AwbState.gains\n\nAnd this \\var ?\n\n\nPaul\n\n> * \\brief White balance gains\n> *\n> - * \\var IPAActiveState::awb.gains.manual\n> - * \\brief Manual white balance gains (set through requests)\n> + * \\var IPAActiveState::awb.manual\n> + * \\brief Manual regulation state (set through requests)\n> *\n> - * \\var IPAActiveState::awb.gains.automatic\n> - * \\brief Automatic white balance gains (computed by the algorithm)\n> + * \\var IPAActiveState::awb.automatic\n> + * \\brief Automatic regulation state (computed by the algorithm)\n> *\n> * \\var IPAActiveState::awb.temperatureK\n> * \\brief Estimated color temperature\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 474f7036f003..6bc922a82971 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -89,10 +89,12 @@ struct IPAActiveState {\n> \t} agc;\n> \n> \tstruct {\n> -\t\tstruct {\n> -\t\t\tRGB manual;\n> -\t\t\tRGB automatic;\n> -\t\t} gains;\n> +\t\tstruct AwbState {\n> +\t\t\tRGB gains;\n> +\t\t};\n> +\n> +\t\tAwbState manual;\n> +\t\tAwbState automatic;\n> \n> \t\tunsigned int temperatureK;\n> \t\tbool autoEnabled;\n> -- \n> 2.43.0\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 80BD0C3200\n\tfor ;\n\tWed, 7 May 2025 16:51:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1FCA68B32;\n\tWed, 7 May 2025 18:51: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 C95C368B2F\n\tfor ;\n\tWed, 7 May 2025 18:51:21 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2001:861:3a80:3300:4f2f:8c2c:b3ef:17d4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 221526D5;\n\tWed, 7 May 2025 18:51:10 +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=\"XU8Mfnxf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746636670;\n\tbh=C4eeMJlsu9oOi0/0Y/t2A3zPm93SpbN+50aqkO5LAcM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XU8MfnxfHTkDfuv9zzN+E8nE7W7HykqnWEH60DSg/FcET0AZc1gcw+rZ774t0Hj86\n\t/KmhHCAeLk8js+Pckae5Nou4GIzywDLe3BNcA3/7AUCTuYwK6xwS/wnD6VcX1kBDvg\n\tl1lNGJOgEdoGp0Fts9RuImr3LScLxagZY/mItzoc=","Date":"Wed, 7 May 2025 18:51:17 +0200","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tLaurent Pinchart ","Subject":"Re: [PATCH v3 09/16] ipa: rkisp1: Refactor automatic/manual\n\tstructure in IPAActiveState","Message-ID":"","References":"<20250403154925.382973-1-stefan.klug@ideasonboard.com>\n\t<20250403154925.382973-10-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250403154925.382973-10-stefan.klug@ideasonboard.com>","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":34282,"web_url":"https://patchwork.libcamera.org/comment/34282/","msgid":"<174772800864.11206.16388253793220801524@localhost>","date":"2025-05-20T08:00:08","subject":"Re: [PATCH v3 09/16] ipa: rkisp1: Refactor automatic/manual\n\tstructure in IPAActiveState","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the review.\n\nQuoting Paul Elder (2025-05-07 18:51:17)\n> On Thu, Apr 03, 2025 at 05:49:14PM +0200, Stefan Klug wrote:\n> > Swap gains and automatic/manual in the IPAActiveState structure. This is\n> > in preparation to adding another member, which is easier in the new\n> > structure. The patch contains no functional changes.\n> > \n> > Signed-off-by: Stefan Klug \n> > Reviewed-by: Kieran Bingham \n> > Reviewed-by: Laurent Pinchart \n> > \n> > ---\n> > \n> > Changes in v2:\n> > - Use one named struct instead of two anonymous ones\n> > \n> > Changes in v3:\n> > - Collected tags\n> > - Updated documentation\n> > ---\n> > src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------\n> > src/ipa/rkisp1/ipa_context.cpp | 13 ++++++++-----\n> > src/ipa/rkisp1/ipa_context.h | 10 ++++++----\n> > 3 files changed, 26 insertions(+), 21 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index eafe93081bb1..a9759e53f593 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -124,8 +124,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> > int Awb::configure(IPAContext &context,\n> > const IPACameraSensorInfo &configInfo)\n> > {\n> > - context.activeState.awb.gains.manual = RGB{ 1.0 };\n> > - context.activeState.awb.gains.automatic =\n> > + context.activeState.awb.manual.gains = RGB{ 1.0 };\n> > + context.activeState.awb.automatic.gains =\n> > awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> > context.activeState.awb.autoEnabled = true;\n> > context.activeState.awb.temperatureK = kDefaultColourTemperature;\n> > @@ -173,8 +173,8 @@ void Awb::queueRequest(IPAContext &context,\n> > const auto &colourTemperature = controls.get(controls::ColourTemperature);\n> > bool update = false;\n> > if (colourGains) {\n> > - awb.gains.manual.r() = (*colourGains)[0];\n> > - awb.gains.manual.b() = (*colourGains)[1];\n> > + awb.manual.gains.r() = (*colourGains)[0];\n> > + awb.manual.gains.b() = (*colourGains)[1];\n> > /*\n> > * \\todo Colour temperature reported in metadata is now\n> > * incorrect, as we can't deduce the temperature from the gains.\n> > @@ -183,17 +183,17 @@ void Awb::queueRequest(IPAContext &context,\n> > update = true;\n> > } else if (colourTemperature) {\n> > const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> > - awb.gains.manual.r() = gains.r();\n> > - awb.gains.manual.b() = gains.b();\n> > + awb.manual.gains.r() = gains.r();\n> > + awb.manual.gains.b() = gains.b();\n> > awb.temperatureK = *colourTemperature;\n> > update = true;\n> > }\n> > \n> > if (update)\n> > LOG(RkISP1Awb, Debug)\n> > - << \"Set colour gains to \" << awb.gains.manual;\n> > + << \"Set colour gains to \" << awb.manual.gains;\n> > \n> > - frameContext.awb.gains = awb.gains.manual;\n> > + frameContext.awb.gains = awb.manual.gains;\n> > frameContext.awb.temperatureK = awb.temperatureK;\n> > }\n> > \n> > @@ -208,7 +208,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> > * most up-to-date automatic values we can read.\n> > */\n> > if (frameContext.awb.autoEnabled) {\n> > - frameContext.awb.gains = context.activeState.awb.gains.automatic;\n> > + frameContext.awb.gains = context.activeState.awb.automatic.gains;\n> > frameContext.awb.temperatureK = context.activeState.awb.temperatureK;\n> > }\n> > \n> > @@ -325,14 +325,14 @@ void Awb::process(IPAContext &context,\n> > /* Filter the values to avoid oscillations. */\n> > double speed = 0.2;\n> > awbResult.gains = awbResult.gains * speed +\n> > - activeState.awb.gains.automatic * (1 - speed);\n> > + activeState.awb.automatic.gains * (1 - speed);\n> > \n> > - activeState.awb.gains.automatic = awbResult.gains;\n> > + activeState.awb.automatic.gains = awbResult.gains;\n> > \n> > LOG(RkISP1Awb, Debug)\n> > << std::showpoint\n> > << \"Means \" << rgbMeans << \", gains \"\n> > - << activeState.awb.gains.automatic << \", temp \"\n> > + << activeState.awb.automatic.gains << \", temp \"\n> > << activeState.awb.temperatureK << \"K\";\n> > }\n> > \n> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > index 99611bd5b390..39b97d143e95 100644\n> > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > @@ -191,14 +191,17 @@ namespace libcamera::ipa::rkisp1 {\n> > * \\var IPAActiveState::awb\n> > * \\brief State for the Automatic White Balance algorithm\n> > *\n> > - * \\struct IPAActiveState::awb.gains\n> > + * \\var IPAActiveState::awb::AwbState\n> \n> Shouldn't this be \\struct ?\n> \n> > + * \\brief Struct for the AWB regulation state\n> > + *\n> > + * \\struct IPAActiveState::awb::AwbState.gains\n> \n> And this \\var ?\n\nI think you are right on both cases. As the rkisp1 is currently excluded\nfrom doxygen generation, these errors don't pop up. We should enable\ndoxygen for that, but that is for another series :-). I'll change that\nwhile applying.\n\nCheers,\nStefan\n\n> \n> \n> Paul\n> \n> > * \\brief White balance gains\n> > *\n> > - * \\var IPAActiveState::awb.gains.manual\n> > - * \\brief Manual white balance gains (set through requests)\n> > + * \\var IPAActiveState::awb.manual\n> > + * \\brief Manual regulation state (set through requests)\n> > *\n> > - * \\var IPAActiveState::awb.gains.automatic\n> > - * \\brief Automatic white balance gains (computed by the algorithm)\n> > + * \\var IPAActiveState::awb.automatic\n> > + * \\brief Automatic regulation state (computed by the algorithm)\n> > *\n> > * \\var IPAActiveState::awb.temperatureK\n> > * \\brief Estimated color temperature\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 474f7036f003..6bc922a82971 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -89,10 +89,12 @@ struct IPAActiveState {\n> > } agc;\n> > \n> > struct {\n> > - struct {\n> > - RGB manual;\n> > - RGB automatic;\n> > - } gains;\n> > + struct AwbState {\n> > + RGB gains;\n> > + };\n> > +\n> > + AwbState manual;\n> > + AwbState automatic;\n> > \n> > unsigned int temperatureK;\n> > bool autoEnabled;\n> > -- \n> > 2.43.0\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 51029BD78E\n\tfor ;\n\tTue, 20 May 2025 08:00:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3562168D7D;\n\tTue, 20 May 2025 10:00:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18AD2614DD\n\tfor ;\n\tTue, 20 May 2025 10:00:12 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:1988:b6b6:ff72:2181])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 506A974C;\n\tTue, 20 May 2025 09:59:51 +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=\"b6QrLNc4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747727991;\n\tbh=WDl8r1jiW/iS6im5JGUJ/P/GvITSMgNISRBTjhyEz1Q=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=b6QrLNc4mspOmbpyF7d4k+S7bunRUvO6zUtCvo1JNYrVLwFpPYElxmDIGC+wGxgeo\n\tGYMVGoRGJM5FETPflogxsuzqqwYKeOqeF24khljLzZdYNrFbN0/sSfar8nC79Dl2tA\n\t3ippQ+Zm3jqdJnDZ910tScXsb2ugdAGVvF5rGFJo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250403154925.382973-1-stefan.klug@ideasonboard.com>\n\t<20250403154925.382973-10-stefan.klug@ideasonboard.com>\n\t","Subject":"Re: [PATCH v3 09/16] ipa: rkisp1: Refactor automatic/manual\n\tstructure in IPAActiveState","From":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tLaurent Pinchart ","To":"Paul Elder ","Date":"Tue, 20 May 2025 10:00:08 +0200","Message-ID":"<174772800864.11206.16388253793220801524@localhost>","User-Agent":"alot/0.12.dev16+g501a9541e2e6.d20250519","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":34310,"web_url":"https://patchwork.libcamera.org/comment/34310/","msgid":"<174784827972.14042.3628650585089011574@calcite>","date":"2025-05-21T17:24:39","subject":"Re: [PATCH v3 09/16] ipa: rkisp1: Refactor automatic/manual\n\tstructure in IPAActiveState","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-05-20 10:00:08)\n> Hi Paul,\n> \n> Thank you for the review.\n> \n> Quoting Paul Elder (2025-05-07 18:51:17)\n> > On Thu, Apr 03, 2025 at 05:49:14PM +0200, Stefan Klug wrote:\n> > > Swap gains and automatic/manual in the IPAActiveState structure. This is\n> > > in preparation to adding another member, which is easier in the new\n> > > structure. The patch contains no functional changes.\n> > > \n> > > Signed-off-by: Stefan Klug \n> > > Reviewed-by: Kieran Bingham \n> > > Reviewed-by: Laurent Pinchart \n> > > \n> > > ---\n> > > \n> > > Changes in v2:\n> > > - Use one named struct instead of two anonymous ones\n> > > \n> > > Changes in v3:\n> > > - Collected tags\n> > > - Updated documentation\n> > > ---\n> > > src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------\n> > > src/ipa/rkisp1/ipa_context.cpp | 13 ++++++++-----\n> > > src/ipa/rkisp1/ipa_context.h | 10 ++++++----\n> > > 3 files changed, 26 insertions(+), 21 deletions(-)\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > index eafe93081bb1..a9759e53f593 100644\n> > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > @@ -124,8 +124,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> > > int Awb::configure(IPAContext &context,\n> > > const IPACameraSensorInfo &configInfo)\n> > > {\n> > > - context.activeState.awb.gains.manual = RGB{ 1.0 };\n> > > - context.activeState.awb.gains.automatic =\n> > > + context.activeState.awb.manual.gains = RGB{ 1.0 };\n> > > + context.activeState.awb.automatic.gains =\n> > > awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> > > context.activeState.awb.autoEnabled = true;\n> > > context.activeState.awb.temperatureK = kDefaultColourTemperature;\n> > > @@ -173,8 +173,8 @@ void Awb::queueRequest(IPAContext &context,\n> > > const auto &colourTemperature = controls.get(controls::ColourTemperature);\n> > > bool update = false;\n> > > if (colourGains) {\n> > > - awb.gains.manual.r() = (*colourGains)[0];\n> > > - awb.gains.manual.b() = (*colourGains)[1];\n> > > + awb.manual.gains.r() = (*colourGains)[0];\n> > > + awb.manual.gains.b() = (*colourGains)[1];\n> > > /*\n> > > * \\todo Colour temperature reported in metadata is now\n> > > * incorrect, as we can't deduce the temperature from the gains.\n> > > @@ -183,17 +183,17 @@ void Awb::queueRequest(IPAContext &context,\n> > > update = true;\n> > > } else if (colourTemperature) {\n> > > const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> > > - awb.gains.manual.r() = gains.r();\n> > > - awb.gains.manual.b() = gains.b();\n> > > + awb.manual.gains.r() = gains.r();\n> > > + awb.manual.gains.b() = gains.b();\n> > > awb.temperatureK = *colourTemperature;\n> > > update = true;\n> > > }\n> > > \n> > > if (update)\n> > > LOG(RkISP1Awb, Debug)\n> > > - << \"Set colour gains to \" << awb.gains.manual;\n> > > + << \"Set colour gains to \" << awb.manual.gains;\n> > > \n> > > - frameContext.awb.gains = awb.gains.manual;\n> > > + frameContext.awb.gains = awb.manual.gains;\n> > > frameContext.awb.temperatureK = awb.temperatureK;\n> > > }\n> > > \n> > > @@ -208,7 +208,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> > > * most up-to-date automatic values we can read.\n> > > */\n> > > if (frameContext.awb.autoEnabled) {\n> > > - frameContext.awb.gains = context.activeState.awb.gains.automatic;\n> > > + frameContext.awb.gains = context.activeState.awb.automatic.gains;\n> > > frameContext.awb.temperatureK = context.activeState.awb.temperatureK;\n> > > }\n> > > \n> > > @@ -325,14 +325,14 @@ void Awb::process(IPAContext &context,\n> > > /* Filter the values to avoid oscillations. */\n> > > double speed = 0.2;\n> > > awbResult.gains = awbResult.gains * speed +\n> > > - activeState.awb.gains.automatic * (1 - speed);\n> > > + activeState.awb.automatic.gains * (1 - speed);\n> > > \n> > > - activeState.awb.gains.automatic = awbResult.gains;\n> > > + activeState.awb.automatic.gains = awbResult.gains;\n> > > \n> > > LOG(RkISP1Awb, Debug)\n> > > << std::showpoint\n> > > << \"Means \" << rgbMeans << \", gains \"\n> > > - << activeState.awb.gains.automatic << \", temp \"\n> > > + << activeState.awb.automatic.gains << \", temp \"\n> > > << activeState.awb.temperatureK << \"K\";\n> > > }\n> > > \n> > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > > index 99611bd5b390..39b97d143e95 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > > @@ -191,14 +191,17 @@ namespace libcamera::ipa::rkisp1 {\n> > > * \\var IPAActiveState::awb\n> > > * \\brief State for the Automatic White Balance algorithm\n> > > *\n> > > - * \\struct IPAActiveState::awb.gains\n> > > + * \\var IPAActiveState::awb::AwbState\n> > \n> > Shouldn't this be \\struct ?\n> > \n> > > + * \\brief Struct for the AWB regulation state\n> > > + *\n> > > + * \\struct IPAActiveState::awb::AwbState.gains\n> > \n> > And this \\var ?\n> \n> I think you are right on both cases. As the rkisp1 is currently excluded\n> from doxygen generation, these errors don't pop up. We should enable\n> doxygen for that, but that is for another series :-). I'll change that\n\nYeah, we can do that on top.\n\n> while applying.\n\nCool :) with that fixed,\n\nReviewed-by: Paul Elder \n\n> \n> Cheers,\n> Stefan\n> \n> > \n> > \n> > Paul\n> > \n> > > * \\brief White balance gains\n> > > *\n> > > - * \\var IPAActiveState::awb.gains.manual\n> > > - * \\brief Manual white balance gains (set through requests)\n> > > + * \\var IPAActiveState::awb.manual\n> > > + * \\brief Manual regulation state (set through requests)\n> > > *\n> > > - * \\var IPAActiveState::awb.gains.automatic\n> > > - * \\brief Automatic white balance gains (computed by the algorithm)\n> > > + * \\var IPAActiveState::awb.automatic\n> > > + * \\brief Automatic regulation state (computed by the algorithm)\n> > > *\n> > > * \\var IPAActiveState::awb.temperatureK\n> > > * \\brief Estimated color temperature\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index 474f7036f003..6bc922a82971 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -89,10 +89,12 @@ struct IPAActiveState {\n> > > } agc;\n> > > \n> > > struct {\n> > > - struct {\n> > > - RGB manual;\n> > > - RGB automatic;\n> > > - } gains;\n> > > + struct AwbState {\n> > > + RGB gains;\n> > > + };\n> > > +\n> > > + AwbState manual;\n> > > + AwbState automatic;\n> > > \n> > > unsigned int temperatureK;\n> > > bool autoEnabled;\n> > > -- \n> > > 2.43.0\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 41CD7C31E9\n\tfor ;\n\tWed, 21 May 2025 17:24:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 11D9A68D92;\n\tWed, 21 May 2025 19:24:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EF52C68C91\n\tfor ;\n\tWed, 21 May 2025 19:24:42 +0200 (CEST)","from pyrite.rasen.tech (unknown [149.232.183.6])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 599936AF;\n\tWed, 21 May 2025 19:24:21 +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=\"kGa+GSWq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747848261;\n\tbh=n4rATef6C0R4zDff34PsPNG0XLO2dN8NtOsB7C/tNLA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=kGa+GSWqvZTRGfAY+brhqX6L/ERZS3Ty8/ZxV2FuRlIUcBGx4riKPWoB3yJLcYLkS\n\tDACp1sMvlVZ+mCjxL9naPGetu+G40vmlCA3rG7WU4Z32vFlk6bJ/MN9vyLxxQDzp1V\n\tmvTa8mZTZoopXBNocgo+MqEUSs1F8YGfmVvcEIBA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<174772800864.11206.16388253793220801524@localhost>","References":"<20250403154925.382973-1-stefan.klug@ideasonboard.com>\n\t<20250403154925.382973-10-stefan.klug@ideasonboard.com>\n\t\n\t<174772800864.11206.16388253793220801524@localhost>","Subject":"Re: [PATCH v3 09/16] ipa: rkisp1: Refactor automatic/manual\n\tstructure in IPAActiveState","From":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tLaurent Pinchart ","To":"Stefan Klug ","Date":"Wed, 21 May 2025 19:24:39 +0200","Message-ID":"<174784827972.14042.3628650585089011574@calcite>","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\" "}}]