[{"id":29766,"web_url":"https://patchwork.libcamera.org/comment/29766/","msgid":"","date":"2024-06-05T08:33:05","subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Wed, Jun 05, 2024 at 08:48:25AM GMT, Stefan Klug wrote:\n> Default control values where not applied to activeState. This had no negative\n> side effects in first place, as the hardware defaults where used. The issue\n> became visible, when only one of the controls was set at runtime. In that case\n> the params for the other values where overwritten with 0 (reset value of\n> activeState) resulting in a black image.\n>\n> While at it, only add the controls to the controls map if the algorithm is\n> contained in the tuning file.\n>\n> Signed-off-by: Stefan Klug \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> ---\n>\n> This patch depends on https://patchwork.libcamera.org/patch/20194/ which needs\n> to be merged first.\n>\n>\n> src/ipa/rkisp1/algorithms/cproc.cpp | 59 +++++++++++++++++++++++++++--\n> src/ipa/rkisp1/algorithms/cproc.h | 3 ++\n> src/ipa/rkisp1/rkisp1.cpp | 3 --\n> 3 files changed, 59 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> index 68bb8180..0f122504 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> @@ -33,6 +33,56 @@ namespace ipa::rkisp1::algorithms {\n>\n> LOG_DEFINE_CATEGORY(RkISP1CProc)\n>\n> +static int convertBrightness(const float v)\n> +{\n> +\treturn std::clamp(std::lround(v * 128), -128, 127);\n> +}\n> +\n> +static int convertContrast(const float v)\n> +{\n> +\treturn std::clamp(std::lround(v * 128), 0, 255);\n> +}\n> +\n> +static int convertSaturation(const float v)\n> +{\n> +\treturn std::clamp(std::lround(v * 128), 0, 255);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int ColorProcessing::init([[maybe_unused]] IPAContext &context,\n> +\t\t\t [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> +\tcontext.ctrlMap[&controls::Brightness] = ControlInfo(-1.0f, 0.993f, 0.0f);\n> +\tcontext.ctrlMap[&controls::Contrast] = ControlInfo(0.0f, 1.993f, 1.0f);\n> +\tcontext.ctrlMap[&controls::Saturation] = ControlInfo(0.0f, 1.993f, 1.0f);\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the Gamma given a configInfo\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] configInfo The IPA configuration data\n> + *\n> + * \\return 0\n> + */\n> +int ColorProcessing::configure([[maybe_unused]] IPAContext &context,\n> +\t\t\t [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> +{\n> +\tauto &cproc = context.activeState.cproc;\n> +\n> +\tcproc.brightness = convertBrightness(\n> +\t\tcontext.ctrlMap[&controls::Brightness].def().get());\n> +\tcproc.contrast = convertContrast(\n> +\t\tcontext.ctrlMap[&controls::Contrast].def().get());\n> +\tcproc.saturation = convertSaturation(\n> +\t\tcontext.ctrlMap[&controls::Saturation].def().get());\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> */\n> @@ -44,9 +94,12 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> \tauto &cproc = context.activeState.cproc;\n> \tbool update = false;\n>\n> +\tif (frame == 0)\n> +\t\tupdate = true;\n> +\n> \tconst auto &brightness = controls.get(controls::Brightness);\n> \tif (brightness) {\n> -\t\tint value = std::clamp(std::lround(*brightness * 128), -128, 127);\n> +\t\tint value = convertBrightness(*brightness);\n> \t\tif (cproc.brightness != value) {\n> \t\t\tcproc.brightness = value;\n> \t\t\tupdate = true;\n> @@ -57,7 +110,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n>\n> \tconst auto &contrast = controls.get(controls::Contrast);\n> \tif (contrast) {\n> -\t\tint value = std::clamp(std::lround(*contrast * 128), 0, 255);\n> +\t\tint value = convertContrast(*contrast);\n> \t\tif (cproc.contrast != value) {\n> \t\t\tcproc.contrast = value;\n> \t\t\tupdate = true;\n> @@ -68,7 +121,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n>\n> \tconst auto saturation = controls.get(controls::Saturation);\n> \tif (saturation) {\n> -\t\tint value = std::clamp(std::lround(*saturation * 128), 0, 255);\n> +\t\tint value = convertSaturation(*saturation);\n> \t\tif (cproc.saturation != value) {\n> \t\t\tcproc.saturation = value;\n> \t\t\tupdate = true;\n> diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h\n> index bafba5cc..e50e7200 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.h\n> +++ b/src/ipa/rkisp1/algorithms/cproc.h\n> @@ -21,6 +21,9 @@ public:\n> \tColorProcessing() = default;\n> \t~ColorProcessing() = default;\n>\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> +\tint configure(IPAContext &context,\n> +\t\t const IPACameraSensorInfo &configInfo) override;\n> \tvoid queueRequest(IPAContext &context, const uint32_t frame,\n> \t\t\t IPAFrameContext &frameContext,\n> \t\t\t const ControlList &controls) override;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 17474408..62d56a3a 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -109,9 +109,6 @@ 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, 0.0f) },\n> -\t{ &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> -\t{ &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> \t{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> \t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\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 5D3DABDE6B\n\tfor ;\n\tWed, 5 Jun 2024 08:33:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B9F86543C;\n\tWed, 5 Jun 2024 10:33:11 +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 E9EB5634B5\n\tfor ;\n\tWed, 5 Jun 2024 10:33:09 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 698E7EA2;\n\tWed, 5 Jun 2024 10:33:01 +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=\"AUYxjcTr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1717576381;\n\tbh=DVTUj7oCpeeO+Neiw3EenEP+jT5RI7mFmv+E+e7ghWQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AUYxjcTrf0Nl3oGAl2qS576g88Y+wIyhoFQcJnozsuWBnL1hOyPn5FmXAG2Ena3ED\n\tuZnlmlwFD/xAT9khYuZDiiQSoamStHcV8OJJP0t7umNi2LBJ3j9rvh0WcB1nSoThm6\n\tU6EmQ3INbfYWSzZVnJ3fof8+Jp8oWBIhG51RjO+o=","Date":"Wed, 5 Jun 2024 10:33:05 +0200","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","Message-ID":"","References":"<20240605064829.81288-1-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240605064829.81288-1-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":29767,"web_url":"https://patchwork.libcamera.org/comment/29767/","msgid":"<171757680409.205609.4731431453450997834@ping.linuxembedded.co.uk>","date":"2024-06-05T08:40:04","subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2024-06-05 07:48:25)\n> Default control values where not applied to activeState. This had no negative\n\ns/where/were/\n\n> side effects in first place, as the hardware defaults where used. The issue\n\ns/in first/in the first/\ns/where/were/\n\n> became visible, when only one of the controls was set at runtime. In that case\n> the params for the other values where overwritten with 0 (reset value of\n\ns/where/were/ ;-)\n\n> activeState) resulting in a black image.\n> \n> While at it, only add the controls to the controls map if the algorithm is\n> contained in the tuning file.\n\nI really like this bit. I would have thought it could be a distinct\npatch but I'll not worry too much if it isn't. But this should be done\nfor all controls in rkisp1Controls if they are used/controlled by an\nalgorithm. Keeping the initialisation of the control and mangement of it\nclose by is far better. (and means the control will only be reported if\nthe algo is enabled of course)\n\n\n\n> \n> Signed-off-by: Stefan Klug \n> \n> ---\n> \n> This patch depends on https://patchwork.libcamera.org/patch/20194/ which needs\n> to be merged first.\n> \n> \n> src/ipa/rkisp1/algorithms/cproc.cpp | 59 +++++++++++++++++++++++++++--\n> src/ipa/rkisp1/algorithms/cproc.h | 3 ++\n> src/ipa/rkisp1/rkisp1.cpp | 3 --\n> 3 files changed, 59 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> index 68bb8180..0f122504 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> @@ -33,6 +33,56 @@ namespace ipa::rkisp1::algorithms {\n> \n> LOG_DEFINE_CATEGORY(RkISP1CProc)\n> \n> +static int convertBrightness(const float v)\n> +{\n> + return std::clamp(std::lround(v * 128), -128, 127);\n> +}\n> +\n> +static int convertContrast(const float v)\n> +{\n> + return std::clamp(std::lround(v * 128), 0, 255);\n> +}\n> +\n> +static int convertSaturation(const float v)\n\nI wondered about having these take a ControlValue so the\ncasting/converting could be done here. Might shorten the lines where\nconvert* is used? But entirely optional.\n\n\n> +{\n> + return std::clamp(std::lround(v * 128), 0, 255);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int ColorProcessing::init([[maybe_unused]] IPAContext &context,\n> + [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> + context.ctrlMap[&controls::Brightness] = ControlInfo(-1.0f, 0.993f, 0.0f);\n> + context.ctrlMap[&controls::Contrast] = ControlInfo(0.0f, 1.993f, 1.0f);\n> + context.ctrlMap[&controls::Saturation] = ControlInfo(0.0f, 1.993f, 1.0f);\n\nI'm so happy to see the control defaults and definitions move into the\nfile that owns and manipulates them!\n\n> +\n> + return 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the Gamma given a configInfo\n\nGamma ?\n\nCould be just a copydoc if there's nothing more to add.\n\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] configInfo The IPA configuration data\n> + *\n> + * \\return 0\n> + */\n> +int ColorProcessing::configure([[maybe_unused]] IPAContext &context,\n> + [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> +{\n> + auto &cproc = context.activeState.cproc;\n> +\n> + cproc.brightness = convertBrightness(\n> + context.ctrlMap[&controls::Brightness].def().get());\n> + cproc.contrast = convertContrast(\n> + context.ctrlMap[&controls::Contrast].def().get());\n> + cproc.saturation = convertSaturation(\n> + context.ctrlMap[&controls::Saturation].def().get());\n> +\n\nTempting to make the default values class consts such as\n\tstatic const float kDefaultBrightness = 0.0f;\n\tstatic const float kDefaultContrast = 1.0f;\n\tstatic const float kDefaultSaturuation = 1.0f;\n\nand then reference those directly to avoid the lookups to what we know\nare otherwise constant?\n\nI'm pretty sure handling all the above is easy so with those:\n\n\nReviewed-by: Kieran Bingham \n\n\n> + return 0;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> */\n> @@ -44,9 +94,12 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> auto &cproc = context.activeState.cproc;\n> bool update = false;\n> \n> + if (frame == 0)\n> + update = true;\n> +\n> const auto &brightness = controls.get(controls::Brightness);\n> if (brightness) {\n> - int value = std::clamp(std::lround(*brightness * 128), -128, 127);\n> + int value = convertBrightness(*brightness);\n> if (cproc.brightness != value) {\n> cproc.brightness = value;\n> update = true;\n> @@ -57,7 +110,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> \n> const auto &contrast = controls.get(controls::Contrast);\n> if (contrast) {\n> - int value = std::clamp(std::lround(*contrast * 128), 0, 255);\n> + int value = convertContrast(*contrast);\n> if (cproc.contrast != value) {\n> cproc.contrast = value;\n> update = true;\n> @@ -68,7 +121,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> \n> const auto saturation = controls.get(controls::Saturation);\n> if (saturation) {\n> - int value = std::clamp(std::lround(*saturation * 128), 0, 255);\n> + int value = convertSaturation(*saturation);\n> if (cproc.saturation != value) {\n> cproc.saturation = value;\n> update = true;\n> diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h\n> index bafba5cc..e50e7200 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.h\n> +++ b/src/ipa/rkisp1/algorithms/cproc.h\n> @@ -21,6 +21,9 @@ public:\n> ColorProcessing() = default;\n> ~ColorProcessing() = default;\n> \n> + int init(IPAContext &context, const YamlObject &tuningData) override;\n> + int configure(IPAContext &context,\n> + const IPACameraSensorInfo &configInfo) override;\n> void queueRequest(IPAContext &context, const uint32_t frame,\n> IPAFrameContext &frameContext,\n> const ControlList &controls) override;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 17474408..62d56a3a 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -109,9 +109,6 @@ const ControlInfoMap::Map rkisp1Controls{\n> { &controls::AeEnable, ControlInfo(false, true) },\n> { &controls::AwbEnable, ControlInfo(false, true) },\n> { &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> - { &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> - { &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> - { &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\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 69305BD87C\n\tfor ;\n\tWed, 5 Jun 2024 08:40:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74F086543F;\n\tWed, 5 Jun 2024 10:40:08 +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 156626543A\n\tfor ;\n\tWed, 5 Jun 2024 10:40:07 +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 9ADCCEA2;\n\tWed, 5 Jun 2024 10:39:58 +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=\"QYisXIPg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1717576798;\n\tbh=2QtxHXraXTlgwqCt+9A7eZ8rLPX/bS2CmtEJbo+fnQ8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=QYisXIPgDicqREOb8YKu8XC/VZbdidv8Czv0ygo6kUHElVvCSHWMigGXDHtwgm/Py\n\tiyDzvCr8hxsCwGRW/8asD9T8qelGt4/4sV/qNt66PHwPL9g3lGQkIhA6eW2Ij/0FsC\n\tzwxXY0vhA+D03hn5ipRsft9vdzZg4EVRhqt0YyJQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240605064829.81288-1-stefan.klug@ideasonboard.com>","References":"<20240605064829.81288-1-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 05 Jun 2024 09:40:04 +0100","Message-ID":"<171757680409.205609.4731431453450997834@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":29768,"web_url":"https://patchwork.libcamera.org/comment/29768/","msgid":"<171757689874.205609.4261436660380809951@ping.linuxembedded.co.uk>","date":"2024-06-05T08:41:38","subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-06-05 09:40:04)\n> Quoting Stefan Klug (2024-06-05 07:48:25)\n...\n> and then reference those directly to avoid the lookups to what we know\n> are otherwise constant?\n> \n> I'm pretty sure handling all the above is easy so with those:\n\n('with those' meaning my suggestions above are optional / up to you -\nnot ... you must take my suggestions :D)\n\n\n> Reviewed-by: Kieran Bingham ","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 05549BDE6B\n\tfor ;\n\tWed, 5 Jun 2024 08:41:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1CAA6543F;\n\tWed, 5 Jun 2024 10:41:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 301896543A\n\tfor ;\n\tWed, 5 Jun 2024 10:41:41 +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 EA232EA2;\n\tWed, 5 Jun 2024 10:41:32 +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=\"MshpLUg7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1717576893;\n\tbh=w9O5nYIfcnkfFC5/DiZWdIRAmQqiGIDtsGOzqOT9DWc=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=MshpLUg7yjy0zBW7rsCFf+xbSdzCi1Y8aflAnhBHw8k88qj9mVKA5ak5ebzeNUClH\n\tjtMfK2ASK2RiatqpsYt69dMHUEB2Vxgl/rZSVQJuWvmqmRKpaOpHGOU3zHufPsmEt4\n\tgp7S3bfqnPoL+SGkQtcolard+uaWIniHwnIHahOI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<171757680409.205609.4731431453450997834@ping.linuxembedded.co.uk>","References":"<20240605064829.81288-1-stefan.klug@ideasonboard.com>\n\t<171757680409.205609.4731431453450997834@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 05 Jun 2024 09:41:38 +0100","Message-ID":"<171757689874.205609.4261436660380809951@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":29777,"web_url":"https://patchwork.libcamera.org/comment/29777/","msgid":"","date":"2024-06-05T09:33:52","subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Kieran,\n\nthanks for the review and especially for the s/where/were/. I need to\nput an autocorrect there :-)\n\nThe suggested change to kDefaultXXX makes it really easier to digest.\n\nFor reference I added the patch that will go in after gamma below.\n\nCheers,\nStefan\n\n---\n\nSubject: [PATCH v2] pipeline: rkisp1: cproc: Fix default value handling\n\nDefault control values were not applied to activeState. This had no negative\nside effects in the first place, as the hardware defaults were used. The issue\nbecame visible, when only one of the controls was set at runtime. In that case\nthe params for the other values were overwritten with 0 (reset value of\nactiveState) resulting in a black image.\n\nWhile at it, only add the controls to the controls map if the algorithm is\ncontained in the tuning file.\n\nSigned-off-by: Stefan Klug \nReviewed-by: Jacopo Mondi \nReviewed-by: Kieran Bingham \n---\n src/ipa/rkisp1/algorithms/cproc.cpp | 58 +++++++++++++++++++++++++++--\n src/ipa/rkisp1/algorithms/cproc.h | 3 ++\n src/ipa/rkisp1/rkisp1.cpp | 3 --\n 3 files changed, 58 insertions(+), 6 deletions(-)\n\ndiff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\nindex 68bb8180..18c7b719 100644\n--- a/src/ipa/rkisp1/algorithms/cproc.cpp\n+++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n@@ -33,6 +33,55 @@ namespace ipa::rkisp1::algorithms {\n \n LOG_DEFINE_CATEGORY(RkISP1CProc)\n \n+constexpr float kDefaultBrightness = 0.0f;\n+constexpr float kDefaultContrast = 1.0f;\n+constexpr float kDefaultSaturation = 1.0f;\n+\n+static int convertBrightness(const float v)\n+{\n+\treturn std::clamp(std::lround(v * 128), -128, 127);\n+}\n+\n+static int convertContrast(const float v)\n+{\n+\treturn std::clamp(std::lround(v * 128), 0, 255);\n+}\n+\n+static int convertSaturation(const float v)\n+{\n+\treturn std::clamp(std::lround(v * 128), 0, 255);\n+}\n+\n+/**\n+ * \\copydoc libcamera::ipa::Algorithm::init\n+ */\n+int ColorProcessing::init([[maybe_unused]] IPAContext &context,\n+\t\t\t [[maybe_unused]] const YamlObject &tuningData)\n+{\n+\tauto &cmap = context.ctrlMap;\n+\n+\tcmap[&controls::Brightness] = ControlInfo(-1.0f, 0.993f, kDefaultBrightness);\n+\tcmap[&controls::Contrast] = ControlInfo(0.0f, 1.993f, kDefaultContrast);\n+\tcmap[&controls::Saturation] = ControlInfo(0.0f, 1.993f, kDefaultSaturation);\n+\n+\treturn 0;\n+}\n+\n+/**\n+ * \\copydoc libcamera::ipa::Algorithm::configure\n+ */\n+int ColorProcessing::configure([[maybe_unused]] IPAContext &context,\n+\t\t\t [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n+{\n+\tauto &cproc = context.activeState.cproc;\n+\n+\tcproc.brightness = convertBrightness(kDefaultBrightness);\n+\tcproc.contrast = convertContrast(kDefaultContrast);\n+\tcproc.saturation = convertSaturation(kDefaultSaturation);\n+\n+\treturn 0;\n+}\n+\n /**\n * \\copydoc libcamera::ipa::Algorithm::queueRequest\n */\n@@ -44,9 +93,12 @@ void ColorProcessing::queueRequest(IPAContext &context,\n \tauto &cproc = context.activeState.cproc;\n \tbool update = false;\n \n+\tif (frame == 0)\n+\t\tupdate = true;\n+\n \tconst auto &brightness = controls.get(controls::Brightness);\n \tif (brightness) {\n-\t\tint value = std::clamp(std::lround(*brightness * 128), -128, 127);\n+\t\tint value = convertBrightness(*brightness);\n \t\tif (cproc.brightness != value) {\n \t\t\tcproc.brightness = value;\n \t\t\tupdate = true;\n@@ -57,7 +109,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n \n \tconst auto &contrast = controls.get(controls::Contrast);\n \tif (contrast) {\n-\t\tint value = std::clamp(std::lround(*contrast * 128), 0, 255);\n+\t\tint value = convertContrast(*contrast);\n \t\tif (cproc.contrast != value) {\n \t\t\tcproc.contrast = value;\n \t\t\tupdate = true;\n@@ -68,7 +120,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n \n \tconst auto saturation = controls.get(controls::Saturation);\n \tif (saturation) {\n-\t\tint value = std::clamp(std::lround(*saturation * 128), 0, 255);\n+\t\tint value = convertSaturation(*saturation);\n \t\tif (cproc.saturation != value) {\n \t\t\tcproc.saturation = value;\n \t\t\tupdate = true;\ndiff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h\nindex bafba5cc..e50e7200 100644\n--- a/src/ipa/rkisp1/algorithms/cproc.h\n+++ b/src/ipa/rkisp1/algorithms/cproc.h\n@@ -21,6 +21,9 @@ public:\n \tColorProcessing() = default;\n \t~ColorProcessing() = default;\n \n+\tint init(IPAContext &context, const YamlObject &tuningData) override;\n+\tint configure(IPAContext &context,\n+\t\t const IPACameraSensorInfo &configInfo) override;\n \tvoid queueRequest(IPAContext &context, const uint32_t frame,\n \t\t\t IPAFrameContext &frameContext,\n \t\t\t const ControlList &controls) override;\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 17474408..62d56a3a 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -109,9 +109,6 @@ 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, 0.0f) },\n-\t{ &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n-\t{ &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n \t{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n \t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\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 A404CBDE6B\n\tfor ;\n\tWed, 5 Jun 2024 09:33:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB1DC6543A;\n\tWed, 5 Jun 2024 11:33:56 +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 712EC634DB\n\tfor ;\n\tWed, 5 Jun 2024 11:33:55 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:dcc4:f2dc:93ce:ff8f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1831414B0;\n\tWed, 5 Jun 2024 11:33:47 +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=\"bLZFqy2M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1717580027;\n\tbh=JEn838nOZZ/oNHyOB3c29qInfwozcfdWZOKM5SfnrhA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bLZFqy2Mu08Hmem7M5bjXScvyUIXoP9iK/7kmqfAx/x9Vxhxvgp3Z2/qctiYWRi8E\n\te1cKZVWyRC7D3iYOCqDkuUMvANRZ3d7sxu3VR7kvIvfAo1OoZwJzvl6OGj0YrL3CP5\n\tcjmXlxXBsaXCuBSWXTgcqap5ztKkwezvy1igv3+g=","Date":"Wed, 5 Jun 2024 11:33:52 +0200","From":"Stefan Klug ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","Message-ID":"","References":"<20240605064829.81288-1-stefan.klug@ideasonboard.com>\n\t<171757680409.205609.4731431453450997834@ping.linuxembedded.co.uk>\n\t<171757689874.205609.4261436660380809951@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171757689874.205609.4261436660380809951@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":29841,"web_url":"https://patchwork.libcamera.org/comment/29841/","msgid":"<20240611200926.GA8290@pendragon.ideasonboard.com>","date":"2024-06-11T20:09:26","subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jun 05, 2024 at 09:40:04AM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2024-06-05 07:48:25)\n> > Default control values where not applied to activeState. This had no negative\n> \n> s/where/were/\n> \n> > side effects in first place, as the hardware defaults where used. The issue\n> \n> s/in first/in the first/\n> s/where/were/\n> \n> > became visible, when only one of the controls was set at runtime. In that case\n> > the params for the other values where overwritten with 0 (reset value of\n> \n> s/where/were/ ;-)\n> \n> > activeState) resulting in a black image.\n> > \n> > While at it, only add the controls to the controls map if the algorithm is\n> > contained in the tuning file.\n> \n> I really like this bit. I would have thought it could be a distinct\n> patch but I'll not worry too much if it isn't.\n\nYes, for next time splitting such changes in two patches would be\npreferred.\n\n> But this should be done\n> for all controls in rkisp1Controls if they are used/controlled by an\n> algorithm. Keeping the initialisation of the control and mangement of it\n> close by is far better. (and means the control will only be reported if\n> the algo is enabled of course)\n> \n> > Signed-off-by: Stefan Klug \n> > \n> > ---\n> > \n> > This patch depends on https://patchwork.libcamera.org/patch/20194/ which needs\n> > to be merged first.\n> > \n> > \n> > src/ipa/rkisp1/algorithms/cproc.cpp | 59 +++++++++++++++++++++++++++--\n> > src/ipa/rkisp1/algorithms/cproc.h | 3 ++\n> > src/ipa/rkisp1/rkisp1.cpp | 3 --\n> > 3 files changed, 59 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> > index 68bb8180..0f122504 100644\n> > --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> > @@ -33,6 +33,56 @@ namespace ipa::rkisp1::algorithms {\n> > \n> > LOG_DEFINE_CATEGORY(RkISP1CProc)\n> > \n> > +static int convertBrightness(const float v)\n> > +{\n> > + return std::clamp(std::lround(v * 128), -128, 127);\n> > +}\n> > +\n> > +static int convertContrast(const float v)\n> > +{\n> > + return std::clamp(std::lround(v * 128), 0, 255);\n> > +}\n> > +\n> > +static int convertSaturation(const float v)\n> \n> I wondered about having these take a ControlValue so the\n> casting/converting could be done here. Might shorten the lines where\n> convert* is used? But entirely optional.\n> \n> > +{\n> > + return std::clamp(std::lround(v * 128), 0, 255);\n> > +}\n\nWe follow the C++ way of using anonymous namespaces to make limit symbol\nvisibility to the TU:\n\nnamespace {\n\nint convertBrightness(const float v)\n{\n\treturn std::clamp(std::lround(v * 128), -128, 127);\n}\n\nint convertContrast(const float v)\n{\n\treturn std::clamp(std::lround(v * 128), 0, 255);\n}\n\nint convertSaturation(const float v)\n{\n\treturn std::clamp(std::lround(v * 128), 0, 255);\n}\n\n} /* namespace */\n\nThis should encompass the constexpr kDefault* mentioned below.\n\nThe convertContrast() and convertSaturation() functions are identical,\nhow about merging them into a single convertContrastSaturation() ?\n\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::init\n> > + */\n> > +int ColorProcessing::init([[maybe_unused]] IPAContext &context,\n\nYou can drop [[maybe_unused]].\n\n> > + [[maybe_unused]] const YamlObject &tuningData)\n> > +{\n> > + context.ctrlMap[&controls::Brightness] = ControlInfo(-1.0f, 0.993f, 0.0f);\n> > + context.ctrlMap[&controls::Contrast] = ControlInfo(0.0f, 1.993f, 1.0f);\n> > + context.ctrlMap[&controls::Saturation] = ControlInfo(0.0f, 1.993f, 1.0f);\n> \n> I'm so happy to see the control defaults and definitions move into the\n> file that owns and manipulates them!\n> \n> > +\n> > + return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Configure the Gamma given a configInfo\n> \n> Gamma ?\n> \n> Could be just a copydoc if there's nothing more to add.\n> \n> > + * \\param[in] context The shared IPA context\n> > + * \\param[in] configInfo The IPA configuration data\n> > + *\n> > + * \\return 0\n> > + */\n> > +int ColorProcessing::configure([[maybe_unused]] IPAContext &context,\n\nSame here.\n\nAs this patch has been merged already, could you fix these small issues\n(at least the ones you agree with) in follow-up patches ?\n\n> > + [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> > +{\n> > + auto &cproc = context.activeState.cproc;\n> > +\n> > + cproc.brightness = convertBrightness(\n> > + context.ctrlMap[&controls::Brightness].def().get());\n> > + cproc.contrast = convertContrast(\n> > + context.ctrlMap[&controls::Contrast].def().get());\n> > + cproc.saturation = convertSaturation(\n> > + context.ctrlMap[&controls::Saturation].def().get());\n> > +\n> \n> Tempting to make the default values class consts such as\n> \tstatic const float kDefaultBrightness = 0.0f;\n> \tstatic const float kDefaultContrast = 1.0f;\n> \tstatic const float kDefaultSaturuation = 1.0f;\n> \n> and then reference those directly to avoid the lookups to what we know\n> are otherwise constant?\n> \n> I'm pretty sure handling all the above is easy so with those:\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> \n> > + return 0;\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > */\n> > @@ -44,9 +94,12 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> > auto &cproc = context.activeState.cproc;\n> > bool update = false;\n> > \n> > + if (frame == 0)\n> > + update = true;\n> > +\n> > const auto &brightness = controls.get(controls::Brightness);\n> > if (brightness) {\n> > - int value = std::clamp(std::lround(*brightness * 128), -128, 127);\n> > + int value = convertBrightness(*brightness);\n> > if (cproc.brightness != value) {\n> > cproc.brightness = value;\n> > update = true;\n> > @@ -57,7 +110,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> > \n> > const auto &contrast = controls.get(controls::Contrast);\n> > if (contrast) {\n> > - int value = std::clamp(std::lround(*contrast * 128), 0, 255);\n> > + int value = convertContrast(*contrast);\n> > if (cproc.contrast != value) {\n> > cproc.contrast = value;\n> > update = true;\n> > @@ -68,7 +121,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> > \n> > const auto saturation = controls.get(controls::Saturation);\n> > if (saturation) {\n> > - int value = std::clamp(std::lround(*saturation * 128), 0, 255);\n> > + int value = convertSaturation(*saturation);\n> > if (cproc.saturation != value) {\n> > cproc.saturation = value;\n> > update = true;\n> > diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h\n> > index bafba5cc..e50e7200 100644\n> > --- a/src/ipa/rkisp1/algorithms/cproc.h\n> > +++ b/src/ipa/rkisp1/algorithms/cproc.h\n> > @@ -21,6 +21,9 @@ public:\n> > ColorProcessing() = default;\n> > ~ColorProcessing() = default;\n> > \n> > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > + int configure(IPAContext &context,\n> > + const IPACameraSensorInfo &configInfo) override;\n> > void queueRequest(IPAContext &context, const uint32_t frame,\n> > IPAFrameContext &frameContext,\n> > const ControlList &controls) override;\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 17474408..62d56a3a 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -109,9 +109,6 @@ const ControlInfoMap::Map rkisp1Controls{\n> > { &controls::AeEnable, ControlInfo(false, true) },\n> > { &controls::AwbEnable, ControlInfo(false, true) },\n> > { &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> > - { &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> > - { &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > - { &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\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 70D18BD87C\n\tfor ;\n\tTue, 11 Jun 2024 20:09:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE8F565456;\n\tTue, 11 Jun 2024 22:09:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 92CFC61A26\n\tfor ;\n\tTue, 11 Jun 2024 22:09:46 +0200 (CEST)","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 5EA8D29A;\n\tTue, 11 Jun 2024 22:09:33 +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=\"rgV/GxQw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718136573;\n\tbh=dPdC7J/u4lrVB+xtdbLEf8y9VdyLeghVrcPkvAjqT24=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rgV/GxQwD1yCMoJ5LudRhCmFT+tyXwDh9tBc38pgz4LRsSBGjBcjchsEsXROax5NQ\n\t+RzeYTa4tP5xIOBJ4OMRpKvYrLBINRTGEKOIrVznQcf0zaYS2LiS1FT4ZkBsUK41vQ\n\tb0H0hgqkvXAGyMtDIXhUFXvZKxIQQxzxqmb2wY7c=","Date":"Tue, 11 Jun 2024 23:09:26 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] pipeline: rkisp1: cproc: Fix default value handling","Message-ID":"<20240611200926.GA8290@pendragon.ideasonboard.com>","References":"<20240605064829.81288-1-stefan.klug@ideasonboard.com>\n\t<171757680409.205609.4731431453450997834@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171757680409.205609.4731431453450997834@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\" "}}]