[{"id":25035,"web_url":"https://patchwork.libcamera.org/comment/25035/","msgid":"<166371515409.18961.10228058604577514168@Monstersaurus>","date":"2022-09-20T23:05:54","subject":"Re: [libcamera-devel] [PATCH v4 22/32] ipa: rkisp1: dpf: Store\n\tper-frame information in frame context","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:50)\n> Rework the algorithm's usage of the active state, to store the value of\n> controls for the last queued request in the queueRequest() function, and\n> store a copy of the values in the corresponding frame context. The\n> latter is used in the prepare() function to populate the ISP parameters\n> with values corresponding to the right frame.\n> \n\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/rkisp1/algorithms/dpf.cpp | 22 +++++++++++-----------\n> src/ipa/rkisp1/ipa_context.cpp | 15 ++++++++++++---\n> src/ipa/rkisp1/ipa_context.h | 6 +++++-\n> 3 files changed, 28 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index 94c35c36570c..5d44480c5059 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -176,10 +176,11 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> */\n> void Dpf::queueRequest(IPAContext &context,\n> [[maybe_unused]] const uint32_t frame,\n> - [[maybe_unused]] RkISP1FrameContext &frameContext,\n> + RkISP1FrameContext &frameContext,\n> const ControlList &controls)\n> {\n> auto &dpf = context.activeState.dpf;\n> + bool update = false;\n> \n> const auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> if (denoise) {\n> @@ -188,34 +189,35 @@ void Dpf::queueRequest(IPAContext &context,\n> switch (*denoise) {\n> case controls::draft::NoiseReductionModeOff:\n> dpf.denoise = false;\n> - dpf.updateParams = true;\n> + update = true;\n> break;\n> case controls::draft::NoiseReductionModeMinimal:\n> case controls::draft::NoiseReductionModeHighQuality:\n> case controls::draft::NoiseReductionModeFast:\n> dpf.denoise = true;\n> - dpf.updateParams = true;\n> + update = true;\n> break;\n> default:\n> LOG(RkISP1Dpf, Error)\n> << \"Unsupported denoise value \"\n> << *denoise;\n> + break;\n> }\n> }\n> +\n> + frameContext.dpf.denoise = dpf.denoise;\n> + frameContext.dpf.update = update;\n> }\n> \n> /**\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> - [[maybe_unused]] RkISP1FrameContext &frameContext,\n> - rkisp1_params_cfg *params)\n> + RkISP1FrameContext &frameContext, rkisp1_params_cfg *params)\n> {\n> if (!initialized_)\n> return;\n> \n> - auto &dpf = context.activeState.dpf;\n> -\n> if (frame == 0) {\n> params->others.dpf_config = config_;\n> params->others.dpf_strength_config = strengthConfig_;\n> @@ -245,12 +247,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;\n> }\n> \n> - if (dpf.updateParams) {\n> + if (frameContext.dpf.update) {\n> params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;\n> - if (dpf.denoise)\n> + if (frameContext.dpf.denoise)\n> params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;\n> -\n> - dpf.updateParams = false;\n> }\n> }\n> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 936b77709417..b0210a978559 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -168,9 +168,6 @@ namespace libcamera::ipa::rkisp1 {\n> *\n> * \\var IPAActiveState::dpf.denoise\n> * \\brief Indicates if denoise is activated\n> - *\n> - * \\var IPAActiveState::dpf.updateParams\n> - * \\brief Indicates if ISP parameters need to be updated\n> */\n> \n> /**\n> @@ -251,6 +248,18 @@ namespace libcamera::ipa::rkisp1 {\n> * compared to the previous frame\n> */\n> \n> +/**\n> + * \\var RkISP1FrameContext::dpf\n> + * \\brief Denoise Pre-Filter parameters for this frame\n> + *\n> + * \\var RkISP1FrameContext::dpf.denoise\n> + * \\brief Indicates if denoise is activated\n> + *\n> + * \\var RkISP1FrameContext::dpf.update\n> + * \\brief Indicates if the denoise pre-filter parameters have been updated\n> + * compared to the previous frame\n> + */\n> +\n> /**\n> * \\var RkISP1FrameContext::sensor\n> * \\brief Sensor configuration that used been used for this frame\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 78edb607d039..c22e1f099c23 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -79,7 +79,6 @@ struct IPAActiveState {\n> \n> struct {\n> bool denoise;\n> - bool updateParams;\n> } dpf;\n> \n> struct {\n> @@ -113,6 +112,11 @@ struct RkISP1FrameContext : public FrameContext {\n> bool update;\n> } cproc;\n> \n> + struct {\n> + bool denoise;\n> + bool update;\n> + } dpf;\n> +\n> struct {\n> uint32_t exposure;\n> double gain;\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 52B00C0DA4\n\tfor ;\n\tTue, 20 Sep 2022 23:05:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 998E5621D0;\n\tWed, 21 Sep 2022 01:05:58 +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 22AFB61F7D\n\tfor ;\n\tWed, 21 Sep 2022 01:05:57 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A782415;\n\tWed, 21 Sep 2022 01:05:56 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663715158;\n\tbh=mftLFhtN9A8+1fOLLXWtCN+bMyZX+6Btg12hF8jkjqI=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=BbYJnJK/sn3w1MgqjidAzxxwdD1+vsrSe0KpoH8aQqy/JKoqzcNzTiE6wr+eixNzm\n\txMP+KcVuFNb+lwlCyvIsCUjQoWcN2W/zbweKlu2KM00oKfjLi/8Jr89SlTXpfkf6hj\n\t2iJTyfy0sFasZcvKtlhIEUfcw15LD3f9hkREhjI5YUACLhH42ou9Kxk+7VaW0Y9GH5\n\txaeESd6r6ruTBh9W/JQFKXrV/PZ4SAb3rzoAV2VNxixgAddX8w5dehvmUBF0m1T++n\n\t4oZH687SCyTMP7s5BUatC09rNJ3tGQaZNYK7ZWWOi6hinCwrv4Lf14951ucEbWHln6\n\tWlrzSdjBmeoaA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663715156;\n\tbh=mftLFhtN9A8+1fOLLXWtCN+bMyZX+6Btg12hF8jkjqI=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=HCVDtCj7H/96rMWbX8ZlW4Fr0jK791RmgxmBiBpe9e8djaWzGTAwodlNyvqcRybM4\n\t+XYqIEqYC0/W1elVPATSNqIrJ5Uad8yG5ZeJjZwBog6YbLpAy/YtzJmNOk7FisJfqH\n\t2jaSA3FDlzylzkhupvO9ah4nbmwlte8j/UL4oGmo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HCVDtCj7\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220908014200.28728-23-laurent.pinchart@ideasonboard.com>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-23-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 21 Sep 2022 00:05:54 +0100","Message-ID":"<166371515409.18961.10228058604577514168@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 22/32] ipa: rkisp1: dpf: Store\n\tper-frame information in frame context","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25068,"web_url":"https://patchwork.libcamera.org/comment/25068/","msgid":"<20220921204041.objpd5yplgn5xpaz@uno.localdomain>","date":"2022-09-21T20:40:41","subject":"Re: [libcamera-devel] [PATCH v4 22/32] ipa: rkisp1: dpf: Store\n\tper-frame information in frame context","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Thu, Sep 08, 2022 at 04:41:50AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Rework the algorithm's usage of the active state, to store the value of\n> controls for the last queued request in the queueRequest() function, and\n> store a copy of the values in the corresponding frame context. The\n> latter is used in the prepare() function to populate the ISP parameters\n> with values corresponding to the right frame.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/rkisp1/algorithms/dpf.cpp | 22 +++++++++++-----------\n> src/ipa/rkisp1/ipa_context.cpp | 15 ++++++++++++---\n> src/ipa/rkisp1/ipa_context.h | 6 +++++-\n> 3 files changed, 28 insertions(+), 15 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index 94c35c36570c..5d44480c5059 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -176,10 +176,11 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> */\n> void Dpf::queueRequest(IPAContext &context,\n> \t\t [[maybe_unused]] const uint32_t frame,\n> -\t\t [[maybe_unused]] RkISP1FrameContext &frameContext,\n> +\t\t RkISP1FrameContext &frameContext,\n> \t\t const ControlList &controls)\n> {\n> \tauto &dpf = context.activeState.dpf;\n> +\tbool update = false;\n>\n> \tconst auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> \tif (denoise) {\n> @@ -188,34 +189,35 @@ void Dpf::queueRequest(IPAContext &context,\n> \t\tswitch (*denoise) {\n> \t\tcase controls::draft::NoiseReductionModeOff:\n> \t\t\tdpf.denoise = false;\n> -\t\t\tdpf.updateParams = true;\n> +\t\t\tupdate = true;\n\nDo we update unconditionally or should we check if the state has\nchanged since last time and the application is not re-sending the same\ncontrol. As this is not compliant with the API assumptions that\ncontrols should be sent only when they change, I think it's fair to\nassume we need to update ?\n\nIf that's the case\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n> \t\t\tbreak;\n> \t\tcase controls::draft::NoiseReductionModeMinimal:\n> \t\tcase controls::draft::NoiseReductionModeHighQuality:\n> \t\tcase controls::draft::NoiseReductionModeFast:\n> \t\t\tdpf.denoise = true;\n> -\t\t\tdpf.updateParams = true;\n> +\t\t\tupdate = true;\n> \t\t\tbreak;\n> \t\tdefault:\n> \t\t\tLOG(RkISP1Dpf, Error)\n> \t\t\t\t<< \"Unsupported denoise value \"\n> \t\t\t\t<< *denoise;\n> +\t\t\tbreak;\n> \t\t}\n> \t}\n> +\n> +\tframeContext.dpf.denoise = dpf.denoise;\n> +\tframeContext.dpf.update = update;\n> }\n>\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> -\t\t [[maybe_unused]] RkISP1FrameContext &frameContext,\n> -\t\t rkisp1_params_cfg *params)\n> +\t\t RkISP1FrameContext &frameContext, rkisp1_params_cfg *params)\n> {\n> \tif (!initialized_)\n> \t\treturn;\n>\n> -\tauto &dpf = context.activeState.dpf;\n> -\n> \tif (frame == 0) {\n> \t\tparams->others.dpf_config = config_;\n> \t\tparams->others.dpf_strength_config = strengthConfig_;\n> @@ -245,12 +247,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> \t\t\t\t\t RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;\n> \t}\n>\n> -\tif (dpf.updateParams) {\n> +\tif (frameContext.dpf.update) {\n> \t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;\n> -\t\tif (dpf.denoise)\n> +\t\tif (frameContext.dpf.denoise)\n> \t\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;\n> -\n> -\t\tdpf.updateParams = false;\n> \t}\n> }\n>\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 936b77709417..b0210a978559 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -168,9 +168,6 @@ namespace libcamera::ipa::rkisp1 {\n> *\n> * \\var IPAActiveState::dpf.denoise\n> * \\brief Indicates if denoise is activated\n> - *\n> - * \\var IPAActiveState::dpf.updateParams\n> - * \\brief Indicates if ISP parameters need to be updated\n> */\n>\n> /**\n> @@ -251,6 +248,18 @@ namespace libcamera::ipa::rkisp1 {\n> * compared to the previous frame\n> */\n>\n> +/**\n> + * \\var RkISP1FrameContext::dpf\n> + * \\brief Denoise Pre-Filter parameters for this frame\n> + *\n> + * \\var RkISP1FrameContext::dpf.denoise\n> + * \\brief Indicates if denoise is activated\n> + *\n> + * \\var RkISP1FrameContext::dpf.update\n> + * \\brief Indicates if the denoise pre-filter parameters have been updated\n> + * compared to the previous frame\n> + */\n> +\n> /**\n> * \\var RkISP1FrameContext::sensor\n> * \\brief Sensor configuration that used been used for this frame\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 78edb607d039..c22e1f099c23 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -79,7 +79,6 @@ struct IPAActiveState {\n>\n> \tstruct {\n> \t\tbool denoise;\n> -\t\tbool updateParams;\n> \t} dpf;\n>\n> \tstruct {\n> @@ -113,6 +112,11 @@ struct RkISP1FrameContext : public FrameContext {\n> \t\tbool update;\n> \t} cproc;\n>\n> +\tstruct {\n> +\t\tbool denoise;\n> +\t\tbool update;\n> +\t} dpf;\n> +\n> \tstruct {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> --\n> Regards,\n>\n> Laurent Pinchart\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 90E9CC3272\n\tfor ;\n\tWed, 21 Sep 2022 20:40:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDD7E621F8;\n\tWed, 21 Sep 2022 22:40:44 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::225])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60C6B600AA\n\tfor ;\n\tWed, 21 Sep 2022 22:40:43 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id C990C1C0005;\n\tWed, 21 Sep 2022 20:40:42 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663792845;\n\tbh=c0ZvZ2+m1MeWU3yr8UrQR1icRr8PptxCOj4Ba9Sip0Q=;\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=IuqHOX7Z5fmo2RbNSjEEXZkxblayNjAJhm9yzYFiJlynB7qJQt3xAyfZWaJ8nuoIp\n\txuXn2NuZLRtBLCaWQiDh3nVc8rT82oDFdO5VCXw4MZE9FJDO1isbGFxLfqFrlPyewo\n\t7nblhxDE9hPD2844A+dTRHu6hB6gc2CYwnYh2GRDtEDj9rH5qJDunWhk7t1yn6atd9\n\tj0Cy6TA4Yz59joMBTYSazjFWIo5KsA47O2cd3govIs4Q0NAVz7gEdYXpLEaxSyC3HL\n\tYuhbTlIPaYM9chiof4GM71fq4FHBFMnVHcYyaJRYxe6eQ5rQw69GsXyUvMdbh02pUp\n\tM+LzACCZsfdrA==","Date":"Wed, 21 Sep 2022 22:40:41 +0200","To":"Laurent Pinchart ","Message-ID":"<20220921204041.objpd5yplgn5xpaz@uno.localdomain>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-23-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220908014200.28728-23-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 22/32] ipa: rkisp1: dpf: Store\n\tper-frame information in frame context","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":25092,"web_url":"https://patchwork.libcamera.org/comment/25092/","msgid":"","date":"2022-09-22T20:16:00","subject":"Re: [libcamera-devel] [PATCH v4 22/32] ipa: rkisp1: dpf: Store\n\tper-frame information in frame context","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Sep 21, 2022 at 10:40:41PM +0200, Jacopo Mondi wrote:\n> On Thu, Sep 08, 2022 at 04:41:50AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Rework the algorithm's usage of the active state, to store the value of\n> > controls for the last queued request in the queueRequest() function, and\n> > store a copy of the values in the corresponding frame context. The\n> > latter is used in the prepare() function to populate the ISP parameters\n> > with values corresponding to the right frame.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/rkisp1/algorithms/dpf.cpp | 22 +++++++++++-----------\n> > src/ipa/rkisp1/ipa_context.cpp | 15 ++++++++++++---\n> > src/ipa/rkisp1/ipa_context.h | 6 +++++-\n> > 3 files changed, 28 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > index 94c35c36570c..5d44480c5059 100644\n> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > @@ -176,10 +176,11 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> > */\n> > void Dpf::queueRequest(IPAContext &context,\n> > \t\t [[maybe_unused]] const uint32_t frame,\n> > -\t\t [[maybe_unused]] RkISP1FrameContext &frameContext,\n> > +\t\t RkISP1FrameContext &frameContext,\n> > \t\t const ControlList &controls)\n> > {\n> > \tauto &dpf = context.activeState.dpf;\n> > +\tbool update = false;\n> >\n> > \tconst auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> > \tif (denoise) {\n> > @@ -188,34 +189,35 @@ void Dpf::queueRequest(IPAContext &context,\n> > \t\tswitch (*denoise) {\n> > \t\tcase controls::draft::NoiseReductionModeOff:\n> > \t\t\tdpf.denoise = false;\n> > -\t\t\tdpf.updateParams = true;\n> > +\t\t\tupdate = true;\n> \n> Do we update unconditionally or should we check if the state has\n> changed since last time and the application is not re-sending the same\n> control. As this is not compliant with the API assumptions that\n> controls should be sent only when they change, I think it's fair to\n> assume we need to update ?\n\nNo, you're right, I think it's useful to check if the value has changed\nan only update when it does. It's just a few CPU cycles here, and will\nsave way more cycles in the kernel.\n\n> If that's the case\n> Reviewed-by: Jacopo Mondi \n> \n> > \t\t\tbreak;\n> > \t\tcase controls::draft::NoiseReductionModeMinimal:\n> > \t\tcase controls::draft::NoiseReductionModeHighQuality:\n> > \t\tcase controls::draft::NoiseReductionModeFast:\n> > \t\t\tdpf.denoise = true;\n> > -\t\t\tdpf.updateParams = true;\n> > +\t\t\tupdate = true;\n> > \t\t\tbreak;\n> > \t\tdefault:\n> > \t\t\tLOG(RkISP1Dpf, Error)\n> > \t\t\t\t<< \"Unsupported denoise value \"\n> > \t\t\t\t<< *denoise;\n> > +\t\t\tbreak;\n> > \t\t}\n> > \t}\n> > +\n> > +\tframeContext.dpf.denoise = dpf.denoise;\n> > +\tframeContext.dpf.update = update;\n> > }\n> >\n> > /**\n> > * \\copydoc libcamera::ipa::Algorithm::prepare\n> > */\n> > void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> > -\t\t [[maybe_unused]] RkISP1FrameContext &frameContext,\n> > -\t\t rkisp1_params_cfg *params)\n> > +\t\t RkISP1FrameContext &frameContext, rkisp1_params_cfg *params)\n> > {\n> > \tif (!initialized_)\n> > \t\treturn;\n> >\n> > -\tauto &dpf = context.activeState.dpf;\n> > -\n> > \tif (frame == 0) {\n> > \t\tparams->others.dpf_config = config_;\n> > \t\tparams->others.dpf_strength_config = strengthConfig_;\n> > @@ -245,12 +247,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> > \t\t\t\t\t RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;\n> > \t}\n> >\n> > -\tif (dpf.updateParams) {\n> > +\tif (frameContext.dpf.update) {\n> > \t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;\n> > -\t\tif (dpf.denoise)\n> > +\t\tif (frameContext.dpf.denoise)\n> > \t\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;\n> > -\n> > -\t\tdpf.updateParams = false;\n> > \t}\n> > }\n> >\n> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > index 936b77709417..b0210a978559 100644\n> > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > @@ -168,9 +168,6 @@ namespace libcamera::ipa::rkisp1 {\n> > *\n> > * \\var IPAActiveState::dpf.denoise\n> > * \\brief Indicates if denoise is activated\n> > - *\n> > - * \\var IPAActiveState::dpf.updateParams\n> > - * \\brief Indicates if ISP parameters need to be updated\n> > */\n> >\n> > /**\n> > @@ -251,6 +248,18 @@ namespace libcamera::ipa::rkisp1 {\n> > * compared to the previous frame\n> > */\n> >\n> > +/**\n> > + * \\var RkISP1FrameContext::dpf\n> > + * \\brief Denoise Pre-Filter parameters for this frame\n> > + *\n> > + * \\var RkISP1FrameContext::dpf.denoise\n> > + * \\brief Indicates if denoise is activated\n> > + *\n> > + * \\var RkISP1FrameContext::dpf.update\n> > + * \\brief Indicates if the denoise pre-filter parameters have been updated\n> > + * compared to the previous frame\n> > + */\n> > +\n> > /**\n> > * \\var RkISP1FrameContext::sensor\n> > * \\brief Sensor configuration that used been used for this frame\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 78edb607d039..c22e1f099c23 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -79,7 +79,6 @@ struct IPAActiveState {\n> >\n> > \tstruct {\n> > \t\tbool denoise;\n> > -\t\tbool updateParams;\n> > \t} dpf;\n> >\n> > \tstruct {\n> > @@ -113,6 +112,11 @@ struct RkISP1FrameContext : public FrameContext {\n> > \t\tbool update;\n> > \t} cproc;\n> >\n> > +\tstruct {\n> > +\t\tbool denoise;\n> > +\t\tbool update;\n> > +\t} dpf;\n> > +\n> > \tstruct {\n> > \t\tuint32_t exposure;\n> > \t\tdouble gain;","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 4BB34C0DA4\n\tfor ;\n\tThu, 22 Sep 2022 20:16:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC1E062218;\n\tThu, 22 Sep 2022 22:16:17 +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 B2AB96219A\n\tfor ;\n\tThu, 22 Sep 2022 22:16:16 +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 DA64B6BE;\n\tThu, 22 Sep 2022 22:16:15 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663877777;\n\tbh=pXrg5C1FZKAg9+BUj92ZEovV+zxKT99+cnAe+GogLS4=;\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=SrPNvGJyczg9SPKdEbWMuCJLphNuJKENmkH4Ij+morlR4FBzC89444mri0d4G92gU\n\t0ANZ2QY4AMmqHZZv2I9tw9C58btO4/dYCkqhvdXpc8XjipgRAgGRUtEjflc5XNi2I7\n\trqSAxGpbECehRvzMEn9DQ0UjWac3c9b+KIkUF70ExCd6cCDHFexI33z1rAE63bRYVL\n\tS7nb97Yh+o4k3hbditXv5XPp5CCLGhEvt61TVh/RvCI2V4Abf88BTY0c8dSGe2kHvW\n\t9Ldk4jySs/X9OaiNd+Q/YDqtcTh1M6svcSSJmdhVSWcAyNgz97DM7gw5WiLFoQpVrk\n\t34r02783KmX5w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663877776;\n\tbh=pXrg5C1FZKAg9+BUj92ZEovV+zxKT99+cnAe+GogLS4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SKnZeCfXp5+z4Pag4ODLmnLBrLHW7oMDxVJ53h1yYWst2Q6cc/Nwfj38fRUFI8WnL\n\tSeAOpJCFOEHHJdc+ROHLTkTORi6JK+qzvJnbTeKGiiZZx9f6DrcE6sk6cb4172Mv21\n\tMk38wXm20IxtVp+LeCyT4yBDlo4Mnh7WLqAOmxeE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SKnZeCfX\"; dkim-atps=neutral","Date":"Thu, 22 Sep 2022 23:16:00 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-23-laurent.pinchart@ideasonboard.com>\n\t<20220921204041.objpd5yplgn5xpaz@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220921204041.objpd5yplgn5xpaz@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v4 22/32] ipa: rkisp1: dpf: Store\n\tper-frame information in frame context","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\" "}}]