[{"id":38643,"web_url":"https://patchwork.libcamera.org/comment/38643/","msgid":"<630c06f9-cc8a-45c3-9991-c95c7a68d803@ideasonboard.com>","date":"2026-04-24T07:58:22","subject":"Re: [PATCH v3 33/37] libcamera: global_configuration: Override\n\toptions with environment variables","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 04. 24. 1:00 keltezéssel, Laurent Pinchart írta:\n> libcamera supports several environment variables that can override\n> configuration options. This requires components that use such overrides\n> to call the special envOption() and envListOption() functions, spreading\n> knowledge of the overrides through the code base. This will hinder\n> future enhancements to the global configuration, such as implementing\n> per-camera options that will override pipeline handler-level options. To\n> prepare for that, move handling of the environment variables to the\n> GlobalConfiguration class.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> ---\n> Changes since v1:\n> \n> - Avoid copy in range-based loop\n> - Drop unneeded constructor\n> - Erase existing node before overriding\n> - Move override code to constructor\n> ---\n\nLooks ok to me.\n\nReviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\n\n>   .../libcamera/internal/global_configuration.h |   7 -\n>   src/libcamera/camera_manager.cpp              |   4 +-\n>   src/libcamera/global_configuration.cpp        | 180 +++++++++++-------\n>   src/libcamera/ipa_manager.cpp                 |   9 +-\n>   src/libcamera/ipa_proxy.cpp                   |   6 +-\n>   src/libcamera/software_isp/software_isp.cpp   |   4 +-\n>   6 files changed, 124 insertions(+), 86 deletions(-)\n> \n> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n> index 5eb646e33fc2..0d2ccb8a1808 100644\n> --- a/include/libcamera/internal/global_configuration.h\n> +++ b/include/libcamera/internal/global_configuration.h\n> @@ -42,13 +42,6 @@ public:\n> \n>   \tstd::optional<std::vector<std::string>> listOption(\n>   \t\tconst std::initializer_list<std::string_view> confPath) const;\n> -\tstd::optional<std::string> envOption(\n> -\t\tconst char *const envVariable,\n> -\t\tconst std::initializer_list<std::string_view> confPath) const;\n> -\tstd::optional<std::vector<std::string>> envListOption(\n> -\t\tconst char *const envVariable,\n> -\t\tconst std::initializer_list<std::string_view> confPath,\n> -\t\tconst std::string delimiter = \":\") const;\n> \n>   private:\n>   \tvoid load();\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index f774bd84291b..0dd4e0c590a1 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -114,9 +114,7 @@ void CameraManager::Private::createPipelineHandlers()\n>   \t * there is no configuration file.\n>   \t */\n>   \tconst auto pipesList =\n> -\t\tconfiguration().envListOption(\"LIBCAMERA_PIPELINES_MATCH_LIST\",\n> -\t\t\t\t\t      { \"pipelines_match_list\" },\n> -\t\t\t\t\t      \",\");\n> +\t\tconfiguration().listOption({ \"pipelines_match_list\" });\n>   \tif (pipesList.has_value()) {\n>   \t\t/*\n>   \t\t * When a list of preferred pipelines is defined, iterate\n> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp\n> index 62b9762d3e31..46a285c3c4ea 100644\n> --- a/src/libcamera/global_configuration.cpp\n> +++ b/src/libcamera/global_configuration.cpp\n> @@ -7,6 +7,7 @@\n> \n>   #include \"libcamera/internal/global_configuration.h\"\n> \n> +#include <array>\n>   #include <filesystem>\n>   #include <memory>\n>   #include <optional>\n> @@ -30,6 +31,99 @@ const std::vector<std::filesystem::path> globalConfigurationFiles = {\n>   \tstd::filesystem::path(LIBCAMERA_DATA_DIR) / \"configuration.yaml\",\n>   };\n> \n> +class EnvironmentProcessor\n> +{\n> +public:\n> +\tvirtual ~EnvironmentProcessor() = default;\n> +\n> +\tvirtual void process(ValueNode &node, const char *env) = 0;\n> +};\n> +\n> +/* A processor that sets a fixed value. */\n> +template<typename T>\n> +class EnvironmentFixedProcessor : public EnvironmentProcessor\n> +{\n> +public:\n> +\tEnvironmentFixedProcessor(const T &value)\n> +\t\t: value_(value)\n> +\t{\n> +\t}\n> +\n> +\tvoid process(ValueNode &node, [[maybe_unused]] const char *env) override\n> +\t{\n> +\t\tnode.set(value_);\n> +\t}\n> +\n> +private:\n> +\tT value_;\n> +};\n> +\n> +/*\n> + * A processor that parses the environment variable as a list of strings with a\n> + * custom delimiter.\n> + */\n> +class EnvironmentListProcessor : public EnvironmentProcessor\n> +{\n> +public:\n> +\tEnvironmentListProcessor(const char *delimiter)\n> +\t\t: delimiter_(delimiter)\n> +\t{\n> +\t}\n> +\n> +\tvoid process(ValueNode &node, const char *env) override\n> +\t{\n> +\t\tfor (auto &&value : utils::split(env, delimiter_))\n> +\t\t\tnode.add(std::make_unique<ValueNode>(std::move(value)));\n> +\t}\n> +\n> +private:\n> +\tconst std::string delimiter_;\n> +};\n> +\n> +/* A processor that copies the value of the environment variable. */\n> +class EnvironmentValueProcessor : public EnvironmentProcessor\n> +{\n> +public:\n> +\tvoid process(ValueNode &node, const char *env) override\n> +\t{\n> +\t\tnode.set(std::string{ env });\n> +\t}\n> +};\n> +\n> +struct EnvironmentOverride {\n> +\tconst char *variable;\n> +\tstd::initializer_list<std::string_view> path;\n> +\tstd::unique_ptr<EnvironmentProcessor> processor;\n> +};\n> +\n> +const std::array<EnvironmentOverride, 6> environmentOverrides{ {\n> +\t{\n> +\t\t\"LIBCAMERA_IPA_CONFIG_PATH\",\n> +\t\t{ \"ipa\", \"config_paths\" },\n> +\t\tstd::make_unique<EnvironmentListProcessor>(\":\"),\n> +\t}, {\n> +\t\t\"LIBCAMERA_IPA_FORCE_ISOLATION\",\n> +\t\t{ \"ipa\", \"force_isolation\" },\n> +\t\tstd::make_unique<EnvironmentFixedProcessor<bool>>(true),\n> +\t}, {\n> +\t\t\"LIBCAMERA_IPA_MODULE_PATH\",\n> +\t\t{ \"ipa\", \"module_paths\" },\n> +\t\tstd::make_unique<EnvironmentListProcessor>(\":\"),\n> +\t}, {\n> +\t\t\"LIBCAMERA_IPA_PROXY_PATH\",\n> +\t\t{ \"ipa\", \"proxy_paths\" },\n> +\t\tstd::make_unique<EnvironmentListProcessor>(\":\"),\n> +\t}, {\n> +\t\t\"LIBCAMERA_PIPELINES_MATCH_LIST\",\n> +\t\t{ \"pipelines_match_list\" },\n> +\t\tstd::make_unique<EnvironmentListProcessor>(\",\"),\n> +\t}, {\n> +\t\t\"LIBCAMERA_SOFTISP_MODE\",\n> +\t\t{ \"software_isp\", \"mode\" },\n> +\t\tstd::make_unique<EnvironmentValueProcessor>(),\n> +\t},\n> +} };\n> +\n>   } /* namespace */\n> \n>   LOG_DEFINE_CATEGORY(Configuration)\n> @@ -50,9 +144,9 @@ LOG_DEFINE_CATEGORY(Configuration)\n>    * reported and no configuration file is used. This is to prevent libcamera from\n>    * using an unintended configuration file.\n>    *\n> - * The configuration can be accessed using the provided helpers, namely\n> - * option(), envOption(), listOption() and envListOption() to access individual\n> - * options, or configuration() to access the whole configuration.\n> + * The configuration can be accessed using the provided helpers, namely option()\n> + * and listOption() to access individual options, or configuration() to access\n> + * the whole configuration.\n>    */\n> \n>   /**\n> @@ -66,6 +160,25 @@ GlobalConfiguration::GlobalConfiguration()\n>   \t\tconfiguration_->add(\"version\", std::make_unique<ValueNode>(1));\n>   \t\tconfiguration_->add(\"configuration\", std::make_unique<ValueNode>());\n>   \t}\n> +\n> +\t/* Process environment variables that override configuration options. */\n> +\tValueNode *cfg = configuration_->at(\"configuration\");\n> +\n> +\tfor (const EnvironmentOverride &envOverride : environmentOverrides) {\n> +\t\tconst char *envValue = utils::secure_getenv(envOverride.variable);\n> +\t\tif (!envValue || !envValue[0])\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::unique_ptr<ValueNode> node = std::make_unique<ValueNode>();\n> +\t\tenvOverride.processor->process(*node.get(), envValue);\n> +\n> +\t\tcfg->erase(envOverride.path);\n> +\n> +\t\tif (!cfg->add(envOverride.path, std::move(node)))\n> +\t\t\tLOG(Configuration, Error)\n> +\t\t\t\t<< \"Failed to override \"\n> +\t\t\t\t<< utils::join(envOverride.path, \"/\");\n> +\t}\n>   }\n> \n>   void GlobalConfiguration::load()\n> @@ -185,65 +298,4 @@ std::optional<std::vector<std::string>> GlobalConfiguration::listOption(\n>   \treturn c->get<std::vector<std::string>>();\n>   }\n> \n> -/**\n> - * \\brief Retrieve the value of environment variable with a fallback on the configuration file\n> - * \\param[in] envVariable Environment variable to get the value from\n> - * \\param[in] confPath The sequence of YAML section names to fall back on when\n> - * \\a envVariable is unavailable\n> - *\n> - * This helper looks first at the given environment variable and if it is\n> - * defined then it returns its value (even if it is empty). Otherwise it looks\n> - * for \\a confPath the same way as in GlobalConfiguration::option. Only string\n> - * values are supported.\n> - *\n> - * \\note Support for using environment variables to configure libcamera behavior\n> - * is provided here mostly for backward compatibility reasons. Introducing new\n> - * configuration environment variables is discouraged.\n> - *\n> - * \\return The value retrieved from the given environment if it is set,\n> - * otherwise the value from the configuration file if it exists, or no value if\n> - * it does not\n> - */\n> -std::optional<std::string> GlobalConfiguration::envOption(\n> -\tconst char *envVariable,\n> -\tconst std::initializer_list<std::string_view> confPath) const\n> -{\n> -\tconst char *envValue = utils::secure_getenv(envVariable);\n> -\tif (envValue)\n> -\t\treturn std::optional{ std::string{ envValue } };\n> -\treturn option<std::string>(confPath);\n> -}\n> -\n> -/**\n> - * \\brief Retrieve the value of the configuration option from a file or environment\n> - * \\param[in] envVariable Environment variable to get the value from\n> - * \\param[in] confPath The same as in GlobalConfiguration::option\n> - * \\param[in] delimiter Items separator in the environment variable\n> - *\n> - * This helper looks first at the given environment variable and if it is\n> - * defined (even if it is empty) then it splits its value by semicolons and\n> - * returns the resulting list of strings. Otherwise it looks for \\a confPath the\n> - * same way as in GlobalConfiguration::option, value of which must be a list of\n> - * strings.\n> - *\n> - * \\note Support for using environment variables to configure libcamera behavior\n> - * is provided here mostly for backward compatibility reasons. Introducing new\n> - * configuration environment variables is discouraged.\n> - *\n> - * \\return A vector of strings retrieved from the given environment option or\n> - * configuration file or no value if not found; the vector may be empty\n> - */\n> -std::optional<std::vector<std::string>> GlobalConfiguration::envListOption(\n> -\tconst char *const envVariable,\n> -\tconst std::initializer_list<std::string_view> confPath,\n> -\tconst std::string delimiter) const\n> -{\n> -\tconst char *envValue = utils::secure_getenv(envVariable);\n> -\tif (envValue) {\n> -\t\tauto items = utils::split(envValue, delimiter);\n> -\t\treturn std::vector<std::string>(items.begin(), items.end());\n> -\t}\n> -\treturn listOption(confPath);\n> -}\n> -\n>   } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index dd1f483beec3..a351f4f7b581 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -114,18 +114,15 @@ IPAManager::IPAManager(const CameraManager &cm)\n>   \tif (!pubKey_.isValid())\n>   \t\tLOG(IPAManager, Warning) << \"Public key not valid\";\n> \n> -\tchar *force = utils::secure_getenv(\"LIBCAMERA_IPA_FORCE_ISOLATION\");\n> -\tforceIsolation_ = (force && force[0] != '\\0') ||\n> -\t\t\t  (!force && configuration.option<bool>({ \"ipa\", \"force_isolation\" })\n> -\t\t\t\t\t     .value_or(false));\n> +\tforceIsolation_ = configuration.option<bool>({ \"ipa\", \"force_isolation\" })\n> +\t\t\t\t       .value_or(false);\n>   #endif\n> \n>   \tunsigned int ipaCount = 0;\n> \n>   \t/* User-specified paths take precedence. */\n>   \tconst auto modulePaths =\n> -\t\tconfiguration.envListOption(\n> -\t\t\t\t     \"LIBCAMERA_IPA_MODULE_PATH\", { \"ipa\", \"module_paths\" })\n> +\t\tconfiguration.listOption({ \"ipa\", \"module_paths\" })\n>   \t\t\t.value_or(std::vector<std::string>());\n>   \tfor (const auto &dir : modulePaths) {\n>   \t\tif (dir.empty())\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 6c8780a012d5..bc8ff090fa86 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -124,11 +124,9 @@ IPAProxy::IPAProxy(IPAModule *ipam, const CameraManager &cm)\n>   {\n>   \tconst GlobalConfiguration &configuration = cm._d()->configuration();\n> \n> -\tconfigPaths_ = configuration.envListOption(\"LIBCAMERA_IPA_CONFIG_PATH\",\n> -\t\t\t\t\t\t   { \"ipa\", \"config_paths\" })\n> +\tconfigPaths_ = configuration.listOption({ \"ipa\", \"config_paths\" })\n>   \t\t\t\t    .value_or(std::vector<std::string>());\n> -\texecPaths_ = configuration.envListOption(\"LIBCAMERA_IPA_PROXY_PATH\",\n> -\t\t\t\t\t\t { \"ipa\", \"proxy_paths\" })\n> +\texecPaths_ = configuration.listOption({ \"ipa\", \"proxy_paths\" })\n>   \t\t\t\t  .value_or(std::vector<std::string>());\n>   }\n> \n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index cd0e9d06a1e1..d227bd8e325f 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -106,11 +106,11 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n> \n>   #if HAVE_DEBAYER_EGL\n>   \tconst GlobalConfiguration &configuration = cm._d()->configuration();\n> -\tstd::optional<std::string> softISPMode = configuration.envOption(\"LIBCAMERA_SOFTISP_MODE\", { \"software_isp\", \"mode\" });\n> +\tstd::optional<std::string> softISPMode = configuration.option<std::string>({ \"software_isp\", \"mode\" });\n>   \tif (softISPMode) {\n>   \t\tif (softISPMode != \"gpu\" && softISPMode != \"cpu\") {\n>   \t\t\tLOG(SoftwareIsp, Error)\n> -\t\t\t\t<< \"Invalid LIBCAMERA_SOFTISP_MODE \\\"\"\n> +\t\t\t\t<< \"Invalid software ISP mode \\\"\"\n>   \t\t\t\t<< softISPMode.value()\n>   \t\t\t\t<< \"\\\", must be \\\"cpu\\\" or \\\"gpu\\\"\";\n>   \t\t\treturn;\n> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 BF79FBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Apr 2026 07:58:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E93AD62F6A;\n\tFri, 24 Apr 2026 09:58:27 +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 E31F462CE6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Apr 2026 09:58:26 +0200 (CEST)","from [192.168.33.62] (185.221.140.120.nat.pool.zt.hu\n\t[185.221.140.120])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1844E802;\n\tFri, 24 Apr 2026 09:56:47 +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=\"RSZMBxjL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1777017407;\n\tbh=t+Z1e88ESEl0Ly8FfJFGomQNziBmtgqJGJwBpouzurY=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=RSZMBxjL7dAzmzqTcm4eKstFcmppNjH+FT0QC6CZ8jHCsSLpPEPklXxSnSvG36esq\n\txKDeVZfCJXYUxGPAIIdKy/uQ3zpoRIWcfEcoc7vOoMpq2G71wC4IrBZRr5CP9HM6Tl\n\tPbG2ENotm5ePf7wFdtASiq6HBr7FtkjDmEXTeT5g=","Message-ID":"<630c06f9-cc8a-45c3-9991-c95c7a68d803@ideasonboard.com>","Date":"Fri, 24 Apr 2026 09:58:22 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 33/37] libcamera: global_configuration: Override\n\toptions with environment variables","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260423230059.3180987-1-laurent.pinchart@ideasonboard.com>\n\t<YGWUEUxe8QoQMBTKwDLDUiE9SvpDZYUOrpcGj0HLYBIy1Hsz-sZWOwKQN0-RE8qz6ygG8K2PtNtQOB-KVoU9hQ==@protonmail.internalid>\n\t<20260423230059.3180987-34-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260423230059.3180987-34-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]