[{"id":37637,"web_url":"https://patchwork.libcamera.org/comment/37637/","msgid":"<6d507c3d-248f-4c4d-bc57-f8dd099efade@ideasonboard.com>","date":"2026-01-14T09:33:44","subject":"Re: [PATCH 36/36] pipeline: simple: Turn devices configuration\n\toption into dictionary","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 13. 1:08 keltezéssel, Laurent Pinchart írta:\n> The pipelines.simple.devices configuration option contains a list of\n> devices, each of them being a dictionary with a \"driver\" element that\n> acts as a unique key to index the list. Turn it into a YAML dictionary\n> to ensure uniqueness of the key, and simplify access in the pipeline\n> handler.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> This simplification will preclude indexing the list of devices with a\n> compound key. For instance, if we were to extend the simple pipeline\n> handler to match not only on a driver name but on a (driver, driver\n> version) pair, the current configuration file structure would allow\n> expressing this with\n> \n> pipelines:\n> simple:\n> devices:\n> - driver: mxc-isi\n> \t driver_version: 1\n> \t software_isp: true\n> - driver: mxc-isi\n> \t driver_version: 2\n> \t software_isp: false\n> \n> The new structure would conceptually allow us to write\n> \n> pipelines:\n> simple:\n> devices:\n> ? driver: mxc-isi\n> \t driver_version: 1\n> \t: software_isp: true\n> ? driver: mxc-isi\n> \t driver_version: 2\n> \t: software_isp: false\n> \n> For those not familiar with the explicit syntax for mappings, this would\n> translate to the following Python data if dict objects were hashable in\n> Python:\n> \n> {\n> 'pipelines': {\n> 'simple': {\n> \t 'devices': {\n> \t { 'driver': 'mxc-isi', 'driver_version': 1 }: {\n> \t\t 'software_isp': True,\n> \t\t},\n> \t { 'driver': 'mxc-isi', 'driver_version': 2 }: {\n> \t\t 'software_isp': False,\n> \t\t},\n> \t },\n> \t},\n> },\n> }\n> \n> Yes, YAML can use complex data as keys in mappings. I would really not\n> want to have to support that in libcamera though.\n\nGenerally I feel like adopting a different approach would be more flexible.\nSpecifically, the the user could define a list of rules that have associated\nconditions and actions. For example:\n\n - if:\n driver: mxc-isi\n driver_version: 1\n then:\n software_isp: true\n - if:\n driver: mxc-isi\n driver_version: '>=2'\n then:\n software_isp: false\n\nthen these rules are evaluated in order of appearance to get the final list of properties/options\nfor the given device. I believe this is easier to handle in a generic way than compound keys and\nit allows one to support e.g. regex matching, number ranges, etc. should the need arise.\n\n\n> \n> Opinions will be appreciated.\n> ---\n> Documentation/runtime_configuration.rst | 6 +++---\n> src/libcamera/pipeline/simple/simple.cpp | 20 +++++++++-----------\n> 2 files changed, 12 insertions(+), 14 deletions(-)\n> \n> diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst\n> index 19c2309ac94f..a904b09a1d35 100644\n> --- a/Documentation/runtime_configuration.rst\n> +++ b/Documentation/runtime_configuration.rst\n> @@ -44,7 +44,7 @@ file structure:\n> pipelines:\n> simple:\n> devices:\n> - - driver: # driver name, e.g. `mxc-isi`\n> + # driver name, e.g. `mxc-isi`:\n> software_isp: # true/false\n> software_isp:\n> copy_input_buffer: # true/false\n> @@ -77,7 +77,7 @@ Configuration file example\n> pipelines:\n> simple:\n> devices:\n> - - driver: mxc-isi\n> + mxc-isi:\n> software_isp: true\n> software_isp:\n> copy_input_buffer: false\n> @@ -139,7 +139,7 @@ LIBCAMERA__TUNING_FILE\n> \n> Example value: ``/usr/local/share/libcamera/ipa/rpi/vc4/custom_sensor.json``\n> \n> -pipelines.simple.devices.driver, pipelines.simple.devices.software_isp\n> +pipelines.simple.devices..software_isp\n> Override whether software ISP is enabled for the given driver.\n> \n> Example `driver` value: ``mxc-isi``\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 0ae9e081f01a..c949f3a69d16 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -1879,18 +1879,16 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr media,\n> \t}\n> \n> \tswIspEnabled_ = info.swIspEnabled;\n> +\n> \tconst GlobalConfiguration &configuration = cameraManager()->_d()->configuration();\n> -\tfor (GlobalConfiguration::Option entry :\n> -\t configuration.configuration()[\"pipelines\"][\"simple\"][\"devices\"]\n> -\t\t .asList()) {\n> -\t\tauto name = entry[\"driver\"].get();\n> -\t\tif (name == info.driver) {\n> -\t\t\tswIspEnabled_ = entry[\"software_isp\"].get().value_or(swIspEnabled_);\n> -\t\t\tLOG(SimplePipeline, Debug)\n> -\t\t\t\t<< \"Configuration file overrides software ISP for \"\n> -\t\t\t\t<< info.driver << \" to \" << swIspEnabled_;\n> -\t\t\tbreak;\n> -\t\t}\n> +\tGlobalConfiguration::Option &cfg =\n> +\t\tconfiguration.configuration()[\"pipelines\"][\"simple\"][\"devices\"][info.driver];\n> +\n> +\tif (cfg) {\n> +\t\tswIspEnabled_ = cfg[\"software_isp\"].get().value_or(swIspEnabled_);\n> +\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t<< \"Configuration file overrides software ISP for \"\n> +\t\t\t<< info.driver << \" to \" << swIspEnabled_;\n\nMaybe\n\n if (auto x = cfg[\"software_isp\"].get())\n\ncould be used in the condition. Since the current and proposed versions both print the message\nabout the override even if no override actually happens (due to missing/malformed `sofware_isp` key).\n\nReviewed-by: Barnabás Pőcze \n\n\n> \t}\n> \n> \t/* Locate the sensors. */","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 E7305BDCBF\n\tfor ;\n\tWed, 14 Jan 2026 09:33:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 313F461FA3;\n\tWed, 14 Jan 2026 10:33:50 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D1441615B2\n\tfor ;\n\tWed, 14 Jan 2026 10:33:47 +0100 (CET)","from [192.168.33.18] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 211F7316;\n\tWed, 14 Jan 2026 10:33:21 +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=\"brF+Vxbs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768383201;\n\tbh=a4qYZrsAQ18z0RNWQTMDKYqFZkIqzVAWMACNfUXRDx8=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=brF+Vxbs9QkDtBTUSBxQE7+pCqNGJrtD2OjnwGaxXx2N3IuVMUswAhDkA21JBF3Dt\n\tccSMSmjQNGgvVeIAef1TF0C8MKTmogxO48F2Zpt7lb+2wLkPUmWinAQaYybQAnIKiy\n\tS/X78mUXdTCRSJEJ1Z7YbbFxDoD/7t37xjEmYni0=","Message-ID":"<6d507c3d-248f-4c4d-bc57-f8dd099efade@ideasonboard.com>","Date":"Wed, 14 Jan 2026 10:33:44 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 36/36] pipeline: simple: Turn devices configuration\n\toption into dictionary","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-37-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260113000808.15395-37-laurent.pinchart@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":37713,"web_url":"https://patchwork.libcamera.org/comment/37713/","msgid":"<20260118231445.GB27500@pendragon.ideasonboard.com>","date":"2026-01-18T23:14:45","subject":"Re: [PATCH 36/36] pipeline: simple: Turn devices configuration\n\toption into dictionary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jan 14, 2026 at 10:33:44AM +0100, Barnabás Pőcze wrote:\n> 2026. 01. 13. 1:08 keltezéssel, Laurent Pinchart írta:\n> > The pipelines.simple.devices configuration option contains a list of\n> > devices, each of them being a dictionary with a \"driver\" element that\n> > acts as a unique key to index the list. Turn it into a YAML dictionary\n> > to ensure uniqueness of the key, and simplify access in the pipeline\n> > handler.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > This simplification will preclude indexing the list of devices with a\n> > compound key. For instance, if we were to extend the simple pipeline\n> > handler to match not only on a driver name but on a (driver, driver\n> > version) pair, the current configuration file structure would allow\n> > expressing this with\n> > \n> > pipelines:\n> > simple:\n> > devices:\n> > - driver: mxc-isi\n> > driver_version: 1\n> > software_isp: true\n> > - driver: mxc-isi\n> > driver_version: 2\n> > software_isp: false\n> > \n> > The new structure would conceptually allow us to write\n> > \n> > pipelines:\n> > simple:\n> > devices:\n> > ? driver: mxc-isi\n> > driver_version: 1\n> > : software_isp: true\n> > ? driver: mxc-isi\n> > driver_version: 2\n> > : software_isp: false\n> > \n> > For those not familiar with the explicit syntax for mappings, this would\n> > translate to the following Python data if dict objects were hashable in\n> > Python:\n> > \n> > {\n> > 'pipelines': {\n> > 'simple': {\n> > 'devices': {\n> > { 'driver': 'mxc-isi', 'driver_version': 1 }: {\n> > 'software_isp': True,\n> > },\n> > { 'driver': 'mxc-isi', 'driver_version': 2 }: {\n> > 'software_isp': False,\n> > },\n> > },\n> > },\n> > },\n> > }\n> > \n> > Yes, YAML can use complex data as keys in mappings. I would really not\n> > want to have to support that in libcamera though.\n> \n> Generally I feel like adopting a different approach would be more flexible.\n> Specifically, the the user could define a list of rules that have associated\n> conditions and actions. For example:\n> \n> - if:\n> driver: mxc-isi\n> driver_version: 1\n> then:\n> software_isp: true\n> - if:\n> driver: mxc-isi\n> driver_version: '>=2'\n> then:\n> software_isp: false\n> \n> then these rules are evaluated in order of appearance to get the final list of properties/options\n> for the given device. I believe this is easier to handle in a generic way than compound keys and\n> it allows one to support e.g. regex matching, number ranges, etc. should the need arise.\n\nCompound keys is definitely not something I would want to implement, the\ncomplexity is just not worth it. Rules would be easier, but they would\nstill require quite a bit of implementation effort. Given your R-b tag\nbelow I assume you're not recommending to implement this right now.\n\n> > \n> > Opinions will be appreciated.\n> > ---\n> > Documentation/runtime_configuration.rst | 6 +++---\n> > src/libcamera/pipeline/simple/simple.cpp | 20 +++++++++-----------\n> > 2 files changed, 12 insertions(+), 14 deletions(-)\n> > \n> > diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst\n> > index 19c2309ac94f..a904b09a1d35 100644\n> > --- a/Documentation/runtime_configuration.rst\n> > +++ b/Documentation/runtime_configuration.rst\n> > @@ -44,7 +44,7 @@ file structure:\n> > pipelines:\n> > simple:\n> > devices:\n> > - - driver: # driver name, e.g. `mxc-isi`\n> > + # driver name, e.g. `mxc-isi`:\n> > software_isp: # true/false\n> > software_isp:\n> > copy_input_buffer: # true/false\n> > @@ -77,7 +77,7 @@ Configuration file example\n> > pipelines:\n> > simple:\n> > devices:\n> > - - driver: mxc-isi\n> > + mxc-isi:\n> > software_isp: true\n> > software_isp:\n> > copy_input_buffer: false\n> > @@ -139,7 +139,7 @@ LIBCAMERA__TUNING_FILE\n> > \n> > Example value: ``/usr/local/share/libcamera/ipa/rpi/vc4/custom_sensor.json``\n> > \n> > -pipelines.simple.devices.driver, pipelines.simple.devices.software_isp\n> > +pipelines.simple.devices..software_isp\n> > Override whether software ISP is enabled for the given driver.\n> > \n> > Example `driver` value: ``mxc-isi``\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 0ae9e081f01a..c949f3a69d16 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -1879,18 +1879,16 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr media,\n> > \t}\n> > \n> > \tswIspEnabled_ = info.swIspEnabled;\n> > +\n> > \tconst GlobalConfiguration &configuration = cameraManager()->_d()->configuration();\n> > -\tfor (GlobalConfiguration::Option entry :\n> > -\t configuration.configuration()[\"pipelines\"][\"simple\"][\"devices\"]\n> > -\t\t .asList()) {\n> > -\t\tauto name = entry[\"driver\"].get();\n> > -\t\tif (name == info.driver) {\n> > -\t\t\tswIspEnabled_ = entry[\"software_isp\"].get().value_or(swIspEnabled_);\n> > -\t\t\tLOG(SimplePipeline, Debug)\n> > -\t\t\t\t<< \"Configuration file overrides software ISP for \"\n> > -\t\t\t\t<< info.driver << \" to \" << swIspEnabled_;\n> > -\t\t\tbreak;\n> > -\t\t}\n> > +\tGlobalConfiguration::Option &cfg =\n> > +\t\tconfiguration.configuration()[\"pipelines\"][\"simple\"][\"devices\"][info.driver];\n> > +\n> > +\tif (cfg) {\n> > +\t\tswIspEnabled_ = cfg[\"software_isp\"].get().value_or(swIspEnabled_);\n> > +\t\tLOG(SimplePipeline, Debug)\n> > +\t\t\t<< \"Configuration file overrides software ISP for \"\n> > +\t\t\t<< info.driver << \" to \" << swIspEnabled_;\n> \n> Maybe\n> \n> if (auto x = cfg[\"software_isp\"].get())\n> \n> could be used in the condition. Since the current and proposed versions both print the message\n> about the override even if no override actually happens (due to missing/malformed `sofware_isp` key).\n\nGood point. I'll fix it.\n\n> Reviewed-by: Barnabás Pőcze \n> \n> > \t}\n> > \n> > \t/* Locate the sensors. */","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 53F97BDCBE\n\tfor ;\n\tSun, 18 Jan 2026 23:15:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8928961FC3;\n\tMon, 19 Jan 2026 00:15:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27CB361F61\n\tfor ;\n\tMon, 19 Jan 2026 00:15:11 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id CDD2CC1;\n\tMon, 19 Jan 2026 00:14:40 +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=\"pKpJfcqK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768778081;\n\tbh=509eKnO0Or+qVZypegiSC4HckUYo5TPT/qS7f2TGVVc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pKpJfcqKSpPub+00XpQemoGlZxNwTrog7oM+9qObJhq2i9hwJKPtlLjsY86E154va\n\tSSYhy1RiuiCTyZam1qHfywEoXcOqFTMIehtpytpNKeB9DyTDae1zjMGtTGQ+zeozrD\n\t8zJ1Rg3S3w/AGdM6gbsyIKUP5OdqMfuMjd+caY78=","Date":"Mon, 19 Jan 2026 01:14:45 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 36/36] pipeline: simple: Turn devices configuration\n\toption into dictionary","Message-ID":"<20260118231445.GB27500@pendragon.ideasonboard.com>","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-37-laurent.pinchart@ideasonboard.com>\n\t<6d507c3d-248f-4c4d-bc57-f8dd099efade@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<6d507c3d-248f-4c4d-bc57-f8dd099efade@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\" "}}]