[{"id":24716,"web_url":"https://patchwork.libcamera.org/comment/24716/","msgid":"<20220819131120.ecnqn275fpux6hjq@uno.localdomain>","date":"2022-08-19T13:11:20","subject":"Re: [libcamera-devel] [PATCH v2] ipa: rkisp1: Add manual color gains","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul\n\nOn Thu, Aug 18, 2022 at 06:01:08PM +0900, Paul Elder via libcamera-devel wrote:\n> Add support for manually controlling the color gains on the rkisp1 IPA.\n> To that end, add and plumb the AwbEnable and ColourGains controls. As\n> per-frame controls aren't supported yet in the rkisp1 IPA, simply apply\n> and perform checks on the controls immediately.\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n>\n\nThe patch looks sane to me!\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> Changes in v2:\n> - s/enabled/autoEnabled\n> - add documentation\n> - fixup construction of the ControlInfo\n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 36 +++++++++++++++++++++++++++++--\n> src/ipa/rkisp1/algorithms/awb.h | 3 +++\n> src/ipa/rkisp1/ipa_context.cpp | 3 +++\n> src/ipa/rkisp1/ipa_context.h | 1 +\n> src/ipa/rkisp1/rkisp1.cpp | 2 ++\n> 5 files changed, 43 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 9f00364d..a3c14a9a 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -12,6 +12,7 @@\n>\n> #include \n>\n> +#include \n> #include \n>\n> /**\n> @@ -38,6 +39,7 @@ int Awb::configure(IPAContext &context,\n> \tcontext.frameContext.awb.gains.red = 1.0;\n> \tcontext.frameContext.awb.gains.blue = 1.0;\n> \tcontext.frameContext.awb.gains.green = 1.0;\n> +\tcontext.frameContext.awb.autoEnabled = true;\n>\n> \t/*\n> \t * Define the measurement window for AWB as a centered rectangle\n> @@ -116,6 +118,34 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> \tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB;\n> }\n>\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void Awb::queueRequest(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t const ControlList &controls)\n> +{\n> +\tauto &awb = context.frameContext.awb;\n> +\n> +\tconst auto &awbEnable = controls.get(controls::AwbEnable);\n> +\tif (awbEnable && *awbEnable != awb.autoEnabled) {\n> +\t\tawb.autoEnabled = *awbEnable;\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< (*awbEnable ? \"Enabling\" : \"Disabling\") << \" AWB\";\n> +\t}\n> +\n> +\tconst auto &colourGains = controls.get(controls::ColourGains);\n> +\tif (colourGains && !awb.autoEnabled) {\n> +\t\tawb.gains.red = (*colourGains)[0];\n> +\t\tawb.gains.blue = (*colourGains)[1];\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< \"Set colour gains to red: \" << awb.gains.red\n> +\t\t\t<< \", blue: \" << awb.gains.blue;\n> +\t}\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::process\n> */\n> @@ -164,8 +194,10 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n> \t * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n> \t * fractional part.\n> \t */\n> -\tframeContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> -\tframeContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> +\tif (frameContext.awb.autoEnabled) {\n> +\t\tframeContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> +\t\tframeContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> +\t}\n> \t/* Hardcode the green gain to 1.0. */\n> \tframeContext.awb.gains.green = 1.0;\n>\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 667a8beb..4332fac7 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -21,6 +21,9 @@ public:\n>\n> \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> \tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +\tvoid queueRequest(IPAContext &context,\n> +\t\t\t const uint32_t frame,\n> +\t\t\t const ControlList &controls);\n> \tvoid process(IPAContext &context, IPAFrameContext *frameCtx,\n> \t\t const rkisp1_stat_buffer *stats) override;\n>\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index ef8bb8e9..290569d0 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -134,6 +134,9 @@ namespace libcamera::ipa::rkisp1 {\n> *\n> * \\var IPAFrameContext::awb.temperatureK\n> * \\brief Estimated color temperature\n> + *\n> + * \\var IPAFrameContext::awb.autoEnabled\n> + * \\brief Whether the Auto White Balance algorithm is enabled\n> */\n>\n> /**\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 2bdb6a81..68f92297 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -55,6 +55,7 @@ struct IPAFrameContext {\n> \t\t} gains;\n>\n> \t\tdouble temperatureK;\n> +\t\tbool autoEnabled;\n> \t} awb;\n>\n> \tstruct {\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 17d42d38..27b4212b 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -92,6 +92,8 @@ namespace {\n> /* List of controls handled by the RkISP1 IPA */\n> const ControlInfoMap::Map rkisp1Controls{\n> \t{ &controls::AeEnable, ControlInfo(false, true) },\n> +\t{ &controls::AwbEnable, ControlInfo(false, true) },\n> +\t{ &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> \t{ &controls::Brightness, ControlInfo(-1.0f, 0.993f) },\n> \t{ &controls::Contrast, ControlInfo(0.0f, 1.993f) },\n> \t{ &controls::Saturation, ControlInfo(0.0f, 1.993f) },\n> --\n> 2.30.2\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 4D9E6BE173\n\tfor ;\n\tFri, 19 Aug 2022 13:11:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A06DD61FC0;\n\tFri, 19 Aug 2022 15:11:26 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3EF6061FA8\n\tfor ;\n\tFri, 19 Aug 2022 15:11:25 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 9AF6D240004;\n\tFri, 19 Aug 2022 13:11:23 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660914686;\n\tbh=VFYhiqo/paDD51kSZpWSA+3KGOGwdnS4iSVS9F/oiVc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=wlDkqxhT7lefUMS43EyGTarnBE4ZCMexJQc4vAH1tMhBRaUFSZCXYEeBfQBKTzS6E\n\ttt/jv9bcghUTULANOQ6DbSUQmFFdPA+/IdEj4GjIImMg2ZFtVuN99jHBdtOkQXdCKK\n\twilopcWIGaAR+HyTYSqUf+ZSFpz51aE69zYYi/pILfSInlD0ykeAgxlpqSldcUozSZ\n\ttfx3aHVlPpLCxVC6f30Uv22MbVlQEkBhfKin79M0KguNca0Wn/wELKYUDbUicmQxSJ\n\tEHIX7f6AdgDBsrXNoWHoL6c7lYjGjNfKshFxRcpxciy0PEI/MlduBlBl/kch5kzKCj\n\tc4E0ahL1LbVRg==","Date":"Fri, 19 Aug 2022 15:11:20 +0200","To":"Paul Elder ","Message-ID":"<20220819131120.ecnqn275fpux6hjq@uno.localdomain>","References":"<20220818090108.3004198-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220818090108.3004198-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] ipa: rkisp1: Add manual color gains","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24717,"web_url":"https://patchwork.libcamera.org/comment/24717/","msgid":"","date":"2022-08-19T13:46:10","subject":"Re: [libcamera-devel] [PATCH v2] ipa: rkisp1: Add manual color gains","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Thu, Aug 18, 2022 at 06:01:08PM +0900, Paul Elder wrote:\n> Add support for manually controlling the color gains on the rkisp1 IPA.\n> To that end, add and plumb the AwbEnable and ColourGains controls. As\n> per-frame controls aren't supported yet in the rkisp1 IPA, simply apply\n> and perform checks on the controls immediately.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> Changes in v2:\n> - s/enabled/autoEnabled\n> - add documentation\n> - fixup construction of the ControlInfo\n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 36 +++++++++++++++++++++++++++++--\n> src/ipa/rkisp1/algorithms/awb.h | 3 +++\n> src/ipa/rkisp1/ipa_context.cpp | 3 +++\n> src/ipa/rkisp1/ipa_context.h | 1 +\n> src/ipa/rkisp1/rkisp1.cpp | 2 ++\n> 5 files changed, 43 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 9f00364d..a3c14a9a 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -12,6 +12,7 @@\n> \n> #include \n> \n> +#include \n> #include \n> \n> /**\n> @@ -38,6 +39,7 @@ int Awb::configure(IPAContext &context,\n> \tcontext.frameContext.awb.gains.red = 1.0;\n> \tcontext.frameContext.awb.gains.blue = 1.0;\n> \tcontext.frameContext.awb.gains.green = 1.0;\n> +\tcontext.frameContext.awb.autoEnabled = true;\n> \n> \t/*\n> \t * Define the measurement window for AWB as a centered rectangle\n> @@ -116,6 +118,34 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> \tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB;\n> }\n> \n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void Awb::queueRequest(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t const ControlList &controls)\n> +{\n> +\tauto &awb = context.frameContext.awb;\n> +\n> +\tconst auto &awbEnable = controls.get(controls::AwbEnable);\n> +\tif (awbEnable && *awbEnable != awb.autoEnabled) {\n> +\t\tawb.autoEnabled = *awbEnable;\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< (*awbEnable ? \"Enabling\" : \"Disabling\") << \" AWB\";\n> +\t}\n> +\n> +\tconst auto &colourGains = controls.get(controls::ColourGains);\n> +\tif (colourGains && !awb.autoEnabled) {\n> +\t\tawb.gains.red = (*colourGains)[0];\n> +\t\tawb.gains.blue = (*colourGains)[1];\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< \"Set colour gains to red: \" << awb.gains.red\n> +\t\t\t<< \", blue: \" << awb.gains.blue;\n> +\t}\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::process\n> */\n> @@ -164,8 +194,10 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n> \t * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n> \t * fractional part.\n> \t */\n> -\tframeContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> -\tframeContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> +\tif (frameContext.awb.autoEnabled) {\n> +\t\tframeContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> +\t\tframeContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> +\t}\n> \t/* Hardcode the green gain to 1.0. */\n> \tframeContext.awb.gains.green = 1.0;\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 667a8beb..4332fac7 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -21,6 +21,9 @@ public:\n> \n> \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> \tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +\tvoid queueRequest(IPAContext &context,\n> +\t\t\t const uint32_t frame,\n> +\t\t\t const ControlList &controls);\n\nMissing override. I'll fix it when applying.\n\n> \tvoid process(IPAContext &context, IPAFrameContext *frameCtx,\n> \t\t const rkisp1_stat_buffer *stats) override;\n> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index ef8bb8e9..290569d0 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -134,6 +134,9 @@ namespace libcamera::ipa::rkisp1 {\n> *\n> * \\var IPAFrameContext::awb.temperatureK\n> * \\brief Estimated color temperature\n> + *\n> + * \\var IPAFrameContext::awb.autoEnabled\n> + * \\brief Whether the Auto White Balance algorithm is enabled\n> */\n> \n> /**\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 2bdb6a81..68f92297 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -55,6 +55,7 @@ struct IPAFrameContext {\n> \t\t} gains;\n> \n> \t\tdouble temperatureK;\n> +\t\tbool autoEnabled;\n> \t} awb;\n> \n> \tstruct {\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 17d42d38..27b4212b 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -92,6 +92,8 @@ namespace {\n> /* List of controls handled by the RkISP1 IPA */\n> const ControlInfoMap::Map rkisp1Controls{\n> \t{ &controls::AeEnable, ControlInfo(false, true) },\n> +\t{ &controls::AwbEnable, ControlInfo(false, true) },\n> +\t{ &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> \t{ &controls::Brightness, ControlInfo(-1.0f, 0.993f) },\n> \t{ &controls::Contrast, ControlInfo(0.0f, 1.993f) },\n> \t{ &controls::Saturation, ControlInfo(0.0f, 1.993f) },","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 6FA78C3272\n\tfor ;\n\tFri, 19 Aug 2022 13:46:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C7DF61FC0;\n\tFri, 19 Aug 2022 15:46:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 484CE61FA8\n\tfor ;\n\tFri, 19 Aug 2022 15:46:14 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A8BB33F1;\n\tFri, 19 Aug 2022 15:46:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660916775;\n\tbh=CEmM5H2STLG24YbKavPE1+o61dbYgDqt/MJN4oLzMyA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=bGZpivvwoEGLampq9R3SPols1tHSjB9bN7vy1/EeO55RBki3mWhVL1Nz1aKW7keAQ\n\t5eTSkACa8hetFUALh8zUbQsIeglGRV+YPWZu+DDt2HFwBXWFUCfuYgLDnJhPDUfuc1\n\t8qDXJhLUn5zWf8dto5xxX/NYvzrveDt3BaEYXWDlGdK3EFc93cnN4Gwyz8Xk3xIXYL\n\tR48ZfkiwPaZtBQphDx/kXZm1tY8PRFhl2qF3pTXjUGpmXSTH+hekaNw/GtMmI/UqYB\n\tNH2irK+J7E7cKiMo9sO6h19rxJsQKf0JG2OL0XfzVtQ4wjqpwqTuP+qLG9YoWXaEK6\n\tiksfPe048CZHw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660916773;\n\tbh=CEmM5H2STLG24YbKavPE1+o61dbYgDqt/MJN4oLzMyA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=d1lz2JXUZSa3Wj8y11J1vzKjI8tJnUuqnY6EVYzL2Z36IrWCeriuaM72IxzTfJn2K\n\ty5UXp1f3a+l3pglcPTHaMVqVflxC86zZjFK88KDWbuH1jUV7xBnFT8H2bBtrDXnSjy\n\to2zfhg399eJT94uc/957Krv10idX4n4blACJW7ww="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"d1lz2JXU\"; dkim-atps=neutral","Date":"Fri, 19 Aug 2022 16:46:10 +0300","To":"Paul Elder ","Message-ID":"","References":"<20220818090108.3004198-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220818090108.3004198-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] ipa: rkisp1: Add manual color gains","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]