[{"id":34150,"web_url":"https://patchwork.libcamera.org/comment/34150/","msgid":"","date":"2025-05-07T17:27:08","subject":"Re: [PATCH v3 13/16] libipa: awb: Make result of gainsFromColourTemp\n\toptional","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:18PM +0200, Stefan Klug wrote:\n> In the grey world AWB case, if no colour gains are contained in the\n> tuning file, the colour gains get reset to 1 when the colour temperature\n> is set manually. This is unexpected and undesirable. Allow the\n> gainsFromColourTemp() function to return a std::nullopt to handle that\n> case.\n> \n> While at it, remove an unnecessary import from rkisp1/algorithms/awb.h.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v2:\n> - Added this patch\n> \n> Changes in v3:\n> - Added #include to libipa/awb.h\n> - Removed stray include from rkisp1/awb.h\n> - Reordered colour temperature assignment for better clarity\n> ---\n> src/ipa/libipa/awb.cpp | 2 +-\n> src/ipa/libipa/awb.h | 3 ++-\n> src/ipa/libipa/awb_bayes.cpp | 4 ++--\n> src/ipa/libipa/awb_bayes.h | 2 +-\n> src/ipa/libipa/awb_grey.cpp | 6 +++---\n> src/ipa/libipa/awb_grey.h | 2 +-\n> src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++++------\n> src/ipa/rkisp1/algorithms/awb.h | 2 --\n> 8 files changed, 22 insertions(+), 17 deletions(-)\n> \n> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> index 925fac232709..214bac8b56a6 100644\n> --- a/src/ipa/libipa/awb.cpp\n> +++ b/src/ipa/libipa/awb.cpp\n> @@ -114,7 +114,7 @@ namespace ipa {\n> * does not take any statistics into account. It is used to compute the colour\n> * gains when the user manually specifies a colour temperature.\n> *\n> - * \\return The colour gains\n> + * \\return The colour gains or std::nullopt if the conversion is not possible\n> */\n> \n> /**\n> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> index 4bab7a451ce5..f4a86038635f 100644\n> --- a/src/ipa/libipa/awb.h\n> +++ b/src/ipa/libipa/awb.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -39,7 +40,7 @@ public:\n> \n> \tvirtual int init(const YamlObject &tuningData) = 0;\n> \tvirtual AwbResult calculateAwb(const AwbStats &stats, unsigned int lux) = 0;\n> -\tvirtual RGB gainsFromColourTemperature(double colourTemperature) = 0;\n> +\tvirtual std::optional> gainsFromColourTemperature(double colourTemperature) = 0;\n> \n> \tconst ControlInfoMap::Map &controls() const\n> \t{\n> diff --git a/src/ipa/libipa/awb_bayes.cpp b/src/ipa/libipa/awb_bayes.cpp\n> index d1d0eaf0e68f..d2bcbd83d7f8 100644\n> --- a/src/ipa/libipa/awb_bayes.cpp\n> +++ b/src/ipa/libipa/awb_bayes.cpp\n> @@ -270,7 +270,7 @@ void AwbBayes::handleControls(const ControlList &controls)\n> \t}\n> }\n> \n> -RGB AwbBayes::gainsFromColourTemperature(double colourTemperature)\n> +std::optional> AwbBayes::gainsFromColourTemperature(double colourTemperature)\n> {\n> \t/*\n> \t * \\todo In the RaspberryPi code, the ct curve was interpolated in\n> @@ -278,7 +278,7 @@ RGB AwbBayes::gainsFromColourTemperature(double colourTemperature)\n> \t * intuitive, as the gains are in linear space. But I can't prove it.\n> \t */\n> \tconst auto &gains = colourGainCurve_.getInterpolated(colourTemperature);\n> -\treturn { { gains[0], 1.0, gains[1] } };\n> +\treturn RGB{ { gains[0], 1.0, gains[1] } };\n> }\n> \n> AwbResult AwbBayes::calculateAwb(const AwbStats &stats, unsigned int lux)\n> diff --git a/src/ipa/libipa/awb_bayes.h b/src/ipa/libipa/awb_bayes.h\n> index bb933038d6d2..47ef3cce4d58 100644\n> --- a/src/ipa/libipa/awb_bayes.h\n> +++ b/src/ipa/libipa/awb_bayes.h\n> @@ -27,7 +27,7 @@ public:\n> \n> \tint init(const YamlObject &tuningData) override;\n> \tAwbResult calculateAwb(const AwbStats &stats, unsigned int lux) override;\n> -\tRGB gainsFromColourTemperature(double temperatureK) override;\n> +\tstd::optional> gainsFromColourTemperature(double temperatureK) override;\n> \tvoid handleControls(const ControlList &controls) override;\n> \n> private:\n> diff --git a/src/ipa/libipa/awb_grey.cpp b/src/ipa/libipa/awb_grey.cpp\n> index d3d2132b8869..d252edb2b6c6 100644\n> --- a/src/ipa/libipa/awb_grey.cpp\n> +++ b/src/ipa/libipa/awb_grey.cpp\n> @@ -98,15 +98,15 @@ AwbResult AwbGrey::calculateAwb(const AwbStats &stats, [[maybe_unused]] unsigned\n> * \\return The colour gains if a colour temperature curve is available,\n> * [1, 1, 1] otherwise.\n> */\n> -RGB AwbGrey::gainsFromColourTemperature(double colourTemperature)\n> +std::optional> AwbGrey::gainsFromColourTemperature(double colourTemperature)\n> {\n> \tif (!colourGainCurve_) {\n> \t\tLOG(Awb, Error) << \"No gains defined\";\n> -\t\treturn RGB({ 1.0, 1.0, 1.0 });\n> +\t\treturn std::nullopt;\n> \t}\n> \n> \tauto gains = colourGainCurve_->getInterpolated(colourTemperature);\n> -\treturn { { gains[0], 1.0, gains[1] } };\n> +\treturn RGB{ { gains[0], 1.0, gains[1] } };\n> }\n> \n> } /* namespace ipa */\n> diff --git a/src/ipa/libipa/awb_grey.h b/src/ipa/libipa/awb_grey.h\n> index 7ec7bfa5da9a..f82a368d11cc 100644\n> --- a/src/ipa/libipa/awb_grey.h\n> +++ b/src/ipa/libipa/awb_grey.h\n> @@ -25,7 +25,7 @@ public:\n> \n> \tint init(const YamlObject &tuningData) override;\n> \tAwbResult calculateAwb(const AwbStats &stats, unsigned int lux) override;\n> -\tRGB gainsFromColourTemperature(double colourTemperature) override;\n> +\tstd::optional> gainsFromColourTemperature(double colourTemperature) override;\n> \n> private:\n> \tstd::optional>> colourGainCurve_;\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 0795b8e5b1e1..f5415a0bb99d 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -125,8 +125,12 @@ int Awb::configure(IPAContext &context,\n> \t\t const IPACameraSensorInfo &configInfo)\n> {\n> \tcontext.activeState.awb.manual.gains = RGB{ 1.0 };\n> -\tcontext.activeState.awb.automatic.gains =\n> -\t\tawbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> +\tauto gains = awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> +\tif (gains)\n> +\t\tcontext.activeState.awb.automatic.gains = *gains;\n> +\telse\n> +\t\tcontext.activeState.awb.automatic.gains = RGB{ 1.0 };\n> +\n> \tcontext.activeState.awb.autoEnabled = true;\n> \tcontext.activeState.awb.manual.temperatureK = kDefaultColourTemperature;\n> \tcontext.activeState.awb.automatic.temperatureK = kDefaultColourTemperature;\n> @@ -183,11 +187,13 @@ void Awb::queueRequest(IPAContext &context,\n> \t\t */\n> \t\tupdate = true;\n> \t} else if (colourTemperature) {\n> -\t\tconst auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> -\t\tawb.manual.gains.r() = gains.r();\n> -\t\tawb.manual.gains.b() = gains.b();\n> \t\tawb.manual.temperatureK = *colourTemperature;\n> -\t\tupdate = true;\n> +\t\tconst auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> +\t\tif (gains) {\n> +\t\t\tawb.manual.gains.r() = gains->r();\n> +\t\t\tawb.manual.gains.b() = gains->b();\n> +\t\t\tupdate = true;\n> +\t\t}\n> \t}\n> \n> \tif (update)\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 7e6c3862e699..02651cc732bf 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -7,8 +7,6 @@\n> \n> #pragma once\n> \n> -#include \n> -\n> #include \"libcamera/internal/vector.h\"\n> \n> #include \"libipa/awb.h\"\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 909AEC3200\n\tfor ;\n\tWed, 7 May 2025 17:27:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E907268B2F;\n\tWed, 7 May 2025 19:27:13 +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 95B3E68AD8\n\tfor ;\n\tWed, 7 May 2025 19:27:12 +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 0A4BBC59;\n\tWed, 7 May 2025 19:27:00 +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=\"EoKJPw9L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746638821;\n\tbh=k+NDyqojlmCQmSGTcn2mEYKxXGoE88+w0hvkQKEq7gQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EoKJPw9Lp/O7UxBVBvkd6dvpcgw9mEot0ozuFxU2xxfN6lsNu+UQH86DiJFxOnrxW\n\tUOLT8db9TdghLaJPjyei5gu0Yo6mnQoEUSSj6zK/zXPR7FmCauAuuXQxJ+jKyams9y\n\tmKUK1StGxZBoS5aCf2iqCW88vj33Lp9Jp5dgbmys=","Date":"Wed, 7 May 2025 19:27:08 +0200","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","Subject":"Re: [PATCH v3 13/16] libipa: awb: Make result of gainsFromColourTemp\n\toptional","Message-ID":"","References":"<20250403154925.382973-1-stefan.klug@ideasonboard.com>\n\t<20250403154925.382973-14-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250403154925.382973-14-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\" "}}]