[{"id":36937,"web_url":"https://patchwork.libcamera.org/comment/36937/","msgid":"<176364739095.1127434.7880224954107702978@ping.linuxembedded.co.uk>","date":"2025-11-20T14:03:10","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Isaac Scott (2025-11-20 13:41:01)\n> It is beneficial to have the option during development to disable and\n> enable algorithms via the tuning file without having to delete their\n> entries.\n> \n> Add support for an optional \"enabled\" parameter to accomplish this.\n> \n> Usage example:\n> version: 1\n> algorithms:\n> - Agc:\n> enabled: true\n> - Awb:\n> enabled: false\n> \n> This will enable AGC, and disable AWB. If the enabled flag is not\n> present, the algorithm will be enabled.\n> \n> Signed-off-by: Isaac Scott \n> \n> ---\n> \n> v1 of this patch can be found here:\n> \n> https://patchwork.libcamera.org/patch/23299/\n> \n> Changelog:\n> \n> v1 -> v2:\n> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> ---\n> src/ipa/libipa/module.h | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n> \n> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> index 0fb51916f..f8c4bb664 100644\n> --- a/src/ipa/libipa/module.h\n> +++ b/src/ipa/libipa/module.h\n> @@ -55,6 +55,15 @@ public:\n> return -EINVAL;\n> }\n> \n> + if (algo.contains(\"enabled\")) {\n> + if (algo[\"enabled\"].get() == false) {\n\n\nCan we simplify this to a single if ?\n\nI think something like this might work, could you test/check it please?\n\n\t\t\tif (algo[\"enabled\"].get(true) == false) {\n\t\t\t\tLOG() << ...;\n\t\t\t\tcontinue;\n\n\t\t\t}\n\n--\nKieran\n\n> + LOG(IPAModuleAlgo, Debug)\n> + << \"Algorithm \" << i\n> + << \" is disabled via tuning file\";\n> + continue;\n> + }\n> + }\n> +\n> int ret = createAlgorithm(context, algo);\n> if (ret) {\n> algorithms_.clear();\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 BC55EC3333\n\tfor ;\n\tThu, 20 Nov 2025 14:03:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DEE82609D8;\n\tThu, 20 Nov 2025 15:03:15 +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 32D086069A\n\tfor ;\n\tThu, 20 Nov 2025 15:03:14 +0100 (CET)","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 88906F09;\n\tThu, 20 Nov 2025 15:01:08 +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=\"cq+hEFzz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763647268;\n\tbh=hhFC31SMhAGB28uA2qVCSZnMAZhWurb+teP8MT66Dx4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=cq+hEFzzpYZDeDyqH7r4JG+I7Bg1P6A2pbPQro4dSGlQr1AjNbTz2ob95iTljs598\n\tVqaWgeM0j2UnHiUdbhZJlZC/T78b5tqorzEHZ52PLN3UsqTfDOxqobjH6GeM4linPM\n\tMIsOdQb8gMc9FqSFRtwyVLF20lnYxcDxg/6dEh7s=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","From":"Kieran Bingham ","Cc":"Isaac Scott ","To":"Isaac Scott ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 20 Nov 2025 14:03:10 +0000","Message-ID":"<176364739095.1127434.7880224954107702978@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":36938,"web_url":"https://patchwork.libcamera.org/comment/36938/","msgid":"<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>","date":"2025-11-20T14:04:51","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n> It is beneficial to have the option during development to disable and\n> enable algorithms via the tuning file without having to delete their\n> entries.\n> \n> Add support for an optional \"enabled\" parameter to accomplish this.\n> \n> Usage example:\n> version: 1\n> algorithms:\n> - Agc:\n> enabled: true\n> - Awb:\n> enabled: false\n> \n> This will enable AGC, and disable AWB. If the enabled flag is not\n> present, the algorithm will be enabled.\n> \n> Signed-off-by: Isaac Scott \n> \n> ---\n> \n> v1 of this patch can be found here:\n> \n> https://patchwork.libcamera.org/patch/23299/\n> \n> Changelog:\n> \n> v1 -> v2:\n> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> ---\n> src/ipa/libipa/module.h | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n> \n> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> index 0fb51916f..f8c4bb664 100644\n> --- a/src/ipa/libipa/module.h\n> +++ b/src/ipa/libipa/module.h\n> @@ -55,6 +55,15 @@ public:\n> \t\t\t\treturn -EINVAL;\n> \t\t\t}\n> \n> +\t\t\tif (algo.contains(\"enabled\")) {\n> +\t\t\t\tif (algo[\"enabled\"].get() == false) {\n\nThe operator[] returns a special empty object if the key is not present,\nso I think the above can be simplified as follows:\n\nif (!algo[\"enabled\"].get(true)) {\n ...\n continue;\n}\n\n\nIn any case I'm a bit concerned about the `createAlgorithm()` function:\n\n const auto &[name, algoData] = *data.asDict().begin();\n\nthis seems very fragile, e.g.\n\n - enabled: true\n Awb:\n\nwouldn't work.\n\n\nRegards,\nBarnabás Pőcze\n\n> +\t\t\t\t\tLOG(IPAModuleAlgo, Debug)\n> +\t\t\t\t\t\t<< \"Algorithm \" << i\n> +\t\t\t\t\t\t<< \" is disabled via tuning file\";\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> \t\t\tint ret = createAlgorithm(context, algo);\n> \t\t\tif (ret) {\n> \t\t\t\talgorithms_.clear();","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 0413EC3333\n\tfor ;\n\tThu, 20 Nov 2025 14:04:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3B9960A81;\n\tThu, 20 Nov 2025 15:04:58 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D009C6069A\n\tfor ;\n\tThu, 20 Nov 2025 15:04:56 +0100 (CET)","from [192.168.33.37] (185.221.143.100.nat.pool.zt.hu\n\t[185.221.143.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3553C9CA;\n\tThu, 20 Nov 2025 15:02:50 +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=\"R2CjEvVQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763647371;\n\tbh=Fl1N+o/acfA71pD5kWuQX1XNtGkOTOo6wnCznP7ics4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=R2CjEvVQuYAcrgozrzAz7r38GLDxlvM+nd1hAA20hrNZAOiLoAEspfXO5XKApT51g\n\t4k0vtM7QMwc4JxXz+pqRgUuZyaDH1tsnDpD7YY6mInIU1u0cKarC6zM3mHhMsudyCd\n\t8e4XXe/518shJNpe/9/cnlqUSieLaP5Low7PAm/k=","Message-ID":"<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>","Date":"Thu, 20 Nov 2025 15:04:51 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","To":"Isaac Scott ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251120134101.72892-1-isaac.scott@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":36939,"web_url":"https://patchwork.libcamera.org/comment/36939/","msgid":"<20251120140810.GL10711@pendragon.ideasonboard.com>","date":"2025-11-20T14:08:10","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Isaac,\n\nOn Thu, Nov 20, 2025 at 01:41:01PM +0000, Isaac Scott wrote:\n> It is beneficial to have the option during development to disable and\n> enable algorithms via the tuning file without having to delete their\n> entries.\n> \n> Add support for an optional \"enabled\" parameter to accomplish this.\n> \n> Usage example:\n> version: 1\n> algorithms:\n> - Agc:\n> enabled: true\n> - Awb:\n> enabled: false\n> \n> This will enable AGC, and disable AWB. If the enabled flag is not\n> present, the algorithm will be enabled.\n> \n> Signed-off-by: Isaac Scott \n> \n> ---\n> \n> v1 of this patch can be found here:\n> \n> https://patchwork.libcamera.org/patch/23299/\n> \n> Changelog:\n> \n> v1 -> v2:\n> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n\nYou seem to have missed the comment I left in the v1 review about\ndocumenting the property.\n\n> ---\n> src/ipa/libipa/module.h | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n> \n> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> index 0fb51916f..f8c4bb664 100644\n> --- a/src/ipa/libipa/module.h\n> +++ b/src/ipa/libipa/module.h\n> @@ -55,6 +55,15 @@ public:\n> \t\t\t\treturn -EINVAL;\n> \t\t\t}\n> \n> +\t\t\tif (algo.contains(\"enabled\")) {\n> +\t\t\t\tif (algo[\"enabled\"].get() == false) {\n> +\t\t\t\t\tLOG(IPAModuleAlgo, Debug)\n> +\t\t\t\t\t\t<< \"Algorithm \" << i\n> +\t\t\t\t\t\t<< \" is disabled via tuning file\";\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> \t\t\tint ret = createAlgorithm(context, algo);\n> \t\t\tif (ret) {\n> \t\t\t\talgorithms_.clear();","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 C271EC3335\n\tfor ;\n\tThu, 20 Nov 2025 14:08:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED90E60AA0;\n\tThu, 20 Nov 2025 15:08:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24DB3609D8\n\tfor ;\n\tThu, 20 Nov 2025 15:08:34 +0100 (CET)","from pendragon.ideasonboard.com (fs276ed015.tkyc509.ap.nuro.jp\n\t[39.110.208.21])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id AF759B5;\n\tThu, 20 Nov 2025 15:06:27 +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=\"ZCDefKhR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763647588;\n\tbh=p6QEHQADjwqcjwDDIaVYNVzhHTg700siriCksOPPw40=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZCDefKhRCvmdlOVgcynHhc74JKvYaHXo7mjyFkwIT0thpo77kgPpaj8Ho97D1eeY/\n\tZVLwUBJlAzb5lS/AwNbNr4OamRz+co3tV9vSt0+xDeByHeERqDp5W/YwHkRbQ4v5kJ\n\tat4rAfHOtFylugswQMpVei6K4NC2FlSuKUroYCDY=","Date":"Thu, 20 Nov 2025 23:08:10 +0900","From":"Laurent Pinchart ","To":"Isaac Scott ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","Message-ID":"<20251120140810.GL10711@pendragon.ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20251120134101.72892-1-isaac.scott@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":36940,"web_url":"https://patchwork.libcamera.org/comment/36940/","msgid":"<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>","date":"2025-11-20T14:18:56","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Barnabás,\n\nThanks for the review!\n\nQuoting Barnabás Pőcze (2025-11-20 14:04:51)\n> Hi\n> \n> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n> > It is beneficial to have the option during development to disable and\n> > enable algorithms via the tuning file without having to delete their\n> > entries.\n> > \n> > Add support for an optional \"enabled\" parameter to accomplish this.\n> > \n> > Usage example:\n> > version: 1\n> > algorithms:\n> > - Agc:\n> > enabled: true\n> > - Awb:\n> > enabled: false\n> > \n> > This will enable AGC, and disable AWB. If the enabled flag is not\n> > present, the algorithm will be enabled.\n> > \n> > Signed-off-by: Isaac Scott \n> > \n> > ---\n> > \n> > v1 of this patch can be found here:\n> > \n> > https://patchwork.libcamera.org/patch/23299/\n> > \n> > Changelog:\n> > \n> > v1 -> v2:\n> > - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> > - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> > ---\n> > src/ipa/libipa/module.h | 9 +++++++++\n> > 1 file changed, 9 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> > index 0fb51916f..f8c4bb664 100644\n> > --- a/src/ipa/libipa/module.h\n> > +++ b/src/ipa/libipa/module.h\n> > @@ -55,6 +55,15 @@ public:\n> > return -EINVAL;\n> > }\n> > \n> > + if (algo.contains(\"enabled\")) {\n> > + if (algo[\"enabled\"].get() == false) {\n> \n> The operator[] returns a special empty object if the key is not present,\n> so I think the above can be simplified as follows:\n\nAh, interesting, that makes it much cleaner!\n\n> \n> if (!algo[\"enabled\"].get(true)) {\n> ...\n> continue;\n> }\n> \n> \n> In any case I'm a bit concerned about the `createAlgorithm()` function:\n> \n> const auto &[name, algoData] = *data.asDict().begin();\n> \n> this seems very fragile, e.g.\n> \n> - enabled: true\n> Awb:\n> \n> wouldn't work.\n\nIn that case, wouldn't that be undesirable anyway? Shouldn't algorithm\nnames come first?\n\nBest wishes,\nIsaac\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> > + LOG(IPAModuleAlgo, Debug)\n> > + << \"Algorithm \" << i\n> > + << \" is disabled via tuning file\";\n> > + continue;\n> > + }\n> > + }\n> > +\n> > int ret = createAlgorithm(context, algo);\n> > if (ret) {\n> > algorithms_.clear();\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 4EA01C3333\n\tfor ;\n\tThu, 20 Nov 2025 14:19:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B6906069A;\n\tThu, 20 Nov 2025 15:19:01 +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 3A1556069A\n\tfor ;\n\tThu, 20 Nov 2025 15:19:00 +0100 (CET)","from thinkpad.ideasonboard.com\n\t(cpc90716-aztw32-2-0-cust408.18-1.cable.virginm.net [86.26.101.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B8EC1B5;\n\tThu, 20 Nov 2025 15:16:54 +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=\"ZjFLmEoq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763648214;\n\tbh=bOmRCEHAXK6AdgdwdY21pZ5RAkPJ1Ykp9DpO70stHIE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=ZjFLmEoqcjTX07CiOy/SM00itp6canlsmdqBb7fN/bPgiWXQDkFEhT86cocGzHPFW\n\tLUqgNs/fM73AYd1xxf9OZtvLWBiDfSbRp/NlFfDZCZup8v+hhUvL2lLhClB5JQcqCa\n\trXCXHprziPjxPqfgBcRquEbWWRlUVNZ0sXas2f6Q=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","From":"Isaac Scott ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 20 Nov 2025 14:18:56 +0000","Message-ID":"<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>","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":36941,"web_url":"https://patchwork.libcamera.org/comment/36941/","msgid":"<176364839773.75776.14088718772286763906@isaac-ThinkPad-T16-Gen-2>","date":"2025-11-20T14:19:57","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the review!\n\nQuoting Laurent Pinchart (2025-11-20 14:08:10)\n> Hi Isaac,\n> \n> On Thu, Nov 20, 2025 at 01:41:01PM +0000, Isaac Scott wrote:\n> > It is beneficial to have the option during development to disable and\n> > enable algorithms via the tuning file without having to delete their\n> > entries.\n> > \n> > Add support for an optional \"enabled\" parameter to accomplish this.\n> > \n> > Usage example:\n> > version: 1\n> > algorithms:\n> > - Agc:\n> > enabled: true\n> > - Awb:\n> > enabled: false\n> > \n> > This will enable AGC, and disable AWB. If the enabled flag is not\n> > present, the algorithm will be enabled.\n> > \n> > Signed-off-by: Isaac Scott \n> > \n> > ---\n> > \n> > v1 of this patch can be found here:\n> > \n> > https://patchwork.libcamera.org/patch/23299/\n> > \n> > Changelog:\n> > \n> > v1 -> v2:\n> > - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> > - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> \n> You seem to have missed the comment I left in the v1 review about\n> documenting the property.\n\nAh - you're right, apologies!\n\nI'll make sure to add it in v3.\n\nBest wishes,\nIsaac\n\n> \n> > ---\n> > src/ipa/libipa/module.h | 9 +++++++++\n> > 1 file changed, 9 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> > index 0fb51916f..f8c4bb664 100644\n> > --- a/src/ipa/libipa/module.h\n> > +++ b/src/ipa/libipa/module.h\n> > @@ -55,6 +55,15 @@ public:\n> > return -EINVAL;\n> > }\n> > \n> > + if (algo.contains(\"enabled\")) {\n> > + if (algo[\"enabled\"].get() == false) {\n> > + LOG(IPAModuleAlgo, Debug)\n> > + << \"Algorithm \" << i\n> > + << \" is disabled via tuning file\";\n> > + continue;\n> > + }\n> > + }\n> > +\n> > int ret = createAlgorithm(context, algo);\n> > if (ret) {\n> > algorithms_.clear();\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 9669AC3333\n\tfor ;\n\tThu, 20 Nov 2025 14:20:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B4B560A81;\n\tThu, 20 Nov 2025 15:20:02 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 247706069A\n\tfor ;\n\tThu, 20 Nov 2025 15:20:01 +0100 (CET)","from thinkpad.ideasonboard.com\n\t(cpc90716-aztw32-2-0-cust408.18-1.cable.virginm.net [86.26.101.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4AC2B5;\n\tThu, 20 Nov 2025 15:17:55 +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=\"ZKbpef5V\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763648276;\n\tbh=uzEVFFSqe8p2iBBQsDd5Acgn6KuO9abG/xILm6HSxDI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ZKbpef5VS11/1DRcjDjE5ps7gjv7U1WRpTQeydXcCFvZlYw/PbyVyAo72p/z++FPS\n\tPv/RaAcf0wdj6/6q4qdhoel4zuEoioC1e3A/lMryJ7Id7xVTGPfzAYPwNr8tGd/JRC\n\tIhZqGaHVzMHNnmSWXePhb/NGWh8+henUdjjKJpkc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251120140810.GL10711@pendragon.ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<20251120140810.GL10711@pendragon.ideasonboard.com>","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","From":"Isaac Scott ","Cc":"libcamera-devel@lists.libcamera.org","To":"Laurent Pinchart ","Date":"Thu, 20 Nov 2025 14:19:57 +0000","Message-ID":"<176364839773.75776.14088718772286763906@isaac-ThinkPad-T16-Gen-2>","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":36942,"web_url":"https://patchwork.libcamera.org/comment/36942/","msgid":"<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>","date":"2025-11-20T14:20:26","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 11. 20. 15:18 keltezéssel, Isaac Scott írta:\n> Hi Barnabás,\n> \n> Thanks for the review!\n> \n> Quoting Barnabás Pőcze (2025-11-20 14:04:51)\n>> Hi\n>>\n>> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n>>> It is beneficial to have the option during development to disable and\n>>> enable algorithms via the tuning file without having to delete their\n>>> entries.\n>>>\n>>> Add support for an optional \"enabled\" parameter to accomplish this.\n>>>\n>>> Usage example:\n>>> version: 1\n>>> algorithms:\n>>> - Agc:\n>>> enabled: true\n>>> - Awb:\n>>> enabled: false\n>>>\n>>> This will enable AGC, and disable AWB. If the enabled flag is not\n>>> present, the algorithm will be enabled.\n>>>\n>>> Signed-off-by: Isaac Scott \n>>>\n>>> ---\n>>>\n>>> v1 of this patch can be found here:\n>>>\n>>> https://patchwork.libcamera.org/patch/23299/\n>>>\n>>> Changelog:\n>>>\n>>> v1 -> v2:\n>>> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n>>> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n>>> ---\n>>> src/ipa/libipa/module.h | 9 +++++++++\n>>> 1 file changed, 9 insertions(+)\n>>>\n>>> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n>>> index 0fb51916f..f8c4bb664 100644\n>>> --- a/src/ipa/libipa/module.h\n>>> +++ b/src/ipa/libipa/module.h\n>>> @@ -55,6 +55,15 @@ public:\n>>> return -EINVAL;\n>>> }\n>>> \n>>> + if (algo.contains(\"enabled\")) {\n>>> + if (algo[\"enabled\"].get() == false) {\n>>\n>> The operator[] returns a special empty object if the key is not present,\n>> so I think the above can be simplified as follows:\n> \n> Ah, interesting, that makes it much cleaner!\n> \n>>\n>> if (!algo[\"enabled\"].get(true)) {\n>> ...\n>> continue;\n>> }\n>>\n>>\n>> In any case I'm a bit concerned about the `createAlgorithm()` function:\n>>\n>> const auto &[name, algoData] = *data.asDict().begin();\n>>\n>> this seems very fragile, e.g.\n>>\n>> - enabled: true\n>> Awb:\n>>\n>> wouldn't work.\n> \n> In that case, wouldn't that be undesirable anyway? Shouldn't algorithm\n> names come first?\n\nI'm not sure. I think generally the expectation is that a json/yaml dictionary\nis unordered, i.e. the order does not matter.\n\n\n> \n> Best wishes,\n> Isaac\n> \n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>> + LOG(IPAModuleAlgo, Debug)\n>>> + << \"Algorithm \" << i\n>>> + << \" is disabled via tuning file\";\n>>> + continue;\n>>> + }\n>>> + }\n>>> +\n>>> int ret = createAlgorithm(context, algo);\n>>> if (ret) {\n>>> algorithms_.clear();\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 92E03C3333\n\tfor ;\n\tThu, 20 Nov 2025 14:20:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 43539609DE;\n\tThu, 20 Nov 2025 15:20:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 21A506069A\n\tfor ;\n\tThu, 20 Nov 2025 15:20:30 +0100 (CET)","from [192.168.33.37] (185.221.143.100.nat.pool.zt.hu\n\t[185.221.143.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D63EEB5;\n\tThu, 20 Nov 2025 15:18:24 +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=\"LRt0gQN0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763648305;\n\tbh=PLjDsucx9fb/1LOd5WZj0nenG/yVl0S3g+udFsd9np4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=LRt0gQN0rT1yMp/e8o1IU88fIbBQUUqDOCjf4D7JKEm7hfuaoovUQJ1PrDiNxviHJ\n\tdqYxwBiDFMVReomac4z4JT/05auQF1eHBX3F4BAJxwacumD4NnlPL3ig457WIdc7NU\n\t6Z2WAFtXISnX8ddWT08356vjhPOQh1+ckl3AMPU0=","Message-ID":"<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>","Date":"Thu, 20 Nov 2025 15:20:26 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","To":"Isaac Scott ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>\n\t<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>","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":36943,"web_url":"https://patchwork.libcamera.org/comment/36943/","msgid":"<176364845194.75776.18120611917162525704@isaac-ThinkPad-T16-Gen-2>","date":"2025-11-20T14:20:51","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Kieran,\n\nThanks for the review!\n\nQuoting Kieran Bingham (2025-11-20 14:03:10)\n> Quoting Isaac Scott (2025-11-20 13:41:01)\n> > It is beneficial to have the option during development to disable and\n> > enable algorithms via the tuning file without having to delete their\n> > entries.\n> > \n> > Add support for an optional \"enabled\" parameter to accomplish this.\n> > \n> > Usage example:\n> > version: 1\n> > algorithms:\n> > - Agc:\n> > enabled: true\n> > - Awb:\n> > enabled: false\n> > \n> > This will enable AGC, and disable AWB. If the enabled flag is not\n> > present, the algorithm will be enabled.\n> > \n> > Signed-off-by: Isaac Scott \n> > \n> > ---\n> > \n> > v1 of this patch can be found here:\n> > \n> > https://patchwork.libcamera.org/patch/23299/\n> > \n> > Changelog:\n> > \n> > v1 -> v2:\n> > - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> > - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> > ---\n> > src/ipa/libipa/module.h | 9 +++++++++\n> > 1 file changed, 9 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> > index 0fb51916f..f8c4bb664 100644\n> > --- a/src/ipa/libipa/module.h\n> > +++ b/src/ipa/libipa/module.h\n> > @@ -55,6 +55,15 @@ public:\n> > return -EINVAL;\n> > }\n> > \n> > + if (algo.contains(\"enabled\")) {\n> > + if (algo[\"enabled\"].get() == false) {\n> \n> \n> Can we simplify this to a single if ?\n> \n> I think something like this might work, could you test/check it please?\n> \n> if (algo[\"enabled\"].get(true) == false) {\n> LOG() << ...;\n> continue;\n> \n> }\n\nI'll give it a go - that would be much cleaner!\n\nBest wishes,\nIsaac\n\n> \n> --\n> Kieran\n> \n> > + LOG(IPAModuleAlgo, Debug)\n> > + << \"Algorithm \" << i\n> > + << \" is disabled via tuning file\";\n> > + continue;\n> > + }\n> > + }\n> > +\n> > int ret = createAlgorithm(context, algo);\n> > if (ret) {\n> > algorithms_.clear();\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 2BE89C3333\n\tfor ;\n\tThu, 20 Nov 2025 14:20:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D077E60AA3;\n\tThu, 20 Nov 2025 15:20:56 +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 B4D8D6069A\n\tfor ;\n\tThu, 20 Nov 2025 15:20:55 +0100 (CET)","from thinkpad.ideasonboard.com\n\t(cpc90716-aztw32-2-0-cust408.18-1.cable.virginm.net [86.26.101.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7718AB5;\n\tThu, 20 Nov 2025 15:18:50 +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=\"SCHCufjY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763648330;\n\tbh=tB7r4xQqll/xCfF8IZfqnCLPg3y4dn6tOfIbFGl+Qdg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=SCHCufjYi6UKfZBh+wXA0xThRExE00cAH0XvRbn+HBf18sAEAOcPZ0P74M3W39VeT\n\tMeNHMFm8lyZNteYQNE5MCimqFGmssW3DBjPS+a4oR5johUKXKezA+Rmp4rGNOHYpWa\n\tfNAN3nrBVgZzy6ye5Nqd3r19hm56hLPQzrQ/QNcU=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<176364739095.1127434.7880224954107702978@ping.linuxembedded.co.uk>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<176364739095.1127434.7880224954107702978@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","From":"Isaac Scott ","Cc":"","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 20 Nov 2025 14:20:51 +0000","Message-ID":"<176364845194.75776.18120611917162525704@isaac-ThinkPad-T16-Gen-2>","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":36944,"web_url":"https://patchwork.libcamera.org/comment/36944/","msgid":"<20251120142725.GB6375@pendragon.ideasonboard.com>","date":"2025-11-20T14:27:25","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Nov 20, 2025 at 02:03:10PM +0000, Kieran Bingham wrote:\n> Quoting Isaac Scott (2025-11-20 13:41:01)\n> > It is beneficial to have the option during development to disable and\n> > enable algorithms via the tuning file without having to delete their\n> > entries.\n> > \n> > Add support for an optional \"enabled\" parameter to accomplish this.\n> > \n> > Usage example:\n> > version: 1\n> > algorithms:\n> > - Agc:\n> > enabled: true\n> > - Awb:\n> > enabled: false\n> > \n> > This will enable AGC, and disable AWB. If the enabled flag is not\n> > present, the algorithm will be enabled.\n> > \n> > Signed-off-by: Isaac Scott \n> > \n> > ---\n> > \n> > v1 of this patch can be found here:\n> > \n> > https://patchwork.libcamera.org/patch/23299/\n> > \n> > Changelog:\n> > \n> > v1 -> v2:\n> > - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> > - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> > ---\n> > src/ipa/libipa/module.h | 9 +++++++++\n> > 1 file changed, 9 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> > index 0fb51916f..f8c4bb664 100644\n> > --- a/src/ipa/libipa/module.h\n> > +++ b/src/ipa/libipa/module.h\n> > @@ -55,6 +55,15 @@ public:\n> > return -EINVAL;\n> > }\n> > \n> > + if (algo.contains(\"enabled\")) {\n> > + if (algo[\"enabled\"].get() == false) {\n> \n> \n> Can we simplify this to a single if ?\n> \n> I think something like this might work, could you test/check it please?\n> \n> \t\t\tif (algo[\"enabled\"].get(true) == false) {\n\n\t\t\tif (!algo[\"enabled\"].get(true)) {\n\n> \t\t\t\tLOG() << ...;\n> \t\t\t\tcontinue;\n> \n> \t\t\t}\n> \n> > + LOG(IPAModuleAlgo, Debug)\n> > + << \"Algorithm \" << i\n> > + << \" is disabled via tuning file\";\n> > + continue;\n> > + }\n> > + }\n> > +\n> > int ret = createAlgorithm(context, algo);\n> > if (ret) {\n> > algorithms_.clear();","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 F12DAC3335\n\tfor ;\n\tThu, 20 Nov 2025 14:27:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3951760A86;\n\tThu, 20 Nov 2025 15:27:49 +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 719226069A\n\tfor ;\n\tThu, 20 Nov 2025 15:27:48 +0100 (CET)","from pendragon.ideasonboard.com (fs276ed015.tkyc509.ap.nuro.jp\n\t[39.110.208.21])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 93A24B5;\n\tThu, 20 Nov 2025 15:25:42 +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=\"FPU/NoA0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763648743;\n\tbh=KnhDHEF8fN5SEcLYzxch75JoxAYyEULH5bkq6s2jvg8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FPU/NoA0EjL9ENKpiwcU5NvBSGPhu13NO1EBcohdlbBG14KySclaZ3HoPY+OWQcB7\n\tMXcR9/9Y9GBU4+OU5xkTvG+MrkjUgMiVQzWFQbNgZJwidOz48ZNreoOqQaUzDtI5Ae\n\tmmjPfDxuKXiCQBCvjfMm8LTZRuNUIJQrF3lxhPx4=","Date":"Thu, 20 Nov 2025 23:27:25 +0900","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Isaac Scott ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","Message-ID":"<20251120142725.GB6375@pendragon.ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<176364739095.1127434.7880224954107702978@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<176364739095.1127434.7880224954107702978@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":36946,"web_url":"https://patchwork.libcamera.org/comment/36946/","msgid":"<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>","date":"2025-11-20T15:36:03","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi,\n\nQuoting Barnabás Pőcze (2025-11-20 14:20:26)\n> 2025. 11. 20. 15:18 keltezéssel, Isaac Scott írta:\n> > Hi Barnabás,\n> > \n> > Thanks for the review!\n> > \n> > Quoting Barnabás Pőcze (2025-11-20 14:04:51)\n> >> Hi\n> >>\n> >> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n> >>> It is beneficial to have the option during development to disable and\n> >>> enable algorithms via the tuning file without having to delete their\n> >>> entries.\n> >>>\n> >>> Add support for an optional \"enabled\" parameter to accomplish this.\n> >>>\n> >>> Usage example:\n> >>> version: 1\n> >>> algorithms:\n> >>> - Agc:\n> >>> enabled: true\n> >>> - Awb:\n> >>> enabled: false\n> >>>\n> >>> This will enable AGC, and disable AWB. If the enabled flag is not\n> >>> present, the algorithm will be enabled.\n> >>>\n> >>> Signed-off-by: Isaac Scott \n> >>>\n> >>> ---\n> >>>\n> >>> v1 of this patch can be found here:\n> >>>\n> >>> https://patchwork.libcamera.org/patch/23299/\n> >>>\n> >>> Changelog:\n> >>>\n> >>> v1 -> v2:\n> >>> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> >>> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> >>> ---\n> >>> src/ipa/libipa/module.h | 9 +++++++++\n> >>> 1 file changed, 9 insertions(+)\n> >>>\n> >>> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> >>> index 0fb51916f..f8c4bb664 100644\n> >>> --- a/src/ipa/libipa/module.h\n> >>> +++ b/src/ipa/libipa/module.h\n> >>> @@ -55,6 +55,15 @@ public:\n> >>> return -EINVAL;\n> >>> }\n> >>> \n> >>> + if (algo.contains(\"enabled\")) {\n> >>> + if (algo[\"enabled\"].get() == false) {\n> >>\n> >> The operator[] returns a special empty object if the key is not present,\n> >> so I think the above can be simplified as follows:\n> > \n> > Ah, interesting, that makes it much cleaner!\n> > \n> >>\n> >> if (!algo[\"enabled\"].get(true)) {\n> >> ...\n> >> continue;\n> >> }\n> >>\n> >>\n> >> In any case I'm a bit concerned about the `createAlgorithm()` function:\n> >>\n> >> const auto &[name, algoData] = *data.asDict().begin();\n> >>\n> >> this seems very fragile, e.g.\n> >>\n> >> - enabled: true\n> >> Awb:\n> >>\n> >> wouldn't work.\n> > \n> > In that case, wouldn't that be undesirable anyway? Shouldn't algorithm\n> > names come first?\n> \n> I'm not sure. I think generally the expectation is that a json/yaml dictionary\n> is unordered, i.e. the order does not matter.\n> \n\nHmmm ok, I'll see if theres an improvement I can make there for an\nadditional patch.\n\nThanks,\nIsaac\n\n> \n> > \n> > Best wishes,\n> > Isaac\n> > \n> >>\n> >>\n> >> Regards,\n> >> Barnabás Pőcze\n> >>\n> >>> + LOG(IPAModuleAlgo, Debug)\n> >>> + << \"Algorithm \" << i\n> >>> + << \" is disabled via tuning file\";\n> >>> + continue;\n> >>> + }\n> >>> + }\n> >>> +\n> >>> int ret = createAlgorithm(context, algo);\n> >>> if (ret) {\n> >>> algorithms_.clear();\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 3A13DBD80A\n\tfor ;\n\tThu, 20 Nov 2025 15:36:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C0D4609D8;\n\tThu, 20 Nov 2025 16:36:08 +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 8C8A16069A\n\tfor ;\n\tThu, 20 Nov 2025 16:36:06 +0100 (CET)","from thinkpad.ideasonboard.com\n\t(cpc90716-aztw32-2-0-cust408.18-1.cable.virginm.net [86.26.101.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C2E559CA;\n\tThu, 20 Nov 2025 16:34:00 +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=\"rG2y5XsG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763652840;\n\tbh=0aqHV29HqvEOcuQ2eG67rMpDomcEK5QWP7hWZJR7yXk=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=rG2y5XsGxM2WS5PxC9TUIHBjhYyqpJC9USTUdOJYgzoA09ZD3bvf+wfN5R7i1x/Ha\n\tC4ZUwF3XuX9nfcNdsQidXU4mAWqMLcfIRqsI5D/mT4m8xt+LnRhpeS0Ne6YnaHdYk8\n\t9ZAlnf85fuBIkO9Ns0g0h0gIqSGe97HpTqH4n9n0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>\n\t<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>\n\t<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","From":"Isaac Scott ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 20 Nov 2025 15:36:03 +0000","Message-ID":"<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>","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":36948,"web_url":"https://patchwork.libcamera.org/comment/36948/","msgid":"<20251120160908.GM10711@pendragon.ideasonboard.com>","date":"2025-11-20T16:09:08","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Nov 20, 2025 at 03:36:03PM +0000, Isaac Scott wrote:\n> Quoting Barnabás Pőcze (2025-11-20 14:20:26)\n> > 2025. 11. 20. 15:18 keltezéssel, Isaac Scott írta:\n> > > Quoting Barnabás Pőcze (2025-11-20 14:04:51)\n> > >> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n> > >>> It is beneficial to have the option during development to disable and\n> > >>> enable algorithms via the tuning file without having to delete their\n> > >>> entries.\n> > >>>\n> > >>> Add support for an optional \"enabled\" parameter to accomplish this.\n> > >>>\n> > >>> Usage example:\n> > >>> version: 1\n> > >>> algorithms:\n> > >>> - Agc:\n> > >>> enabled: true\n> > >>> - Awb:\n> > >>> enabled: false\n> > >>>\n> > >>> This will enable AGC, and disable AWB. If the enabled flag is not\n> > >>> present, the algorithm will be enabled.\n> > >>>\n> > >>> Signed-off-by: Isaac Scott \n> > >>>\n> > >>> ---\n> > >>>\n> > >>> v1 of this patch can be found here:\n> > >>>\n> > >>> https://patchwork.libcamera.org/patch/23299/\n> > >>>\n> > >>> Changelog:\n> > >>>\n> > >>> v1 -> v2:\n> > >>> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> > >>> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> > >>> ---\n> > >>> src/ipa/libipa/module.h | 9 +++++++++\n> > >>> 1 file changed, 9 insertions(+)\n> > >>>\n> > >>> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> > >>> index 0fb51916f..f8c4bb664 100644\n> > >>> --- a/src/ipa/libipa/module.h\n> > >>> +++ b/src/ipa/libipa/module.h\n> > >>> @@ -55,6 +55,15 @@ public:\n> > >>> return -EINVAL;\n> > >>> }\n> > >>> \n> > >>> + if (algo.contains(\"enabled\")) {\n> > >>> + if (algo[\"enabled\"].get() == false) {\n> > >>\n> > >> The operator[] returns a special empty object if the key is not present,\n> > >> so I think the above can be simplified as follows:\n> > > \n> > > Ah, interesting, that makes it much cleaner!\n> > > \n> > >>\n> > >> if (!algo[\"enabled\"].get(true)) {\n> > >> ...\n> > >> continue;\n> > >> }\n> > >>\n> > >>\n> > >> In any case I'm a bit concerned about the `createAlgorithm()` function:\n> > >>\n> > >> const auto &[name, algoData] = *data.asDict().begin();\n> > >>\n> > >> this seems very fragile, e.g.\n> > >>\n> > >> - enabled: true\n> > >> Awb:\n> > >>\n> > >> wouldn't work.\n> > > \n> > > In that case, wouldn't that be undesirable anyway? Shouldn't algorithm\n> > > names come first?\n> > \n> > I'm not sure. I think generally the expectation is that a json/yaml dictionary\n> > is unordered, i.e. the order does not matter.\n> \n> Hmmm ok, I'll see if theres an improvement I can make there for an\n> additional patch.\n\nOops, I haven't noticed that, my bad. The enabled property should be a\nchild of the algorithm:\n\n - Awb:\n enabled: false\n\n> > >>> + LOG(IPAModuleAlgo, Debug)\n> > >>> + << \"Algorithm \" << i\n> > >>> + << \" is disabled via tuning file\";\n> > >>> + continue;\n> > >>> + }\n> > >>> + }\n> > >>> +\n> > >>> int ret = createAlgorithm(context, algo);\n> > >>> if (ret) {\n> > >>> algorithms_.clear();","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 BF12DBD80A\n\tfor ;\n\tThu, 20 Nov 2025 16:09:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A35360A80;\n\tThu, 20 Nov 2025 17:09:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD8656069A\n\tfor ;\n\tThu, 20 Nov 2025 17:09:36 +0100 (CET)","from pendragon.ideasonboard.com (fs276ed015.tkyc509.ap.nuro.jp\n\t[39.110.208.21])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 6598BB5;\n\tThu, 20 Nov 2025 17:07:30 +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=\"VCsaqawJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763654851;\n\tbh=L+GCk/VqYbVg+ebWEpZIvkZfyQCOpeG49Z7MUU9nxe0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VCsaqawJ4zIzN+xxMFcNo6Z3FEWYxqthJOS3VHN+S3Rr9Y2WrbqrR9qlY7H5ZBFIX\n\tKZVHXQrbJaMWYMr9W7danbig8MhQFCgE2y1HcyGL5Bw2vqQ69+jdcfWDYS0cydz73p\n\t74e+y3O+nVKzQlbvlJOy1DB++iTyR6VhXFkMw1Mg=","Date":"Fri, 21 Nov 2025 01:09:08 +0900","From":"Laurent Pinchart ","To":"Isaac Scott ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","Message-ID":"<20251120160908.GM10711@pendragon.ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>\n\t<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>\n\t<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>\n\t<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>","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":36949,"web_url":"https://patchwork.libcamera.org/comment/36949/","msgid":"<42056baf-bc1a-4699-9507-a711c5e34f4e@ideasonboard.com>","date":"2025-11-20T16:23:10","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 11. 20. 17:09 keltezéssel, Laurent Pinchart írta:\n> On Thu, Nov 20, 2025 at 03:36:03PM +0000, Isaac Scott wrote:\n>> Quoting Barnabás Pőcze (2025-11-20 14:20:26)\n>>> 2025. 11. 20. 15:18 keltezéssel, Isaac Scott írta:\n>>>> Quoting Barnabás Pőcze (2025-11-20 14:04:51)\n>>>>> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n>>>>>> It is beneficial to have the option during development to disable and\n>>>>>> enable algorithms via the tuning file without having to delete their\n>>>>>> entries.\n>>>>>>\n>>>>>> Add support for an optional \"enabled\" parameter to accomplish this.\n>>>>>>\n>>>>>> Usage example:\n>>>>>> version: 1\n>>>>>> algorithms:\n>>>>>> - Agc:\n>>>>>> enabled: true\n>>>>>> - Awb:\n>>>>>> enabled: false\n>>>>>>\n>>>>>> This will enable AGC, and disable AWB. If the enabled flag is not\n>>>>>> present, the algorithm will be enabled.\n>>>>>>\n>>>>>> Signed-off-by: Isaac Scott \n>>>>>>\n>>>>>> ---\n>>>>>>\n>>>>>> v1 of this patch can be found here:\n>>>>>>\n>>>>>> https://patchwork.libcamera.org/patch/23299/\n>>>>>>\n>>>>>> Changelog:\n>>>>>>\n>>>>>> v1 -> v2:\n>>>>>> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n>>>>>> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n>>>>>> ---\n>>>>>> src/ipa/libipa/module.h | 9 +++++++++\n>>>>>> 1 file changed, 9 insertions(+)\n>>>>>>\n>>>>>> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n>>>>>> index 0fb51916f..f8c4bb664 100644\n>>>>>> --- a/src/ipa/libipa/module.h\n>>>>>> +++ b/src/ipa/libipa/module.h\n>>>>>> @@ -55,6 +55,15 @@ public:\n>>>>>> return -EINVAL;\n>>>>>> }\n>>>>>> \n>>>>>> + if (algo.contains(\"enabled\")) {\n>>>>>> + if (algo[\"enabled\"].get() == false) {\n>>>>>\n>>>>> The operator[] returns a special empty object if the key is not present,\n>>>>> so I think the above can be simplified as follows:\n>>>>\n>>>> Ah, interesting, that makes it much cleaner!\n>>>>\n>>>>>\n>>>>> if (!algo[\"enabled\"].get(true)) {\n>>>>> ...\n>>>>> continue;\n>>>>> }\n>>>>>\n>>>>>\n>>>>> In any case I'm a bit concerned about the `createAlgorithm()` function:\n>>>>>\n>>>>> const auto &[name, algoData] = *data.asDict().begin();\n>>>>>\n>>>>> this seems very fragile, e.g.\n>>>>>\n>>>>> - enabled: true\n>>>>> Awb:\n>>>>>\n>>>>> wouldn't work.\n>>>>\n>>>> In that case, wouldn't that be undesirable anyway? Shouldn't algorithm\n>>>> names come first?\n>>>\n>>> I'm not sure. I think generally the expectation is that a json/yaml dictionary\n>>> is unordered, i.e. the order does not matter.\n>>\n>> Hmmm ok, I'll see if theres an improvement I can make there for an\n>> additional patch.\n> \n> Oops, I haven't noticed that, my bad. The enabled property should be a\n> child of the algorithm:\n> \n> - Awb:\n> enabled: false\n> \n\nHmm, to me the \"obvious\" choice is to have it \"outside\" as not to mix the two\ntypes of parameters. E.g.\n\n - name: Awb\n params:\n # params for Awb algorithm\n enabled: true\n # other \"external\" parameters for the algorithm loading code\n\nI've felt like the current design (array of dictionaries with a single key) is an mix of a\n\"dictionary\" and an \"array\" approach, and it seems to me that both would have advantages over\nthe current format. But admittedly I have not investigated why this specific format was\nselected, so I'm probably missing something.\n\n>>>>>> + LOG(IPAModuleAlgo, Debug)\n>>>>>> + << \"Algorithm \" << i\n>>>>>> + << \" is disabled via tuning file\";\n>>>>>> + continue;\n>>>>>> + }\n>>>>>> + }\n>>>>>> +\n>>>>>> int ret = createAlgorithm(context, algo);\n>>>>>> if (ret) {\n>>>>>> algorithms_.clear();\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 03FB4BD80A\n\tfor ;\n\tThu, 20 Nov 2025 16:23:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 499F2609D8;\n\tThu, 20 Nov 2025 17:23:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EFA256069A\n\tfor ;\n\tThu, 20 Nov 2025 17:23:13 +0100 (CET)","from [192.168.33.37] (185.221.143.100.nat.pool.zt.hu\n\t[185.221.143.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7FBB1C77;\n\tThu, 20 Nov 2025 17:21:08 +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=\"L7x8ia+9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763655668;\n\tbh=E+MOU2GOx35q6wHGcfRy+e2LRHwT9xytN08To975k0Y=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=L7x8ia+9ZE1X9b9qnn5HT5qM8KcQ/7gqjiPhLdZ+KdXRWbMV+qYNBIV81V2stT+v5\n\tXWPZvAgnt7t4nVE1lg7kyTTVvCI+NVOcKHcZeHv47xzGnErgvcKrbgHWVgLMjf/mus\n\tP7JUoHOCLjrbpN4n7XFcfBOvA54KmJvamB/RLpS8=","Message-ID":"<42056baf-bc1a-4699-9507-a711c5e34f4e@ideasonboard.com>","Date":"Thu, 20 Nov 2025 17:23:10 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","To":"Laurent Pinchart ,\n\tIsaac Scott ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>\n\t<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>\n\t<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>\n\t<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>\n\t<20251120160908.GM10711@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251120160908.GM10711@pendragon.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":36950,"web_url":"https://patchwork.libcamera.org/comment/36950/","msgid":"<20251120163313.GO10711@pendragon.ideasonboard.com>","date":"2025-11-20T16:33:13","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Nov 20, 2025 at 05:23:10PM +0100, Barnabás Pőcze wrote:\n> 2025. 11. 20. 17:09 keltezéssel, Laurent Pinchart írta:\n> > On Thu, Nov 20, 2025 at 03:36:03PM +0000, Isaac Scott wrote:\n> >> Quoting Barnabás Pőcze (2025-11-20 14:20:26)\n> >>> 2025. 11. 20. 15:18 keltezéssel, Isaac Scott írta:\n> >>>> Quoting Barnabás Pőcze (2025-11-20 14:04:51)\n> >>>>> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n> >>>>>> It is beneficial to have the option during development to disable and\n> >>>>>> enable algorithms via the tuning file without having to delete their\n> >>>>>> entries.\n> >>>>>>\n> >>>>>> Add support for an optional \"enabled\" parameter to accomplish this.\n> >>>>>>\n> >>>>>> Usage example:\n> >>>>>> version: 1\n> >>>>>> algorithms:\n> >>>>>> - Agc:\n> >>>>>> enabled: true\n> >>>>>> - Awb:\n> >>>>>> enabled: false\n> >>>>>>\n> >>>>>> This will enable AGC, and disable AWB. If the enabled flag is not\n> >>>>>> present, the algorithm will be enabled.\n> >>>>>>\n> >>>>>> Signed-off-by: Isaac Scott \n> >>>>>>\n> >>>>>> ---\n> >>>>>>\n> >>>>>> v1 of this patch can be found here:\n> >>>>>>\n> >>>>>> https://patchwork.libcamera.org/patch/23299/\n> >>>>>>\n> >>>>>> Changelog:\n> >>>>>>\n> >>>>>> v1 -> v2:\n> >>>>>> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> >>>>>> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> >>>>>> ---\n> >>>>>> src/ipa/libipa/module.h | 9 +++++++++\n> >>>>>> 1 file changed, 9 insertions(+)\n> >>>>>>\n> >>>>>> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> >>>>>> index 0fb51916f..f8c4bb664 100644\n> >>>>>> --- a/src/ipa/libipa/module.h\n> >>>>>> +++ b/src/ipa/libipa/module.h\n> >>>>>> @@ -55,6 +55,15 @@ public:\n> >>>>>> return -EINVAL;\n> >>>>>> }\n> >>>>>> \n> >>>>>> + if (algo.contains(\"enabled\")) {\n> >>>>>> + if (algo[\"enabled\"].get() == false) {\n> >>>>>\n> >>>>> The operator[] returns a special empty object if the key is not present,\n> >>>>> so I think the above can be simplified as follows:\n> >>>>\n> >>>> Ah, interesting, that makes it much cleaner!\n> >>>>\n> >>>>>\n> >>>>> if (!algo[\"enabled\"].get(true)) {\n> >>>>> ...\n> >>>>> continue;\n> >>>>> }\n> >>>>>\n> >>>>>\n> >>>>> In any case I'm a bit concerned about the `createAlgorithm()` function:\n> >>>>>\n> >>>>> const auto &[name, algoData] = *data.asDict().begin();\n> >>>>>\n> >>>>> this seems very fragile, e.g.\n> >>>>>\n> >>>>> - enabled: true\n> >>>>> Awb:\n> >>>>>\n> >>>>> wouldn't work.\n> >>>>\n> >>>> In that case, wouldn't that be undesirable anyway? Shouldn't algorithm\n> >>>> names come first?\n> >>>\n> >>> I'm not sure. I think generally the expectation is that a json/yaml dictionary\n> >>> is unordered, i.e. the order does not matter.\n> >>\n> >> Hmmm ok, I'll see if theres an improvement I can make there for an\n> >> additional patch.\n> > \n> > Oops, I haven't noticed that, my bad. The enabled property should be a\n> > child of the algorithm:\n> > \n> > - Awb:\n> > enabled: false\n> > \n> \n> Hmm, to me the \"obvious\" choice is to have it \"outside\" as not to mix the two\n> types of parameters. E.g.\n> \n> - name: Awb\n> params:\n> # params for Awb algorithm\n> enabled: true\n> # other \"external\" parameters for the algorithm loading code\n> \n> I've felt like the current design (array of dictionaries with a single key) is an mix of a\n> \"dictionary\" and an \"array\" approach, and it seems to me that both would have advantages over\n> the current format. But admittedly I have not investigated why this specific format was\n> selected, so I'm probably missing something.\n\nThe sequence is required as mappings are unordered in yaml, as you've\nmentioned. Using the algorithm name as the key to the single-item\nmapping is a shortcut to avoid being too verbose. Other options could be\npossible.\n\n> >>>>>> + LOG(IPAModuleAlgo, Debug)\n> >>>>>> + << \"Algorithm \" << i\n> >>>>>> + << \" is disabled via tuning file\";\n> >>>>>> + continue;\n> >>>>>> + }\n> >>>>>> + }\n> >>>>>> +\n> >>>>>> int ret = createAlgorithm(context, algo);\n> >>>>>> if (ret) {\n> >>>>>> algorithms_.clear();","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 5F459BD80A\n\tfor ;\n\tThu, 20 Nov 2025 16:33:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 96E3360A80;\n\tThu, 20 Nov 2025 17:33:42 +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 A81F86069A\n\tfor ;\n\tThu, 20 Nov 2025 17:33:41 +0100 (CET)","from pendragon.ideasonboard.com (fs276ed015.tkyc509.ap.nuro.jp\n\t[39.110.208.21])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1F732B5;\n\tThu, 20 Nov 2025 17:31:34 +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=\"seVMUqPj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763656296;\n\tbh=sPg6ifv4AvbV1HLOSm2Jw1lo4g7OvvEWhjN4vFqsNXQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=seVMUqPjAGAspngLYnljuDDtMXkjevI/4eOm7GufmUGzAiXENM69ZM42+6K4s1Mh3\n\tjHCCiGK9xlukoFVGacuNyjjQAjuC4OfizbCDF6X9vGYNMWdkUk7zonzJpqcwDKh5nw\n\tYRyGqfPJKY201vVpO3smX0iiWS5THWUvYXEoVGyw=","Date":"Fri, 21 Nov 2025 01:33:13 +0900","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Isaac Scott ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","Message-ID":"<20251120163313.GO10711@pendragon.ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>\n\t<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>\n\t<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>\n\t<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>\n\t<20251120160908.GM10711@pendragon.ideasonboard.com>\n\t<42056baf-bc1a-4699-9507-a711c5e34f4e@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<42056baf-bc1a-4699-9507-a711c5e34f4e@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":36951,"web_url":"https://patchwork.libcamera.org/comment/36951/","msgid":"<57c04bbf-54c4-4ff1-a8cc-e08ff91fcaed@ideasonboard.com>","date":"2025-11-20T16:38:46","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 11. 20. 17:33 keltezéssel, Laurent Pinchart írta:\n> On Thu, Nov 20, 2025 at 05:23:10PM +0100, Barnabás Pőcze wrote:\n>> 2025. 11. 20. 17:09 keltezéssel, Laurent Pinchart írta:\n>>> On Thu, Nov 20, 2025 at 03:36:03PM +0000, Isaac Scott wrote:\n>>>> Quoting Barnabás Pőcze (2025-11-20 14:20:26)\n>>>>> 2025. 11. 20. 15:18 keltezéssel, Isaac Scott írta:\n>>>>>> Quoting Barnabás Pőcze (2025-11-20 14:04:51)\n>>>>>>> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n>>>>>>>> It is beneficial to have the option during development to disable and\n>>>>>>>> enable algorithms via the tuning file without having to delete their\n>>>>>>>> entries.\n>>>>>>>>\n>>>>>>>> Add support for an optional \"enabled\" parameter to accomplish this.\n>>>>>>>>\n>>>>>>>> Usage example:\n>>>>>>>> version: 1\n>>>>>>>> algorithms:\n>>>>>>>> - Agc:\n>>>>>>>> enabled: true\n>>>>>>>> - Awb:\n>>>>>>>> enabled: false\n>>>>>>>>\n>>>>>>>> This will enable AGC, and disable AWB. If the enabled flag is not\n>>>>>>>> present, the algorithm will be enabled.\n>>>>>>>>\n>>>>>>>> Signed-off-by: Isaac Scott \n>>>>>>>>\n>>>>>>>> ---\n>>>>>>>>\n>>>>>>>> v1 of this patch can be found here:\n>>>>>>>>\n>>>>>>>> https://patchwork.libcamera.org/patch/23299/\n>>>>>>>>\n>>>>>>>> Changelog:\n>>>>>>>>\n>>>>>>>> v1 -> v2:\n>>>>>>>> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n>>>>>>>> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n>>>>>>>> ---\n>>>>>>>> src/ipa/libipa/module.h | 9 +++++++++\n>>>>>>>> 1 file changed, 9 insertions(+)\n>>>>>>>>\n>>>>>>>> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n>>>>>>>> index 0fb51916f..f8c4bb664 100644\n>>>>>>>> --- a/src/ipa/libipa/module.h\n>>>>>>>> +++ b/src/ipa/libipa/module.h\n>>>>>>>> @@ -55,6 +55,15 @@ public:\n>>>>>>>> return -EINVAL;\n>>>>>>>> }\n>>>>>>>> \n>>>>>>>> + if (algo.contains(\"enabled\")) {\n>>>>>>>> + if (algo[\"enabled\"].get() == false) {\n>>>>>>>\n>>>>>>> The operator[] returns a special empty object if the key is not present,\n>>>>>>> so I think the above can be simplified as follows:\n>>>>>>\n>>>>>> Ah, interesting, that makes it much cleaner!\n>>>>>>\n>>>>>>>\n>>>>>>> if (!algo[\"enabled\"].get(true)) {\n>>>>>>> ...\n>>>>>>> continue;\n>>>>>>> }\n>>>>>>>\n>>>>>>>\n>>>>>>> In any case I'm a bit concerned about the `createAlgorithm()` function:\n>>>>>>>\n>>>>>>> const auto &[name, algoData] = *data.asDict().begin();\n>>>>>>>\n>>>>>>> this seems very fragile, e.g.\n>>>>>>>\n>>>>>>> - enabled: true\n>>>>>>> Awb:\n>>>>>>>\n>>>>>>> wouldn't work.\n>>>>>>\n>>>>>> In that case, wouldn't that be undesirable anyway? Shouldn't algorithm\n>>>>>> names come first?\n>>>>>\n>>>>> I'm not sure. I think generally the expectation is that a json/yaml dictionary\n>>>>> is unordered, i.e. the order does not matter.\n>>>>\n>>>> Hmmm ok, I'll see if theres an improvement I can make there for an\n>>>> additional patch.\n>>>\n>>> Oops, I haven't noticed that, my bad. The enabled property should be a\n>>> child of the algorithm:\n>>>\n>>> - Awb:\n>>> enabled: false\n>>>\n>>\n>> Hmm, to me the \"obvious\" choice is to have it \"outside\" as not to mix the two\n>> types of parameters. E.g.\n>>\n>> - name: Awb\n>> params:\n>> # params for Awb algorithm\n>> enabled: true\n>> # other \"external\" parameters for the algorithm loading code\n>>\n>> I've felt like the current design (array of dictionaries with a single key) is an mix of a\n>> \"dictionary\" and an \"array\" approach, and it seems to me that both would have advantages over\n>> the current format. But admittedly I have not investigated why this specific format was\n>> selected, so I'm probably missing something.\n> \n> The sequence is required as mappings are unordered in yaml, as you've\n> mentioned. Using the algorithm name as the key to the single-item\n> mapping is a shortcut to avoid being too verbose. Other options could be\n> possible.\n\nYes, but the ordering is essentially determined by the IPA module in question, no?\nSo regardless of the order in the tuning file, I think it is expected that\nthings will work the same (and correctly), right?\n\n\n> \n>>>>>>>> + LOG(IPAModuleAlgo, Debug)\n>>>>>>>> + << \"Algorithm \" << i\n>>>>>>>> + << \" is disabled via tuning file\";\n>>>>>>>> + continue;\n>>>>>>>> + }\n>>>>>>>> + }\n>>>>>>>> +\n>>>>>>>> int ret = createAlgorithm(context, algo);\n>>>>>>>> if (ret) {\n>>>>>>>> algorithms_.clear();\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 D2FBCC3333\n\tfor ;\n\tThu, 20 Nov 2025 16:38:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1442B60A9D;\n\tThu, 20 Nov 2025 17:38:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEACA6069A\n\tfor ;\n\tThu, 20 Nov 2025 17:38:50 +0100 (CET)","from [192.168.33.37] (185.221.143.100.nat.pool.zt.hu\n\t[185.221.143.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 18185B5;\n\tThu, 20 Nov 2025 17:36:45 +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=\"YZV6BKgm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763656605;\n\tbh=eu/C2hh/G40F4pQtNxpt6/Z1E7pYWiyFcHhnn3x70wA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=YZV6BKgmtpk0+0ywd9DkKegSHwkVQtG0kykWd06GHCf8YvCP4YIR2HxvIWfhmZR2B\n\t8AhmuT/5Kc9iIJnl11JTuGAs3CAQ6C25lRtPb0mOBSEy9qqhvFqVycgpO9WPZ60Iiz\n\t8RdIqaFKFosNHR2/VaNUzQYKFKcl9RjfIPPPttmE=","Message-ID":"<57c04bbf-54c4-4ff1-a8cc-e08ff91fcaed@ideasonboard.com>","Date":"Thu, 20 Nov 2025 17:38:46 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","To":"Laurent Pinchart ","Cc":"Isaac Scott ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>\n\t<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>\n\t<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>\n\t<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>\n\t<20251120160908.GM10711@pendragon.ideasonboard.com>\n\t<42056baf-bc1a-4699-9507-a711c5e34f4e@ideasonboard.com>\n\t<20251120163313.GO10711@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251120163313.GO10711@pendragon.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":36963,"web_url":"https://patchwork.libcamera.org/comment/36963/","msgid":"<20251121032850.GA9186@pendragon.ideasonboard.com>","date":"2025-11-21T03:28:50","subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Nov 20, 2025 at 05:38:46PM +0100, Barnabás Pőcze wrote:\n> 2025. 11. 20. 17:33 keltezéssel, Laurent Pinchart írta:\n> > On Thu, Nov 20, 2025 at 05:23:10PM +0100, Barnabás Pőcze wrote:\n> >> 2025. 11. 20. 17:09 keltezéssel, Laurent Pinchart írta:\n> >>> On Thu, Nov 20, 2025 at 03:36:03PM +0000, Isaac Scott wrote:\n> >>>> Quoting Barnabás Pőcze (2025-11-20 14:20:26)\n> >>>>> 2025. 11. 20. 15:18 keltezéssel, Isaac Scott írta:\n> >>>>>> Quoting Barnabás Pőcze (2025-11-20 14:04:51)\n> >>>>>>> 2025. 11. 20. 14:41 keltezéssel, Isaac Scott írta:\n> >>>>>>>> It is beneficial to have the option during development to disable and\n> >>>>>>>> enable algorithms via the tuning file without having to delete their\n> >>>>>>>> entries.\n> >>>>>>>>\n> >>>>>>>> Add support for an optional \"enabled\" parameter to accomplish this.\n> >>>>>>>>\n> >>>>>>>> Usage example:\n> >>>>>>>> version: 1\n> >>>>>>>> algorithms:\n> >>>>>>>> - Agc:\n> >>>>>>>> enabled: true\n> >>>>>>>> - Awb:\n> >>>>>>>> enabled: false\n> >>>>>>>>\n> >>>>>>>> This will enable AGC, and disable AWB. If the enabled flag is not\n> >>>>>>>> present, the algorithm will be enabled.\n> >>>>>>>>\n> >>>>>>>> Signed-off-by: Isaac Scott \n> >>>>>>>>\n> >>>>>>>> ---\n> >>>>>>>>\n> >>>>>>>> v1 of this patch can be found here:\n> >>>>>>>>\n> >>>>>>>> https://patchwork.libcamera.org/patch/23299/\n> >>>>>>>>\n> >>>>>>>> Changelog:\n> >>>>>>>>\n> >>>>>>>> v1 -> v2:\n> >>>>>>>> - Moved parsing to createAlgorithms() instead of createAlgorithm()\n> >>>>>>>> - Changed \"disabled\" flag to \"enabled: [boolean]\"\n> >>>>>>>> ---\n> >>>>>>>> src/ipa/libipa/module.h | 9 +++++++++\n> >>>>>>>> 1 file changed, 9 insertions(+)\n> >>>>>>>>\n> >>>>>>>> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> >>>>>>>> index 0fb51916f..f8c4bb664 100644\n> >>>>>>>> --- a/src/ipa/libipa/module.h\n> >>>>>>>> +++ b/src/ipa/libipa/module.h\n> >>>>>>>> @@ -55,6 +55,15 @@ public:\n> >>>>>>>> return -EINVAL;\n> >>>>>>>> }\n> >>>>>>>> \n> >>>>>>>> + if (algo.contains(\"enabled\")) {\n> >>>>>>>> + if (algo[\"enabled\"].get() == false) {\n> >>>>>>>\n> >>>>>>> The operator[] returns a special empty object if the key is not present,\n> >>>>>>> so I think the above can be simplified as follows:\n> >>>>>>\n> >>>>>> Ah, interesting, that makes it much cleaner!\n> >>>>>>\n> >>>>>>>\n> >>>>>>> if (!algo[\"enabled\"].get(true)) {\n> >>>>>>> ...\n> >>>>>>> continue;\n> >>>>>>> }\n> >>>>>>>\n> >>>>>>>\n> >>>>>>> In any case I'm a bit concerned about the `createAlgorithm()` function:\n> >>>>>>>\n> >>>>>>> const auto &[name, algoData] = *data.asDict().begin();\n> >>>>>>>\n> >>>>>>> this seems very fragile, e.g.\n> >>>>>>>\n> >>>>>>> - enabled: true\n> >>>>>>> Awb:\n> >>>>>>>\n> >>>>>>> wouldn't work.\n> >>>>>>\n> >>>>>> In that case, wouldn't that be undesirable anyway? Shouldn't algorithm\n> >>>>>> names come first?\n> >>>>>\n> >>>>> I'm not sure. I think generally the expectation is that a json/yaml dictionary\n> >>>>> is unordered, i.e. the order does not matter.\n> >>>>\n> >>>> Hmmm ok, I'll see if theres an improvement I can make there for an\n> >>>> additional patch.\n> >>>\n> >>> Oops, I haven't noticed that, my bad. The enabled property should be a\n> >>> child of the algorithm:\n> >>>\n> >>> - Awb:\n> >>> enabled: false\n> >>>\n> >>\n> >> Hmm, to me the \"obvious\" choice is to have it \"outside\" as not to mix the two\n> >> types of parameters. E.g.\n> >>\n> >> - name: Awb\n> >> params:\n> >> # params for Awb algorithm\n> >> enabled: true\n> >> # other \"external\" parameters for the algorithm loading code\n> >>\n> >> I've felt like the current design (array of dictionaries with a single key) is an mix of a\n> >> \"dictionary\" and an \"array\" approach, and it seems to me that both would have advantages over\n> >> the current format. But admittedly I have not investigated why this specific format was\n> >> selected, so I'm probably missing something.\n> > \n> > The sequence is required as mappings are unordered in yaml, as you've\n> > mentioned. Using the algorithm name as the key to the single-item\n> > mapping is a shortcut to avoid being too verbose. Other options could be\n> > possible.\n> \n> Yes, but the ordering is essentially determined by the IPA module in question, no?\n> So regardless of the order in the tuning file, I think it is expected that\n> things will work the same (and correctly), right?\n\nNot necessarily. There are dependencies between algorithms, which\ncurrently need to be handled by ordering them correctly in tuning files.\nIt would be nice to declare those dependencies programmatically to avoid\nrelying on the tuning file order, but even then, different orderings may\nresult in different outcomes. This hasn't been researched in details.\n\n> >>>>>>>> + LOG(IPAModuleAlgo, Debug)\n> >>>>>>>> + << \"Algorithm \" << i\n> >>>>>>>> + << \" is disabled via tuning file\";\n> >>>>>>>> + continue;\n> >>>>>>>> + }\n> >>>>>>>> + }\n> >>>>>>>> +\n> >>>>>>>> int ret = createAlgorithm(context, algo);\n> >>>>>>>> if (ret) {\n> >>>>>>>> algorithms_.clear();","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 47B96C3334\n\tfor ;\n\tFri, 21 Nov 2025 03:29:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3202060A8B;\n\tFri, 21 Nov 2025 04:29:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 85048606A0\n\tfor ;\n\tFri, 21 Nov 2025 04:29:16 +0100 (CET)","from pendragon.ideasonboard.com (fs276ed015.tkyc509.ap.nuro.jp\n\t[39.110.208.21])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B0A59E7C;\n\tFri, 21 Nov 2025 04:27:09 +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=\"STRvkAXs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763695630;\n\tbh=hqK36r8j4UA8eJO4xHsRpwKXEt45DBxBo5FfyYkotK0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=STRvkAXs9f1yrTL1ljcoj8yUHG/xW5q7VwCKdRSkgLNJ1JJsuLTL0bHKsUGUUudZN\n\tc8iXHTZaBIdELstqTF7Ag+Te4k+V/aGL/DP1DbA5rjZUdz02tl/1Dshp5p2t0ihWgs\n\tspy/wz+yB23sDzQXx4ZNGD5tioHPSmCRLmMi8vTw=","Date":"Fri, 21 Nov 2025 12:28:50 +0900","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Isaac Scott ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libipa: module: Allow algorithms to be disabled via\n\tthe tuning file","Message-ID":"<20251121032850.GA9186@pendragon.ideasonboard.com>","References":"<20251120134101.72892-1-isaac.scott@ideasonboard.com>\n\t<49de9d43-3e1c-484b-90cb-9df476259fbd@ideasonboard.com>\n\t<176364833675.75776.5932656295120311951@isaac-ThinkPad-T16-Gen-2>\n\t<2477ffa1-91ee-4d77-97f7-f57d30bba9e7@ideasonboard.com>\n\t<176365296318.85746.5658557852574592247@isaac-ThinkPad-T16-Gen-2>\n\t<20251120160908.GM10711@pendragon.ideasonboard.com>\n\t<42056baf-bc1a-4699-9507-a711c5e34f4e@ideasonboard.com>\n\t<20251120163313.GO10711@pendragon.ideasonboard.com>\n\t<57c04bbf-54c4-4ff1-a8cc-e08ff91fcaed@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<57c04bbf-54c4-4ff1-a8cc-e08ff91fcaed@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\" "}}]