[{"id":36447,"web_url":"https://patchwork.libcamera.org/comment/36447/","msgid":"","date":"2025-10-24T16:13:16","subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 14. 9:52 keltezéssel, Stefan Klug írta:\n> Implement the LensShadingEnable control for rkisp1.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/dpf.cpp | 2 +-\n> src/ipa/rkisp1/algorithms/lsc.cpp | 41 ++++++++++++++++++++++++++---\n> src/ipa/rkisp1/algorithms/lsc.h | 3 +++\n> src/ipa/rkisp1/ipa_context.h | 12 ++++++---\n> src/libcamera/control_ids_core.yaml | 3 +++\n> 5 files changed, 52 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index cb6095daeeed..79cb4829fdbb 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -229,7 +229,7 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> \t\t*config = config_;\n> \n> \t\tconst auto &awb = context.configuration.awb;\n> -\t\tconst auto &lsc = context.configuration.lsc;\n> +\t\tconst auto &lsc = context.activeState.lsc;\n> \n> \t\tauto &mode = config->gain.mode;\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 68cf35064182..ddb7154a0211 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -366,6 +366,8 @@ LensShadingCorrection::LensShadingCorrection()\n> int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \t\t\t\tconst YamlObject &tuningData)\n> {\n> +\tcontext.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, true);\n\nMaybe I'm missing something but it seems this control is only applicable for processed\nstreams, shouldn't it be hidden based on the configuration?\n\n\n> +\n> \txSize_ = parseSizes(tuningData, \"x-size\");\n> \tySize_ = parseSizes(tuningData, \"y-size\");\n> \n> @@ -449,7 +451,7 @@ int LensShadingCorrection::configure(IPAContext &context,\n> \n> \tsets_.setData(std::move(shadingData));\n> \n> -\tcontext.configuration.lsc.enabled = true;\n> +\tcontext.activeState.lsc.enabled = true;\n> \treturn 0;\n> }\n> \n> @@ -470,6 +472,27 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> \tstd::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);\n> }\n> \n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void LensShadingCorrection::queueRequest(IPAContext &context,\n> +\t\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t\t\t const ControlList &controls)\n> +{\n> +\tauto &lsc = context.activeState.lsc;\n> +\n> +\tconst auto &lscEnable = controls.get(controls::LensShadingEnable);\n> +\tif (lscEnable && *lscEnable != lsc.enabled) {\n> +\t\tlsc.enabled = *lscEnable;\n> +\n> +\t\tLOG(RkISP1Lsc, Debug)\n> +\t\t\t<< (*lscEnable ? \"Enabling\" : \"Disabling\") << \" Lsc\";\n> +\n> +\t\tframeContext.lsc.update = true;\n> +\t}\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> @@ -479,16 +502,26 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n> \t\t\t\t RkISP1Params *params)\n> {\n> \tuint32_t ct = frameContext.awb.temperatureK;\n> -\tif (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n> +\n> +\tif (!frameContext.lsc.update && !context.activeState.lsc.enabled)\n> +\t\treturn;\n> +\n> +\tif (!frameContext.lsc.update &&\n> +\t std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n> \t kColourTemperatureChangeThreshhold)\n> \t\treturn;\n> +\n> \tunsigned int quantizedCt;\n> \tconst Components &set = sets_.getInterpolated(ct, &quantizedCt);\n> -\tif (lastAppliedQuantizedCt_ == quantizedCt)\n> +\tif (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt)\n> \t\treturn;\n\nMaybe there is way to have just a single `frameContext.lsc.update` check,\nwhich would be easier to understand in my opinion. At least it seems easily\ndoable for the first two `if`s.\n\n\n> \n> \tauto config = params->block();\n> -\tconfig.setEnabled(true);\n> +\tconfig.setEnabled(context.activeState.lsc.enabled);\n> +\n> +\tif (!context.activeState.lsc.enabled)\n> +\t\treturn;\n> +\n> \tsetParameters(*config);\n> \tcopyTable(*config, set);\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> index 43fee337931f..a3d8042130c0 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.h\n> +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> @@ -29,6 +29,9 @@ public:\n> \tvoid prepare(IPAContext &context, const uint32_t frame,\n> \t\t IPAFrameContext &frameContext,\n> \t\t RkISP1Params *params) 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> \n> \tstruct Components {\n> \t\tstd::vector r;\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index f85a130d9c23..5ec87ae4e5a3 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -55,10 +55,6 @@ struct IPASessionConfiguration {\n> \t\tbool supported;\n> \t} compress;\n> \n> -\tstruct {\n> -\t\tbool enabled;\n> -\t} lsc;\n> -\n> \tstruct {\n> \t\tutils::Duration minExposureTime;\n> \t\tutils::Duration maxExposureTime;\n> @@ -139,6 +135,10 @@ struct IPAActiveState {\n> \t\tdouble gain;\n> \t\tdouble strength;\n> \t} wdr;\n> +\n> +\tstruct {\n> +\t\tbool enabled;\n> +\t} lsc;\n> };\n> \n> struct IPAFrameContext : public FrameContext {\n> @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext {\n> \t\tdouble strength;\n> \t\tdouble gain;\n> \t} wdr;\n> +\n> +\tstruct {\n> +\t\tbool update;\n> +\t} lsc;\n> };\n> \n> struct IPAContext {\n> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\n> index 9244b7eddb77..3b062df64bf5 100644\n> --- a/src/libcamera/control_ids_core.yaml\n> +++ b/src/libcamera/control_ids_core.yaml\n> @@ -1352,4 +1352,7 @@ controls:\n> description: |\n> Enable or disable the lens shading algorithm.\n> \n> + This control is only available when there are valid lens shading\n> + correction parameters available in the tuning file.\n> +\n> ...","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 644BCC3259\n\tfor ;\n\tFri, 24 Oct 2025 16:13:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1A7BC609B8;\n\tFri, 24 Oct 2025 18:13:23 +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 351F7608DC\n\tfor ;\n\tFri, 24 Oct 2025 18:13:21 +0200 (CEST)","from [192.168.33.13] (185.221.141.231.nat.pool.zt.hu\n\t[185.221.141.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 71655A8F;\n\tFri, 24 Oct 2025 18:11:35 +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=\"ohE5DMBi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761322295;\n\tbh=7L4np2Nm2zQDQWLe8Aj3YkJi6JV59AXI5unwyrZ90rk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ohE5DMBiAM7U1Oexd3VebYWbyGfQ22N4V8T4BwZLjZdesnmYneIVzfan2+4vP67lY\n\t4fcRMCcjlUd1l8yEq33s9RAzZwcdD8prAks8yGJDuckfErMy6rbthS0nekppaRlzXD\n\toV2dFNtZ4r0doLN9Rc0zFj45giO0WrX1ct6jMqyY=","Message-ID":"","Date":"Fri, 24 Oct 2025 18:13:16 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-13-stefan.klug@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251014075252.2876485-13-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36454,"web_url":"https://patchwork.libcamera.org/comment/36454/","msgid":"","date":"2025-10-24T21:09:02","subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"On 2025-10-14 03:52, Stefan Klug wrote:\n> Implement the LensShadingEnable control for rkisp1. Signed-off-by: \n> Stefan Klug --- \n> src/ipa/rkisp1/algorithms/dpf.cpp | 2 +- \n> src/ipa/rkisp1/algorithms/lsc.cpp | 41 ++++++++++++++++++++++++++--- \n> src/ipa/rkisp1/algorithms/lsc.h | 3 +++ src/ipa/rkisp1/ipa_context.h | \n> 12 ++++++--- src/libcamera/control_ids_core.yaml | 3 +++ 5 files \n> changed, 52 insertions(+), 9 deletions(-) diff --git \n> a/src/ipa/rkisp1/algorithms/dpf.cpp \n> b/src/ipa/rkisp1/algorithms/dpf.cpp index cb6095daeeed..79cb4829fdbb \n> 100644 --- a/src/ipa/rkisp1/algorithms/dpf.cpp +++ \n> b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -229,7 +229,7 @@ void \n> Dpf::prepare(IPAContext &context, const uint32_t frame, *config = \n> config_; const auto &awb = context.configuration.awb; - const auto \n> &lsc = context.configuration.lsc; + const auto &lsc = \n> context.activeState.lsc; auto &mode = config->gain.mode; diff --git \n> a/src/ipa/rkisp1/algorithms/lsc.cpp \n> b/src/ipa/rkisp1/algorithms/lsc.cpp index 68cf35064182..ddb7154a0211 \n> 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ \n> b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -366,6 +366,8 @@ \n> LensShadingCorrection::LensShadingCorrection() int \n> LensShadingCorrection::init([[maybe_unused]] IPAContext &context, \n> const YamlObject &tuningData) {\n\n                 auto lscEnable = parseYaml(tuningData);  or default as true\n\n                I think if Yaml has lsc enable toggle . it would be better .\n\n> + context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, \n> true, true);\n\n\t context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, lscEnable);\n\n> + xSize_ = parseSizes(tuningData, \"x-size\"); ySize_ = \n> parseSizes(tuningData, \"y-size\"); @@ -449,7 +451,7 @@ int \n> LensShadingCorrection::configure(IPAContext &context, \n> sets_.setData(std::move(shadingData)); - \n> context.configuration.lsc.enabled = true; + \n> context.activeState.lsc.enabled = true;\n\ncontext.activeState.lsc.enabled = lscEnable;\n\n> return 0; } @@ -470,6 +472,27 @@ void \n> LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config, \n> std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]); } \n> +/** + * \\copydoc libcamera::ipa::Algorithm::queueRequest + */ +void \n> LensShadingCorrection::queueRequest(IPAContext &context, + \n> [[maybe_unused]] const uint32_t frame, + IPAFrameContext \n> &frameContext, + const ControlList &controls) +{ + auto &lsc = \n> context.activeState.lsc; + + const auto &lscEnable = \n> controls.get(controls::LensShadingEnable); + if (lscEnable && \n> *lscEnable != lsc.enabled) { + lsc.enabled = *lscEnable; + + \n> LOG(RkISP1Lsc, Debug) + << (*lscEnable ? \"Enabling\" : \"Disabling\") << \n> \" Lsc\"; + + frameContext.lsc.update = true; + } +} + /** * \\copydoc \n> libcamera::ipa::Algorithm::prepare */ @@ -479,16 +502,26 @@ void \n> LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, \n> RkISP1Params *params) { uint32_t ct = frameContext.awb.temperatureK; - \n> if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) \n> < + + if (!frameContext.lsc.update && \n> !context.activeState.lsc.enabled) + return; + + if \n> (!frameContext.lsc.update && + std::abs(static_cast(ct) - \n> static_cast(lastAppliedCt_)) < \n> kColourTemperatureChangeThreshhold) return; + unsigned int \n> quantizedCt; const Components &set = sets_.getInterpolated(ct, \n> &quantizedCt); - if (lastAppliedQuantizedCt_ == quantizedCt) + if \n> (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt) \n> return; auto config = params->block(); - \n> config.setEnabled(true); + \n> config.setEnabled(context.activeState.lsc.enabled); + + if \n> (!context.activeState.lsc.enabled) + return; + setParameters(*config); \n> copyTable(*config, set); diff --git a/src/ipa/rkisp1/algorithms/lsc.h \n> b/src/ipa/rkisp1/algorithms/lsc.h index 43fee337931f..a3d8042130c0 \n> 100644 --- a/src/ipa/rkisp1/algorithms/lsc.h +++ \n> b/src/ipa/rkisp1/algorithms/lsc.h @@ -29,6 +29,9 @@ public: void \n> prepare(IPAContext &context, const uint32_t frame, IPAFrameContext \n> &frameContext, RkISP1Params *params) override; + void \n> queueRequest(IPAContext &context, const uint32_t frame, + \n> IPAFrameContext &frameContext, + const ControlList &controls) \n> override; struct Components { std::vector r; diff --git \n> a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index \n> f85a130d9c23..5ec87ae4e5a3 100644 --- a/src/ipa/rkisp1/ipa_context.h \n> +++ b/src/ipa/rkisp1/ipa_context.h @@ -55,10 +55,6 @@ struct \n> IPASessionConfiguration { bool supported; } compress; - struct { - \n> bool enabled; - } lsc; - struct { utils::Duration minExposureTime; \n> utils::Duration maxExposureTime; @@ -139,6 +135,10 @@ struct \n> IPAActiveState { double gain; double strength; } wdr; + + struct { + \n> bool enabled; + } lsc; }; struct IPAFrameContext : public FrameContext \n> { @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext { \n> double strength; double gain; } wdr; + + struct { + bool update; + } \n> lsc; }; struct IPAContext { diff --git \n> a/src/libcamera/control_ids_core.yaml \n> b/src/libcamera/control_ids_core.yaml index 9244b7eddb77..3b062df64bf5 \n> 100644 --- a/src/libcamera/control_ids_core.yaml +++ \n> b/src/libcamera/control_ids_core.yaml @@ -1352,4 +1352,7 @@ controls: \n> description: | Enable or disable the lens shading algorithm. + This \n> control is only available when there are valid lens shading + \n> correction parameters available in the tuning file. + ...","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 A5C0EC3259\n\tfor ;\n\tFri, 24 Oct 2025 21:09:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9241760A70;\n\tFri, 24 Oct 2025 23:09:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 142D4609B1\n\tfor ;\n\tFri, 24 Oct 2025 23:09:16 +0200 (CEST)","from [192.168.31.114] (unknown [209.216.122.90])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70BD1AB4\n\tfor ;\n\tFri, 24 Oct 2025 23:07:29 +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=\"b9h35jgG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761340049;\n\tbh=T71+xzZSBAcpvZpmUGr8dGK/EUlDPmRFlxtWqAzMXvk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=b9h35jgGr67WHLDiQ9L0gYtBiz98ms+YF0H+Kl8b9t58XJLwWejVCeJYVaahOKxW7\n\tExpqxwEsNoHRRscSk6t50c+66o1akgaVwE4Q2Lr+srtc1uwbdhiySXc/XaSRnTwGYa\n\tWNlIyxKH1Sn19gm925yecHEJDswpb9SBvG89znYg=","Content-Type":"multipart/alternative;\n\tboundary=\"------------aqMO0724d3oTUHwGcjGL9Uil\"","Message-ID":"","Date":"Fri, 24 Oct 2025 17:09:02 -0400","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","To":"libcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-13-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"rui wang ","In-Reply-To":"<20251014075252.2876485-13-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":37530,"web_url":"https://patchwork.libcamera.org/comment/37530/","msgid":"<176787416144.21455.18374123203880679933@localhost>","date":"2026-01-08T12:09:21","subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the review.\n\nQuoting Barnabás Pőcze (2025-10-24 18:13:16)\n> 2025. 10. 14. 9:52 keltezéssel, Stefan Klug írta:\n> > Implement the LensShadingEnable control for rkisp1.\n> > \n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/ipa/rkisp1/algorithms/dpf.cpp | 2 +-\n> > src/ipa/rkisp1/algorithms/lsc.cpp | 41 ++++++++++++++++++++++++++---\n> > src/ipa/rkisp1/algorithms/lsc.h | 3 +++\n> > src/ipa/rkisp1/ipa_context.h | 12 ++++++---\n> > src/libcamera/control_ids_core.yaml | 3 +++\n> > 5 files changed, 52 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > index cb6095daeeed..79cb4829fdbb 100644\n> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > @@ -229,7 +229,7 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> > *config = config_;\n> > \n> > const auto &awb = context.configuration.awb;\n> > - const auto &lsc = context.configuration.lsc;\n> > + const auto &lsc = context.activeState.lsc;\n> > \n> > auto &mode = config->gain.mode;\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index 68cf35064182..ddb7154a0211 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -366,6 +366,8 @@ LensShadingCorrection::LensShadingCorrection()\n> > int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > const YamlObject &tuningData)\n> > {\n> > + context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, true);\n> \n> Maybe I'm missing something but it seems this control is only applicable for processed\n> streams, shouldn't it be hidden based on the configuration?\n\nThere is Algorithm::supportsRaw_ which defaults to false. But that only\nguards the configure step. So the control is added to the info map\nunconditionally. But that is the case for all algorithms already. As we\ndon't have a solution for thread safe modification of the control info\nmap and do not advertise to reload the map after camera configure time,\nI think we should keep it that way for now. But the issue is there. \n\n> \n> \n> > +\n> > xSize_ = parseSizes(tuningData, \"x-size\");\n> > ySize_ = parseSizes(tuningData, \"y-size\");\n> > \n> > @@ -449,7 +451,7 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > \n> > sets_.setData(std::move(shadingData));\n> > \n> > - context.configuration.lsc.enabled = true;\n> > + context.activeState.lsc.enabled = true;\n> > return 0;\n> > }\n> > \n> > @@ -470,6 +472,27 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> > std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);\n> > }\n> > \n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > + */\n> > +void LensShadingCorrection::queueRequest(IPAContext &context,\n> > + [[maybe_unused]] const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + const ControlList &controls)\n> > +{\n> > + auto &lsc = context.activeState.lsc;\n> > +\n> > + const auto &lscEnable = controls.get(controls::LensShadingEnable);\n> > + if (lscEnable && *lscEnable != lsc.enabled) {\n> > + lsc.enabled = *lscEnable;\n> > +\n> > + LOG(RkISP1Lsc, Debug)\n> > + << (*lscEnable ? \"Enabling\" : \"Disabling\") << \" Lsc\";\n> > +\n> > + frameContext.lsc.update = true;\n> > + }\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::ipa::Algorithm::prepare\n> > */\n> > @@ -479,16 +502,26 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > RkISP1Params *params)\n> > {\n> > uint32_t ct = frameContext.awb.temperatureK;\n> > - if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n> > +\n> > + if (!frameContext.lsc.update && !context.activeState.lsc.enabled)\n> > + return;\n> > +\n> > + if (!frameContext.lsc.update &&\n> > + std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n> > kColourTemperatureChangeThreshhold)\n> > return;\n> > +\n> > unsigned int quantizedCt;\n> > const Components &set = sets_.getInterpolated(ct, &quantizedCt);\n> > - if (lastAppliedQuantizedCt_ == quantizedCt)\n> > + if (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt)\n> > return;\n> \n> Maybe there is way to have just a single `frameContext.lsc.update` check,\n> which would be easier to understand in my opinion. At least it seems easily\n> doable for the first two `if`s.\n\nYes that wasn't nice. I think I found a nicer solution for v2.\n\nBest regards,\nStefan\n\n> \n> \n> > \n> > auto config = params->block();\n> > - config.setEnabled(true);\n> > + config.setEnabled(context.activeState.lsc.enabled);\n> > +\n> > + if (!context.activeState.lsc.enabled)\n> > + return;\n> > +\n> > setParameters(*config);\n> > copyTable(*config, set);\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> > index 43fee337931f..a3d8042130c0 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.h\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> > @@ -29,6 +29,9 @@ public:\n> > void prepare(IPAContext &context, const uint32_t frame,\n> > IPAFrameContext &frameContext,\n> > RkISP1Params *params) override;\n> > + void queueRequest(IPAContext &context, const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + const ControlList &controls) override;\n> > \n> > struct Components {\n> > std::vector r;\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index f85a130d9c23..5ec87ae4e5a3 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -55,10 +55,6 @@ struct IPASessionConfiguration {\n> > bool supported;\n> > } compress;\n> > \n> > - struct {\n> > - bool enabled;\n> > - } lsc;\n> > -\n> > struct {\n> > utils::Duration minExposureTime;\n> > utils::Duration maxExposureTime;\n> > @@ -139,6 +135,10 @@ struct IPAActiveState {\n> > double gain;\n> > double strength;\n> > } wdr;\n> > +\n> > + struct {\n> > + bool enabled;\n> > + } lsc;\n> > };\n> > \n> > struct IPAFrameContext : public FrameContext {\n> > @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext {\n> > double strength;\n> > double gain;\n> > } wdr;\n> > +\n> > + struct {\n> > + bool update;\n> > + } lsc;\n> > };\n> > \n> > struct IPAContext {\n> > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\n> > index 9244b7eddb77..3b062df64bf5 100644\n> > --- a/src/libcamera/control_ids_core.yaml\n> > +++ b/src/libcamera/control_ids_core.yaml\n> > @@ -1352,4 +1352,7 @@ controls:\n> > description: |\n> > Enable or disable the lens shading algorithm.\n> > \n> > + This control is only available when there are valid lens shading\n> > + correction parameters available in the tuning file.\n> > +\n> > ...\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 862D0BE08B\n\tfor ;\n\tThu, 8 Jan 2026 12:09:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF53761FA0;\n\tThu, 8 Jan 2026 13:09:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27764615B2\n\tfor ;\n\tThu, 8 Jan 2026 13:09:25 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:d809:713a:29af:b139])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 796F2BE1;\n\tThu, 8 Jan 2026 13:09:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ln3eGZ6f\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1767874142;\n\tbh=NPIOPLJXqRJ3Ek6t5iFuAYQ2q4eb/hThLhtn04BO3Js=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=ln3eGZ6fbTKk7zYNpd7PkR81Gh8WRyQIDmx9k1ogrHQwfrPGACjYWz3EZ93hFp3ja\n\tQ89u6msUi/lhw1IczJDLwmLdvY++/ok8dusz0mjM23Ncebw+U0jtJjSYk/FNw2O+we\n\t/m2NrR52s3YoCxIZMeL2P3XJuiWHYx4oipe3gpbY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-13-stefan.klug@ideasonboard.com>\n\t","Subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","From":"Stefan Klug ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 08 Jan 2026 13:09:21 +0100","Message-ID":"<176787416144.21455.18374123203880679933@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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":37531,"web_url":"https://patchwork.libcamera.org/comment/37531/","msgid":"<176787439188.21455.10794608635026889412@localhost>","date":"2026-01-08T12:13:11","subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Rui,\n\nQuoting rui wang (2025-10-24 23:09:02)\n> \n> On 2025-10-14 03:52, Stefan Klug wrote:\n> > Implement the LensShadingEnable control for rkisp1. Signed-off-by: \n> > Stefan Klug --- \n> > src/ipa/rkisp1/algorithms/dpf.cpp | 2 +- \n> > src/ipa/rkisp1/algorithms/lsc.cpp | 41 ++++++++++++++++++++++++++--- \n> > src/ipa/rkisp1/algorithms/lsc.h | 3 +++ src/ipa/rkisp1/ipa_context.h | \n> > 12 ++++++--- src/libcamera/control_ids_core.yaml | 3 +++ 5 files \n> > changed, 52 insertions(+), 9 deletions(-) diff --git \n> > a/src/ipa/rkisp1/algorithms/dpf.cpp \n> > b/src/ipa/rkisp1/algorithms/dpf.cpp index cb6095daeeed..79cb4829fdbb \n> > 100644 --- a/src/ipa/rkisp1/algorithms/dpf.cpp +++ \n> > b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -229,7 +229,7 @@ void \n> > Dpf::prepare(IPAContext &context, const uint32_t frame, *config = \n> > config_; const auto &awb = context.configuration.awb; - const auto \n> > &lsc = context.configuration.lsc; + const auto &lsc = \n> > context.activeState.lsc; auto &mode = config->gain.mode; diff --git \n> > a/src/ipa/rkisp1/algorithms/lsc.cpp \n> > b/src/ipa/rkisp1/algorithms/lsc.cpp index 68cf35064182..ddb7154a0211 \n> > 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ \n> > b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -366,6 +366,8 @@ \n> > LensShadingCorrection::LensShadingCorrection() int \n> > LensShadingCorrection::init([[maybe_unused]] IPAContext &context, \n> > const YamlObject &tuningData) {\n\nThe formatting is broken in your response mail. Please check your email\nclient.\n\n> \n>                 auto lscEnable = parseYaml(tuningData);  or default as true\n> \n>                I think if Yaml has lsc enable toggle . it would be better .\n\nWe don't usually do that. If a algorithm is configured, it gets\nenabled. I guess the only usecase for that would be during development\nbut is easy to workaround. And it somehow collides with the enabled\nflag Isaac wants to integrate.\n\nBest regards,\nStefan\n\n> \n> > + context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, \n> > true, true);\n> \n> context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, lscEnable);\n> \n> > + xSize_ = parseSizes(tuningData, \"x-size\"); ySize_ = \n> > parseSizes(tuningData, \"y-size\"); @@ -449,7 +451,7 @@ int \n> > LensShadingCorrection::configure(IPAContext &context, \n> > sets_.setData(std::move(shadingData)); - \n> > context.configuration.lsc.enabled = true; + \n> > context.activeState.lsc.enabled = true;\n> \n> context.activeState.lsc.enabled = lscEnable;\n> \n> > return 0; } @@ -470,6 +472,27 @@ void \n> > LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config, \n> > std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]); } \n> > +/** + * \\copydoc libcamera::ipa::Algorithm::queueRequest + */ +void \n> > LensShadingCorrection::queueRequest(IPAContext &context, + \n> > [[maybe_unused]] const uint32_t frame, + IPAFrameContext \n> > &frameContext, + const ControlList &controls) +{ + auto &lsc = \n> > context.activeState.lsc; + + const auto &lscEnable = \n> > controls.get(controls::LensShadingEnable); + if (lscEnable && \n> > *lscEnable != lsc.enabled) { + lsc.enabled = *lscEnable; + + \n> > LOG(RkISP1Lsc, Debug) + << (*lscEnable ? \"Enabling\" : \"Disabling\") << \n> > \" Lsc\"; + + frameContext.lsc.update = true; + } +} + /** * \\copydoc \n> > libcamera::ipa::Algorithm::prepare */ @@ -479,16 +502,26 @@ void \n> > LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, \n> > RkISP1Params *params) { uint32_t ct = frameContext.awb.temperatureK; - \n> > if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) \n> > < + + if (!frameContext.lsc.update && \n> > !context.activeState.lsc.enabled) + return; + + if \n> > (!frameContext.lsc.update && + std::abs(static_cast(ct) - \n> > static_cast(lastAppliedCt_)) < \n> > kColourTemperatureChangeThreshhold) return; + unsigned int \n> > quantizedCt; const Components &set = sets_.getInterpolated(ct, \n> > &quantizedCt); - if (lastAppliedQuantizedCt_ == quantizedCt) + if \n> > (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt) \n> > return; auto config = params->block(); - \n> > config.setEnabled(true); + \n> > config.setEnabled(context.activeState.lsc.enabled); + + if \n> > (!context.activeState.lsc.enabled) + return; + setParameters(*config); \n> > copyTable(*config, set); diff --git a/src/ipa/rkisp1/algorithms/lsc.h \n> > b/src/ipa/rkisp1/algorithms/lsc.h index 43fee337931f..a3d8042130c0 \n> > 100644 --- a/src/ipa/rkisp1/algorithms/lsc.h +++ \n> > b/src/ipa/rkisp1/algorithms/lsc.h @@ -29,6 +29,9 @@ public: void \n> > prepare(IPAContext &context, const uint32_t frame, IPAFrameContext \n> > &frameContext, RkISP1Params *params) override; + void \n> > queueRequest(IPAContext &context, const uint32_t frame, + \n> > IPAFrameContext &frameContext, + const ControlList &controls) \n> > override; struct Components { std::vector r; diff --git \n> > a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index \n> > f85a130d9c23..5ec87ae4e5a3 100644 --- a/src/ipa/rkisp1/ipa_context.h \n> > +++ b/src/ipa/rkisp1/ipa_context.h @@ -55,10 +55,6 @@ struct \n> > IPASessionConfiguration { bool supported; } compress; - struct { - \n> > bool enabled; - } lsc; - struct { utils::Duration minExposureTime; \n> > utils::Duration maxExposureTime; @@ -139,6 +135,10 @@ struct \n> > IPAActiveState { double gain; double strength; } wdr; + + struct { + \n> > bool enabled; + } lsc; }; struct IPAFrameContext : public FrameContext \n> > { @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext { \n> > double strength; double gain; } wdr; + + struct { + bool update; + } \n> > lsc; }; struct IPAContext { diff --git \n> > a/src/libcamera/control_ids_core.yaml \n> > b/src/libcamera/control_ids_core.yaml index 9244b7eddb77..3b062df64bf5 \n> > 100644 --- a/src/libcamera/control_ids_core.yaml +++ \n> > b/src/libcamera/control_ids_core.yaml @@ -1352,4 +1352,7 @@ controls: \n> > description: | Enable or disable the lens shading algorithm. + This \n> > control is only available when there are valid lens shading + \n> > correction parameters available in the tuning file. + ...","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 5C3B5BDCBF\n\tfor ;\n\tThu, 8 Jan 2026 12:13:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3FCB61FA0;\n\tThu, 8 Jan 2026 13:13:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F6F8615B2\n\tfor ;\n\tThu, 8 Jan 2026 13:13:15 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:d809:713a:29af:b139])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 078AD56D;\n\tThu, 8 Jan 2026 13:12:52 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fHesBfzY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1767874373;\n\tbh=sKI07yKsGgHMMWn2ubsxrBhVSrhI62b+/VxL+JN3+T8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=fHesBfzYJsnQy0rrn8M078dZiSjQFy9JEmFjVX8kNZ19xNkICFccOHZJI3Z1C5dMX\n\tOJMwd+iXQdfwkgf1FaP1FBn5Q4nD8ZOSU0Uphe65HhsuSs5ngCZdIpMb2plahiH+DW\n\tL8aHE4CNDWLEwZgEVzqp09wNxIlACh6pI7+8asqk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-13-stefan.klug@ideasonboard.com>\n\t","Subject":"Re: [PATCH v1 12/12] ipa: rkisp1: Add LensShadingEnable control","From":"Stefan Klug ","To":"libcamera-devel@lists.libcamera.org, rui wang ","Date":"Thu, 08 Jan 2026 13:13:11 +0100","Message-ID":"<176787439188.21455.10794608635026889412@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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\" "}}]