[{"id":34500,"web_url":"https://patchwork.libcamera.org/comment/34500/","msgid":"<20250616222929.GG32454@pendragon.ideasonboard.com>","date":"2025-06-16T22:29:29","subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Mon, Jun 16, 2025 at 10:47:20AM +0200, Milan Zamazal wrote:\n> Currently, libcamera can be configured in runtime using several\n> environment variables. With introducing more and more variables, this\n> mechanism reaches its limits. It would be simpler and more flexible if\n> it was possible to configure libcamera in a single file.\n> \n> For example, there was a request to define pipeline precedence in\n> runtime. We want to compile in multiple pipelines, in order to have\n> them accessible within single packages in distributions. And then being\n> able to select among the pipelines manually as needed based on the\n> particular hardware or operating system environment. Having the\n> configuration file then allows easy switching between hardware, GPU or\n> CPU IPAs. Another possible use case is tuning image output, especially\n> with software ISP, to user liking. For example, some users may prefer\n> higher contrast without the need to use the corresponding knobs, if\n> present at all, in every application.\n\nThat's an interesting use case, but I don't think it necessarily belong\nto the global configuration file. In the context of a desktop\ndistribution where libcamera is used through pipewire, user preferences\nare I think best handled through pipewire. This isn't a topic that needs\nto be decided now, I foresee lots of interesting discussions in the\nfuture about what should go to the configuration file and what\nshouldn't.\n\n> The configuration file can also\n> be used to enable or disable experimental features and avoid the need to\n> track local patches changing configuration options hard-wired in the\n> code when working on new features.\n> \n> This patch introduces basic support for configuration files.\n> GlobalConfiguration class reads, stores and accesses the configuration.\n> GlobalConfiguration instances are meant to be stored to and accessed\n> from instances of another class, preferably from a single instance.\n> GlobalConfiguration is defined in base because it will be accessed from\n> code in base, e.g. logging.\n\nAs mentioned in the review of 01/13, moving YamlParser to base is\nproblematic. Let's see in the appropriate patch if we can configure the\nlogger without having configuration file support in base.\n\n> libcamera configuration can be specified using a system-wide\n> configuration file or a user configuration file. The user configuration\n> file takes precedence if present. There is currently no way to merge\n> multiple configuration files, the one found is used as the only\n> configuration file.\n\nIs this an issue ? I'm wondering if we need a default configuration in\n/usr/share/libcamera and a system-specific override in /etc/libcamera\n(or somewhere else).\n\n> If no configuration file is present, nothing\n> changes to the current libcamera behavior (except for some log\n> messages related to configuration file lookup).\n> \n> The configuration file is a YAML file. We already have a mechanism for\n> handling YAML configuration files in libcamera and the given\n> infrastructure can be reused for the purpose. However, the\n> configuration type is abstracted to make contingent future change of the\n> underlying class easier while retaining (most of) the original API.\n\nWhat changes are you expecting ?\n\n> \n> The configuration is versioned. This has currently no particular\n> meaning but is likely to have its purpose in future, especially once\n> configuration validation is introduced.\n> \n> The configuration YAML file looks as follows:\n> \n> ---\n> version: 1\n> configuration:\n> WHATEVER CONFIGURATION NEEDED\n> \n> This patch introduces just the basic idea. Actually using the\n> configuration in the corresponding places (everything what is currently\n> configurable via environment variables should be configurable in the\n> file configuration) and other enhancements are implemented in the\n> followup patches.\n\nI probably won't have many comments on this patch yet, I'll review the\nremaining of the series first to see how the class gets used. Skimming\nthrough the series though, I was expecting this patch to introduce a\nfully-fledged class, but I see that quite a few extensions to the class\nare spread in other patches. I don't mind too much either way though.\n\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> .../libcamera/internal/global_configuration.h | 34 ++++\n> include/libcamera/internal/meson.build | 1 +\n> src/libcamera/base/global_configuration.cpp | 150 ++++++++++++++++++\n> src/libcamera/base/meson.build | 1 +\n> 4 files changed, 186 insertions(+)\n> create mode 100644 include/libcamera/internal/global_configuration.h\n> create mode 100644 src/libcamera/base/global_configuration.cpp\n> \n> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n> new file mode 100644\n> index 000000000..8d0410ac2\n> --- /dev/null\n> +++ b/include/libcamera/internal/global_configuration.h\n> @@ -0,0 +1,34 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024-2025 Red Hat, inc.\n> + *\n> + * Global configuration handling\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +namespace libcamera {\n> +\n> +class GlobalConfiguration\n> +{\n> +public:\n> +\tusing Configuration = const YamlObject &;\n> +\n> +\tGlobalConfiguration();\n> +\n> +\tunsigned int version() const;\n> +\tConfiguration configuration() const;\n> +\n> +private:\n> +\tbool loadFile(const std::filesystem::path &fileName);\n> +\tvoid load();\n> +\tConfiguration get() const;\n> +\n> +\tstd::unique_ptr yamlConfiguration;\n\nMissing trailing underscore.\n\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 33f318b2b..951bfee49 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -22,6 +22,7 @@ libcamera_internal_headers = files([\n> 'dma_buf_allocator.h',\n> 'formats.h',\n> 'framebuffer.h',\n> + 'global_configuration.h',\n\nThis would belong to include/libcamera/base/ if global_configuration.cpp\nis in base too.\n\n> 'ipa_data_serializer.h',\n> 'ipa_manager.h',\n> 'ipa_module.h',\n> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\n> new file mode 100644\n> index 000000000..1ddf5bc33\n> --- /dev/null\n> +++ b/src/libcamera/base/global_configuration.cpp\n> @@ -0,0 +1,150 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024-2025 Red Hat, inc.\n> + *\n> + * Global configuration handling\n> + */\n> +\n> +#include \"libcamera/internal/global_configuration.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace {\n> +const std::vector\n> +\tglobalConfigurationFiles = {\n> +\t\tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n\nGiven that distributions install libcamera with prefix set to /usr, this\nwould be /usr/etc/libcamera/configuration.yaml, which is not a valid\nlocation according to\nhttps://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s09.html.\n\n> +\t\tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n> +\t};\n\nYou can write\n\nconst std::vector globalConfigurationFiles = {\n\tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n\tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n};\n\nAnd you could move this to a static const variable to\nGlobalConfiguration::load(), the only place where it is used. But\nlooking at the rest of the series, globalConfigurationFiles becomes\nunused somewhere, and should be dropped in the corresponding patch.\n\n> +}\n> +\n> +LOG_DEFINE_CATEGORY(Configuration)\n> +\n> +/**\n> + * \\class GlobalConfiguration\n> + * \\brief Support for global libcamera configuration\n> + *\n> + * The configuration file is a YAML file and the configuration itself is stored\n> + * under `configuration' top-level item.\n> + *\n> + * The configuration file is looked up in user's home directory first and if it\n> + * is not found then in system-wide configuration directories. If multiple\n> + * configuration files exist then only the first one found is used and no\n> + * configuration merging is performed.\n> + */\n> +\n> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n> +{\n> +\tFile file(fileName);\n> +\tif (!file.exists()) {\n> +\t\treturn false;\n> +\t}\n\nNo need for curly braces.\n\n> +\n> +\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n> +\t\tLOG(Configuration, Warning)\n> +\t\t\t<< \"Failed to open configuration file \" << fileName;\n> +\t\treturn true;\n> +\t}\n> +\n> +\tauto root = YamlParser::parse(file);\n\nMaybe\n\n\tyamlConfiguration_ = YamlParser::parse(file);\n\n?\n\n> +\tif (!root) {\n> +\t\tLOG(Configuration, Warning)\n> +\t\t\t<< \"Failed to parse configuration file \" << fileName;\n> +\t\treturn true;\n> +\t}\n> +\tyamlConfiguration = std::move(root);\n> +\n> +\treturn true;\n> +}\n> +\n> +void GlobalConfiguration::load()\n> +{\n> +\tstd::filesystem::path userConfigurationDirectory;\n> +\tchar *xdgConfigHome = utils::secure_getenv(\"XDG_CONFIG_HOME\");\n\nconst\n\n> +\tif (xdgConfigHome) {\n> +\t\tuserConfigurationDirectory = xdgConfigHome;\n> +\t} else {\n> +\t\tconst char *home = utils::secure_getenv(\"HOME\");\n> +\t\tif (home)\n> +\t\t\tuserConfigurationDirectory =\n> +\t\t\t\tstd::filesystem::path(home) / \".config\";\n> +\t}\n> +\n> +\tif (!userConfigurationDirectory.empty()) {\n> +\t\tstd::filesystem::path user_configuration_file =\n\ncamelCase\n\n> +\t\t\tuserConfigurationDirectory / \"libcamera\" / \"configuration.yaml\";\n> +\t\tif (loadFile(user_configuration_file))\n> +\t\t\treturn;\n\nIf the file exists but can't be loaded, should we make it a fatal error\n? Continuing by ignoring malformed configuration files means we'll run\nwith a configuration different from what the user expects.\n\n> +\t}\n> +\n> +\tfor (const auto &path : globalConfigurationFiles)\n> +\t\tif (loadFile(path))\n> +\t\t\treturn;\n\nPlease use curly braces for the outer scope.\n\n> +}\n> +\n> +GlobalConfiguration::Configuration GlobalConfiguration::get() const\n> +{\n> +\treturn *yamlConfiguration;\n\nI think you can drop this private function and access yamlConfiguration_\ndirectly internally.\n\n> +}\n> +\n> +/**\n> + * \\brief Initialize the global configuration\n> + */\n> +GlobalConfiguration::GlobalConfiguration()\n> +{\n> +\tload();\n> +}\n> +\n> +/**\n> + * \\typedef GlobalConfiguration::Configuration\n> + * \\brief Type representing global libcamera configuration\n> + *\n> + * All code outside GlobalConfiguration must use this type declaration and not\n> + * the underlying type.\n\nWhat's the reason for that ?\n\n> + */\n> +\n> +/**\n> + * \\brief Return configuration version\n> + *\n> + * The version is (optionally) declared in the configuration file in the\n> + * top-level section `version', alongside `configuration'. This has currently no\n> + * real use but may be needed in future if configuration incompatibilities\n> + * occur.\n> + *\n> + * \\return Configuration version as declared in the configuration file or 0 if\n> + * no version is declared there\n> + */\n> +unsigned int GlobalConfiguration::version() const\n> +{\n> +\treturn get()[\"version\"].get().value_or(0);\n> +}\n> +\n> +/**\n> + * \\brief Return libcamera global configuration\n> + *\n> + * This returns the whole configuration stored in the top-level section\n> + * `configuration' of the YAML configuration file.\n> + *\n> + * The requested part of the configuration can be accessed using \\a YamlObject\n> + * methods.\n> + *\n> + * \\note \\a YamlObject type itself shouldn't be used in type declarations to\n> + * avoid trouble if we decide to change the underlying data objects in future.\n> + *\n> + * \\return The whole configuration section\n> + */\n> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const\n> +{\n> +\treturn get()[\"configuration\"];\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> index 94843eb95..4c0032845 100644\n> --- a/src/libcamera/base/meson.build\n> +++ b/src/libcamera/base/meson.build\n> @@ -16,6 +16,7 @@ libcamera_base_internal_sources = files([\n> 'event_dispatcher_poll.cpp',\n> 'event_notifier.cpp',\n> 'file.cpp',\n> + 'global_configuration.cpp',\n> 'log.cpp',\n> 'memfd.cpp',\n> 'message.cpp',","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 B5446BDE6B\n\tfor ;\n\tMon, 16 Jun 2025 22:29:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A81AE68DCB;\n\tTue, 17 Jun 2025 00:29:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFDD761548\n\tfor ;\n\tTue, 17 Jun 2025 00:29:45 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D5B66493;\n\tTue, 17 Jun 2025 00:29:33 +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=\"gctTdE5f\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750112974;\n\tbh=43H56AyFEqfzkdsalWwdTr+JJAIgS+/pREEokJOmEUc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gctTdE5f9KnhoPxfQEUreBN7oKdO9ADGhoc6bheIU6yrF1nYOxDaQN05MgSfdjvDp\n\t0OM7CqjehhcFPa0uyNf4YP6AIAB4zB4+/H2+CGBforyoT459DMbYx0Y9GsIEP5f6lS\n\tjiMY2RkDrnDK2cMM4buRhx3m7gC88nyeYOH6F2OI=","Date":"Tue, 17 Jun 2025 01:29:29 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t","Subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","Message-ID":"<20250616222929.GG32454@pendragon.ideasonboard.com>","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-3-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250616084733.18707-3-mzamazal@redhat.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":34540,"web_url":"https://patchwork.libcamera.org/comment/34540/","msgid":"<880716f2-2499-46f5-ba3f-39a6c084156f@ideasonboard.com>","date":"2025-06-18T11:00:47","subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","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. 06. 17. 0:29 keltezéssel, Laurent Pinchart írta:\n> Hi Milan,\n> \n> Thank you for the patch.\n> \n> On Mon, Jun 16, 2025 at 10:47:20AM +0200, Milan Zamazal wrote:\n>> Currently, libcamera can be configured in runtime using several\n>> environment variables. With introducing more and more variables, this\n>> mechanism reaches its limits. It would be simpler and more flexible if\n>> it was possible to configure libcamera in a single file.\n>>\n>> For example, there was a request to define pipeline precedence in\n>> runtime. We want to compile in multiple pipelines, in order to have\n>> them accessible within single packages in distributions. And then being\n>> able to select among the pipelines manually as needed based on the\n>> particular hardware or operating system environment. Having the\n>> configuration file then allows easy switching between hardware, GPU or\n>> CPU IPAs. Another possible use case is tuning image output, especially\n>> with software ISP, to user liking. For example, some users may prefer\n>> higher contrast without the need to use the corresponding knobs, if\n>> present at all, in every application.\n> \n> That's an interesting use case, but I don't think it necessarily belong\n> to the global configuration file. In the context of a desktop\n> distribution where libcamera is used through pipewire, user preferences\n> are I think best handled through pipewire. This isn't a topic that needs\n> to be decided now, I foresee lots of interesting discussions in the\n> future about what should go to the configuration file and what\n> shouldn't.\n> \n>> The configuration file can also\n>> be used to enable or disable experimental features and avoid the need to\n>> track local patches changing configuration options hard-wired in the\n>> code when working on new features.\n>>\n>> This patch introduces basic support for configuration files.\n>> GlobalConfiguration class reads, stores and accesses the configuration.\n>> GlobalConfiguration instances are meant to be stored to and accessed\n>> from instances of another class, preferably from a single instance.\n>> GlobalConfiguration is defined in base because it will be accessed from\n>> code in base, e.g. logging.\n> \n> As mentioned in the review of 01/13, moving YamlParser to base is\n> problematic. Let's see in the appropriate patch if we can configure the\n> logger without having configuration file support in base.\n> \n>> libcamera configuration can be specified using a system-wide\n>> configuration file or a user configuration file. The user configuration\n>> file takes precedence if present. There is currently no way to merge\n>> multiple configuration files, the one found is used as the only\n>> configuration file.\n> \n> Is this an issue ? I'm wondering if we need a default configuration in\n> /usr/share/libcamera and a system-specific override in /etc/libcamera\n> (or somewhere else).\n> \n>> If no configuration file is present, nothing\n>> changes to the current libcamera behavior (except for some log\n>> messages related to configuration file lookup).\n>>\n>> The configuration file is a YAML file. We already have a mechanism for\n>> handling YAML configuration files in libcamera and the given\n>> infrastructure can be reused for the purpose. However, the\n>> configuration type is abstracted to make contingent future change of the\n>> underlying class easier while retaining (most of) the original API.\n> \n> What changes are you expecting ?\n> \n>>\n>> The configuration is versioned. This has currently no particular\n>> meaning but is likely to have its purpose in future, especially once\n>> configuration validation is introduced.\n>>\n>> The configuration YAML file looks as follows:\n>>\n>> ---\n>> version: 1\n>> configuration:\n>> WHATEVER CONFIGURATION NEEDED\n>>\n>> This patch introduces just the basic idea. Actually using the\n>> configuration in the corresponding places (everything what is currently\n>> configurable via environment variables should be configurable in the\n>> file configuration) and other enhancements are implemented in the\n>> followup patches.\n> \n> I probably won't have many comments on this patch yet, I'll review the\n> remaining of the series first to see how the class gets used. Skimming\n> through the series though, I was expecting this patch to introduce a\n> fully-fledged class, but I see that quite a few extensions to the class\n> are spread in other patches. I don't mind too much either way though.\n> \n>>\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> .../libcamera/internal/global_configuration.h | 34 ++++\n>> include/libcamera/internal/meson.build | 1 +\n>> src/libcamera/base/global_configuration.cpp | 150 ++++++++++++++++++\n>> src/libcamera/base/meson.build | 1 +\n>> 4 files changed, 186 insertions(+)\n>> create mode 100644 include/libcamera/internal/global_configuration.h\n>> create mode 100644 src/libcamera/base/global_configuration.cpp\n>>\n>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>> new file mode 100644\n>> index 000000000..8d0410ac2\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/global_configuration.h\n>> @@ -0,0 +1,34 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024-2025 Red Hat, inc.\n>> + *\n>> + * Global configuration handling\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/yaml_parser.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +class GlobalConfiguration\n>> +{\n>> +public:\n>> +\tusing Configuration = const YamlObject &;\n>> +\n>> +\tGlobalConfiguration();\n>> +\n>> +\tunsigned int version() const;\n>> +\tConfiguration configuration() const;\n>> +\n>> +private:\n>> +\tbool loadFile(const std::filesystem::path &fileName);\n>> +\tvoid load();\n>> +\tConfiguration get() const;\n>> +\n>> +\tstd::unique_ptr yamlConfiguration;\n> \n> Missing trailing underscore.\n> \n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n>> index 33f318b2b..951bfee49 100644\n>> --- a/include/libcamera/internal/meson.build\n>> +++ b/include/libcamera/internal/meson.build\n>> @@ -22,6 +22,7 @@ libcamera_internal_headers = files([\n>> 'dma_buf_allocator.h',\n>> 'formats.h',\n>> 'framebuffer.h',\n>> + 'global_configuration.h',\n> \n> This would belong to include/libcamera/base/ if global_configuration.cpp\n> is in base too.\n> \n>> 'ipa_data_serializer.h',\n>> 'ipa_manager.h',\n>> 'ipa_module.h',\n>> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\n>> new file mode 100644\n>> index 000000000..1ddf5bc33\n>> --- /dev/null\n>> +++ b/src/libcamera/base/global_configuration.cpp\n>> @@ -0,0 +1,150 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024-2025 Red Hat, inc.\n>> + *\n>> + * Global configuration handling\n>> + */\n>> +\n>> +#include \"libcamera/internal/global_configuration.h\"\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \"libcamera/internal/yaml_parser.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace {\n>> +const std::vector\n>> +\tglobalConfigurationFiles = {\n>> +\t\tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n> \n> Given that distributions install libcamera with prefix set to /usr, this\n> would be /usr/etc/libcamera/configuration.yaml, which is not a valid\n> location according to\n> https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s09.html.\n\nLooks good to me. `LIBCAMERA_SYSCONF_DIR` becomes `/etc/libcamera/` if you set `-D prefix=/usr`.\nSee https://mesonbuild.com/Release-notes-for-0-44-0.html#prefixdependent-defaults-for-sysconfdir-localstatedir-and-sharedstatedir\n\nI would probably change the lookup order as follows:\n\n - LIBCAMERA_SYSCONF_DIR (/etc/libcamera usually)\n - LIBCAMERA_DATA_DIR (/usr/share/libcamera usually)\n\nI am not sure that we want to hard-code \"/etc/libcamera\" there.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>> +\t\tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n>> +\t};\n> \n> You can write\n> \n> const std::vector globalConfigurationFiles = {\n> \tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n> \tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n> };\n> \n> And you could move this to a static const variable to\n> GlobalConfiguration::load(), the only place where it is used. But\n> looking at the rest of the series, globalConfigurationFiles becomes\n> unused somewhere, and should be dropped in the corresponding patch.\n> \n>> +}\n>> +\n>> +LOG_DEFINE_CATEGORY(Configuration)\n>> +\n>> +/**\n>> + * \\class GlobalConfiguration\n>> + * \\brief Support for global libcamera configuration\n>> + *\n>> + * The configuration file is a YAML file and the configuration itself is stored\n>> + * under `configuration' top-level item.\n>> + *\n>> + * The configuration file is looked up in user's home directory first and if it\n>> + * is not found then in system-wide configuration directories. If multiple\n>> + * configuration files exist then only the first one found is used and no\n>> + * configuration merging is performed.\n>> + */\n>> +\n>> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n>> +{\n>> +\tFile file(fileName);\n>> +\tif (!file.exists()) {\n>> +\t\treturn false;\n>> +\t}\n> \n> No need for curly braces.\n> \n>> +\n>> +\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n>> +\t\tLOG(Configuration, Warning)\n>> +\t\t\t<< \"Failed to open configuration file \" << fileName;\n>> +\t\treturn true;\n>> +\t}\n>> +\n>> +\tauto root = YamlParser::parse(file);\n> \n> Maybe\n> \n> \tyamlConfiguration_ = YamlParser::parse(file);\n> \n> ?\n> \n>> +\tif (!root) {\n>> +\t\tLOG(Configuration, Warning)\n>> +\t\t\t<< \"Failed to parse configuration file \" << fileName;\n>> +\t\treturn true;\n>> +\t}\n>> +\tyamlConfiguration = std::move(root);\n>> +\n>> +\treturn true;\n>> +}\n>> +\n>> +void GlobalConfiguration::load()\n>> +{\n>> +\tstd::filesystem::path userConfigurationDirectory;\n>> +\tchar *xdgConfigHome = utils::secure_getenv(\"XDG_CONFIG_HOME\");\n> \n> const\n> \n>> +\tif (xdgConfigHome) {\n>> +\t\tuserConfigurationDirectory = xdgConfigHome;\n>> +\t} else {\n>> +\t\tconst char *home = utils::secure_getenv(\"HOME\");\n>> +\t\tif (home)\n>> +\t\t\tuserConfigurationDirectory =\n>> +\t\t\t\tstd::filesystem::path(home) / \".config\";\n>> +\t}\n>> +\n>> +\tif (!userConfigurationDirectory.empty()) {\n>> +\t\tstd::filesystem::path user_configuration_file =\n> \n> camelCase\n> \n>> +\t\t\tuserConfigurationDirectory / \"libcamera\" / \"configuration.yaml\";\n>> +\t\tif (loadFile(user_configuration_file))\n>> +\t\t\treturn;\n> \n> If the file exists but can't be loaded, should we make it a fatal error\n> ? Continuing by ignoring malformed configuration files means we'll run\n> with a configuration different from what the user expects.\n> \n>> +\t}\n>> +\n>> +\tfor (const auto &path : globalConfigurationFiles)\n>> +\t\tif (loadFile(path))\n>> +\t\t\treturn;\n> \n> Please use curly braces for the outer scope.\n> \n>> +}\n>> +\n>> +GlobalConfiguration::Configuration GlobalConfiguration::get() const\n>> +{\n>> +\treturn *yamlConfiguration;\n> \n> I think you can drop this private function and access yamlConfiguration_\n> directly internally.\n> \n>> +}\n>> +\n>> +/**\n>> + * \\brief Initialize the global configuration\n>> + */\n>> +GlobalConfiguration::GlobalConfiguration()\n>> +{\n>> +\tload();\n>> +}\n>> +\n>> +/**\n>> + * \\typedef GlobalConfiguration::Configuration\n>> + * \\brief Type representing global libcamera configuration\n>> + *\n>> + * All code outside GlobalConfiguration must use this type declaration and not\n>> + * the underlying type.\n> \n> What's the reason for that ?\n> \n>> + */\n>> +\n>> +/**\n>> + * \\brief Return configuration version\n>> + *\n>> + * The version is (optionally) declared in the configuration file in the\n>> + * top-level section `version', alongside `configuration'. This has currently no\n>> + * real use but may be needed in future if configuration incompatibilities\n>> + * occur.\n>> + *\n>> + * \\return Configuration version as declared in the configuration file or 0 if\n>> + * no version is declared there\n>> + */\n>> +unsigned int GlobalConfiguration::version() const\n>> +{\n>> +\treturn get()[\"version\"].get().value_or(0);\n>> +}\n>> +\n>> +/**\n>> + * \\brief Return libcamera global configuration\n>> + *\n>> + * This returns the whole configuration stored in the top-level section\n>> + * `configuration' of the YAML configuration file.\n>> + *\n>> + * The requested part of the configuration can be accessed using \\a YamlObject\n>> + * methods.\n>> + *\n>> + * \\note \\a YamlObject type itself shouldn't be used in type declarations to\n>> + * avoid trouble if we decide to change the underlying data objects in future.\n>> + *\n>> + * \\return The whole configuration section\n>> + */\n>> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const\n>> +{\n>> +\treturn get()[\"configuration\"];\n>> +}\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n>> index 94843eb95..4c0032845 100644\n>> --- a/src/libcamera/base/meson.build\n>> +++ b/src/libcamera/base/meson.build\n>> @@ -16,6 +16,7 @@ libcamera_base_internal_sources = files([\n>> 'event_dispatcher_poll.cpp',\n>> 'event_notifier.cpp',\n>> 'file.cpp',\n>> + 'global_configuration.cpp',\n>> 'log.cpp',\n>> 'memfd.cpp',\n>> 'message.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 11599BDE6B\n\tfor ;\n\tWed, 18 Jun 2025 11:00:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 379FB68DCC;\n\tWed, 18 Jun 2025 13:00:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 895DA68DC1\n\tfor ;\n\tWed, 18 Jun 2025 13:00:50 +0200 (CEST)","from [192.168.33.16] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 262D9752;\n\tWed, 18 Jun 2025 13:00:37 +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=\"hqwIYbc+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750244437;\n\tbh=dvbrbS894UUVUAanSGvz6XDSV+F9Uo4QoVZWDK6XYFc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=hqwIYbc+jORxJrPOGZUvvM56tn8qghgfgQQADwGmRKCbW5yoRXM0OEGv/n4DelYES\n\tC4Zj48T+M6UEY40bDEgbg+uM6UsUT2Te1uEpauSf/Yy89jdrVVmjgtwWEzSZE72mhn\n\tkAVfHAbR/dWp43dqlf1NDhEXb6AT7yYBmKqhe+10=","Message-ID":"<880716f2-2499-46f5-ba3f-39a6c084156f@ideasonboard.com>","Date":"Wed, 18 Jun 2025 13:00:47 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","To":"Laurent Pinchart ,\n\tMilan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-3-mzamazal@redhat.com>\n\t<20250616222929.GG32454@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250616222929.GG32454@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":34587,"web_url":"https://patchwork.libcamera.org/comment/34587/","msgid":"<857c16f7az.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-20T21:10:28","subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for review.\n\nLaurent Pinchart writes:\n\n> Hi Milan,\n>\n> Thank you for the patch.\n>\n> On Mon, Jun 16, 2025 at 10:47:20AM +0200, Milan Zamazal wrote:\n>> Currently, libcamera can be configured in runtime using several\n>> environment variables. With introducing more and more variables, this\n>> mechanism reaches its limits. It would be simpler and more flexible if\n>> it was possible to configure libcamera in a single file.\n>> \n>> For example, there was a request to define pipeline precedence in\n>> runtime. We want to compile in multiple pipelines, in order to have\n>> them accessible within single packages in distributions. And then being\n>> able to select among the pipelines manually as needed based on the\n>> particular hardware or operating system environment. Having the\n>> configuration file then allows easy switching between hardware, GPU or\n>> CPU IPAs. Another possible use case is tuning image output, especially\n>> with software ISP, to user liking. For example, some users may prefer\n>> higher contrast without the need to use the corresponding knobs, if\n>> present at all, in every application.\n>\n> That's an interesting use case, but I don't think it necessarily belong\n> to the global configuration file. In the context of a desktop\n> distribution where libcamera is used through pipewire, user preferences\n> are I think best handled through pipewire. This isn't a topic that needs\n> to be decided now, I foresee lots of interesting discussions in the\n> future about what should go to the configuration file and what\n> shouldn't.\n\nYes, I'll drop that part from the commit message.\n\n>> The configuration file can also\n>> be used to enable or disable experimental features and avoid the need to\n>> track local patches changing configuration options hard-wired in the\n>> code when working on new features.\n>> \n>> This patch introduces basic support for configuration files.\n>> GlobalConfiguration class reads, stores and accesses the configuration.\n>> GlobalConfiguration instances are meant to be stored to and accessed\n>> from instances of another class, preferably from a single instance.\n>> GlobalConfiguration is defined in base because it will be accessed from\n>> code in base, e.g. logging.\n>\n> As mentioned in the review of 01/13, moving YamlParser to base is\n> problematic. Let's see in the appropriate patch if we can configure the\n> logger without having configuration file support in base.\n\nI'm not going to introduce logging configuration in this series. I\nthought there was something else needing the configuration base but\napparently it's not any more.\n\n>> libcamera configuration can be specified using a system-wide\n>> configuration file or a user configuration file. The user configuration\n>> file takes precedence if present. There is currently no way to merge\n>> multiple configuration files, the one found is used as the only\n>> configuration file.\n>\n> Is this an issue ? \n\nI don't think so but it should be mentioned to be clear.\n\n> I'm wondering if we need a default configuration in\n> /usr/share/libcamera and a system-specific override in /etc/libcamera\n> (or somewhere else).\n\nAt the moment, we don't need a default configuration in\n/usr/share/libcamera, maybe distributions could utilise it but they can\nsimply patch libcamera if they need to change e.g. some path.\n/etc/libcamera is needed, for system-specific defaults.\n\n>> If no configuration file is present, nothing\n>> changes to the current libcamera behavior (except for some log\n>> messages related to configuration file lookup).\n>> \n>> The configuration file is a YAML file. We already have a mechanism for\n>> handling YAML configuration files in libcamera and the given\n>> infrastructure can be reused for the purpose. However, the\n>> configuration type is abstracted to make contingent future change of the\n>> underlying class easier while retaining (most of) the original API.\n>\n> What changes are you expecting ?\n\nIf we ever decide not to use YamlObject& as the underlying data\nstructure, this makes the change easier. And conceptually, there is no\nreason why the configuration type should be YamlObject&, it just happens\nto be a good match in the current implementation.\n\n>> The configuration is versioned. This has currently no particular\n>> meaning but is likely to have its purpose in future, especially once\n>> configuration validation is introduced.\n>> \n>> The configuration YAML file looks as follows:\n>> \n>> ---\n>> version: 1\n>> configuration:\n>> WHATEVER CONFIGURATION NEEDED\n>> \n>> This patch introduces just the basic idea. Actually using the\n>> configuration in the corresponding places (everything what is currently\n>> configurable via environment variables should be configurable in the\n>> file configuration) and other enhancements are implemented in the\n>> followup patches.\n>\n> I probably won't have many comments on this patch yet, I'll review the\n> remaining of the series first to see how the class gets used. Skimming\n> through the series though, I was expecting this patch to introduce a\n> fully-fledged class, but I see that quite a few extensions to the class\n> are spread in other patches. I don't mind too much either way though.\n\nIt may be a bit messy currently, I can change it if it is easier to\nreview etc., once it's unlikely major rewrite of the code is needed.\n\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> .../libcamera/internal/global_configuration.h | 34 ++++\n>> include/libcamera/internal/meson.build | 1 +\n>> src/libcamera/base/global_configuration.cpp | 150 ++++++++++++++++++\n>> src/libcamera/base/meson.build | 1 +\n>> 4 files changed, 186 insertions(+)\n>> create mode 100644 include/libcamera/internal/global_configuration.h\n>> create mode 100644 src/libcamera/base/global_configuration.cpp\n>> \n>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>> new file mode 100644\n>> index 000000000..8d0410ac2\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/global_configuration.h\n>> @@ -0,0 +1,34 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024-2025 Red Hat, inc.\n>> + *\n>> + * Global configuration handling\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/yaml_parser.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +class GlobalConfiguration\n>> +{\n>> +public:\n>> +\tusing Configuration = const YamlObject &;\n>> +\n>> +\tGlobalConfiguration();\n>> +\n>> +\tunsigned int version() const;\n>> +\tConfiguration configuration() const;\n>> +\n>> +private:\n>> +\tbool loadFile(const std::filesystem::path &fileName);\n>> +\tvoid load();\n>> +\tConfiguration get() const;\n>> +\n>> +\tstd::unique_ptr yamlConfiguration;\n>\n> Missing trailing underscore.\n\nWill fix.\n\n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n>> index 33f318b2b..951bfee49 100644\n>> --- a/include/libcamera/internal/meson.build\n>> +++ b/include/libcamera/internal/meson.build\n>> @@ -22,6 +22,7 @@ libcamera_internal_headers = files([\n>> 'dma_buf_allocator.h',\n>> 'formats.h',\n>> 'framebuffer.h',\n>> + 'global_configuration.h',\n>\n> This would belong to include/libcamera/base/ if global_configuration.cpp\n> is in base too.\n>\n>> 'ipa_data_serializer.h',\n>> 'ipa_manager.h',\n>> 'ipa_module.h',\n>> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\n>> new file mode 100644\n>> index 000000000..1ddf5bc33\n>> --- /dev/null\n>> +++ b/src/libcamera/base/global_configuration.cpp\n>> @@ -0,0 +1,150 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024-2025 Red Hat, inc.\n>> + *\n>> + * Global configuration handling\n>> + */\n>> +\n>> +#include \"libcamera/internal/global_configuration.h\"\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \"libcamera/internal/yaml_parser.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace {\n>> +const std::vector\n>> +\tglobalConfigurationFiles = {\n>> +\t\tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n>\n> Given that distributions install libcamera with prefix set to /usr, this\n> would be /usr/etc/libcamera/configuration.yaml, which is not a valid\n> location according to\n> https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s09.html.\n\nClarified by Barnabás.\n\n>> +\t\tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n>> +\t};\n>\n> You can write\n>\n> const std::vector globalConfigurationFiles = {\n> \tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n> \tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n> };\n>\n> And you could move this to a static const variable to\n> GlobalConfiguration::load(), the only place where it is used. But\n> looking at the rest of the series, globalConfigurationFiles becomes\n> unused somewhere, and should be dropped in the corresponding patch.\n\nAh, right, will do.\n\n>> +}\n>> +\n>> +LOG_DEFINE_CATEGORY(Configuration)\n>> +\n>> +/**\n>> + * \\class GlobalConfiguration\n>> + * \\brief Support for global libcamera configuration\n>> + *\n>> + * The configuration file is a YAML file and the configuration itself is stored\n>> + * under `configuration' top-level item.\n>> + *\n>> + * The configuration file is looked up in user's home directory first and if it\n>> + * is not found then in system-wide configuration directories. If multiple\n>> + * configuration files exist then only the first one found is used and no\n>> + * configuration merging is performed.\n>> + */\n>> +\n>> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n>> +{\n>> +\tFile file(fileName);\n>> +\tif (!file.exists()) {\n>> +\t\treturn false;\n>> +\t}\n>\n> No need for curly braces.\n\nOK.\n\n>> +\n>> +\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n>> +\t\tLOG(Configuration, Warning)\n>> +\t\t\t<< \"Failed to open configuration file \" << fileName;\n>> +\t\treturn true;\n>> +\t}\n>> +\n>> +\tauto root = YamlParser::parse(file);\n>\n> Maybe\n>\n> \tyamlConfiguration_ = YamlParser::parse(file);\n>\n> ?\n\nYes, should work.\n\n>> +\tif (!root) {\n>> +\t\tLOG(Configuration, Warning)\n>> +\t\t\t<< \"Failed to parse configuration file \" << fileName;\n>> +\t\treturn true;\n>> +\t}\n>> +\tyamlConfiguration = std::move(root);\n>> +\n>> +\treturn true;\n>> +}\n>> +\n>> +void GlobalConfiguration::load()\n>> +{\n>> +\tstd::filesystem::path userConfigurationDirectory;\n>> +\tchar *xdgConfigHome = utils::secure_getenv(\"XDG_CONFIG_HOME\");\n>\n> const\n\nOK.\n\n>> +\tif (xdgConfigHome) {\n>> +\t\tuserConfigurationDirectory = xdgConfigHome;\n>> +\t} else {\n>> +\t\tconst char *home = utils::secure_getenv(\"HOME\");\n>> +\t\tif (home)\n>> +\t\t\tuserConfigurationDirectory =\n>> +\t\t\t\tstd::filesystem::path(home) / \".config\";\n>> +\t}\n>> +\n>> +\tif (!userConfigurationDirectory.empty()) {\n>> +\t\tstd::filesystem::path user_configuration_file =\n>\n> camelCase\n\nOK.\n\n>> +\t\t\tuserConfigurationDirectory / \"libcamera\" / \"configuration.yaml\";\n>> +\t\tif (loadFile(user_configuration_file))\n>> +\t\t\treturn;\n>\n> If the file exists but can't be loaded, should we make it a fatal error\n> ? Continuing by ignoring malformed configuration files means we'll run\n> with a configuration different from what the user expects.\n\nI think so, I don't recall why I made it just a warning. But: If it is\nfatal, how to propagate it in the current implementation, considering\nthe configuration is used already in CameraManager::Private constructor?\nHow to fail early enough?\n\n>> +\t}\n>> +\n>> +\tfor (const auto &path : globalConfigurationFiles)\n>> +\t\tif (loadFile(path))\n>> +\t\t\treturn;\n>\n> Please use curly braces for the outer scope.\n\nOK.\n\n>> +}\n>> +\n>> +GlobalConfiguration::Configuration GlobalConfiguration::get() const\n>> +{\n>> +\treturn *yamlConfiguration;\n>\n> I think you can drop this private function and access yamlConfiguration_\n> directly internally.\n\nOK.\n\n>> +}\n>> +\n>> +/**\n>> + * \\brief Initialize the global configuration\n>> + */\n>> +GlobalConfiguration::GlobalConfiguration()\n>> +{\n>> +\tload();\n>> +}\n>> +\n>> +/**\n>> + * \\typedef GlobalConfiguration::Configuration\n>> + * \\brief Type representing global libcamera configuration\n>> + *\n>> + * All code outside GlobalConfiguration must use this type declaration and not\n>> + * the underlying type.\n>\n> What's the reason for that ?\n\nI don't think YamlObject& is /the/ type for configuration, see my\nresponse to the related question in the commit message above.\n\n>> + */\n>> +\n>> +/**\n>> + * \\brief Return configuration version\n>> + *\n>> + * The version is (optionally) declared in the configuration file in the\n>> + * top-level section `version', alongside `configuration'. This has currently no\n>> + * real use but may be needed in future if configuration incompatibilities\n>> + * occur.\n>> + *\n>> + * \\return Configuration version as declared in the configuration file or 0 if\n>> + * no version is declared there\n>> + */\n>> +unsigned int GlobalConfiguration::version() const\n>> +{\n>> +\treturn get()[\"version\"].get().value_or(0);\n>> +}\n>> +\n>> +/**\n>> + * \\brief Return libcamera global configuration\n>> + *\n>> + * This returns the whole configuration stored in the top-level section\n>> + * `configuration' of the YAML configuration file.\n>> + *\n>> + * The requested part of the configuration can be accessed using \\a YamlObject\n>> + * methods.\n>> + *\n>> + * \\note \\a YamlObject type itself shouldn't be used in type declarations to\n>> + * avoid trouble if we decide to change the underlying data objects in future.\n>> + *\n>> + * \\return The whole configuration section\n>> + */\n>> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const\n>> +{\n>> +\treturn get()[\"configuration\"];\n>> +}\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n>> index 94843eb95..4c0032845 100644\n>> --- a/src/libcamera/base/meson.build\n>> +++ b/src/libcamera/base/meson.build\n>> @@ -16,6 +16,7 @@ libcamera_base_internal_sources = files([\n>> 'event_dispatcher_poll.cpp',\n>> 'event_notifier.cpp',\n>> 'file.cpp',\n>> + 'global_configuration.cpp',\n>> 'log.cpp',\n>> 'memfd.cpp',\n>> 'message.cpp',","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 ADABFC3237\n\tfor ;\n\tFri, 20 Jun 2025 21:10:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7D3661538;\n\tFri, 20 Jun 2025 23:10:36 +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 9563461535\n\tfor ;\n\tFri, 20 Jun 2025 23:10:34 +0200 (CEST)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-119-K9VfpGIzPaObRbzYY1rR9A-1; Fri, 20 Jun 2025 17:10:31 -0400","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-3a578958000so840017f8f.3\n\tfor ;\n\tFri, 20 Jun 2025 14:10:31 -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\tffacd0b85a97d-3a6d0f1ac5asm3074100f8f.33.2025.06.20.14.10.28\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 20 Jun 2025 14:10:29 -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=\"KS4yZt0i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1750453833;\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=mHKYB6TEqHCcY47V2/tTqotnG0zjl6wQ09A93O985W4=;\n\tb=KS4yZt0i3SdeEEGZbVW54gmhDWwMZcnj2wTHiP8Ep7AprnHjGHdOTofp5sL56uMmRaI6HJ\n\tJfa/uzJh5Q95UD1RCFwYvSYoM+7HmWsRpYKyyZnMnzzrKMwN/ShYchvca0i5ZUfTKRiCH3\n\toHr1AlRav41qH3NxGt9SzhcpFLM75WY=","X-MC-Unique":"K9VfpGIzPaObRbzYY1rR9A-1","X-Mimecast-MFC-AGG-ID":"K9VfpGIzPaObRbzYY1rR9A_1750453830","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750453830; x=1751058630;\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=mXOWjK1WKn42kfUe+hVr20JdwLOZrq0y1MZpp4eF4hk=;\n\tb=kYvW/YT77pu25R/u8j//MirNBEa3sZwsVB7yyyt17xW8xOeA6eGfS2kwEOFt3fqHxg\n\tBqxcdNIvcB3xT7krZ5UNfwj+2TPafKR2aPEzRRFi4zGE902s0LxZ98PrZfmWeisazECv\n\tTooHZgcE1+HloN0V1DD1BlpWlJBJcGnJ/+Mtne+zxbR41Th4uDfa+kdI+ibj7sYViRkt\n\tXUwnZZ9qd5s32AjAj7eVoM33Yc9znkgz2eLlQvjIvr46DFZZu3poXCoGltNV91bkhGu7\n\t7Q1WXDvd+18H/Tp+Z+ITI5J2DcHmxXnoDxYYxC9BdlSZFd5xb3rH+9AtUTYuMnJC6Kxc\n\tdgRA==","X-Gm-Message-State":"AOJu0YyPq3l1n3eByAWqxLpL/+yK/VY/LEmEdl8mMsUUoAXXlx2S0COa\n\tjDkxuI9CuhjZZMGmWU55dKKBRW+l6vKv3CNMo7N5W3D1mpvMAqJme91bpenaRNEWffW1MJZsYZl\n\tC/uTMvdVSNu9Jz1TL3pQHpPPjtDg7QnsiJRhyBBFWtHyJmjWXXEZazOeOaMeJh1vjV7GJvJzxu6\n\tY=","X-Gm-Gg":"ASbGncsOKgB2aZyDoRj7VWqH1qdc1e2p1ZlFJaVZlu3IZPryLtyvebw3YWPLeIOlB6T\n\t5+9HbL5cPWQcyh78SSYXA8wdQPomtFDs+WEHlA1uh8jDbljWY6dc6bUEU6VVoFar/+W85UFj8c1\n\trsnwbJgVY/dSETpT2i5cW79t60cmcRYHcNfOK69U9o2MAMb6yNXauEs5ZW0nIkRS5bknun+Snuy\n\tDRLGPUUhr691cIiRriCqP5s6amQc5wgW9R3CtOUtgAnprDMPRK4M6Am8RjbnsUo0j5400QALqI2\n\tsYDB6pf7vlOnApR52x30jHXeiV7qp84MzfDJWLdaz7RFqiUInxxoAw3jZUPMv+q9dXTsR+lOmRw\n\t=","X-Received":["by 2002:a05:6000:2481:b0:3a5:2ec5:35b8 with SMTP id\n\tffacd0b85a97d-3a6d11910f4mr3437711f8f.11.1750453830293; \n\tFri, 20 Jun 2025 14:10:30 -0700 (PDT)","by 2002:a05:6000:2481:b0:3a5:2ec5:35b8 with SMTP id\n\tffacd0b85a97d-3a6d11910f4mr3437699f8f.11.1750453829721; \n\tFri, 20 Jun 2025 14:10:29 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGSkqzC1HK02LGA/vJUA8dY1vRonnwL5+5Q0Y77PxyCZzD1nQzSEiJzDzAMe0X4ylZFoE5e7A==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ","Subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","In-Reply-To":"<20250616222929.GG32454@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Tue, 17 Jun 2025 01:29:29 +0300\")","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-3-mzamazal@redhat.com>\n\t<20250616222929.GG32454@pendragon.ideasonboard.com>","Date":"Fri, 20 Jun 2025 23:10:28 +0200","Message-ID":"<857c16f7az.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":"ks9LzWT63ICQcaXQsQCI0Zr6hrGt7Tr2rIKRJt5mcz4_1750453830","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":34588,"web_url":"https://patchwork.libcamera.org/comment/34588/","msgid":"<855xgqf74u.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-20T21:14:09","subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","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. 06. 17. 0:29 keltezéssel, Laurent Pinchart írta:\n>> Hi Milan,\n>> Thank you for the patch.\n>> On Mon, Jun 16, 2025 at 10:47:20AM +0200, Milan Zamazal wrote:\n>>> Currently, libcamera can be configured in runtime using several\n>>> environment variables. With introducing more and more variables, this\n>>> mechanism reaches its limits. It would be simpler and more flexible if\n>>> it was possible to configure libcamera in a single file.\n>>>\n>>> For example, there was a request to define pipeline precedence in\n>>> runtime. We want to compile in multiple pipelines, in order to have\n>>> them accessible within single packages in distributions. And then being\n>>> able to select among the pipelines manually as needed based on the\n>>> particular hardware or operating system environment. Having the\n>>> configuration file then allows easy switching between hardware, GPU or\n>>> CPU IPAs. Another possible use case is tuning image output, especially\n>>> with software ISP, to user liking. For example, some users may prefer\n>>> higher contrast without the need to use the corresponding knobs, if\n>>> present at all, in every application.\n>> That's an interesting use case, but I don't think it necessarily belong\n>> to the global configuration file. In the context of a desktop\n>> distribution where libcamera is used through pipewire, user preferences\n>> are I think best handled through pipewire. This isn't a topic that needs\n>> to be decided now, I foresee lots of interesting discussions in the\n>> future about what should go to the configuration file and what\n>> shouldn't.\n>> \n>>> The configuration file can also\n>>> be used to enable or disable experimental features and avoid the need to\n>>> track local patches changing configuration options hard-wired in the\n>>> code when working on new features.\n>>>\n>>> This patch introduces basic support for configuration files.\n>>> GlobalConfiguration class reads, stores and accesses the configuration.\n>>> GlobalConfiguration instances are meant to be stored to and accessed\n>>> from instances of another class, preferably from a single instance.\n>>> GlobalConfiguration is defined in base because it will be accessed from\n>>> code in base, e.g. logging.\n>> As mentioned in the review of 01/13, moving YamlParser to base is\n>> problematic. Let's see in the appropriate patch if we can configure the\n>> logger without having configuration file support in base.\n>> \n>>> libcamera configuration can be specified using a system-wide\n>>> configuration file or a user configuration file. The user configuration\n>>> file takes precedence if present. There is currently no way to merge\n>>> multiple configuration files, the one found is used as the only\n>>> configuration file.\n>> Is this an issue ? I'm wondering if we need a default configuration in\n>> /usr/share/libcamera and a system-specific override in /etc/libcamera\n>> (or somewhere else).\n>> \n>>> If no configuration file is present, nothing\n>>> changes to the current libcamera behavior (except for some log\n>>> messages related to configuration file lookup).\n>>>\n>>> The configuration file is a YAML file. We already have a mechanism for\n>>> handling YAML configuration files in libcamera and the given\n>>> infrastructure can be reused for the purpose. However, the\n>>> configuration type is abstracted to make contingent future change of the\n>>> underlying class easier while retaining (most of) the original API.\n>> What changes are you expecting ?\n>> \n>>>\n>>> The configuration is versioned. This has currently no particular\n>>> meaning but is likely to have its purpose in future, especially once\n>>> configuration validation is introduced.\n>>>\n>>> The configuration YAML file looks as follows:\n>>>\n>>> ---\n>>> version: 1\n>>> configuration:\n>>> WHATEVER CONFIGURATION NEEDED\n>>>\n>>> This patch introduces just the basic idea. Actually using the\n>>> configuration in the corresponding places (everything what is currently\n>>> configurable via environment variables should be configurable in the\n>>> file configuration) and other enhancements are implemented in the\n>>> followup patches.\n>> I probably won't have many comments on this patch yet, I'll review the\n>> remaining of the series first to see how the class gets used. Skimming\n>> through the series though, I was expecting this patch to introduce a\n>> fully-fledged class, but I see that quite a few extensions to the class\n>> are spread in other patches. I don't mind too much either way though.\n>> \n>>>\n>>> Signed-off-by: Milan Zamazal \n>>> ---\n>>> .../libcamera/internal/global_configuration.h | 34 ++++\n>>> include/libcamera/internal/meson.build | 1 +\n>>> src/libcamera/base/global_configuration.cpp | 150 ++++++++++++++++++\n>>> src/libcamera/base/meson.build | 1 +\n>>> 4 files changed, 186 insertions(+)\n>>> create mode 100644 include/libcamera/internal/global_configuration.h\n>>> create mode 100644 src/libcamera/base/global_configuration.cpp\n>>>\n>>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>>> new file mode 100644\n>>> index 000000000..8d0410ac2\n>>> --- /dev/null\n>>> +++ b/include/libcamera/internal/global_configuration.h\n>>> @@ -0,0 +1,34 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2024-2025 Red Hat, inc.\n>>> + *\n>>> + * Global configuration handling\n>>> + */\n>>> +\n>>> +#pragma once\n>>> +\n>>> +#include \n>>> +\n>>> +#include \"libcamera/internal/yaml_parser.h\"\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +class GlobalConfiguration\n>>> +{\n>>> +public:\n>>> +\tusing Configuration = const YamlObject &;\n>>> +\n>>> +\tGlobalConfiguration();\n>>> +\n>>> +\tunsigned int version() const;\n>>> +\tConfiguration configuration() const;\n>>> +\n>>> +private:\n>>> +\tbool loadFile(const std::filesystem::path &fileName);\n>>> +\tvoid load();\n>>> +\tConfiguration get() const;\n>>> +\n>>> +\tstd::unique_ptr yamlConfiguration;\n>> Missing trailing underscore.\n>> \n>>> +};\n>>> +\n>>> +} /* namespace libcamera */\n>>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n>>> index 33f318b2b..951bfee49 100644\n>>> --- a/include/libcamera/internal/meson.build\n>>> +++ b/include/libcamera/internal/meson.build\n>>> @@ -22,6 +22,7 @@ libcamera_internal_headers = files([\n>>> 'dma_buf_allocator.h',\n>>> 'formats.h',\n>>> 'framebuffer.h',\n>>> + 'global_configuration.h',\n>> This would belong to include/libcamera/base/ if global_configuration.cpp\n>> is in base too.\n>> \n>>> 'ipa_data_serializer.h',\n>>> 'ipa_manager.h',\n>>> 'ipa_module.h',\n>>> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\n>>> new file mode 100644\n>>> index 000000000..1ddf5bc33\n>>> --- /dev/null\n>>> +++ b/src/libcamera/base/global_configuration.cpp\n>>> @@ -0,0 +1,150 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2024-2025 Red Hat, inc.\n>>> + *\n>>> + * Global configuration handling\n>>> + */\n>>> +\n>>> +#include \"libcamera/internal/global_configuration.h\"\n>>> +\n>>> +#include \n>>> +#include \n>>> +#include \n>>> +\n>>> +#include \n>>> +#include \n>>> +#include \n>>> +\n>>> +#include \"libcamera/internal/yaml_parser.h\"\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +namespace {\n>>> +const std::vector\n>>> +\tglobalConfigurationFiles = {\n>>> +\t\tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n>> Given that distributions install libcamera with prefix set to /usr, this\n>> would be /usr/etc/libcamera/configuration.yaml, which is not a valid\n>> location according to\n>> https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s09.html.\n>\n> Looks good to me. `LIBCAMERA_SYSCONF_DIR` becomes `/etc/libcamera/` if you set `-D prefix=/usr`.\n> See\n> https://mesonbuild.com/Release-notes-for-0-44-0.html#prefixdependent-defaults-for-sysconfdir-localstatedir-and-sharedstatedir\n\nThank you for clarification.\n\n> I would probably change the lookup order as follows:\n>\n> - LIBCAMERA_SYSCONF_DIR (/etc/libcamera usually)\n> - LIBCAMERA_DATA_DIR (/usr/share/libcamera usually)\n>\n> I am not sure that we want to hard-code \"/etc/libcamera\" there.\n\nWith the default /usr/local prefix, LIBCAMERA_SYSCONF_DIR is\n/usr/local/etc, which makes sense. Imagine somebody installs libcamera\nfrom the sources to /usr/local, the idea is that /etc/libcamera should\nstill be looked up and have a preference. Good or bad?\n\nAs for LIBCAMERA_DATA_DIR, it doesn't harm to look up the configuration\nthere, although it doesn't seem to be needed at the moment. I'm OK with\neither adding or not adding it.\n\n> Regards,\n> Barnabás Pőcze\n>\n>> \n>>> +\t\tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n>>> +\t};\n>> You can write\n>> const std::vector globalConfigurationFiles = {\n>> \tstd::filesystem::path(LIBCAMERA_SYSCONF_DIR) / \"configuration.yaml\",\n>> \tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n>> };\n>> And you could move this to a static const variable to\n>> GlobalConfiguration::load(), the only place where it is used. But\n>> looking at the rest of the series, globalConfigurationFiles becomes\n>> unused somewhere, and should be dropped in the corresponding patch.\n>> \n>>> +}\n>>> +\n>>> +LOG_DEFINE_CATEGORY(Configuration)\n>>> +\n>>> +/**\n>>> + * \\class GlobalConfiguration\n>>> + * \\brief Support for global libcamera configuration\n>>> + *\n>>> + * The configuration file is a YAML file and the configuration itself is stored\n>>> + * under `configuration' top-level item.\n>>> + *\n>>> + * The configuration file is looked up in user's home directory first and if it\n>>> + * is not found then in system-wide configuration directories. If multiple\n>>> + * configuration files exist then only the first one found is used and no\n>>> + * configuration merging is performed.\n>>> + */\n>>> +\n>>> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n>>> +{\n>>> +\tFile file(fileName);\n>>> +\tif (!file.exists()) {\n>>> +\t\treturn false;\n>>> +\t}\n>> No need for curly braces.\n>> \n>>> +\n>>> +\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n>>> +\t\tLOG(Configuration, Warning)\n>>> +\t\t\t<< \"Failed to open configuration file \" << fileName;\n>>> +\t\treturn true;\n>>> +\t}\n>>> +\n>>> +\tauto root = YamlParser::parse(file);\n>> Maybe\n>> \tyamlConfiguration_ = YamlParser::parse(file);\n>> ?\n>> \n>>> +\tif (!root) {\n>>> +\t\tLOG(Configuration, Warning)\n>>> +\t\t\t<< \"Failed to parse configuration file \" << fileName;\n>>> +\t\treturn true;\n>>> +\t}\n>>> +\tyamlConfiguration = std::move(root);\n>>> +\n>>> +\treturn true;\n>>> +}\n>>> +\n>>> +void GlobalConfiguration::load()\n>>> +{\n>>> +\tstd::filesystem::path userConfigurationDirectory;\n>>> +\tchar *xdgConfigHome = utils::secure_getenv(\"XDG_CONFIG_HOME\");\n>> const\n>> \n>>> +\tif (xdgConfigHome) {\n>>> +\t\tuserConfigurationDirectory = xdgConfigHome;\n>>> +\t} else {\n>>> +\t\tconst char *home = utils::secure_getenv(\"HOME\");\n>>> +\t\tif (home)\n>>> +\t\t\tuserConfigurationDirectory =\n>>> +\t\t\t\tstd::filesystem::path(home) / \".config\";\n>>> +\t}\n>>> +\n>>> +\tif (!userConfigurationDirectory.empty()) {\n>>> +\t\tstd::filesystem::path user_configuration_file =\n>> camelCase\n>> \n>>> +\t\t\tuserConfigurationDirectory / \"libcamera\" / \"configuration.yaml\";\n>>> +\t\tif (loadFile(user_configuration_file))\n>>> +\t\t\treturn;\n>> If the file exists but can't be loaded, should we make it a fatal error\n>> ? Continuing by ignoring malformed configuration files means we'll run\n>> with a configuration different from what the user expects.\n>> \n>>> +\t}\n>>> +\n>>> +\tfor (const auto &path : globalConfigurationFiles)\n>>> +\t\tif (loadFile(path))\n>>> +\t\t\treturn;\n>> Please use curly braces for the outer scope.\n>> \n>>> +}\n>>> +\n>>> +GlobalConfiguration::Configuration GlobalConfiguration::get() const\n>>> +{\n>>> +\treturn *yamlConfiguration;\n>> I think you can drop this private function and access yamlConfiguration_\n>> directly internally.\n>> \n>>> +}\n>>> +\n>>> +/**\n>>> + * \\brief Initialize the global configuration\n>>> + */\n>>> +GlobalConfiguration::GlobalConfiguration()\n>>> +{\n>>> +\tload();\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\typedef GlobalConfiguration::Configuration\n>>> + * \\brief Type representing global libcamera configuration\n>>> + *\n>>> + * All code outside GlobalConfiguration must use this type declaration and not\n>>> + * the underlying type.\n>> What's the reason for that ?\n>> \n>>> + */\n>>> +\n>>> +/**\n>>> + * \\brief Return configuration version\n>>> + *\n>>> + * The version is (optionally) declared in the configuration file in the\n>>> + * top-level section `version', alongside `configuration'. This has currently no\n>>> + * real use but may be needed in future if configuration incompatibilities\n>>> + * occur.\n>>> + *\n>>> + * \\return Configuration version as declared in the configuration file or 0 if\n>>> + * no version is declared there\n>>> + */\n>>> +unsigned int GlobalConfiguration::version() const\n>>> +{\n>>> +\treturn get()[\"version\"].get().value_or(0);\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\brief Return libcamera global configuration\n>>> + *\n>>> + * This returns the whole configuration stored in the top-level section\n>>> + * `configuration' of the YAML configuration file.\n>>> + *\n>>> + * The requested part of the configuration can be accessed using \\a YamlObject\n>>> + * methods.\n>>> + *\n>>> + * \\note \\a YamlObject type itself shouldn't be used in type declarations to\n>>> + * avoid trouble if we decide to change the underlying data objects in future.\n>>> + *\n>>> + * \\return The whole configuration section\n>>> + */\n>>> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const\n>>> +{\n>>> +\treturn get()[\"configuration\"];\n>>> +}\n>>> +\n>>> +} /* namespace libcamera */\n>>> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n>>> index 94843eb95..4c0032845 100644\n>>> --- a/src/libcamera/base/meson.build\n>>> +++ b/src/libcamera/base/meson.build\n>>> @@ -16,6 +16,7 @@ libcamera_base_internal_sources = files([\n>>> 'event_dispatcher_poll.cpp',\n>>> 'event_notifier.cpp',\n>>> 'file.cpp',\n>>> + 'global_configuration.cpp',\n>>> 'log.cpp',\n>>> 'memfd.cpp',\n>>> 'message.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 CB48DBDE6B\n\tfor ;\n\tFri, 20 Jun 2025 21:14:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6BC668DD0;\n\tFri, 20 Jun 2025 23:14:17 +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 ADEED61535\n\tfor ;\n\tFri, 20 Jun 2025 23:14:15 +0200 (CEST)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-260-0vuMVFHiM1OcnGrgjSbBrw-1; Fri, 20 Jun 2025 17:14:13 -0400","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-450d244bfabso17763275e9.0\n\tfor ;\n\tFri, 20 Jun 2025 14:14:12 -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\t5b1f17b1804b1-453646cb672sm36160335e9.6.2025.06.20.14.14.10\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 20 Jun 2025 14:14:10 -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=\"SXp2K8iQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1750454054;\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=lH9AEqjNQF/ahTr651joCamiPOtHOOUFvS0bxKkd3gQ=;\n\tb=SXp2K8iQPCgPh7cnm6g2gOJ5XEEBb02Ftkpgywdy/OjpTRT/2t7SO0mp94/+Fdo5I5a7AM\n\tmvx1jbU8foHE/6/S3lakIETqBvGmo83UUJxWo6xqLOCL5ZWFYZsb6HrJtT/NuQ/PiXWi/T\n\t064n6vhIMO5CckHSkqyNLewUgO1nhuY=","X-MC-Unique":"0vuMVFHiM1OcnGrgjSbBrw-1","X-Mimecast-MFC-AGG-ID":"0vuMVFHiM1OcnGrgjSbBrw_1750454052","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750454052; x=1751058852;\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=RGc/PSnGvfuVhPpvA0/STLmn8XFyVsPoZkvI8NeWGz8=;\n\tb=NQn1s4hJ9hiiYAsJiGJAw+4ZMbrVb/pgP8jK+DQ9kaLJ0GhhxYiGTmQSLfBYY41bOE\n\taOxXfH+F7hZqaeSfE5k1oc2xRKn98BXOq6PULtk7eeBtNgTPWXMbD75tCJyQyFCYSQDm\n\tWT8gQBWnPdFH4icTU3tJHebVz1yc2DlvJAKEK0vX5n2oPE7KBOPPtBlr859LbOIEMzyh\n\tdnjz+a0bG14jeR/cT+GPAfVi3v2LS0a2FUono0oRgcwGPZRD5JVnUd+X05XNU8cvYT4C\n\tzP8wHzPsSEZXf9ekowu/R9g9chZ0oTxRdUF2SZVMSuldAw7fViwARKnpSN+miiQNNqpl\n\tJWEw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXSt6DqSMRGvp8/lRIPQjJBIkriTb4OibJalPBwO1QhFN2GOjCbezQU4qw11qucZ4AHWh3QwYIinnFO/RpcKfI=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yz6FNg80ZbuhrNQy+nM/6gdY5xkuR8kI6KKX9mIeF3Qb1QHrpvP\n\tqzSueqe02I9B0eoUPU1XApiw61fLTMvRqEv3RHfrUsGi9w/1DpU4ugLiOUOCD/7pb6VjnKM6fuc\n\t5GEoYyz9SrVikzjTZXR504vZ+zG6RerjxY+8SW/q8p12R6LPjdcxwr2xF8kYa/wqmYJPZe7LaRd\n\tw=","X-Gm-Gg":"ASbGncsDlHg5kESCkp97DmC4SeeRroB3kWZDhollGW0PHmBFy5RqzUGBECbMsJcEtKr\n\tIRIaIqkuKOkDpMqhLxET2WpcRQQumE/ehKkVaJzNPTCpO3Ie53VNkjuewFIrKHp1sReLnYE2j/T\n\tZQtX2KOUxtgFF7WC7e4A2Siputqc7fqhw9h6IcYkKfw6W24Jn8JfCJ2H5gActRdD5xM1cpvQh9W\n\t7iAyhx5DEnJYS3ErmfIzD9DWdlIV9kkhAhhsslxBDOqI8LK2eHzHMTbIf1cQoMAKcmkh1LUAG6X\n\tDSMtYfuanmwXAq1IEXehxvsJugiUim6MyKwJcXod+U18l6taMMerps+MvzHwPFZ0wPolRhchojc\n\t=","X-Received":["by 2002:a05:600c:3115:b0:453:9bf:6f7c with SMTP id\n\t5b1f17b1804b1-453657be1bemr45196775e9.9.1750454051676; \n\tFri, 20 Jun 2025 14:14:11 -0700 (PDT)","by 2002:a05:600c:3115:b0:453:9bf:6f7c with SMTP id\n\t5b1f17b1804b1-453657be1bemr45196585e9.9.1750454051143; \n\tFri, 20 Jun 2025 14:14:11 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IFkpGLgh1jCDPADLOfKR8ueLWtNfK97rbRgf50JnSWBVi6HhgWaJ6/cuduQfMrmGk0cdCHc9g==","From":"Milan Zamazal ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org, Kieran Bingham\n\t","Subject":"Re: [PATCH v10 02/13] config: Introduce global runtime configuration","In-Reply-To":"<880716f2-2499-46f5-ba3f-39a6c084156f@ideasonboard.com> (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Wed,\n\t18 Jun 2025 13:00:47 +0200\")","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-3-mzamazal@redhat.com>\n\t<20250616222929.GG32454@pendragon.ideasonboard.com>\n\t<880716f2-2499-46f5-ba3f-39a6c084156f@ideasonboard.com>","Date":"Fri, 20 Jun 2025 23:14:09 +0200","Message-ID":"<855xgqf74u.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":"9kb2siTdell-uu30OcQP78rLBm8yqda4rGz09nWpwXc_1750454052","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\" "}}]