[{"id":30591,"web_url":"https://patchwork.libcamera.org/comment/30591/","msgid":"<172286541957.1687952.5577482370034389114@ping.linuxembedded.co.uk>","date":"2024-08-05T13:43:39","subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2024-08-05 13:05:03)\n> For the implementation of a manual colour temperature setting, it is\n> necessary to read predefined colour gains per colour temperature from\n> the tuning file. Implement this in a backwards compatible way. If no\n> gains are contained in the tuning file, loading just continues as\n> before.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++++++++++\n> src/ipa/rkisp1/algorithms/awb.h | 6 ++++++\n> 2 files changed, 24 insertions(+)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 4ccafd48dedd..957d24fe3425 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -39,6 +39,24 @@ Awb::Awb()\n> {\n> }\n> \n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> +{\n> + MatrixInterpolator gains;\n> + int ret = gains.readYaml(tuningData[\"gains\"], \"ct\", \"gains\");\n\nDo we have a way to check if there are gains set at all? If there are no\ngains set in the yaml - we wouldn't report a warning at all I'd expect?\n\n(And presumably we wouldn't expose/report a manual color temperature\ncontrol as setable either?)\n\n> + if (ret < 0)\n> + LOG(RkISP1Awb, Warning)\n> + << \"Failed to parse 'gains' \"\n> + << \"parameter from tuning file; \"\n> + << \"rgb gains will not be set based on colour temperature\";\n> + else\n> + gains_ = gains;\n> +\n> + return 0;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::configure\n> */\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 06c92896e2dc..a010e6d1cb3c 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -7,6 +7,10 @@\n> \n> #pragma once\n> \n> +#include \n> +\n> +#include \"libipa/matrix_interpolator.h\"\n> +\n> #include \"algorithm.h\"\n> \n> namespace libcamera {\n> @@ -19,6 +23,7 @@ public:\n> Awb();\n> ~Awb() = default;\n> \n> + int init(IPAContext &context, const YamlObject &tuningData) override;\n> int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> void queueRequest(IPAContext &context, const uint32_t frame,\n> IPAFrameContext &frameContext,\n> @@ -34,6 +39,7 @@ public:\n> private:\n> uint32_t estimateCCT(double red, double green, double blue);\n> \n> + std::optional> gains_;\n> bool rgbMode_;\n> };\n> \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 2DCEFC323E\n\tfor ;\n\tMon, 5 Aug 2024 13:43:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 73D5563369;\n\tMon, 5 Aug 2024 15:43:44 +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 B6BEF6195D\n\tfor ;\n\tMon, 5 Aug 2024 15:43:42 +0200 (CEST)","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 2FA7F18D;\n\tMon, 5 Aug 2024 15:42: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=\"pNo5/opt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722865371;\n\tbh=d4lOKeRTEdjxJFZvnKSolh/bT3l1t1AiaisTunIixjk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pNo5/opteq8fllIT+oBJymGLpUTII8W01SIjdeRUwPOhEPmc2UiBhHzsDLi5pgTB0\n\t58GVgOdeMwwe3qqQ4HKkJA7uAyznO+IEup2q1KHQK15WJrLVKHoRnYQ+2FsCuSwYwZ\n\tZFknZBd7YCl9EOzhGrGBnfiW5d7md6of3tyBStac=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240805120522.1613342-3-stefan.klug@ideasonboard.com>","References":"<20240805120522.1613342-1-stefan.klug@ideasonboard.com>\n\t<20240805120522.1613342-3-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","From":"Kieran Bingham ","Cc":"David Plowman ,\n\tStefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 05 Aug 2024 14:43:39 +0100","Message-ID":"<172286541957.1687952.5577482370034389114@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":30606,"web_url":"https://patchwork.libcamera.org/comment/30606/","msgid":"","date":"2024-08-06T06:26:59","subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Mon, Aug 05, 2024 at 02:43:39PM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2024-08-05 13:05:03)\n> > For the implementation of a manual colour temperature setting, it is\n> > necessary to read predefined colour gains per colour temperature from\n> > the tuning file. Implement this in a backwards compatible way. If no\n> > gains are contained in the tuning file, loading just continues as\n> > before.\n> > \n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++++++++++\n> > src/ipa/rkisp1/algorithms/awb.h | 6 ++++++\n> > 2 files changed, 24 insertions(+)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index 4ccafd48dedd..957d24fe3425 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -39,6 +39,24 @@ Awb::Awb()\n> > {\n> > }\n> > \n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::init\n> > + */\n> > +int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> > +{\n> > + MatrixInterpolator gains;\n> > + int ret = gains.readYaml(tuningData[\"gains\"], \"ct\", \"gains\");\n> \n> Do we have a way to check if there are gains set at all? If there are no\n> gains set in the yaml - we wouldn't report a warning at all I'd expect?\n\nIf there are no gains in the yaml, readYaml returns an error. We could\nadd a check for that specific case and silence the warning. But do we\nreally want to silence that? It basically says \"You have an old tuning\nfile, some features are limited but I'll continue\". So I think it's\nreasonable - up to you.\n\n> \n> (And presumably we wouldn't expose/report a manual color temperature\n> control as setable either?)\n\nThat's a bit difficult here as the temperature also controls the ccm.\nSo we would need to check the existence of ccms also. As the ccms are\nnecessary for a properly tuned sensor, I think it is ok to\nunconditionally add the control. \n\n> \n> > + if (ret < 0)\n> > + LOG(RkISP1Awb, Warning)\n> > + << \"Failed to parse 'gains' \"\n> > + << \"parameter from tuning file; \"\n> > + << \"rgb gains will not be set based on colour temperature\";\n> > + else\n> > + gains_ = gains;\n> > +\n> > + return 0;\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::ipa::Algorithm::configure\n> > */\n> > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> > index 06c92896e2dc..a010e6d1cb3c 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.h\n> > +++ b/src/ipa/rkisp1/algorithms/awb.h\n> > @@ -7,6 +7,10 @@\n> > \n> > #pragma once\n> > \n> > +#include \n> > +\n> > +#include \"libipa/matrix_interpolator.h\"\n> > +\n> > #include \"algorithm.h\"\n> > \n> > namespace libcamera {\n> > @@ -19,6 +23,7 @@ public:\n> > Awb();\n> > ~Awb() = default;\n> > \n> > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> > void queueRequest(IPAContext &context, const uint32_t frame,\n> > IPAFrameContext &frameContext,\n> > @@ -34,6 +39,7 @@ public:\n> > private:\n> > uint32_t estimateCCT(double red, double green, double blue);\n> > \n> > + std::optional> gains_;\n> > bool rgbMode_;\n> > };\n> > \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 3B7ADBE173\n\tfor ;\n\tTue, 6 Aug 2024 06:27:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66A8663394;\n\tTue, 6 Aug 2024 08:27:04 +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 86D6A63369\n\tfor ;\n\tTue, 6 Aug 2024 08:27:02 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7a68:2b6f:8265:aa72])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 69F9C2C5;\n\tTue, 6 Aug 2024 08:26: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=\"uNUy2A7O\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722925570;\n\tbh=/4JfxLts62s0kifzn1Ry9x3dxxGsh+dD2PrtTUhFq6w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uNUy2A7O+W9In4wCwSs/QmzNz/Px3v3we43tlKn7i61oipK47x/5Sh1Mwvu1VnXP/\n\t6tYNkgF5rF5xef8Hcm0FqI/rxqkxQlhrQYg4YcSIJB0pJKf2VvreVS1b9DId/PJfQh\n\tyZTAKgVxjPP+P66y6vvbShM5fZgFWG0wyju8chOk=","Date":"Tue, 6 Aug 2024 08:26:59 +0200","From":"Stefan Klug ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, \n\tDavid Plowman ","Subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","Message-ID":"","References":"<20240805120522.1613342-1-stefan.klug@ideasonboard.com>\n\t<20240805120522.1613342-3-stefan.klug@ideasonboard.com>\n\t<172286541957.1687952.5577482370034389114@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<172286541957.1687952.5577482370034389114@ping.linuxembedded.co.uk>","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":30618,"web_url":"https://patchwork.libcamera.org/comment/30618/","msgid":"<172293434585.1687952.17801092027801694304@ping.linuxembedded.co.uk>","date":"2024-08-06T08:52:25","subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2024-08-06 07:26:59)\n> On Mon, Aug 05, 2024 at 02:43:39PM +0100, Kieran Bingham wrote:\n> > Quoting Stefan Klug (2024-08-05 13:05:03)\n> > > For the implementation of a manual colour temperature setting, it is\n> > > necessary to read predefined colour gains per colour temperature from\n> > > the tuning file. Implement this in a backwards compatible way. If no\n> > > gains are contained in the tuning file, loading just continues as\n> > > before.\n> > > \n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++++++++++\n> > > src/ipa/rkisp1/algorithms/awb.h | 6 ++++++\n> > > 2 files changed, 24 insertions(+)\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > index 4ccafd48dedd..957d24fe3425 100644\n> > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > @@ -39,6 +39,24 @@ Awb::Awb()\n> > > {\n> > > }\n> > > \n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::init\n> > > + */\n> > > +int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> > > +{\n> > > + MatrixInterpolator gains;\n> > > + int ret = gains.readYaml(tuningData[\"gains\"], \"ct\", \"gains\");\n> > \n> > Do we have a way to check if there are gains set at all? If there are no\n> > gains set in the yaml - we wouldn't report a warning at all I'd expect?\n> \n> If there are no gains in the yaml, readYaml returns an error. We could\n> add a check for that specific case and silence the warning. But do we\n> really want to silence that? It basically says \"You have an old tuning\n> file, some features are limited but I'll continue\". So I think it's\n> reasonable - up to you.\n\nI think if we can get the schema validation then we can add this as an\nexpected/required tuning entry then and maybe that also solves the issue\nas then the warning really would be a warning, so I'm fine with leaving\nthis as it is given we're aiming in that direction.\n\n> \n> > \n> > (And presumably we wouldn't expose/report a manual color temperature\n> > control as setable either?)\n> \n> That's a bit difficult here as the temperature also controls the ccm.\n> So we would need to check the existence of ccms also. As the ccms are\n> necessary for a properly tuned sensor, I think it is ok to\n> unconditionally add the control. \n\nOk so we have a dependency on both algo's for the suitable / acceptable\nparameters for the control? That makes me think back to how we determine\nwhat the min/max values can be - but that's in a later patch I think ;-)\n\n> \n> > \n> > > + if (ret < 0)\n> > > + LOG(RkISP1Awb, Warning)\n> > > + << \"Failed to parse 'gains' \"\n> > > + << \"parameter from tuning file; \"\n> > > + << \"rgb gains will not be set based on colour temperature\";\n> > > + else\n> > > + gains_ = gains;\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > /**\n> > > * \\copydoc libcamera::ipa::Algorithm::configure\n> > > */\n> > > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> > > index 06c92896e2dc..a010e6d1cb3c 100644\n> > > --- a/src/ipa/rkisp1/algorithms/awb.h\n> > > +++ b/src/ipa/rkisp1/algorithms/awb.h\n> > > @@ -7,6 +7,10 @@\n> > > \n> > > #pragma once\n> > > \n> > > +#include \n> > > +\n> > > +#include \"libipa/matrix_interpolator.h\"\n> > > +\n> > > #include \"algorithm.h\"\n> > > \n> > > namespace libcamera {\n> > > @@ -19,6 +23,7 @@ public:\n> > > Awb();\n> > > ~Awb() = default;\n> > > \n> > > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> > > void queueRequest(IPAContext &context, const uint32_t frame,\n> > > IPAFrameContext &frameContext,\n> > > @@ -34,6 +39,7 @@ public:\n> > > private:\n> > > uint32_t estimateCCT(double red, double green, double blue);\n> > > \n> > > + std::optional> gains_;\n> > > bool rgbMode_;\n> > > };\n> > > \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 AE1CBBE173\n\tfor ;\n\tTue, 6 Aug 2024 08:52:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6A3D633AC;\n\tTue, 6 Aug 2024 10:52:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22D396337E\n\tfor ;\n\tTue, 6 Aug 2024 10:52:29 +0200 (CEST)","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 7B153922;\n\tTue, 6 Aug 2024 10:51:36 +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=\"FTERi81m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722934296;\n\tbh=5XO4xprhHb0y44233c5HMmrkBDtEfSuA09AfRrCkmU4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=FTERi81mU+Px+mPNTxoOjTpSW1LX989h0SsxiKGmlyjgGp2/T7sT02Rzdub/7K5AG\n\tiUaCAMztcIqPaGqMG0ce9rVs6D5DkQLc6Q+I26fZjtta6hl8jv+zgUEhY7mdFo4Auq\n\tjFeqIKV38j3Z0E+MfQaAuxI5vbND0ooogpj1s470=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240805120522.1613342-1-stefan.klug@ideasonboard.com>\n\t<20240805120522.1613342-3-stefan.klug@ideasonboard.com>\n\t<172286541957.1687952.5577482370034389114@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tDavid Plowman ","To":"Stefan Klug ","Date":"Tue, 06 Aug 2024 09:52:25 +0100","Message-ID":"<172293434585.1687952.17801092027801694304@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":30622,"web_url":"https://patchwork.libcamera.org/comment/30622/","msgid":"<172293466903.1687952.11396833042959897248@ping.linuxembedded.co.uk>","date":"2024-08-06T08:57:49","subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-08-06 09:52:25)\n> Quoting Stefan Klug (2024-08-06 07:26:59)\n> > On Mon, Aug 05, 2024 at 02:43:39PM +0100, Kieran Bingham wrote:\n> > > Quoting Stefan Klug (2024-08-05 13:05:03)\n> > > > For the implementation of a manual colour temperature setting, it is\n> > > > necessary to read predefined colour gains per colour temperature from\n> > > > the tuning file. Implement this in a backwards compatible way. If no\n> > > > gains are contained in the tuning file, loading just continues as\n> > > > before.\n> > > > \n> > > > Signed-off-by: Stefan Klug \n> > > > ---\n> > > > src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++++++++++\n> > > > src/ipa/rkisp1/algorithms/awb.h | 6 ++++++\n> > > > 2 files changed, 24 insertions(+)\n> > > > \n> > > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > index 4ccafd48dedd..957d24fe3425 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > @@ -39,6 +39,24 @@ Awb::Awb()\n> > > > {\n> > > > }\n> > > > \n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::init\n> > > > + */\n> > > > +int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> > > > +{\n> > > > + MatrixInterpolator gains;\n> > > > + int ret = gains.readYaml(tuningData[\"gains\"], \"ct\", \"gains\");\n> > > \n> > > Do we have a way to check if there are gains set at all? If there are no\n> > > gains set in the yaml - we wouldn't report a warning at all I'd expect?\n> > \n> > If there are no gains in the yaml, readYaml returns an error. We could\n> > add a check for that specific case and silence the warning. But do we\n> > really want to silence that? It basically says \"You have an old tuning\n> > file, some features are limited but I'll continue\". So I think it's\n> > reasonable - up to you.\n> \n> I think if we can get the schema validation then we can add this as an\n> expected/required tuning entry then and maybe that also solves the issue\n> as then the warning really would be a warning, so I'm fine with leaving\n> this as it is given we're aiming in that direction.\n> \n> > \n> > > \n> > > (And presumably we wouldn't expose/report a manual color temperature\n> > > control as setable either?)\n> > \n> > That's a bit difficult here as the temperature also controls the ccm.\n> > So we would need to check the existence of ccms also. As the ccms are\n> > necessary for a properly tuned sensor, I think it is ok to\n> > unconditionally add the control. \n> \n> Ok so we have a dependency on both algo's for the suitable / acceptable\n> parameters for the control? That makes me think back to how we determine\n> what the min/max values can be - but that's in a later patch I think ;-)\n\n\nReviewed-by: Kieran Bingham \n\n> \n> > \n> > > \n> > > > + if (ret < 0)\n> > > > + LOG(RkISP1Awb, Warning)\n> > > > + << \"Failed to parse 'gains' \"\n> > > > + << \"parameter from tuning file; \"\n> > > > + << \"rgb gains will not be set based on colour temperature\";\n> > > > + else\n> > > > + gains_ = gains;\n> > > > +\n> > > > + return 0;\n> > > > +}\n> > > > +\n> > > > /**\n> > > > * \\copydoc libcamera::ipa::Algorithm::configure\n> > > > */\n> > > > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> > > > index 06c92896e2dc..a010e6d1cb3c 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/awb.h\n> > > > +++ b/src/ipa/rkisp1/algorithms/awb.h\n> > > > @@ -7,6 +7,10 @@\n> > > > \n> > > > #pragma once\n> > > > \n> > > > +#include \n> > > > +\n> > > > +#include \"libipa/matrix_interpolator.h\"\n> > > > +\n> > > > #include \"algorithm.h\"\n> > > > \n> > > > namespace libcamera {\n> > > > @@ -19,6 +23,7 @@ public:\n> > > > Awb();\n> > > > ~Awb() = default;\n> > > > \n> > > > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> > > > void queueRequest(IPAContext &context, const uint32_t frame,\n> > > > IPAFrameContext &frameContext,\n> > > > @@ -34,6 +39,7 @@ public:\n> > > > private:\n> > > > uint32_t estimateCCT(double red, double green, double blue);\n> > > > \n> > > > + std::optional> gains_;\n> > > > bool rgbMode_;\n> > > > };\n> > > > \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 D9906C323E\n\tfor ;\n\tTue, 6 Aug 2024 08:57:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C6B363394;\n\tTue, 6 Aug 2024 10:57:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C0EF6337E\n\tfor ;\n\tTue, 6 Aug 2024 10:57:51 +0200 (CEST)","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 AB2744CD;\n\tTue, 6 Aug 2024 10:56:59 +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=\"fpi5Pm4I\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722934619;\n\tbh=5lEdZiYfiASm5LCnj18XY+60Ay1p+35HpG+qOgPq7PE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=fpi5Pm4IlSYtViV573YQ1DgdbUPudIJAgKEJmKVVrGdOV5nY7YtcGTZBLx9B4Ls1P\n\tMG3cR0wjwvIKJp06B4WgKy7aK4MDbn6jYOxOOOBuyiIAZUUPpoPwBp6jYU1H4i4JQ1\n\t4VB6PWIZppJXIkvz9BySFb/XCAhbN8IWByOj+QtA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<172293434585.1687952.17801092027801694304@ping.linuxembedded.co.uk>","References":"<20240805120522.1613342-1-stefan.klug@ideasonboard.com>\n\t<20240805120522.1613342-3-stefan.klug@ideasonboard.com>\n\t<172286541957.1687952.5577482370034389114@ping.linuxembedded.co.uk>\n\t\n\t<172293434585.1687952.17801092027801694304@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from\n\ttuning file","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tDavid Plowman ","To":"Stefan Klug ","Date":"Tue, 06 Aug 2024 09:57:49 +0100","Message-ID":"<172293466903.1687952.11396833042959897248@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\" "}}]