[{"id":33738,"web_url":"https://patchwork.libcamera.org/comment/33738/","msgid":"<174302439880.1701483.9800700792129679592@ping.linuxembedded.co.uk>","date":"2025-03-26T21:26:38","subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-03-26 12:48:52)\n> From: Kieran Bingham \n> \n> Provide the determined colour gains back into the metadata\n> to add to completed requests.\n> \n> Signed-off-by: Kieran Bingham \n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/simple/algorithms/awb.cpp | 21 ++++++++++++++++++++-\n> src/ipa/simple/algorithms/awb.h | 6 +++++-\n> src/ipa/simple/ipa_context.h | 4 ++++\n> 3 files changed, 29 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n> index ec77c6e5..55719059 100644\n> --- a/src/ipa/simple/algorithms/awb.cpp\n> +++ b/src/ipa/simple/algorithms/awb.cpp\n> @@ -17,6 +17,8 @@\n> #include \"libipa/colours.h\"\n> #include \"simple/ipa_context.h\"\n> \n> +#include \"control_ids.h\"\n> +\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPASoftAwb)\n> @@ -32,15 +34,32 @@ int Awb::configure(IPAContext &context,\n> return 0;\n> }\n> \n> +void Awb::prepare(IPAContext &context,\n> + [[maybe_unused]] const uint32_t frame,\n> + IPAFrameContext &frameContext,\n> + [[maybe_unused]] DebayerParams *params)\n> +{\n> + auto &gains = context.activeState.awb.gains;\n> + frameContext.gains.red = gains.r();\n> + frameContext.gains.blue = gains.b();\n\nInteresting that this highlights the white balance parameters are\nprobably being used from the wrong 'time' as they should be determined\nhere in prepare... (by taking the most recent active state and combining\nany decision from the controls for any manual white balance).\n\nBut - as this series is just trying to get the reporting in - lets leave\nthat issue for later, and try to keep this moving forwards:\n\n\nReviewed-by: Kieran Bingham \n\n\n\n> +}\n> +\n> void Awb::process(IPAContext &context,\n> [[maybe_unused]] const uint32_t frame,\n> - [[maybe_unused]] IPAFrameContext &frameContext,\n> + IPAFrameContext &frameContext,\n> const SwIspStats *stats,\n> ControlList &metadata)\n> {\n> const SwIspStats::Histogram &histogram = stats->yHistogram;\n> const uint8_t blackLevel = context.activeState.blc.level;\n> \n> + const float maxGain = 1024.0;\n> + const float mdGains[] = {\n> + static_cast(frameContext.gains.red / maxGain),\n> + static_cast(frameContext.gains.blue / maxGain)\n> + };\n> + metadata.set(controls::ColourGains, mdGains);\n> +\n> /*\n> * Black level must be subtracted to get the correct AWB ratios, they\n> * would be off if they were computed from the whole brightness range\n> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h\n> index db1496cd..ad993f39 100644\n> --- a/src/ipa/simple/algorithms/awb.h\n> +++ b/src/ipa/simple/algorithms/awb.h\n> @@ -1,6 +1,6 @@\n> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> /*\n> - * Copyright (C) 2024, Red Hat Inc.\n> + * Copyright (C) 2024-2025 Red Hat Inc.\n> *\n> * Auto white balance\n> */\n> @@ -20,6 +20,10 @@ public:\n> ~Awb() = default;\n> \n> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> + void prepare(IPAContext &context,\n> + const uint32_t frame,\n> + IPAFrameContext &frameContext,\n> + DebayerParams *params) override;\n> void process(IPAContext &context,\n> const uint32_t frame,\n> IPAFrameContext &frameContext,\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index 17bcd4ca..bfac835b 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -70,6 +70,10 @@ struct IPAFrameContext : public FrameContext {\n> int32_t exposure;\n> double gain;\n> } sensor;\n> + struct {\n> + double red;\n> + double blue;\n> + } gains;\n> };\n> \n> struct IPAContext {\n> -- \n> 2.49.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 C43F2C323E\n\tfor ;\n\tWed, 26 Mar 2025 21:26:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D7A6968967;\n\tWed, 26 Mar 2025 22:26:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 61F7168950\n\tfor ;\n\tWed, 26 Mar 2025 22:26:42 +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 0EC5A475;\n\tWed, 26 Mar 2025 22:24:54 +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=\"u7eDgSz5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743024294;\n\tbh=4IwxBllZBNX+3/FjiBa+hCpMzFiQBzP7jTouoXHIMxw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=u7eDgSz5AvTTG70ofYDPvwq6+PxyB7A4vbSQzg58ixNEAL+y3TXAgyxcFgCPObC5T\n\t5WI3rz8DkR1uosmr2aubn9ZMifB3hvYjo/s4mg/r9lYQcMTWurtmxtgowizAusFg/k\n\tiNYn7wl+ZygLC3BJRL8KrXxjGKsnxVkIfjPKGFIA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250326124856.75709-4-mzamazal@redhat.com>","References":"<20250326124856.75709-1-mzamazal@redhat.com>\n\t<20250326124856.75709-4-mzamazal@redhat.com>","Subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","From":"Kieran Bingham ","Cc":"Laurent Pinchart ,\n\tMilan Zamazal ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Wed, 26 Mar 2025 21:26:38 +0000","Message-ID":"<174302439880.1701483.9800700792129679592@ping.linuxembedded.co.uk>","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\" "}},{"id":33744,"web_url":"https://patchwork.libcamera.org/comment/33744/","msgid":"<20250326232233.GD21053@pendragon.ideasonboard.com>","date":"2025-03-26T23:22:33","subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Wed, Mar 26, 2025 at 01:48:52PM +0100, Milan Zamazal wrote:\n> From: Kieran Bingham \n> \n> Provide the determined colour gains back into the metadata\n> to add to completed requests.\n> \n> Signed-off-by: Kieran Bingham \n> Signed-off-by: Milan Zamazal \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/simple/algorithms/awb.cpp | 21 ++++++++++++++++++++-\n> src/ipa/simple/algorithms/awb.h | 6 +++++-\n> src/ipa/simple/ipa_context.h | 4 ++++\n> 3 files changed, 29 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n> index ec77c6e5..55719059 100644\n> --- a/src/ipa/simple/algorithms/awb.cpp\n> +++ b/src/ipa/simple/algorithms/awb.cpp\n> @@ -17,6 +17,8 @@\n> #include \"libipa/colours.h\"\n> #include \"simple/ipa_context.h\"\n> \n> +#include \"control_ids.h\"\n> +\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPASoftAwb)\n> @@ -32,15 +34,32 @@ int Awb::configure(IPAContext &context,\n> \treturn 0;\n> }\n> \n> +void Awb::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t [[maybe_unused]] DebayerParams *params)\n> +{\n> +\tauto &gains = context.activeState.awb.gains;\n> +\tframeContext.gains.red = gains.r();\n> +\tframeContext.gains.blue = gains.b();\n> +}\n> +\n> void Awb::process(IPAContext &context,\n> \t\t [[maybe_unused]] const uint32_t frame,\n> -\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t IPAFrameContext &frameContext,\n> \t\t const SwIspStats *stats,\n> \t\t ControlList &metadata)\n> {\n> \tconst SwIspStats::Histogram &histogram = stats->yHistogram;\n> \tconst uint8_t blackLevel = context.activeState.blc.level;\n> \n> +\tconst float maxGain = 1024.0;\n> +\tconst float mdGains[] = {\n> +\t\tstatic_cast(frameContext.gains.red / maxGain),\n> +\t\tstatic_cast(frameContext.gains.blue / maxGain)\n> +\t};\n> +\tmetadata.set(controls::ColourGains, mdGains);\n> +\n> \t/*\n> \t * Black level must be subtracted to get the correct AWB ratios, they\n> \t * would be off if they were computed from the whole brightness range\n> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h\n> index db1496cd..ad993f39 100644\n> --- a/src/ipa/simple/algorithms/awb.h\n> +++ b/src/ipa/simple/algorithms/awb.h\n> @@ -1,6 +1,6 @@\n> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> /*\n> - * Copyright (C) 2024, Red Hat Inc.\n> + * Copyright (C) 2024-2025 Red Hat Inc.\n> *\n> * Auto white balance\n> */\n> @@ -20,6 +20,10 @@ public:\n> \t~Awb() = default;\n> \n> \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> +\tvoid prepare(IPAContext &context,\n> +\t\t const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t DebayerParams *params) override;\n> \tvoid process(IPAContext &context,\n> \t\t const uint32_t frame,\n> \t\t IPAFrameContext &frameContext,\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index 17bcd4ca..bfac835b 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -70,6 +70,10 @@ struct IPAFrameContext : public FrameContext {\n> \t\tint32_t exposure;\n> \t\tdouble gain;\n> \t} sensor;\n> +\tstruct {\n> +\t\tdouble red;\n> +\t\tdouble blue;\n> +\t} gains;\n> };\n> \n> struct IPAContext {","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 9BB6BC323E\n\tfor ;\n\tWed, 26 Mar 2025 23:23:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE7FC68950;\n\tThu, 27 Mar 2025 00:22:59 +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 8D745600EB\n\tfor ;\n\tThu, 27 Mar 2025 00:22:57 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0F0481D5;\n\tThu, 27 Mar 2025 00:21:09 +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=\"adMXWbjF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743031269;\n\tbh=9ks1UCHoqcXiysCCCi74YQJwxOQVqr1KSeYtPC8cQTs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=adMXWbjF6c5pAZxz08DQj3QPQIh6HWIXWsOlAWy+E2Leq2SDjPc3IUsT6Vwe56nGn\n\t/d2N4mHGupy7MWQ3EMaWMYTHG1ZCO+SAG2ASUMoIniUCs9LxDis9qAgHZrxr2B/nwP\n\tQCfjHXgETcPt2labqztJgi8ZRT/62VEYQgJmQeFo=","Date":"Thu, 27 Mar 2025 01:22:33 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","Subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","Message-ID":"<20250326232233.GD21053@pendragon.ideasonboard.com>","References":"<20250326124856.75709-1-mzamazal@redhat.com>\n\t<20250326124856.75709-4-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250326124856.75709-4-mzamazal@redhat.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":33750,"web_url":"https://patchwork.libcamera.org/comment/33750/","msgid":"<85r02i1b38.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-03-27T20:18:35","subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Kieran,\n\nthank you for review.\n\nKieran Bingham writes:\n\n> Quoting Milan Zamazal (2025-03-26 12:48:52)\n>> From: Kieran Bingham \n>> \n>\n>> Provide the determined colour gains back into the metadata\n>> to add to completed requests.\n>> \n>> Signed-off-by: Kieran Bingham \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/simple/algorithms/awb.cpp | 21 ++++++++++++++++++++-\n>> src/ipa/simple/algorithms/awb.h | 6 +++++-\n>> src/ipa/simple/ipa_context.h | 4 ++++\n>> 3 files changed, 29 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n>> index ec77c6e5..55719059 100644\n>> --- a/src/ipa/simple/algorithms/awb.cpp\n>> +++ b/src/ipa/simple/algorithms/awb.cpp\n>> @@ -17,6 +17,8 @@\n>> #include \"libipa/colours.h\"\n>> #include \"simple/ipa_context.h\"\n>> \n>> +#include \"control_ids.h\"\n>> +\n>> namespace libcamera {\n>> \n>> LOG_DEFINE_CATEGORY(IPASoftAwb)\n>> @@ -32,15 +34,32 @@ int Awb::configure(IPAContext &context,\n>> return 0;\n>> }\n>> \n>> +void Awb::prepare(IPAContext &context,\n>> + [[maybe_unused]] const uint32_t frame,\n>> + IPAFrameContext &frameContext,\n>> + [[maybe_unused]] DebayerParams *params)\n>> +{\n>> + auto &gains = context.activeState.awb.gains;\n>> + frameContext.gains.red = gains.r();\n>> + frameContext.gains.blue = gains.b();\n>\n> Interesting that this highlights the white balance parameters are\n> probably being used from the wrong 'time' as they should be determined\n> here in prepare... (by taking the most recent active state and combining\n> any decision from the controls for any manual white balance).\n\nI'm not sure I understand what you mean. They come from the most recent\nactive state, don't they? We don't support manual AWB balance but if we\ndid the gains could be amended here accordingly. And later used in Lut\nalgorithm. The gains stored here and used to process the frame are then\nreported in metadata for this frame.\n\nWhat's wrong then?\n\n> But - as this series is just trying to get the reporting in - lets leave\n> that issue for later, and try to keep this moving forwards:\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n>\n>> +}\n>> +\n>> void Awb::process(IPAContext &context,\n>> [[maybe_unused]] const uint32_t frame,\n>> - [[maybe_unused]] IPAFrameContext &frameContext,\n>> + IPAFrameContext &frameContext,\n>> const SwIspStats *stats,\n>> ControlList &metadata)\n>> {\n>> const SwIspStats::Histogram &histogram = stats->yHistogram;\n>> const uint8_t blackLevel = context.activeState.blc.level;\n>> \n>> + const float maxGain = 1024.0;\n>> + const float mdGains[] = {\n>> + static_cast(frameContext.gains.red / maxGain),\n>> + static_cast(frameContext.gains.blue / maxGain)\n>> + };\n>> + metadata.set(controls::ColourGains, mdGains);\n>> +\n>> /*\n>> * Black level must be subtracted to get the correct AWB ratios, they\n>> * would be off if they were computed from the whole brightness range\n>> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h\n>> index db1496cd..ad993f39 100644\n>> --- a/src/ipa/simple/algorithms/awb.h\n>> +++ b/src/ipa/simple/algorithms/awb.h\n>> @@ -1,6 +1,6 @@\n>> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> /*\n>> - * Copyright (C) 2024, Red Hat Inc.\n>> + * Copyright (C) 2024-2025 Red Hat Inc.\n>> *\n>> * Auto white balance\n>> */\n>> @@ -20,6 +20,10 @@ public:\n>> ~Awb() = default;\n>> \n>> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>> + void prepare(IPAContext &context,\n>> + const uint32_t frame,\n>> + IPAFrameContext &frameContext,\n>> + DebayerParams *params) override;\n>> void process(IPAContext &context,\n>> const uint32_t frame,\n>> IPAFrameContext &frameContext,\n>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>> index 17bcd4ca..bfac835b 100644\n>> --- a/src/ipa/simple/ipa_context.h\n>> +++ b/src/ipa/simple/ipa_context.h\n>> @@ -70,6 +70,10 @@ struct IPAFrameContext : public FrameContext {\n>> int32_t exposure;\n>> double gain;\n>> } sensor;\n>> + struct {\n>> + double red;\n>> + double blue;\n>> + } gains;\n>> };\n>> \n>> struct IPAContext {\n>> -- \n>> 2.49.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 4A3D2C3213\n\tfor ;\n\tThu, 27 Mar 2025 20:18:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 46AE868971;\n\tThu, 27 Mar 2025 21:18:43 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFD2668947\n\tfor ;\n\tThu, 27 Mar 2025 21:18:41 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-65-DucE8HjeOHyxSL6zuf_EIQ-1; Thu, 27 Mar 2025 16:18:39 -0400","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-43bc97e6360so8311175e9.3\n\tfor ;\n\tThu, 27 Mar 2025 13:18:39 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-43d8f404ac3sm6051465e9.0.2025.03.27.13.18.36\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 27 Mar 2025 13:18:36 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"d0A8C0Kv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1743106720;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=RACPZ6PkdcROpp2zza1sQN7JAbUbQK9aJ+UskBOJ3Zg=;\n\tb=d0A8C0KvDWcnBPQ+98ju7n1xA4n/zWUcI5TyNmPLZ+tNpoAzUoYdolHJAproKd7Vmzoqqa\n\tUvVyyGUaWmgGZcF80VDRLDAJx+xZepZGwuCXfGgJDFzpo2q/wB4cTRmEwD8F8dIc0hHFXK\n\tPpp6p/4Q8WosHSbBgp01T8R23BZ9JWg=","X-MC-Unique":"DucE8HjeOHyxSL6zuf_EIQ-1","X-Mimecast-MFC-AGG-ID":"DucE8HjeOHyxSL6zuf_EIQ_1743106718","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1743106718; x=1743711518;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=RACPZ6PkdcROpp2zza1sQN7JAbUbQK9aJ+UskBOJ3Zg=;\n\tb=o920Iy4LAWofAqONhZT3uWHyWOoCL0yWisWDXFN/IRhjNcXZhGL44tcTDgQ0XF/mjE\n\t9SRsADOXJMMnXEagAiMhqkNqgi7uwFesLVzD0Dn+39BXyp/ggHo7XZi2cIiPcjiuMfYV\n\tUta6OMUcziuVKWONiGark0UpGH2cEbZI+vpZ7PU4M6CSiPHhJBV46h99ZxYRuFye5ZWO\n\tkWTfn0BzL6w/smwLc4waXnfe5QZxbTsGXnnXB4AxBbpyTCEG+9tsCgbz1Uh0e8aJO4c4\n\tSI8w1swCU5p0dLtcQZgp9If9Miz0Ju87bz2QFAd7Nk6J1l/qoECPS9LkeJDJXaX01yS2\n\tWXmg==","X-Gm-Message-State":"AOJu0YxfRen+M0ZNjQj4cd0B9vWFUfNZz5dnwb+S1Vr5LIoT7SJZID0M\n\tYRM6PfqOxWyXvui49LKPaDE8b1n93jOO48Y+5S9jfyxgcChAQefQiYN1vD0vUBN68dDRZieU348\n\tw+tQXSO+4XploWWKaqv+fVQKhwDCI33c7kvuk7/6BQkS7KZUqmFDoP23QCLUR3T+NDS+jROQIO0\n\t0vhaQMCA==","X-Gm-Gg":"ASbGnctkmI1WvanNfJDxmwBMTlFxcelbjr4rd0pTDzzGL2DKljuFu9NXTeEEeTQ972i\n\tsYOzmYsgwkms+XPCYS/pLkRh4n9k6709TnQX0Mmg5FYAWnk7TFNGLb3mwuFH6jKj+Bp+esJWFi1\n\tkIDs8Uhg23GJi69Zy80PVbbcf+5BfnGBxLACcFMtoCHcWKUYCPbi/IV6gCGFy0Zplc4y8Bumoji\n\tg9l264hoViittjTTiXryTubF1fNKnHGgkgW61odO8y78b0pv3uo6B91a3rnFLjXRjrzzTQaxAQD\n\toWSBHifcMOMXJ1XjrFwE46kqWCIZ/o5Os00ltWZVaV9L8U5b/R9n0J2CQdXS2wFWFP0v","X-Received":["by 2002:a05:600c:4e8b:b0:43d:abd:ad0e with SMTP id\n\t5b1f17b1804b1-43d8813c8e7mr40712235e9.18.1743106717852; \n\tThu, 27 Mar 2025 13:18:37 -0700 (PDT)","by 2002:a05:600c:4e8b:b0:43d:abd:ad0e with SMTP id\n\t5b1f17b1804b1-43d8813c8e7mr40712065e9.18.1743106717391; \n\tThu, 27 Mar 2025 13:18:37 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGXGa0s+yWfF5orqPkLVOr4WY/Qi42eu8TwT+ceppnJLw7aSvGa/upH05V9Oc2jjqXHmh961A==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t","Subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","In-Reply-To":"<174302439880.1701483.9800700792129679592@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Wed, 26 Mar 2025 21:26:38 +0000\")","References":"<20250326124856.75709-1-mzamazal@redhat.com>\n\t<20250326124856.75709-4-mzamazal@redhat.com>\n\t<174302439880.1701483.9800700792129679592@ping.linuxembedded.co.uk>","Date":"Thu, 27 Mar 2025 21:18:35 +0100","Message-ID":"<85r02i1b38.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"_2I7Nbug0KsU2f-GI3S_SNGKFNa49jJVlcTqt-D0fXQ_1743106718","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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":33755,"web_url":"https://patchwork.libcamera.org/comment/33755/","msgid":"<174314352359.1242204.14278942393843651679@ping.linuxembedded.co.uk>","date":"2025-03-28T06:32:03","subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-03-27 20:18:35)\n> Hi Kieran,\n> \n> thank you for review.\n> \n> Kieran Bingham writes:\n> \n> > Quoting Milan Zamazal (2025-03-26 12:48:52)\n> >> From: Kieran Bingham \n> >> \n> >\n> >> Provide the determined colour gains back into the metadata\n> >> to add to completed requests.\n> >> \n> >> Signed-off-by: Kieran Bingham \n> >> Signed-off-by: Milan Zamazal \n> >> ---\n> >> src/ipa/simple/algorithms/awb.cpp | 21 ++++++++++++++++++++-\n> >> src/ipa/simple/algorithms/awb.h | 6 +++++-\n> >> src/ipa/simple/ipa_context.h | 4 ++++\n> >> 3 files changed, 29 insertions(+), 2 deletions(-)\n> >> \n> >> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n> >> index ec77c6e5..55719059 100644\n> >> --- a/src/ipa/simple/algorithms/awb.cpp\n> >> +++ b/src/ipa/simple/algorithms/awb.cpp\n> >> @@ -17,6 +17,8 @@\n> >> #include \"libipa/colours.h\"\n> >> #include \"simple/ipa_context.h\"\n> >> \n> >> +#include \"control_ids.h\"\n> >> +\n> >> namespace libcamera {\n> >> \n> >> LOG_DEFINE_CATEGORY(IPASoftAwb)\n> >> @@ -32,15 +34,32 @@ int Awb::configure(IPAContext &context,\n> >> return 0;\n> >> }\n> >> \n> >> +void Awb::prepare(IPAContext &context,\n> >> + [[maybe_unused]] const uint32_t frame,\n> >> + IPAFrameContext &frameContext,\n> >> + [[maybe_unused]] DebayerParams *params)\n> >> +{\n> >> + auto &gains = context.activeState.awb.gains;\n> >> + frameContext.gains.red = gains.r();\n> >> + frameContext.gains.blue = gains.b();\n> >\n> > Interesting that this highlights the white balance parameters are\n> > probably being used from the wrong 'time' as they should be determined\n> > here in prepare... (by taking the most recent active state and combining\n> > any decision from the controls for any manual white balance).\n> \n> I'm not sure I understand what you mean. They come from the most recent\n> active state, don't they? We don't support manual AWB balance but if we\n> did the gains could be amended here accordingly. And later used in Lut\n> algorithm. The gains stored here and used to process the frame are then\n> reported in metadata for this frame.\n> \n> What's wrong then?\n\nI'm sorry - I was probably thrown off by the fact that I couldn't see\nany configuration being set into the output params at this point. If\nit's handled in a separate algo then I guess that's the correct timing\n... but means we should probably have a note in here to say that the\nactual handling of AWB is combined with the LUT component.\n\nBut my original point was likely wrong above.\n\n--\nKieran\n\n\n\n\n> > But - as this series is just trying to get the reporting in - lets leave\n> > that issue for later, and try to keep this moving forwards:\n> >\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> >\n> >\n> >> +}\n> >> +\n> >> void Awb::process(IPAContext &context,\n> >> [[maybe_unused]] const uint32_t frame,\n> >> - [[maybe_unused]] IPAFrameContext &frameContext,\n> >> + IPAFrameContext &frameContext,\n> >> const SwIspStats *stats,\n> >> ControlList &metadata)\n> >> {\n> >> const SwIspStats::Histogram &histogram = stats->yHistogram;\n> >> const uint8_t blackLevel = context.activeState.blc.level;\n> >> \n> >> + const float maxGain = 1024.0;\n> >> + const float mdGains[] = {\n> >> + static_cast(frameContext.gains.red / maxGain),\n> >> + static_cast(frameContext.gains.blue / maxGain)\n> >> + };\n> >> + metadata.set(controls::ColourGains, mdGains);\n> >> +\n> >> /*\n> >> * Black level must be subtracted to get the correct AWB ratios, they\n> >> * would be off if they were computed from the whole brightness range\n> >> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h\n> >> index db1496cd..ad993f39 100644\n> >> --- a/src/ipa/simple/algorithms/awb.h\n> >> +++ b/src/ipa/simple/algorithms/awb.h\n> >> @@ -1,6 +1,6 @@\n> >> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> /*\n> >> - * Copyright (C) 2024, Red Hat Inc.\n> >> + * Copyright (C) 2024-2025 Red Hat Inc.\n> >> *\n> >> * Auto white balance\n> >> */\n> >> @@ -20,6 +20,10 @@ public:\n> >> ~Awb() = default;\n> >> \n> >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> >> + void prepare(IPAContext &context,\n> >> + const uint32_t frame,\n> >> + IPAFrameContext &frameContext,\n> >> + DebayerParams *params) override;\n> >> void process(IPAContext &context,\n> >> const uint32_t frame,\n> >> IPAFrameContext &frameContext,\n> >> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> >> index 17bcd4ca..bfac835b 100644\n> >> --- a/src/ipa/simple/ipa_context.h\n> >> +++ b/src/ipa/simple/ipa_context.h\n> >> @@ -70,6 +70,10 @@ struct IPAFrameContext : public FrameContext {\n> >> int32_t exposure;\n> >> double gain;\n> >> } sensor;\n> >> + struct {\n> >> + double red;\n> >> + double blue;\n> >> + } gains;\n> >> };\n> >> \n> >> struct IPAContext {\n> >> -- \n> >> 2.49.0\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 2468FC323E\n\tfor ;\n\tFri, 28 Mar 2025 06:32:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0FBBB68979;\n\tFri, 28 Mar 2025 07:32:09 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D516F614E7\n\tfor ;\n\tFri, 28 Mar 2025 07:32:06 +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 749DF844;\n\tFri, 28 Mar 2025 07:30: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=\"mfJLH1K5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743143417;\n\tbh=L1Xg6EdW+dCgWXJJCWggtcIWYncALPeMYf+W1yXq/SQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=mfJLH1K5xhtiVdI+WZgOEapCMMZiOmvGlJwJ0SO+1sHLl6/lBAsFuSWLzakmNNfKs\n\tA7dEgs+cNJ76CoCS0BfHYPuoLOjtpZ8UQKU5K16y5/RUMH9KjZdO/1Zxjh8Ufj2vm8\n\tqZmqj1KvU/Q3bSAKVfgPqmtp3H9U0zEVZlFia7Qs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85r02i1b38.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20250326124856.75709-1-mzamazal@redhat.com>\n\t<20250326124856.75709-4-mzamazal@redhat.com>\n\t<174302439880.1701483.9800700792129679592@ping.linuxembedded.co.uk>\n\t<85r02i1b38.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ","To":"Milan Zamazal ","Date":"Fri, 28 Mar 2025 06:32:03 +0000","Message-ID":"<174314352359.1242204.14278942393843651679@ping.linuxembedded.co.uk>","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\" "}}]