[{"id":35273,"web_url":"https://patchwork.libcamera.org/comment/35273/","msgid":"","date":"2025-08-04T16:51:24","subject":"Re: [PATCH v16 00/12] Add global configuration 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. 07. 29. 9:31 keltezéssel, Milan Zamazal írta:\n> This patch series introduces global configuration file for libcamera, to\n> provide runtime configuration means other than environment variables.\n> Instead of, or in addition to, the growing list of configuration\n> environment variables, the whole configuration can be specified in a\n> single configuration file. This is both simpler and more flexible.\n> \n> This is not a replacement for specific configuration files already\n> present in libcamera, with the exception of rpi configuration file (see\n> “Look up rpi configuration in the configuration file” patch).\n> \n> The patches implement what is needed to introduce a configuration file\n> that can handle the current environment variables and software ISP\n> TODOs. They demonstrate how to deal with the key points that must be\n> considered. Logging is omitted from the configuration file for\n> technical reasons. See commit messages for more details.\n> \n> The configuration file is a YAML file. It is looked up in user’s home\n> directory or, if not present, in system-wide libcamera directories.\n> Both the directories and file name of the configuration file can be\n> overridden using newly introduced environment variables.\n> \n> The current configuration environment variables are still supported and\n> take precedence, if defined, over the corresponding entries in the YAML\n> file.\n> \n> This patch series is not exhaustive, there can be future enhancements,\n> most notably configuration file validation to avoid confusions caused by\n> typos etc.\n> \n> Not everything has been tested because some of the patches are related\n> to specific hardware.\n\nI think this is ready to be merged for the most part, but it certainly needs more review\nand probably a bit more testing. I feel like I have been looking at it for too long.\n\nI also think the exact nature of how configuration is distributed to the particular\ncomponents is a question (for later). Currently everything accesses the `GlobalConfiguration`\nobject, and finds the data by its absolute path. I believe using relative paths would\nintroduce less coupling between the parts.\n\nAnother interesting question is how far from the pipeline handler should things stop\nusing the configuration file, and instead take their parameters in native types via\nconstructors, etc. For example, I think that `DebayerCpu` should not be accessing the\nconfiguration file directly, e.g. patch 10/12 \"libcamera: software_isp: Make measurement configurable\"\nintroduces too strong of a coupling between the simple pipeline handler and the\n`DebayerCpu` class in my opinion.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Changes in v16:\n> - Non-code fixes suggested by Barnabás.\n> \n> Changes in v15:\n> - Fixes to reading configuration files suggested by Barnabás.\n> - The outdated example in the documentation fixed.\n> \n> Changes in v14:\n> - Unused expectedTarget variable removed.\n> - Old `ipas' examples in examples removed.\n> - IPAProxy::{configPaths_,execPaths_} have their direct types now,\n> without std::optional.\n> \n> Changes in v13:\n> - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is\n> loaded.\n> - Standard user configuration directory has no longer precedence over\n> LIBCAMERA_CONFIG_DIR.\n> - Improvements to the rpi code suggested by Barnabás.\n> - Documentation fixes suggested by Barnabás.\n> - Make sure yamlConfiguration_ is initialized.\n> \n> Changes in v12:\n> - The commit message of \"Make GlobalConfiguration instance\" updated\n> according to the recent discussions.\n> - Use initializer_list rather than a string with separators for\n> GlobalConfiguration option path arguments.\n> - char *const qualifiers dropped.\n> - rpi can have separate configurations for different targets.\n> - GlobalConfiguration::option template is not type limited anymore.\n> - utils::join used instead of manual string concatenation.\n> - envListOption and listOption return std::optional now.\n> - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped.\n> - Isolation configuration stored to IPAManager to avoid passing the\n> whole configuraton to isSignatureValid.\n> - Similarly to paths in IPAProxy.\n> - `delimiter' argument added to GlobalConfiguration::envListOption and\n> used for LIBCAMERA_PIPELINES_MATCH_LIST processing.\n> - Configuration no longer looked up in /etc/libcamera and additionally\n> looked up in LIBCAMERA_DATA_DIR.\n> - Cosmetic changes suggested by Barnabás.\n> \n> Changes in v11:\n> - global_configuration.cpp moved out of base and yaml_parser.cpp left in\n> its original place.\n> - Configuration initialized as empty if there is no configuration file,\n> preventing a segfault on later access.\n> - Cosmetic changes suggested by Laurent.\n> - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly.\n> - Failing to read an existent configuration file is an error, rather\n> than warning, now. (It should be also fatal, but how to propagate it?)\n> - Slash is used instead of dot to separate configuration names.\n> - rpi configuration read is in the global configuration directly (unless\n> overridden by LIBCAMERA_RPI_CONFIG_FILE) now.\n> \n> Changes in v10:\n> - Minor improvements suggested by Barnabás.\n> \n> Changes in v9:\n> - The configuration instance is now stored in CameraManager and accessed\n> from there rather than being a separate singleton wrapped by global\n> accessors. This solves the ugly problem of delayed initialization but\n> I don’t like anything else about it. I played a bit with the idea of\n> attaching it to Logger instead, see the commit message of patch 03 for\n> some discussion, but stayed with the CameraManager proposal\n> eventually.\n> - I’ve given up on the logging configuration now when the configuration\n> is stored in CameraManager and removed the corresponding patches. I\n> think it’s possible to add it later, but for now, it’s already\n> complicated enough.\n> - Not much tested, let’s see first if the current implementation gets in\n> an acceptable direction.\n> \n> Changes in v8:\n> - Rebased on current master.\n> - Anniversary edition: 400 days since v1 posted. ;-)\n> \n> Changes in v7:\n> - Rebased on current master.\n> - Tuning file path configuration updated for recent changes.\n> A significant change is that the tuning file configuration is no\n> longer sensor dependent as there is apparently no access to the sensor\n> info in the IPA proxy.\n> - Minor improvements of some commit messages.\n> \n> Changes in v6:\n> - Rebased on master.\n> - File names from file header descriptions removed.\n> - Indentation fix in moved code as requested by checkstyle.py.\n> - Unneeded const_cast's removed.\n> - Using GlobalConfiguration namespace rather than a class.\n> - Path configuration options are defined as sequences in YAML.\n> - A patch introducing LIBCAMERA_CONFIG_NAME added.\n> - A patch introducing LIBCAMERA_CONFIG_DIR added.\n> - Miscellaneous minor code changes suggested by Barnabás.\n> \n> Changes in v5:\n> - A pointer is used to store the global configuration singleton rather\n> than a static variable. This makes the things more robust and fixes\n> the problem with re-entrancy on logging and a failing\n> camera_reconfigure test.\n> - In relation to the change above, a new initialization method\n> GlobalConfiguration::initialize() was introduced that replaces the\n> initialization calls in CameraManager and IPAManager.\n> - Logging YAML errors when reading the configuration was also fixed.\n> - Global configuration is placed to base directly, without an\n> intermediate patch.\n> - An `optional' value comparison simplified.\n> - A temporary typo in a comment fixed.\n> - Unused stdint.h include removed.\n> \n> Changes in v4:\n> - Rebased on current master.\n> - Configuration option for LIBCAMERA_IPA_PROXY_PATH added.\n> - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp.\n> \n> Changes in v3:\n> - Added a configuration item for the newly introduced\n> LIBCAMERA_PIPELINES_MATCH_LIST variable.\n> - A minor indentation fix.\n> - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch.\n> - Tuning files are looked up now by particular cameras attached rather than\n> being specified for the whole pipeline.\n> - Helpers use std::string& instead of char* for confPath arguments.\n> - Protection against returning YamlObject::empty as a regular value (the\n> problem has been probably exposed by addition of\n> LIBCAMERA_PIPELINES_MATCH_LIST).\n> \n> Changes in v2:\n> - Rebased on master.\n> - Various cleanups, documentation improvements and minor fixes.\n> - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush).\n> - Two more patches for software ISP configuration added.\n> \n> Milan Zamazal (12):\n> config: Introduce global runtime configuration\n> config: Make GlobalConfiguration instance\n> config: Add configuration retrieval helpers\n> config: Look up rpi configuration in the configuration file\n> config: Look up IPA configurables in configuration file\n> config: Look up pipelines match list in configuration file\n> config: Allow enabling software ISP in runtime\n> config: Add global configuration file documentation\n> libcamera: software_isp: Make input buffer copying configurable\n> libcamera: software_isp: Make measurement configurable\n> config: Make configuration file configurable\n> config: Make configuration directories configurable\n> \n> Documentation/documentation-contents.rst | 2 +-\n> Documentation/index.rst | 2 +-\n> Documentation/meson.build | 2 +-\n> ...ariables.rst => runtime_configuration.rst} | 126 ++++++++-\n> include/libcamera/internal/camera_manager.h | 8 +\n> .../libcamera/internal/global_configuration.h | 62 +++++\n> include/libcamera/internal/ipa_manager.h | 7 +-\n> include/libcamera/internal/ipa_proxy.h | 8 +-\n> include/libcamera/internal/meson.build | 1 +\n> src/libcamera/camera_manager.cpp | 20 +-\n> src/libcamera/global_configuration.cpp | 258 ++++++++++++++++++\n> src/libcamera/ipa_manager.cpp | 39 ++-\n> src/libcamera/ipa_proxy.cpp | 51 ++--\n> src/libcamera/meson.build | 1 +\n> .../pipeline/rpi/common/pipeline_base.cpp | 59 ++--\n> .../pipeline/rpi/common/pipeline_base.h | 3 +-\n> src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +-\n> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +-\n> src/libcamera/pipeline/simple/simple.cpp | 13 +\n> src/libcamera/software_isp/TODO | 36 ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 32 ++-\n> src/libcamera/software_isp/debayer_cpu.h | 10 +-\n> src/libcamera/software_isp/software_isp.cpp | 3 +-\n> .../module_ipa_proxy.cpp.tmpl | 4 +-\n> .../module_ipa_proxy.h.tmpl | 2 +-\n> 25 files changed, 625 insertions(+), 176 deletions(-)\n> rename Documentation/{environment_variables.rst => runtime_configuration.rst} (60%)\n> create mode 100644 include/libcamera/internal/global_configuration.h\n> create mode 100644 src/libcamera/global_configuration.cpp\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 D0468BE086\n\tfor ;\n\tMon, 4 Aug 2025 16:51:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2309E6921A;\n\tMon, 4 Aug 2025 18:51:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AB9C269217\n\tfor ;\n\tMon, 4 Aug 2025 18:51:28 +0200 (CEST)","from [192.168.33.15] (185.182.215.213.nat.pool.zt.hu\n\t[185.182.215.213])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 40B60C6D;\n\tMon, 4 Aug 2025 18:50:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KZoUdbEZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1754326241;\n\tbh=tMi0XzCHLlbUqo1naIKR9VyBnztICnbV0sRcPDFlBnk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=KZoUdbEZghxX9raaoEjG8iy1vgUy7OUViRv/tJKNjOFyMQnQEoCOm+IGZBKh6VkiO\n\tWWCC6HO6qaUNkIC2+TSDrvK/lm9zO+AYhzOsDQ2ub+PYLSjbYB02tF4IK2QVfz4/Vu\n\tK6fF8NP4Fweh34nHNt5FrEc715flrcwT8re2XQas=","Message-ID":"","Date":"Mon, 4 Aug 2025 18:51:24 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v16 00/12] Add global configuration file","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ,\n\tLaurent Pinchart ","References":"<20250729073201.5369-1-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250729073201.5369-1-mzamazal@redhat.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":35275,"web_url":"https://patchwork.libcamera.org/comment/35275/","msgid":"<85jz3iyjsg.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-08-04T19:21:51","subject":"Re: [PATCH v16 00/12] Add global configuration file","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Barnabás,\n\nBarnabás Pőcze writes:\n\n> Hi\n>\n> 2025. 07. 29. 9:31 keltezéssel, Milan Zamazal írta:\n>> This patch series introduces global configuration file for libcamera, to\n>> provide runtime configuration means other than environment variables.\n>> Instead of, or in addition to, the growing list of configuration\n>> environment variables, the whole configuration can be specified in a\n>> single configuration file. This is both simpler and more flexible.\n>> This is not a replacement for specific configuration files already\n>> present in libcamera, with the exception of rpi configuration file (see\n>> “Look up rpi configuration in the configuration file” patch).\n>> The patches implement what is needed to introduce a configuration file\n>> that can handle the current environment variables and software ISP\n>> TODOs. They demonstrate how to deal with the key points that must be\n>> considered. Logging is omitted from the configuration file for\n>> technical reasons. See commit messages for more details.\n>> The configuration file is a YAML file. It is looked up in user’s home\n>> directory or, if not present, in system-wide libcamera directories.\n>> Both the directories and file name of the configuration file can be\n>> overridden using newly introduced environment variables.\n>> The current configuration environment variables are still supported and\n>> take precedence, if defined, over the corresponding entries in the YAML\n>> file.\n>> This patch series is not exhaustive, there can be future enhancements,\n>> most notably configuration file validation to avoid confusions caused by\n>> typos etc.\n>> Not everything has been tested because some of the patches are related\n>> to specific hardware.\n>\n> I think this is ready to be merged for the most part, but it certainly needs more review\n> and probably a bit more testing.\n\nYes.\n\n> I feel like I have been looking at it for too long.\n\nI understand, the same here.\n\nThank you for your reviews and helping move the patches forward.\n\n> I also think the exact nature of how configuration is distributed to the particular\n> components is a question (for later). Currently everything accesses the `GlobalConfiguration`\n> object, and finds the data by its absolute path. I believe using relative paths would\n> introduce less coupling between the parts.\n>\n> Another interesting question is how far from the pipeline handler should things stop\n> using the configuration file, and instead take their parameters in native types via\n> constructors, etc. For example, I think that `DebayerCpu` should not be accessing the\n> configuration file directly, e.g. patch 10/12 \"libcamera: software_isp: Make measurement configurable\"\n> introduces too strong of a coupling between the simple pipeline handler and the\n> `DebayerCpu` class in my opinion.\n\nThese may be valid points but they can be implemented later. From my\nperspective, the most important things at the moment are:\n\n- Not introducing regressions.\n\n- Agreeing on the configuration file structure so that it needn't be\n changed any time soon, which would be annoying for users.\n\n> Regards,\n> Barnabás Pőcze\n>\n>> Changes in v16:\n>> - Non-code fixes suggested by Barnabás.\n>> Changes in v15:\n>> - Fixes to reading configuration files suggested by Barnabás.\n>> - The outdated example in the documentation fixed.\n>> Changes in v14:\n>> - Unused expectedTarget variable removed.\n>> - Old `ipas' examples in examples removed.\n>> - IPAProxy::{configPaths_,execPaths_} have their direct types now,\n>> without std::optional.\n>> Changes in v13:\n>> - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is\n>> loaded.\n>> - Standard user configuration directory has no longer precedence over\n>> LIBCAMERA_CONFIG_DIR.\n>> - Improvements to the rpi code suggested by Barnabás.\n>> - Documentation fixes suggested by Barnabás.\n>> - Make sure yamlConfiguration_ is initialized.\n>> Changes in v12:\n>> - The commit message of \"Make GlobalConfiguration instance\" updated\n>> according to the recent discussions.\n>> - Use initializer_list rather than a string with separators for\n>> GlobalConfiguration option path arguments.\n>> - char *const qualifiers dropped.\n>> - rpi can have separate configurations for different targets.\n>> - GlobalConfiguration::option template is not type limited anymore.\n>> - utils::join used instead of manual string concatenation.\n>> - envListOption and listOption return std::optional now.\n>> - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped.\n>> - Isolation configuration stored to IPAManager to avoid passing the\n>> whole configuraton to isSignatureValid.\n>> - Similarly to paths in IPAProxy.\n>> - `delimiter' argument added to GlobalConfiguration::envListOption and\n>> used for LIBCAMERA_PIPELINES_MATCH_LIST processing.\n>> - Configuration no longer looked up in /etc/libcamera and additionally\n>> looked up in LIBCAMERA_DATA_DIR.\n>> - Cosmetic changes suggested by Barnabás.\n>> Changes in v11:\n>> - global_configuration.cpp moved out of base and yaml_parser.cpp left in\n>> its original place.\n>> - Configuration initialized as empty if there is no configuration file,\n>> preventing a segfault on later access.\n>> - Cosmetic changes suggested by Laurent.\n>> - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly.\n>> - Failing to read an existent configuration file is an error, rather\n>> than warning, now. (It should be also fatal, but how to propagate it?)\n>> - Slash is used instead of dot to separate configuration names.\n>> - rpi configuration read is in the global configuration directly (unless\n>> overridden by LIBCAMERA_RPI_CONFIG_FILE) now.\n>> Changes in v10:\n>> - Minor improvements suggested by Barnabás.\n>> Changes in v9:\n>> - The configuration instance is now stored in CameraManager and accessed\n>> from there rather than being a separate singleton wrapped by global\n>> accessors. This solves the ugly problem of delayed initialization but\n>> I don’t like anything else about it. I played a bit with the idea of\n>> attaching it to Logger instead, see the commit message of patch 03 for\n>> some discussion, but stayed with the CameraManager proposal\n>> eventually.\n>> - I’ve given up on the logging configuration now when the configuration\n>> is stored in CameraManager and removed the corresponding patches. I\n>> think it’s possible to add it later, but for now, it’s already\n>> complicated enough.\n>> - Not much tested, let’s see first if the current implementation gets in\n>> an acceptable direction.\n>> Changes in v8:\n>> - Rebased on current master.\n>> - Anniversary edition: 400 days since v1 posted. ;-)\n>> Changes in v7:\n>> - Rebased on current master.\n>> - Tuning file path configuration updated for recent changes.\n>> A significant change is that the tuning file configuration is no\n>> longer sensor dependent as there is apparently no access to the sensor\n>> info in the IPA proxy.\n>> - Minor improvements of some commit messages.\n>> Changes in v6:\n>> - Rebased on master.\n>> - File names from file header descriptions removed.\n>> - Indentation fix in moved code as requested by checkstyle.py.\n>> - Unneeded const_cast's removed.\n>> - Using GlobalConfiguration namespace rather than a class.\n>> - Path configuration options are defined as sequences in YAML.\n>> - A patch introducing LIBCAMERA_CONFIG_NAME added.\n>> - A patch introducing LIBCAMERA_CONFIG_DIR added.\n>> - Miscellaneous minor code changes suggested by Barnabás.\n>> Changes in v5:\n>> - A pointer is used to store the global configuration singleton rather\n>> than a static variable. This makes the things more robust and fixes\n>> the problem with re-entrancy on logging and a failing\n>> camera_reconfigure test.\n>> - In relation to the change above, a new initialization method\n>> GlobalConfiguration::initialize() was introduced that replaces the\n>> initialization calls in CameraManager and IPAManager.\n>> - Logging YAML errors when reading the configuration was also fixed.\n>> - Global configuration is placed to base directly, without an\n>> intermediate patch.\n>> - An `optional' value comparison simplified.\n>> - A temporary typo in a comment fixed.\n>> - Unused stdint.h include removed.\n>> Changes in v4:\n>> - Rebased on current master.\n>> - Configuration option for LIBCAMERA_IPA_PROXY_PATH added.\n>> - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp.\n>> Changes in v3:\n>> - Added a configuration item for the newly introduced\n>> LIBCAMERA_PIPELINES_MATCH_LIST variable.\n>> - A minor indentation fix.\n>> - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch.\n>> - Tuning files are looked up now by particular cameras attached rather than\n>> being specified for the whole pipeline.\n>> - Helpers use std::string& instead of char* for confPath arguments.\n>> - Protection against returning YamlObject::empty as a regular value (the\n>> problem has been probably exposed by addition of\n>> LIBCAMERA_PIPELINES_MATCH_LIST).\n>> Changes in v2:\n>> - Rebased on master.\n>> - Various cleanups, documentation improvements and minor fixes.\n>> - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush).\n>> - Two more patches for software ISP configuration added.\n>> Milan Zamazal (12):\n>> config: Introduce global runtime configuration\n>> config: Make GlobalConfiguration instance\n>> config: Add configuration retrieval helpers\n>> config: Look up rpi configuration in the configuration file\n>> config: Look up IPA configurables in configuration file\n>> config: Look up pipelines match list in configuration file\n>> config: Allow enabling software ISP in runtime\n>> config: Add global configuration file documentation\n>> libcamera: software_isp: Make input buffer copying configurable\n>> libcamera: software_isp: Make measurement configurable\n>> config: Make configuration file configurable\n>> config: Make configuration directories configurable\n>> Documentation/documentation-contents.rst | 2 +-\n>> Documentation/index.rst | 2 +-\n>> Documentation/meson.build | 2 +-\n>> ...ariables.rst => runtime_configuration.rst} | 126 ++++++++-\n>> include/libcamera/internal/camera_manager.h | 8 +\n>> .../libcamera/internal/global_configuration.h | 62 +++++\n>> include/libcamera/internal/ipa_manager.h | 7 +-\n>> include/libcamera/internal/ipa_proxy.h | 8 +-\n>> include/libcamera/internal/meson.build | 1 +\n>> src/libcamera/camera_manager.cpp | 20 +-\n>> src/libcamera/global_configuration.cpp | 258 ++++++++++++++++++\n>> src/libcamera/ipa_manager.cpp | 39 ++-\n>> src/libcamera/ipa_proxy.cpp | 51 ++--\n>> src/libcamera/meson.build | 1 +\n>> .../pipeline/rpi/common/pipeline_base.cpp | 59 ++--\n>> .../pipeline/rpi/common/pipeline_base.h | 3 +-\n>> src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +-\n>> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +-\n>> src/libcamera/pipeline/simple/simple.cpp | 13 +\n>> src/libcamera/software_isp/TODO | 36 ---\n>> src/libcamera/software_isp/debayer_cpu.cpp | 32 ++-\n>> src/libcamera/software_isp/debayer_cpu.h | 10 +-\n>> src/libcamera/software_isp/software_isp.cpp | 3 +-\n>> .../module_ipa_proxy.cpp.tmpl | 4 +-\n>> .../module_ipa_proxy.h.tmpl | 2 +-\n>> 25 files changed, 625 insertions(+), 176 deletions(-)\n>> rename Documentation/{environment_variables.rst => runtime_configuration.rst} (60%)\n>> create mode 100644 include/libcamera/internal/global_configuration.h\n>> create mode 100644 src/libcamera/global_configuration.cpp\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 23D0BBE086\n\tfor ;\n\tMon, 4 Aug 2025 19:22:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DACC6921D;\n\tMon, 4 Aug 2025 21:22:00 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 350D36920D\n\tfor ;\n\tMon, 4 Aug 2025 21:21:58 +0200 (CEST)","from mail-qt1-f200.google.com (mail-qt1-f200.google.com\n\t[209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-625-UQiJ3YIDP1GKOs4ryAch3g-1; Mon, 04 Aug 2025 15:21:55 -0400","by mail-qt1-f200.google.com with SMTP id\n\td75a77b69052e-4af019842d1so66852551cf.0\n\tfor ;\n\tMon, 04 Aug 2025 12:21:55 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\td75a77b69052e-4aeeedbaa3asm56064261cf.53.2025.08.04.12.21.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 04 Aug 2025 12:21:53 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Ov1NM/JS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1754335317;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=tSn55XdYz6sOLfO9697ZZhohSg+1VAmiSF7Xs4ZmEaQ=;\n\tb=Ov1NM/JSgHdOSuo1yc1CXivUc/VW9ZG1i3wXUcl9GzTimHFqpOxbpKGB0iRYuceT1ctYn5\n\t0aKBtRAsasdggGe17ECDRAaohzzag29d4EV7FuwlAc4/DSLCgtgW3DRWP771BvyjhXLcBh\n\tcFUHQbadrkF/K9E9sOlEcXkumjXPCL0=","X-MC-Unique":"UQiJ3YIDP1GKOs4ryAch3g-1","X-Mimecast-MFC-AGG-ID":"UQiJ3YIDP1GKOs4ryAch3g_1754335315","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1754335315; x=1754940115;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=TvceMnAzs/MGWsJZzZuUszuJwlsIGoEzi91fDD4Vmks=;\n\tb=ukAEuquIDnfegRuSs15rVhxrVhPO9mBeNoZBNu1Hquh+hZ7ycQ0LHN6dAd8Vg//Hc3\n\tK/JL30vMCGGVACXNP+fYnHfQfg4/cI7w27Lvi8Zf12DKHvR+ocZhcVmndCqzSGCqf1wX\n\tD599PV/NCGUYXq/7E8CFs1hqTV59sG9zVJHSYqLVkvBN7V80SuC4B0nTEuLH+mzZzBh6\n\t7eEnw6cw6xBI1GMVGzM3dBO4ETGO4+Rnr06+x30QfArNCNo21+zf0ywwZQHTLeltmJUP\n\tgoqCW0+1RwXiqrkIEHdu4h7PkHk+zGRGqZ/gbR1iQcF4S3MNuu1IWcrxRPmDC/R9kLL0\n\tL3JQ==","X-Gm-Message-State":"AOJu0YxloyqIWChZKGg1DGpjhoHXQzAL/awF+GUzPqk0f1xw6SkCTzzl\n\tkDz2wK5/Ww7i6Vm0QQPM9cw8Z8LCxenztK7fsqS/k7DL9IwRcO7IViLQkdc6bGQbIYBJY2gvMij\n\tbhteVwCi9Lv+Y98pKASrFiiBnrhFPr1KlUL8RfRqgMTWntWvcmHvxmWoKdHnu14n4gw2+jgg1XB\n\t1n2tlHm9E=","X-Gm-Gg":"ASbGncv95eQSrlHyzC8lSrfNbPIP/bYJnUmn+IjUCfDgp+NI4AKyOKLJnC/OwwiXweU\n\tocaaUu55u/i82p//tQAuSCeHPKggdX71jsknUilcbiUYRH5J1yP1UD+/RttSNrj+777Vh+xbdqb\n\txPYjPOquNUmAv/mDdTIVGbOR2noThobokOng1PUluPx9RbgFsfrrsLKYR6WJQQeT6psSoEvR0Fe\n\tvB/Z0PpnmL32GdGAiM175UWidnPF3paqjJcy74lkoGq1n4GXHsKx8ww05vEYMxMhwuGdu82OYdn\n\tJab36NRiYpUhhr+fyZSUpUbBqfLVzH9d8bIbH6KFqvyxTCLvWxzxMWLl/XhHzREP6leXulIEjNQ\n\tiJ4BRNc674KoopdRT","X-Received":["by 2002:a05:622a:15c7:b0:4ab:c00c:250b with SMTP id\n\td75a77b69052e-4af10cf33f6mr157328181cf.40.1754335314616; \n\tMon, 04 Aug 2025 12:21:54 -0700 (PDT)","by 2002:a05:622a:15c7:b0:4ab:c00c:250b with SMTP id\n\td75a77b69052e-4af10cf33f6mr157327621cf.40.1754335313944; \n\tMon, 04 Aug 2025 12:21:53 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGefPBSsW8PjYeQPJyTWLQZx7bYO6h9UQBQBbgj4BlQl7CDd+PF6J6HEWIFZGdIBWIevS5VYA==","From":"Milan Zamazal ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, Laurent Pinchart\n\t","Subject":"Re: [PATCH v16 00/12] Add global configuration file","In-Reply-To":" (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Mon,\n\t4 Aug 2025 18:51:24 +0200\")","References":"<20250729073201.5369-1-mzamazal@redhat.com>\n\t","Date":"Mon, 04 Aug 2025 21:21:51 +0200","Message-ID":"<85jz3iyjsg.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"-PLKqOh0vNV9aCKguqhwvkXZsZfyaJmdNQtb1A_A7tQ_1754335315","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":35741,"web_url":"https://patchwork.libcamera.org/comment/35741/","msgid":"<175741092506.2127323.11668353838526167840@neptunite.rasen.tech>","date":"2025-09-09T09:42:05","subject":"Re: [PATCH v16 00/12] Add global configuration file","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-08-05 04:21:51)\n> Hi Barnabás,\n> \n> Barnabás Pőcze writes:\n> \n> > Hi\n> >\n> > 2025. 07. 29. 9:31 keltezéssel, Milan Zamazal írta:\n> >> This patch series introduces global configuration file for libcamera, to\n> >> provide runtime configuration means other than environment variables.\n> >> Instead of, or in addition to, the growing list of configuration\n> >> environment variables, the whole configuration can be specified in a\n> >> single configuration file. This is both simpler and more flexible.\n> >> This is not a replacement for specific configuration files already\n> >> present in libcamera, with the exception of rpi configuration file (see\n> >> “Look up rpi configuration in the configuration file” patch).\n> >> The patches implement what is needed to introduce a configuration file\n> >> that can handle the current environment variables and software ISP\n> >> TODOs. They demonstrate how to deal with the key points that must be\n> >> considered. Logging is omitted from the configuration file for\n> >> technical reasons. See commit messages for more details.\n> >> The configuration file is a YAML file. It is looked up in user’s home\n> >> directory or, if not present, in system-wide libcamera directories.\n> >> Both the directories and file name of the configuration file can be\n> >> overridden using newly introduced environment variables.\n> >> The current configuration environment variables are still supported and\n> >> take precedence, if defined, over the corresponding entries in the YAML\n> >> file.\n> >> This patch series is not exhaustive, there can be future enhancements,\n> >> most notably configuration file validation to avoid confusions caused by\n> >> typos etc.\n> >> Not everything has been tested because some of the patches are related\n> >> to specific hardware.\n> >\n> > I think this is ready to be merged for the most part, but it certainly needs more review\n> > and probably a bit more testing.\n> \n> Yes.\n> \n> > I feel like I have been looking at it for too long.\n> \n> I understand, the same here.\n> \n> Thank you for your reviews and helping move the patches forward.\n> \n> > I also think the exact nature of how configuration is distributed to the particular\n> > components is a question (for later). Currently everything accesses the `GlobalConfiguration`\n> > object, and finds the data by its absolute path. I believe using relative paths would\n> > introduce less coupling between the parts.\n> >\n> > Another interesting question is how far from the pipeline handler should things stop\n> > using the configuration file, and instead take their parameters in native types via\n> > constructors, etc. For example, I think that `DebayerCpu` should not be accessing the\n> > configuration file directly, e.g. patch 10/12 \"libcamera: software_isp: Make measurement configurable\"\n> > introduces too strong of a coupling between the simple pipeline handler and the\n> > `DebayerCpu` class in my opinion.\n\nOh good point, I missed that. Yeah I agree, maybe there should be a separate\ntop-level category for software_isp.\n\n\nPaul\n\n> \n> These may be valid points but they can be implemented later. From my\n> perspective, the most important things at the moment are:\n> \n> - Not introducing regressions.\n> \n> - Agreeing on the configuration file structure so that it needn't be\n> changed any time soon, which would be annoying for users.\n> \n> > Regards,\n> > Barnabás Pőcze\n> >\n> >> Changes in v16:\n> >> - Non-code fixes suggested by Barnabás.\n> >> Changes in v15:\n> >> - Fixes to reading configuration files suggested by Barnabás.\n> >> - The outdated example in the documentation fixed.\n> >> Changes in v14:\n> >> - Unused expectedTarget variable removed.\n> >> - Old `ipas' examples in examples removed.\n> >> - IPAProxy::{configPaths_,execPaths_} have their direct types now,\n> >> without std::optional.\n> >> Changes in v13:\n> >> - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is\n> >> loaded.\n> >> - Standard user configuration directory has no longer precedence over\n> >> LIBCAMERA_CONFIG_DIR.\n> >> - Improvements to the rpi code suggested by Barnabás.\n> >> - Documentation fixes suggested by Barnabás.\n> >> - Make sure yamlConfiguration_ is initialized.\n> >> Changes in v12:\n> >> - The commit message of \"Make GlobalConfiguration instance\" updated\n> >> according to the recent discussions.\n> >> - Use initializer_list rather than a string with separators for\n> >> GlobalConfiguration option path arguments.\n> >> - char *const qualifiers dropped.\n> >> - rpi can have separate configurations for different targets.\n> >> - GlobalConfiguration::option template is not type limited anymore.\n> >> - utils::join used instead of manual string concatenation.\n> >> - envListOption and listOption return std::optional now.\n> >> - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped.\n> >> - Isolation configuration stored to IPAManager to avoid passing the\n> >> whole configuraton to isSignatureValid.\n> >> - Similarly to paths in IPAProxy.\n> >> - `delimiter' argument added to GlobalConfiguration::envListOption and\n> >> used for LIBCAMERA_PIPELINES_MATCH_LIST processing.\n> >> - Configuration no longer looked up in /etc/libcamera and additionally\n> >> looked up in LIBCAMERA_DATA_DIR.\n> >> - Cosmetic changes suggested by Barnabás.\n> >> Changes in v11:\n> >> - global_configuration.cpp moved out of base and yaml_parser.cpp left in\n> >> its original place.\n> >> - Configuration initialized as empty if there is no configuration file,\n> >> preventing a segfault on later access.\n> >> - Cosmetic changes suggested by Laurent.\n> >> - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly.\n> >> - Failing to read an existent configuration file is an error, rather\n> >> than warning, now. (It should be also fatal, but how to propagate it?)\n> >> - Slash is used instead of dot to separate configuration names.\n> >> - rpi configuration read is in the global configuration directly (unless\n> >> overridden by LIBCAMERA_RPI_CONFIG_FILE) now.\n> >> Changes in v10:\n> >> - Minor improvements suggested by Barnabás.\n> >> Changes in v9:\n> >> - The configuration instance is now stored in CameraManager and accessed\n> >> from there rather than being a separate singleton wrapped by global\n> >> accessors. This solves the ugly problem of delayed initialization but\n> >> I don’t like anything else about it. I played a bit with the idea of\n> >> attaching it to Logger instead, see the commit message of patch 03 for\n> >> some discussion, but stayed with the CameraManager proposal\n> >> eventually.\n> >> - I’ve given up on the logging configuration now when the configuration\n> >> is stored in CameraManager and removed the corresponding patches. I\n> >> think it’s possible to add it later, but for now, it’s already\n> >> complicated enough.\n> >> - Not much tested, let’s see first if the current implementation gets in\n> >> an acceptable direction.\n> >> Changes in v8:\n> >> - Rebased on current master.\n> >> - Anniversary edition: 400 days since v1 posted. ;-)\n> >> Changes in v7:\n> >> - Rebased on current master.\n> >> - Tuning file path configuration updated for recent changes.\n> >> A significant change is that the tuning file configuration is no\n> >> longer sensor dependent as there is apparently no access to the sensor\n> >> info in the IPA proxy.\n> >> - Minor improvements of some commit messages.\n> >> Changes in v6:\n> >> - Rebased on master.\n> >> - File names from file header descriptions removed.\n> >> - Indentation fix in moved code as requested by checkstyle.py.\n> >> - Unneeded const_cast's removed.\n> >> - Using GlobalConfiguration namespace rather than a class.\n> >> - Path configuration options are defined as sequences in YAML.\n> >> - A patch introducing LIBCAMERA_CONFIG_NAME added.\n> >> - A patch introducing LIBCAMERA_CONFIG_DIR added.\n> >> - Miscellaneous minor code changes suggested by Barnabás.\n> >> Changes in v5:\n> >> - A pointer is used to store the global configuration singleton rather\n> >> than a static variable. This makes the things more robust and fixes\n> >> the problem with re-entrancy on logging and a failing\n> >> camera_reconfigure test.\n> >> - In relation to the change above, a new initialization method\n> >> GlobalConfiguration::initialize() was introduced that replaces the\n> >> initialization calls in CameraManager and IPAManager.\n> >> - Logging YAML errors when reading the configuration was also fixed.\n> >> - Global configuration is placed to base directly, without an\n> >> intermediate patch.\n> >> - An `optional' value comparison simplified.\n> >> - A temporary typo in a comment fixed.\n> >> - Unused stdint.h include removed.\n> >> Changes in v4:\n> >> - Rebased on current master.\n> >> - Configuration option for LIBCAMERA_IPA_PROXY_PATH added.\n> >> - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp.\n> >> Changes in v3:\n> >> - Added a configuration item for the newly introduced\n> >> LIBCAMERA_PIPELINES_MATCH_LIST variable.\n> >> - A minor indentation fix.\n> >> - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch.\n> >> - Tuning files are looked up now by particular cameras attached rather than\n> >> being specified for the whole pipeline.\n> >> - Helpers use std::string& instead of char* for confPath arguments.\n> >> - Protection against returning YamlObject::empty as a regular value (the\n> >> problem has been probably exposed by addition of\n> >> LIBCAMERA_PIPELINES_MATCH_LIST).\n> >> Changes in v2:\n> >> - Rebased on master.\n> >> - Various cleanups, documentation improvements and minor fixes.\n> >> - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush).\n> >> - Two more patches for software ISP configuration added.\n> >> Milan Zamazal (12):\n> >> config: Introduce global runtime configuration\n> >> config: Make GlobalConfiguration instance\n> >> config: Add configuration retrieval helpers\n> >> config: Look up rpi configuration in the configuration file\n> >> config: Look up IPA configurables in configuration file\n> >> config: Look up pipelines match list in configuration file\n> >> config: Allow enabling software ISP in runtime\n> >> config: Add global configuration file documentation\n> >> libcamera: software_isp: Make input buffer copying configurable\n> >> libcamera: software_isp: Make measurement configurable\n> >> config: Make configuration file configurable\n> >> config: Make configuration directories configurable\n> >> Documentation/documentation-contents.rst | 2 +-\n> >> Documentation/index.rst | 2 +-\n> >> Documentation/meson.build | 2 +-\n> >> ...ariables.rst => runtime_configuration.rst} | 126 ++++++++-\n> >> include/libcamera/internal/camera_manager.h | 8 +\n> >> .../libcamera/internal/global_configuration.h | 62 +++++\n> >> include/libcamera/internal/ipa_manager.h | 7 +-\n> >> include/libcamera/internal/ipa_proxy.h | 8 +-\n> >> include/libcamera/internal/meson.build | 1 +\n> >> src/libcamera/camera_manager.cpp | 20 +-\n> >> src/libcamera/global_configuration.cpp | 258 ++++++++++++++++++\n> >> src/libcamera/ipa_manager.cpp | 39 ++-\n> >> src/libcamera/ipa_proxy.cpp | 51 ++--\n> >> src/libcamera/meson.build | 1 +\n> >> .../pipeline/rpi/common/pipeline_base.cpp | 59 ++--\n> >> .../pipeline/rpi/common/pipeline_base.h | 3 +-\n> >> src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +-\n> >> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +-\n> >> src/libcamera/pipeline/simple/simple.cpp | 13 +\n> >> src/libcamera/software_isp/TODO | 36 ---\n> >> src/libcamera/software_isp/debayer_cpu.cpp | 32 ++-\n> >> src/libcamera/software_isp/debayer_cpu.h | 10 +-\n> >> src/libcamera/software_isp/software_isp.cpp | 3 +-\n> >> .../module_ipa_proxy.cpp.tmpl | 4 +-\n> >> .../module_ipa_proxy.h.tmpl | 2 +-\n> >> 25 files changed, 625 insertions(+), 176 deletions(-)\n> >> rename Documentation/{environment_variables.rst => runtime_configuration.rst} (60%)\n> >> create mode 100644 include/libcamera/internal/global_configuration.h\n> >> create mode 100644 src/libcamera/global_configuration.cpp\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 739C3C324E\n\tfor ;\n\tTue, 9 Sep 2025 09:42:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 147E369361;\n\tTue, 9 Sep 2025 11:42:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE81369339\n\tfor ;\n\tTue, 9 Sep 2025 11:42:12 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:6f3a:4f34:f1fa:8b3])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 095AB605;\n\tTue, 9 Sep 2025 11:40:58 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LYHyeBKr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757410859;\n\tbh=IdCtrwHyBronjocrVhqoeHVgQTx+Krxu6MzWIGUv4NA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=LYHyeBKrL7CRojppQI+DoWhYiKKikAneqhZs97P0Kd4RWJOplMUw3nXqzUbSC1qDw\n\trRNxJXKE81jIbOG12Q5RE1Zx3CqCnaAz4COx8Pkkr1+9OZTruXVo3n3vJziEalCx7x\n\tM+N3RNzhms0+8GZXmWrRRmaGzMFm2YKB9AgSxIBc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85jz3iyjsg.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20250729073201.5369-1-mzamazal@redhat.com>\n\t\n\t<85jz3iyjsg.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH v16 00/12] Add global configuration file","From":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tLaurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tMilan Zamazal ","Date":"Tue, 09 Sep 2025 18:42:05 +0900","Message-ID":"<175741092506.2127323.11668353838526167840@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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":35755,"web_url":"https://patchwork.libcamera.org/comment/35755/","msgid":"<85zfb3ptyw.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-09-09T14:43:19","subject":"Re: [PATCH v16 00/12] Add global configuration file","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Paul Elder writes:\n\n> Quoting Milan Zamazal (2025-08-05 04:21:51)\n>> Hi Barnabás,\n>> \n>> Barnabás Pőcze writes:\n>> \n>> > Hi\n>> >\n>> > 2025. 07. 29. 9:31 keltezéssel, Milan Zamazal írta:\n>> >> This patch series introduces global configuration file for libcamera, to\n>> >> provide runtime configuration means other than environment variables.\n>> >> Instead of, or in addition to, the growing list of configuration\n>> >> environment variables, the whole configuration can be specified in a\n>> >> single configuration file. This is both simpler and more flexible.\n>> >> This is not a replacement for specific configuration files already\n>> >> present in libcamera, with the exception of rpi configuration file (see\n>> >> “Look up rpi configuration in the configuration file” patch).\n>> >> The patches implement what is needed to introduce a configuration file\n>> >> that can handle the current environment variables and software ISP\n>> >> TODOs. They demonstrate how to deal with the key points that must be\n>> >> considered. Logging is omitted from the configuration file for\n>> >> technical reasons. See commit messages for more details.\n>> >> The configuration file is a YAML file. It is looked up in user’s home\n>> >> directory or, if not present, in system-wide libcamera directories.\n>> >> Both the directories and file name of the configuration file can be\n>> >> overridden using newly introduced environment variables.\n>> >> The current configuration environment variables are still supported and\n>> >> take precedence, if defined, over the corresponding entries in the YAML\n>> >> file.\n>> >> This patch series is not exhaustive, there can be future enhancements,\n>> >> most notably configuration file validation to avoid confusions caused by\n>> >> typos etc.\n>> >> Not everything has been tested because some of the patches are related\n>> >> to specific hardware.\n>> >\n>> > I think this is ready to be merged for the most part, but it certainly needs more review\n>> > and probably a bit more testing.\n>> \n>> Yes.\n>> \n>> > I feel like I have been looking at it for too long.\n>> \n>> I understand, the same here.\n>> \n>> Thank you for your reviews and helping move the patches forward.\n>> \n>> > I also think the exact nature of how configuration is distributed to the particular\n>> > components is a question (for later). Currently everything accesses the `GlobalConfiguration`\n>> > object, and finds the data by its absolute path. I believe using relative paths would\n>> > introduce less coupling between the parts.\n>> >\n>> > Another interesting question is how far from the pipeline handler should things stop\n>> > using the configuration file, and instead take their parameters in native types via\n>> > constructors, etc. For example, I think that `DebayerCpu` should not be accessing the\n>> > configuration file directly, e.g. patch 10/12 \"libcamera: software_isp: Make measurement configurable\"\n>> > introduces too strong of a coupling between the simple pipeline handler and the\n>> > `DebayerCpu` class in my opinion.\n>\n> Oh good point, I missed that. Yeah I agree, maybe there should be a separate\n> top-level category for software_isp.\n\nI like this idea.\n\nGenerally, internal passing values around is not that important at the\nmoment, it can be changed any time later. But I'd like to avoid\nannoying users by changing the structure of the configuration file.\n\n> Paul\n>\n>> \n>> These may be valid points but they can be implemented later. From my\n>> perspective, the most important things at the moment are:\n>> \n>> - Not introducing regressions.\n>> \n>> - Agreeing on the configuration file structure so that it needn't be\n>> changed any time soon, which would be annoying for users.\n>> \n>> > Regards,\n>> > Barnabás Pőcze\n>> >\n>> >> Changes in v16:\n>> >> - Non-code fixes suggested by Barnabás.\n>> >> Changes in v15:\n>> >> - Fixes to reading configuration files suggested by Barnabás.\n>> >> - The outdated example in the documentation fixed.\n>> >> Changes in v14:\n>> >> - Unused expectedTarget variable removed.\n>> >> - Old `ipas' examples in examples removed.\n>> >> - IPAProxy::{configPaths_,execPaths_} have their direct types now,\n>> >> without std::optional.\n>> >> Changes in v13:\n>> >> - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is\n>> >> loaded.\n>> >> - Standard user configuration directory has no longer precedence over\n>> >> LIBCAMERA_CONFIG_DIR.\n>> >> - Improvements to the rpi code suggested by Barnabás.\n>> >> - Documentation fixes suggested by Barnabás.\n>> >> - Make sure yamlConfiguration_ is initialized.\n>> >> Changes in v12:\n>> >> - The commit message of \"Make GlobalConfiguration instance\" updated\n>> >> according to the recent discussions.\n>> >> - Use initializer_list rather than a string with separators for\n>> >> GlobalConfiguration option path arguments.\n>> >> - char *const qualifiers dropped.\n>> >> - rpi can have separate configurations for different targets.\n>> >> - GlobalConfiguration::option template is not type limited anymore.\n>> >> - utils::join used instead of manual string concatenation.\n>> >> - envListOption and listOption return std::optional now.\n>> >> - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped.\n>> >> - Isolation configuration stored to IPAManager to avoid passing the\n>> >> whole configuraton to isSignatureValid.\n>> >> - Similarly to paths in IPAProxy.\n>> >> - `delimiter' argument added to GlobalConfiguration::envListOption and\n>> >> used for LIBCAMERA_PIPELINES_MATCH_LIST processing.\n>> >> - Configuration no longer looked up in /etc/libcamera and additionally\n>> >> looked up in LIBCAMERA_DATA_DIR.\n>> >> - Cosmetic changes suggested by Barnabás.\n>> >> Changes in v11:\n>> >> - global_configuration.cpp moved out of base and yaml_parser.cpp left in\n>> >> its original place.\n>> >> - Configuration initialized as empty if there is no configuration file,\n>> >> preventing a segfault on later access.\n>> >> - Cosmetic changes suggested by Laurent.\n>> >> - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly.\n>> >> - Failing to read an existent configuration file is an error, rather\n>> >> than warning, now. (It should be also fatal, but how to propagate it?)\n>> >> - Slash is used instead of dot to separate configuration names.\n>> >> - rpi configuration read is in the global configuration directly (unless\n>> >> overridden by LIBCAMERA_RPI_CONFIG_FILE) now.\n>> >> Changes in v10:\n>> >> - Minor improvements suggested by Barnabás.\n>> >> Changes in v9:\n>> >> - The configuration instance is now stored in CameraManager and accessed\n>> >> from there rather than being a separate singleton wrapped by global\n>> >> accessors. This solves the ugly problem of delayed initialization but\n>> >> I don’t like anything else about it. I played a bit with the idea of\n>> >> attaching it to Logger instead, see the commit message of patch 03 for\n>> >> some discussion, but stayed with the CameraManager proposal\n>> >> eventually.\n>> >> - I’ve given up on the logging configuration now when the configuration\n>> >> is stored in CameraManager and removed the corresponding patches. I\n>> >> think it’s possible to add it later, but for now, it’s already\n>> >> complicated enough.\n>> >> - Not much tested, let’s see first if the current implementation gets in\n>> >> an acceptable direction.\n>> >> Changes in v8:\n>> >> - Rebased on current master.\n>> >> - Anniversary edition: 400 days since v1 posted. ;-)\n>> >> Changes in v7:\n>> >> - Rebased on current master.\n>> >> - Tuning file path configuration updated for recent changes.\n>> >> A significant change is that the tuning file configuration is no\n>> >> longer sensor dependent as there is apparently no access to the sensor\n>> >> info in the IPA proxy.\n>> >> - Minor improvements of some commit messages.\n>> >> Changes in v6:\n>> >> - Rebased on master.\n>> >> - File names from file header descriptions removed.\n>> >> - Indentation fix in moved code as requested by checkstyle.py.\n>> >> - Unneeded const_cast's removed.\n>> >> - Using GlobalConfiguration namespace rather than a class.\n>> >> - Path configuration options are defined as sequences in YAML.\n>> >> - A patch introducing LIBCAMERA_CONFIG_NAME added.\n>> >> - A patch introducing LIBCAMERA_CONFIG_DIR added.\n>> >> - Miscellaneous minor code changes suggested by Barnabás.\n>> >> Changes in v5:\n>> >> - A pointer is used to store the global configuration singleton rather\n>> >> than a static variable. This makes the things more robust and fixes\n>> >> the problem with re-entrancy on logging and a failing\n>> >> camera_reconfigure test.\n>> >> - In relation to the change above, a new initialization method\n>> >> GlobalConfiguration::initialize() was introduced that replaces the\n>> >> initialization calls in CameraManager and IPAManager.\n>> >> - Logging YAML errors when reading the configuration was also fixed.\n>> >> - Global configuration is placed to base directly, without an\n>> >> intermediate patch.\n>> >> - An `optional' value comparison simplified.\n>> >> - A temporary typo in a comment fixed.\n>> >> - Unused stdint.h include removed.\n>> >> Changes in v4:\n>> >> - Rebased on current master.\n>> >> - Configuration option for LIBCAMERA_IPA_PROXY_PATH added.\n>> >> - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp.\n>> >> Changes in v3:\n>> >> - Added a configuration item for the newly introduced\n>> >> LIBCAMERA_PIPELINES_MATCH_LIST variable.\n>> >> - A minor indentation fix.\n>> >> - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch.\n>> >> - Tuning files are looked up now by particular cameras attached rather than\n>> >> being specified for the whole pipeline.\n>> >> - Helpers use std::string& instead of char* for confPath arguments.\n>> >> - Protection against returning YamlObject::empty as a regular value (the\n>> >> problem has been probably exposed by addition of\n>> >> LIBCAMERA_PIPELINES_MATCH_LIST).\n>> >> Changes in v2:\n>> >> - Rebased on master.\n>> >> - Various cleanups, documentation improvements and minor fixes.\n>> >> - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush).\n>> >> - Two more patches for software ISP configuration added.\n>> >> Milan Zamazal (12):\n>> >> config: Introduce global runtime configuration\n>> >> config: Make GlobalConfiguration instance\n>> >> config: Add configuration retrieval helpers\n>> >> config: Look up rpi configuration in the configuration file\n>> >> config: Look up IPA configurables in configuration file\n>> >> config: Look up pipelines match list in configuration file\n>> >> config: Allow enabling software ISP in runtime\n>> >> config: Add global configuration file documentation\n>> >> libcamera: software_isp: Make input buffer copying configurable\n>> >> libcamera: software_isp: Make measurement configurable\n>> >> config: Make configuration file configurable\n>> >> config: Make configuration directories configurable\n>> >> Documentation/documentation-contents.rst | 2 +-\n>> >> Documentation/index.rst | 2 +-\n>> >> Documentation/meson.build | 2 +-\n>> >> ...ariables.rst => runtime_configuration.rst} | 126 ++++++++-\n>> >> include/libcamera/internal/camera_manager.h | 8 +\n>> >> .../libcamera/internal/global_configuration.h | 62 +++++\n>> >> include/libcamera/internal/ipa_manager.h | 7 +-\n>> >> include/libcamera/internal/ipa_proxy.h | 8 +-\n>> >> include/libcamera/internal/meson.build | 1 +\n>> >> src/libcamera/camera_manager.cpp | 20 +-\n>> >> src/libcamera/global_configuration.cpp | 258 ++++++++++++++++++\n>> >> src/libcamera/ipa_manager.cpp | 39 ++-\n>> >> src/libcamera/ipa_proxy.cpp | 51 ++--\n>> >> src/libcamera/meson.build | 1 +\n>> >> .../pipeline/rpi/common/pipeline_base.cpp | 59 ++--\n>> >> .../pipeline/rpi/common/pipeline_base.h | 3 +-\n>> >> src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +-\n>> >> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +-\n>> >> src/libcamera/pipeline/simple/simple.cpp | 13 +\n>> >> src/libcamera/software_isp/TODO | 36 ---\n>> >> src/libcamera/software_isp/debayer_cpu.cpp | 32 ++-\n>> >> src/libcamera/software_isp/debayer_cpu.h | 10 +-\n>> >> src/libcamera/software_isp/software_isp.cpp | 3 +-\n>> >> .../module_ipa_proxy.cpp.tmpl | 4 +-\n>> >> .../module_ipa_proxy.h.tmpl | 2 +-\n>> >> 25 files changed, 625 insertions(+), 176 deletions(-)\n>> >> rename Documentation/{environment_variables.rst => runtime_configuration.rst} (60%)\n>> >> create mode 100644 include/libcamera/internal/global_configuration.h\n>> >> create mode 100644 src/libcamera/global_configuration.cpp\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 D9E3BBDB13\n\tfor ;\n\tTue, 9 Sep 2025 14:43:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B4E269367;\n\tTue, 9 Sep 2025 16:43:28 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 09D456934B\n\tfor ;\n\tTue, 9 Sep 2025 16:43:25 +0200 (CEST)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-548-EUFPivqxPfClKNL_AQ31Dg-1; Tue, 09 Sep 2025 10:43:23 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-45b9a856d58so45871965e9.0\n\tfor ;\n\tTue, 09 Sep 2025 07:43:23 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb ([85.93.96.130])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-45de16b8b58sm142277755e9.4.2025.09.09.07.43.19\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 09 Sep 2025 07:43:20 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"CSbsBaE9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1757429005;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=HPxsj47GBZYngDA81gil/2g2T7B+KbeYXkSewJhMo9A=;\n\tb=CSbsBaE9xwwnvdd4KUSZxG01ekr/CZBOzf4GaxKH0FcV21QYp3eWqP4Bb6Wr3gU3PAuXgf\n\tQdOiR/xBnL4+gw8orZO/490wGf9pAGmaK8nF4c2Ng9W3A7sWT1S/S5X4kZZYhL2gObEsMd\n\ti9QZz2wrhIgm3z70GZ5Dy+aj0ynaqms=","X-MC-Unique":"EUFPivqxPfClKNL_AQ31Dg-1","X-Mimecast-MFC-AGG-ID":"EUFPivqxPfClKNL_AQ31Dg_1757429002","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1757429002; x=1758033802;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=bbOq4Rr+JhfLriDBuG8Bq7sIpJcEziJyTucDX3t8SDE=;\n\tb=hXZ2k0vmvly4v7CyNNDTb1iQDh5RFTQTiDwY58msEgu2oTkgzWYTeAuWoWgbEiTqY/\n\t1aScLNtARPFuMjKRiPQH9Vgxc53/mQnDaKiqLyuzjegsR5AdfWQUhTPXPgWp8qdbO59A\n\tKGiJ5wZCfP2RbQMChfwbo/1T2KYxVGDoINqlFfXMWpLud10pUz0/U6XqGV/tWgOV891L\n\t08R9dsVUCPk2zYVnpm3+ME/SXLw5iiV6gHuRM8/CTdAL1fOoqzZNZUX5Jncr50dn6/kQ\n\t1O+NyC0EcSiQ9Pcj0uyzgiuH3GKO/gO1O8gebQHnKgz/kMStk8hX/tCcixd8jMh7G3er\n\t4Rjw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCU+c+n+Hjmq6+RBy0w7MNPj30eBo47WDuxuui7sZsP+ylArrh5PJxjjpnDbN4d6tDxkx9W5MFnwX90nB/2248o=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxGTcYr5JYISn21iPL16Tq17SVux5W8vDRibUvRA8SON61VLENs\n\tXwsJ7Hxn26GlB8FfEeAwwbg/2cxurGOZB4dPetVA2p2dJZ0WpjX+2OwQucJkJ5DcvMEK3TsOm+X\n\t6bxv81/2oRJU/90RXEm/IY2cgXMiqwumPFZB+65ClKXTmi7xnlhzapLmuLAe18nlMu6c/QrKwMP\n\ts=","X-Gm-Gg":"ASbGnct6Zrt5zuojnqYAnQAW8zB3O6O8g6vlJ5zqveVUyj2sFc7qdLWD0BXwfBT+xCw\n\t1TnqrAN9Q7aLU08LWpwYmE/kJKXfGrMJARIF92RCMV79PhAYB8jsb49w3VQUauobPgP2/nb7FrY\n\tndJ0sF9hNBRoLuF4lUBTQmnCgahJxtol3GbrWIvtsTt2xWDBNRYbAyIiwnw5Z2SYOZ6TVsHGefW\n\tVfGMMUiBKHnF/J2rNaOr+q/N7tmABtH8Xl9SAsdrNz362F/9xcEmgVxcj+pWWF2Ui2fLOK4XBNc\n\tamOPGzlkeRZycY5120UNyqt69zf+NFqkoku+nMOQjqnasYbPC33XMRGDx1El63wP5A==","X-Received":["by 2002:a05:600c:548a:b0:458:bfb1:1fb6 with SMTP id\n\t5b1f17b1804b1-45ddde6a471mr91342285e9.2.1757429002224; \n\tTue, 09 Sep 2025 07:43:22 -0700 (PDT)","by 2002:a05:600c:548a:b0:458:bfb1:1fb6 with SMTP id\n\t5b1f17b1804b1-45ddde6a471mr91341665e9.2.1757429000564; \n\tTue, 09 Sep 2025 07:43:20 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IE23YV5nxhu5Lj+pwyYsqd5VynJ2BuUqZF8wf9DcDcaG8VIoXoEGotFbWAh1VcnYcBE72NMbQ==","From":"Milan Zamazal ","To":"Paul Elder ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, Laurent Pinchart\n\t","Subject":"Re: [PATCH v16 00/12] Add global configuration file","In-Reply-To":"<175741092506.2127323.11668353838526167840@neptunite.rasen.tech>\n\t(Paul Elder's message of \"Tue, 09 Sep 2025 18:42:05 +0900\")","References":"<20250729073201.5369-1-mzamazal@redhat.com>\n\t\n\t<85jz3iyjsg.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<175741092506.2127323.11668353838526167840@neptunite.rasen.tech>","Date":"Tue, 09 Sep 2025 16:43:19 +0200","Message-ID":"<85zfb3ptyw.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"hYBy6QHjGfL-gAwlK-cTG3AMSx7UUxQrwKB25guHCwY_1757429002","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}}]