[{"id":34697,"web_url":"https://patchwork.libcamera.org/comment/34697/","msgid":"<1791792c-8970-43e4-811e-e0dc5d08f68e@ideasonboard.com>","date":"2025-06-27T14:00:44","subject":"Re: [PATCH v11 05/12] config: Look up IPA configurables in\n\tconfiguration 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. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:\n> This patch adds configuration options for environment variables used in\n> the IPA proxy. Two utility functions configuration retrieval functions\n> are added, to retrieve lists of values.\n> \n> The configuration snippet:\n> \n> configuration:\n> ipa:\n> config_paths:\n> - config path 1\n> - config path 2\n> - ...\n> module_paths:\n> - module path 1\n> - module path 2\n> - ...\n> proxy_paths:\n> - proxy path 1\n> - proxy path 2\n> - ...\n> force_isolation: BOOL\n> ipas:\n> IPA_NAME:\n> tuning_file: TUNING FILE\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> .../libcamera/internal/global_configuration.h | 35 ++++++++-\n> include/libcamera/internal/ipa_manager.h | 8 ++-\n> include/libcamera/internal/ipa_proxy.h | 5 +-\n> src/libcamera/camera_manager.cpp | 2 +-\n> src/libcamera/global_configuration.cpp | 47 +++++++++++-\n> src/libcamera/ipa_manager.cpp | 37 ++++++----\n> src/libcamera/ipa_proxy.cpp | 72 ++++++++++---------\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +-\n> .../pipeline/rpi/common/pipeline_base.cpp | 3 +-\n> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n> src/libcamera/software_isp/software_isp.cpp | 4 +-\n> test/ipa/ipa_interface_test.cpp | 3 +-\n> .../module_ipa_proxy.cpp.tmpl | 4 +-\n> .../module_ipa_proxy.h.tmpl | 2 +-\n> 16 files changed, 169 insertions(+), 65 deletions(-)\n> \n> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n> index 357119628..cc436703e 100644\n> --- a/include/libcamera/internal/global_configuration.h\n> +++ b/include/libcamera/internal/global_configuration.h\n> @@ -11,6 +11,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/yaml_parser.h\"\n> \n> namespace libcamera {\n> @@ -24,9 +26,40 @@ public:\n> \n> \tunsigned int version() const;\n> \tConfiguration configuration() const;\n> -\tstd::optional option(const std::string &confPath) const;\n> +\n> +#ifndef __DOXYGEN__\n> +\ttemplate +\t\t std::enable_if_t<\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v> * = nullptr>\n\nIs this needed? `YamlObject::get()` does not have any constraints,\nso I think it can be omitted here as well?\n\n\n> +#else\n> +\ttemplate\n> +#endif\n> +\tstd::optional option(const std::string &confPath) const\n> +\t{\n> +\t\tconst YamlObject *c = &configuration();\n> +\t\tfor (auto part : utils::split(confPath, \"/\")) {\n> +\t\t\tc = &(*c)[part];\n> +\t\t\tif (!*c)\n> +\t\t\t\treturn {};\n> +\t\t}\n> +\t\treturn c->get();\n> +\t}\n> +\n> +\tstd::vector listOption(const std::string &confPath) const;\n> \tstd::optional envOption(const char *const envVariable,\n> \t\t\t\t\t const std::string &confPath) const;\n> +\tstd::vector envListOption(\n> +\t\tconst char *const envVariable,\n> +\t\tconst std::string &confPath) const;\n> \n> private:\n> \tbool loadFile(const std::filesystem::path &fileName);\n> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n> index a0d448cf9..8ad717919 100644\n> --- a/include/libcamera/internal/ipa_manager.h\n> +++ b/include/libcamera/internal/ipa_manager.h\n> @@ -17,6 +17,7 @@\n> #include \n> \n> #include \"libcamera/internal/camera_manager.h\"\n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/ipa_module.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/pub_key.h\"\n> @@ -28,7 +29,7 @@ LOG_DECLARE_CATEGORY(IPAManager)\n> class IPAManager\n> {\n> public:\n> -\tIPAManager();\n> +\tIPAManager(const GlobalConfiguration &configuration);\n> \t~IPAManager();\n> \n> \ttemplate\n> @@ -42,7 +43,8 @@ public:\n> \t\tif (!m)\n> \t\t\treturn nullptr;\n> \n> -\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m));\n> +\t\tconst GlobalConfiguration &configuration = cm->_d()->configuration();\n> +\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m, configuration), configuration);\n> \t\tif (!proxy->isValid()) {\n> \t\t\tLOG(IPAManager, Error) << \"Failed to load proxy\";\n> \t\t\treturn nullptr;\n> @@ -66,7 +68,7 @@ private:\n> \tIPAModule *module(PipelineHandler *pipe, uint32_t minVersion,\n> \t\t\t uint32_t maxVersion);\n> \n> -\tbool isSignatureValid(IPAModule *ipa) const;\n> +\tbool isSignatureValid(IPAModule *ipa, const GlobalConfiguration &configuration) const;\n> \n> \tstd::vector> modules_;\n> \n> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\n> index 983bcc5fa..01cc5deff 100644\n> --- a/include/libcamera/internal/ipa_proxy.h\n> +++ b/include/libcamera/internal/ipa_proxy.h\n> @@ -11,6 +11,8 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/global_configuration.h\"\n> +\n> namespace libcamera {\n> \n> class IPAModule;\n> @@ -30,10 +32,11 @@ public:\n> \tbool isValid() const { return valid_; }\n> \n> \tstd::string configurationFile(const std::string &name,\n> +\t\t\t\t const GlobalConfiguration &configuration,\n> \t\t\t\t const std::string &fallbackName = std::string()) const;\n> \n> protected:\n> -\tstd::string resolvePath(const std::string &file) const;\n> +\tstd::string resolvePath(const std::string &file, const GlobalConfiguration &configuration) const;\n\nI feel like `IPAProxy` could take the `GlobalConfiguration`\nin the constructor?\n\n\n> \n> \tbool valid_;\n> \tProxyState state_;\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index f3b4ec708..c47fd3677 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -41,7 +41,7 @@ LOG_DEFINE_CATEGORY(Camera)\n> CameraManager::Private::Private()\n> \t: initialized_(false)\n> {\n> -\tipaManager_ = std::make_unique();\n> +\tipaManager_ = std::make_unique(this->configuration());\n> }\n> \n> int CameraManager::Private::start()\n> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp\n> index 068ddd5a5..f7c69890c 100644\n> --- a/src/libcamera/global_configuration.cpp\n> +++ b/src/libcamera/global_configuration.cpp\n> @@ -9,9 +9,11 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -116,13 +118,22 @@ GlobalConfiguration::GlobalConfiguration()\n> */\n> \n> /**\n> + * \\fn std::optional GlobalConfiguration::option(const std::string &confPath) const\n> * \\brief Return value of the configuration option identified by \\a confPath\n> * \\param[in] confPath Sequence of the YAML section names (excluding\n> * `configuration') leading to the requested option separated by slashes\n> * \\return A value if an item corresponding to \\a confPath exists in the\n> * configuration file, no value otherwise\n> */\n> -std::optional GlobalConfiguration::option(const std::string &confPath) const\n> +\n> +/**\n> + * \\brief Return values of the configuration option identified by \\a confPath\n> + * \\tparam T The type of the retrieved configuration value\n> + * \\param[in] confPath Sequence of the YAML section names (excluding\n> + * `configuration') leading to the requested list option, separated by dots\n> + * \\return A vector of strings (empty one if the option is not found)\n> + */\n> +std::vector GlobalConfiguration::listOption(const std::string &confPath) const\n> {\n> \tconst YamlObject *c = &configuration();\n> \tfor (auto part : utils::split(confPath, \"/\")) {\n> @@ -130,7 +141,7 @@ std::optional GlobalConfiguration::option(const std::string &confPa\n> \t\tif (!*c)\n> \t\t\treturn {};\n> \t}\n> -\treturn c->get();\n> +\treturn c->getList().value_or(std::vector());\n> }\n> \n> /**\n> @@ -157,7 +168,37 @@ std::optional GlobalConfiguration::envOption(\n> \tconst char *envValue = utils::secure_getenv(envVariable);\n> \tif (envValue)\n> \t\treturn std::optional{ std::string{ envValue } };\n> -\treturn option(confPath);\n> +\treturn option(confPath);\n> +}\n> +\n> +/**\n> + * \\brief Return values 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> + *\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 (an empty vector is returned if nothing is found)\n\nI think we want to distinguish \"not present\" and \"empty\". I could imagine\nthat someone might want to inhibit some behaviour by setting the env var\nto empty. `envOption()` also differentiates the two. For example, with\nthe new \"layers\" proposal, I could image that we can set the layers in\na configuration file, etc, as well as an environmental variable. In that\ncase doing `LIBCAMERA_LAYERS= cam ...` might be useful for testing to\ntemporarily disable all layers.\n\n\n> + */\n> +std::vector GlobalConfiguration::envListOption(\n> +\tconst char *const envVariable,\n> +\tconst std::string &confPath) const\n> +{\n> +\tconst char *envValue = utils::secure_getenv(envVariable);\n> +\tif (envValue) {\n> +\t\tauto items = utils::split(envValue, \":\");\n> +\t\treturn std::vector(items.begin(), items.end());\n> +\t}\n> +\treturn listOption(confPath);\n> }\n> \n> /**\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 830750dcc..67c1b23d2 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -9,6 +9,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -16,6 +17,7 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/ipa_module.h\"\n> #include \"libcamera/internal/ipa_proxy.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -101,7 +103,7 @@ LOG_DEFINE_CATEGORY(IPAManager)\n> * The IPAManager class is meant to only be instantiated once, by the\n> * CameraManager.\n> */\n> -IPAManager::IPAManager()\n> +IPAManager::IPAManager(const GlobalConfiguration &configuration)\n> {\n> #if HAVE_IPA_PUBKEY\n> \tif (!pubKey_.isValid())\n> @@ -111,18 +113,25 @@ IPAManager::IPAManager()\n> \tunsigned int ipaCount = 0;\n> \n> \t/* User-specified paths take precedence. */\n> -\tconst char *modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n> -\tif (modulePaths) {\n> -\t\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n> -\t\t\tif (dir.empty())\n> -\t\t\t\tcontinue;\n> +\tconst auto modulePaths =\n> +\t\tconfiguration.envListOption(\n> +\t\t\t\"LIBCAMERA_IPA_MODULE_PATH\", \"ipa/module_paths\");\n> +\tfor (const auto &dir : modulePaths) {\n> +\t\tif (dir.empty())\n> +\t\t\tcontinue;\n> \n> -\t\t\tipaCount += addDir(dir.c_str());\n> -\t\t}\n> +\t\tipaCount += addDir(dir.c_str());\n> +\t}\n> \n> -\t\tif (!ipaCount)\n> -\t\t\tLOG(IPAManager, Warning)\n> -\t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n> +\tif (!ipaCount) {\n> +\t\tstd::string paths;\n> +\t\tif (!modulePaths.empty()) {\n> +\t\t\tpaths = std::accumulate(std::next(modulePaths.begin()),\n> +\t\t\t\t\t\tmodulePaths.end(),\n> +\t\t\t\t\t\tmodulePaths[0],\n> +\t\t\t\t\t\t[](std::string s1, std::string s2) { return s1 + \":\" + s2; });\n> +\t\t}\n> +\t\tLOG(IPAManager, Warning) << \"No IPA found in '\" << paths << \"'\";\n\n #include \n ...\n LOG(IPAManager, Warning) << \"No IPA found in '\" << utils::join(modulePaths, \":\") << \"'\";\n\n?\n\n\n> \t}\n> \n> \t/*\n> @@ -276,11 +285,13 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n> */\n> #endif\n> \n> -bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n> +bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa, const GlobalConfiguration &configuration) const\n\nMaybe `configuration.option(\"ipa/force_isolation\").value_or(false)` could be\nsaved into a member variable in the constructor? Then this won't need\nthe configuration parameter.\n\n\n> {\n> #if HAVE_IPA_PUBKEY\n> \tchar *force = utils::secure_getenv(\"LIBCAMERA_IPA_FORCE_ISOLATION\");\n> -\tif (force && force[0] != '\\0') {\n> +\tif ((force && force[0] != '\\0') ||\n> +\t (!force && configuration.option(\"ipa/force_isolation\")\n> +\t\t\t .value_or(false))) {\n> \t\tLOG(IPAManager, Debug)\n> \t\t\t<< \"Isolation of IPA module \" << ipa->path()\n> \t\t\t<< \" forced through environment variable\";\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 9907b9615..77927b0d4 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/ipa_module.h\"\n> \n> /**\n> @@ -71,6 +72,7 @@ IPAProxy::~IPAProxy()\n> /**\n> * \\brief Retrieve the absolute path to an IPA configuration file\n> * \\param[in] name The configuration file name\n> + * \\param[in] configuration The global configuration\n> * \\param[in] fallbackName The name of a fallback configuration file\n> *\n> * This function locates the configuration file for an IPA and returns its\n> @@ -96,6 +98,7 @@ IPAProxy::~IPAProxy()\n> * no configuration file can be found\n> */\n> std::string IPAProxy::configurationFile(const std::string &name,\n> +\t\t\t\t\tconst GlobalConfiguration &configuration,\n> \t\t\t\t\tconst std::string &fallbackName) const\n> {\n> \t/*\n> @@ -105,37 +108,37 @@ std::string IPAProxy::configurationFile(const std::string &name,\n> \tconst std::string ipaName = ipam_->info().name;\n> \n> \t/*\n> -\t * Start with any user override through the module-specific environment\n> -\t * variable. Use the name of the IPA module up to the first '/' to\n> -\t * construct the variable name.\n> +\t * Start with any user override through the module-specific configuration or\n> +\t * environment variable. Use the name of the IPA module up to the first '/'\n> +\t * to construct the configuration and variable names.\n> \t */\n> -\tstd::string ipaEnvName = ipaName.substr(0, ipaName.find('/'));\n> +\tstd::string ipaBaseName = ipaName.substr(0, ipaName.find('/'));\n> +\tstd::string ipaConfigName = \"ipa/ipas/\" + ipaBaseName + \"/tuning_file\";\n\nI feel like we're unnecessarily limiting outself wrt. rpi here. Since the IPA\nnames are \"rpi/vc4\" and \"rpi/pisp\", thus `ipaBaseName == \"rpi\"`. Which means\nthat it is impossible to specify different tuning files for the different\nplatforms. I think this is fine for an env var, but for a configuration file\nI think this flexibility is expected.\n\nBut I don't quite see the point of overriding the tuning file like this from a\nconfiguration file. So maybe leaving just the env var override is sufficient?\n\n\n\n> +\tstd::string ipaEnvName = ipaBaseName;\n> \tstd::transform(ipaEnvName.begin(), ipaEnvName.end(), ipaEnvName.begin(),\n> \t\t [](unsigned char c) { return std::toupper(c); });\n> \tipaEnvName = \"LIBCAMERA_\" + ipaEnvName + \"_TUNING_FILE\";\n> \n> -\tchar const *configFromEnv = utils::secure_getenv(ipaEnvName.c_str());\n> -\tif (configFromEnv && *configFromEnv != '\\0')\n> -\t\treturn { configFromEnv };\n> +\tauto config = configuration.envOption(ipaEnvName.c_str(), ipaConfigName);\n> +\tif (config)\n> +\t\treturn { config.value() };\n> \n> \tstruct stat statbuf;\n> \tint ret;\n> \n> \t/*\n> -\t * Check the directory pointed to by the IPA config path environment\n> -\t * variable next.\n> +\t * Check the directory pointed to by the IPA config path next.\n> \t */\n> -\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> -\tif (confPaths) {\n> -\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> -\t\t\tif (dir.empty())\n> -\t\t\t\tcontinue;\n> -\n> -\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> -\t\t\tret = stat(confPath.c_str(), &statbuf);\n> -\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> -\t\t\t\treturn confPath;\n> -\t\t}\n> +\tauto confPaths =\n> +\t\tconfiguration.envListOption(\n> +\t\t\t\"LIBCAMERA_IPA_CONFIG_PATH\", \"ipa/config_paths\");\n> +\tfor (const auto &dir : confPaths) {\n> +\t\tif (dir.empty())\n> +\t\t\tcontinue;\n> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> +\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\treturn confPath;\n> \t}\n> \n> \tstd::string root = utils::libcameraSourcePath();\n> @@ -178,12 +181,13 @@ std::string IPAProxy::configurationFile(const std::string &name,\n> \t\t<< \"Configuration file '\" << name\n> \t\t<< \"' not found for IPA module '\" << ipaName\n> \t\t<< \"', falling back to '\" << fallbackName << \"'\";\n> -\treturn configurationFile(fallbackName);\n> +\treturn configurationFile(fallbackName, configuration);\n> }\n> \n> /**\n> * \\brief Find a valid full path for a proxy worker for a given executable name\n> * \\param[in] file File name of proxy worker executable\n> + * \\param[in] configuration The global configuration\n> *\n> * A proxy worker's executable could be found in either the global installation\n> * directory, or in the paths specified by the environment variable\n> @@ -195,22 +199,22 @@ std::string IPAProxy::configurationFile(const std::string &name,\n> * \\return The full path to the proxy worker executable, or an empty string if\n> * no valid executable path\n> */\n> -std::string IPAProxy::resolvePath(const std::string &file) const\n> +std::string IPAProxy::resolvePath(const std::string &file, const GlobalConfiguration &configuration) const\n> {\n> \tstd::string proxyFile = \"/\" + file;\n> \n> -\t/* Check env variable first. */\n> -\tconst char *execPaths = utils::secure_getenv(\"LIBCAMERA_IPA_PROXY_PATH\");\n> -\tif (execPaths) {\n> -\t\tfor (const auto &dir : utils::split(execPaths, \":\")) {\n> -\t\t\tif (dir.empty())\n> -\t\t\t\tcontinue;\n> -\n> -\t\t\tstd::string proxyPath = dir;\n> -\t\t\tproxyPath += proxyFile;\n> -\t\t\tif (!access(proxyPath.c_str(), X_OK))\n> -\t\t\t\treturn proxyPath;\n> -\t\t}\n> +\t/* Check the configuration first. */\n> +\tconst auto execPaths =\n> +\t\tconfiguration.envListOption(\n> +\t\t\t\"LIBCAMERA_IPA_PROXY_PATH\", \"ipa/proxy_paths\");\n> +\tfor (const auto &dir : execPaths) {\n> +\t\tif (dir.empty())\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::string proxyPath = dir;\n\nstd::string proxyPath = dir + proxyFile;\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +\t\tproxyPath += proxyFile;\n> +\t\tif (!access(proxyPath.c_str(), X_OK))\n> +\t\t\treturn proxyPath;\n> \t}\n> \n> \t/*\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index e31e3879d..723f7665b 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1183,8 +1183,9 @@ int IPU3CameraData::loadIPA()\n> \t * The API tuning file is made from the sensor name. If the tuning file\n> \t * isn't found, fall back to the 'uncalibrated' file.\n> \t */\n> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n> \tstd::string ipaTuningFile =\n> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n> \n> \tret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },\n> \t\t\t sensorInfo, sensor->controls(), &ipaControls_);\n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index 4acc091bd..d346fa25f 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -388,7 +388,9 @@ int MaliC55CameraData::loadIPA()\n> \n> \tipa_->setSensorControls.connect(this, &MaliC55CameraData::setSensorControls);\n> \n> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n> \tstd::string ipaTuningFile = ipa_->configurationFile(sensor_->model() + \".yaml\",\n> +\t\t\t\t\t\t\t configuration,\n> \t\t\t\t\t\t\t \"uncalibrated.yaml\");\n> \n> \t/* We need to inform the IPA of the sensor configuration */\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 675f0a749..aded4cb43 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -40,6 +40,7 @@\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/media_pipeline.h\"\n> @@ -383,8 +384,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \tipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);\n> \n> \t/* The IPA tuning file is made from the sensor name. */\n> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n> \tstd::string ipaTuningFile =\n> -\t\tipa_->configurationFile(sensor_->model() + \".yaml\", \"uncalibrated.yaml\");\n> +\t\tipa_->configurationFile(sensor_->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n> \n> \tIPACameraSensorInfo sensorInfo{};\n> \tint ret = sensor_->sensorInfo(&sensorInfo);\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index a316ef297..3da919ee7 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1157,7 +1157,8 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> \tstd::string model = sensor_->model();\n> \tif (isMonoSensor(sensor_))\n> \t\tmodel += \"_mono\";\n> -\tstd::string configurationFile = ipa_->configurationFile(model + \".json\");\n> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n> +\tstd::string configurationFile = ipa_->configurationFile(model + \".json\", configuration);\n> \n> \tIPASettings settings(configurationFile, sensor_->model());\n> \tipa::RPi::InitParams params;\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 07273bd2b..555d51bb6 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -494,7 +494,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \n> \tdata->ipa_->paramsComputed.connect(data.get(), &VimcCameraData::paramsComputed);\n> \n> -\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> +\tconst GlobalConfiguration &configuration = manager_->_d()->configuration();\n> +\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\", configuration);\n> \tFlags inFlags = ipa::vimc::TestFlag::Flag2;\n> \tFlags outFlags;\n> \tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() },\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index 28e2a360e..3ce354111 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -21,6 +21,7 @@\n> #include \n> \n> #include \"libcamera/internal/framebuffer.h\"\n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/software_isp/debayer_params.h\"\n> \n> @@ -130,8 +131,9 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n> \t * The API tuning file is made from the sensor name. If the tuning file\n> \t * isn't found, fall back to the 'uncalibrated' file.\n> \t */\n> +\tconst GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration();\n> \tstd::string ipaTuningFile =\n> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n> \n> \tIPACameraSensorInfo sensorInfo{};\n> \tint ret = sensor->sensorInfo(&sensorInfo);\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index b81783664..1afcbf106 100644\n> --- a/test/ipa/ipa_interface_test.cpp\n> +++ b/test/ipa/ipa_interface_test.cpp\n> @@ -106,7 +106,8 @@ protected:\n> \t\t}\n> \n> \t\t/* Test initialization of IPA module. */\n> -\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\");\n> +\t\tconst GlobalConfiguration configuration;\n> +\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\", configuration);\n> \t\tFlags inFlags;\n> \t\tFlags outFlags;\n> \t\tint ret = ipa_->init(IPASettings{ conf, \"vimc\" },\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> index 9a3aadbd2..71e920eb1 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> @@ -45,7 +45,7 @@ namespace {{ns}} {\n> {% endfor %}\n> {%- endif %}\n> \n> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)\n> +{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration)\n> \t: IPAProxy(ipam), isolate_(isolate),\n> \t controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n> {\n> @@ -54,7 +54,7 @@ namespace {{ns}} {\n> \t\t<< ipam->path();\n> \n> \tif (isolate_) {\n> -\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n> +\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\", configuration);\n> \t\tif (proxyWorkerPath.empty()) {\n> \t\t\tLOG(IPAProxy, Error)\n> \t\t\t\t<< \"Failed to get proxy worker path\";\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> index a0312a7c1..057c3ab03 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> @@ -37,7 +37,7 @@ namespace {{ns}} {\n> class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object\n> {\n> public:\n> -\t{{proxy_name}}(IPAModule *ipam, bool isolate);\n> +\t{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration);\n> \t~{{proxy_name}}();\n> \n> {% for method in interface_main.methods %}","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 CCA3AC3237\n\tfor ;\n\tFri, 27 Jun 2025 14:01:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 03F5868DFE;\n\tFri, 27 Jun 2025 16:01:03 +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 76C2068E06\n\tfor ;\n\tFri, 27 Jun 2025 16:00:51 +0200 (CEST)","from [192.168.33.12] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ABAF0162B;\n\tFri, 27 Jun 2025 16:00:31 +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=\"siohZKvq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751032832;\n\tbh=1WiriBsJuLTloOcDr4rhIgJQ2BQ/DII6nIRbRq1jILc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=siohZKvq/XDTuar2M5qrJMGoS9+Atx47TO+EkSYJfXcbJdMZ66kXSU5WwPJzx2cZA\n\tkS39nvnqDMoFdz+wIcdLYl/owseiwaBobURkZcjGxmuRv7b9lx7EXcMNadVuOBOYId\n\tSGVSyDfNrxTf0cGVLqpAN4B4aljoas+HvOvlbF3A=","Message-ID":"<1791792c-8970-43e4-811e-e0dc5d08f68e@ideasonboard.com>","Date":"Fri, 27 Jun 2025 16:00:44 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v11 05/12] config: Look up IPA configurables in\n\tconfiguration file","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ,\n\tLaurent Pinchart ","References":"<20250624083612.27230-1-mzamazal@redhat.com>\n\t<20250624083612.27230-6-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250624083612.27230-6-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":34715,"web_url":"https://patchwork.libcamera.org/comment/34715/","msgid":"<8534bkx5fj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-27T21:04:00","subject":"Re: [PATCH v11 05/12] config: Look up IPA configurables in\n\tconfiguration file","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Barnabás,\n\nthank you for review.\n\nBarnabás Pőcze writes:\n\n> Hi\n>\n> 2025. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:\n>> This patch adds configuration options for environment variables used in\n>> the IPA proxy. Two utility functions configuration retrieval functions\n>> are added, to retrieve lists of values.\n>> The configuration snippet:\n>> configuration:\n>> ipa:\n>> config_paths:\n>> - config path 1\n>> - config path 2\n>> - ...\n>> module_paths:\n>> - module path 1\n>> - module path 2\n>> - ...\n>> proxy_paths:\n>> - proxy path 1\n>> - proxy path 2\n>> - ...\n>> force_isolation: BOOL\n>> ipas:\n>> IPA_NAME:\n>> tuning_file: TUNING FILE\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> .../libcamera/internal/global_configuration.h | 35 ++++++++-\n>> include/libcamera/internal/ipa_manager.h | 8 ++-\n>> include/libcamera/internal/ipa_proxy.h | 5 +-\n>> src/libcamera/camera_manager.cpp | 2 +-\n>> src/libcamera/global_configuration.cpp | 47 +++++++++++-\n>> src/libcamera/ipa_manager.cpp | 37 ++++++----\n>> src/libcamera/ipa_proxy.cpp | 72 ++++++++++---------\n>> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +-\n>> .../pipeline/rpi/common/pipeline_base.cpp | 3 +-\n>> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n>> src/libcamera/software_isp/software_isp.cpp | 4 +-\n>> test/ipa/ipa_interface_test.cpp | 3 +-\n>> .../module_ipa_proxy.cpp.tmpl | 4 +-\n>> .../module_ipa_proxy.h.tmpl | 2 +-\n>> 16 files changed, 169 insertions(+), 65 deletions(-)\n>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>> index 357119628..cc436703e 100644\n>> --- a/include/libcamera/internal/global_configuration.h\n>> +++ b/include/libcamera/internal/global_configuration.h\n>> @@ -11,6 +11,8 @@\n>> #include \n>> #include \n>> +#include \n>> +\n>> #include \"libcamera/internal/yaml_parser.h\"\n>> namespace libcamera {\n>> @@ -24,9 +26,40 @@ public:\n>> \tunsigned int version() const;\n>> \tConfiguration configuration() const;\n>> -\tstd::optional option(const std::string &confPath) const;\n>> +\n>> +#ifndef __DOXYGEN__\n>> +\ttemplate> +\t\t std::enable_if_t<\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v ||\n>> +\t\t\t std::is_same_v> * = nullptr>\n>\n> Is this needed? `YamlObject::get()` does not have any constraints,\n> so I think it can be omitted here as well?\n\nYes; YamlObject::get() used to have the constraints but doesn't have\nthem any more.\n\n>> +#else\n>> +\ttemplate\n>> +#endif\n>> +\tstd::optional option(const std::string &confPath) const\n>> +\t{\n>> +\t\tconst YamlObject *c = &configuration();\n>> +\t\tfor (auto part : utils::split(confPath, \"/\")) {\n>> +\t\t\tc = &(*c)[part];\n>> +\t\t\tif (!*c)\n>> +\t\t\t\treturn {};\n>> +\t\t}\n>> +\t\treturn c->get();\n>> +\t}\n>> +\n>> +\tstd::vector listOption(const std::string &confPath) const;\n>> \tstd::optional envOption(const char *const envVariable,\n>> \t\t\t\t\t const std::string &confPath) const;\n>> +\tstd::vector envListOption(\n>> +\t\tconst char *const envVariable,\n>> +\t\tconst std::string &confPath) const;\n>> private:\n>> \tbool loadFile(const std::filesystem::path &fileName);\n>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n>> index a0d448cf9..8ad717919 100644\n>> --- a/include/libcamera/internal/ipa_manager.h\n>> +++ b/include/libcamera/internal/ipa_manager.h\n>> @@ -17,6 +17,7 @@\n>> #include \n>> #include \"libcamera/internal/camera_manager.h\"\n>> +#include \"libcamera/internal/global_configuration.h\"\n>> #include \"libcamera/internal/ipa_module.h\"\n>> #include \"libcamera/internal/pipeline_handler.h\"\n>> #include \"libcamera/internal/pub_key.h\"\n>> @@ -28,7 +29,7 @@ LOG_DECLARE_CATEGORY(IPAManager)\n>> class IPAManager\n>> {\n>> public:\n>> -\tIPAManager();\n>> +\tIPAManager(const GlobalConfiguration &configuration);\n>> \t~IPAManager();\n>> \ttemplate\n>> @@ -42,7 +43,8 @@ public:\n>> \t\tif (!m)\n>> \t\t\treturn nullptr;\n>> -\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m));\n>> +\t\tconst GlobalConfiguration &configuration = cm->_d()->configuration();\n>> +\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m, configuration), configuration);\n>> \t\tif (!proxy->isValid()) {\n>> \t\t\tLOG(IPAManager, Error) << \"Failed to load proxy\";\n>> \t\t\treturn nullptr;\n>> @@ -66,7 +68,7 @@ private:\n>> \tIPAModule *module(PipelineHandler *pipe, uint32_t minVersion,\n>> \t\t\t uint32_t maxVersion);\n>> -\tbool isSignatureValid(IPAModule *ipa) const;\n>> +\tbool isSignatureValid(IPAModule *ipa, const GlobalConfiguration &configuration) const;\n>> \tstd::vector> modules_;\n>> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\n>> index 983bcc5fa..01cc5deff 100644\n>> --- a/include/libcamera/internal/ipa_proxy.h\n>> +++ b/include/libcamera/internal/ipa_proxy.h\n>> @@ -11,6 +11,8 @@\n>> #include \n>> +#include \"libcamera/internal/global_configuration.h\"\n>> +\n>> namespace libcamera {\n>> class IPAModule;\n>> @@ -30,10 +32,11 @@ public:\n>> \tbool isValid() const { return valid_; }\n>> \tstd::string configurationFile(const std::string &name,\n>> +\t\t\t\t const GlobalConfiguration &configuration,\n>> \t\t\t\t const std::string &fallbackName = std::string()) const;\n>> protected:\n>> -\tstd::string resolvePath(const std::string &file) const;\n>> +\tstd::string resolvePath(const std::string &file, const GlobalConfiguration &configuration) const;\n>\n> I feel like `IPAProxy` could take the `GlobalConfiguration`\n> in the constructor?\n\nI.e. copy the configuration and changing std::unique_ptr\ninside to shared_ptr? Possible, I think, and I don't have a particular\npreference -- up to your judgement what's better.\n\n>> \tbool valid_;\n>> \tProxyState state_;\n>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>> index f3b4ec708..c47fd3677 100644\n>> --- a/src/libcamera/camera_manager.cpp\n>> +++ b/src/libcamera/camera_manager.cpp\n>> @@ -41,7 +41,7 @@ LOG_DEFINE_CATEGORY(Camera)\n>> CameraManager::Private::Private()\n>> \t: initialized_(false)\n>> {\n>> -\tipaManager_ = std::make_unique();\n>> +\tipaManager_ = std::make_unique(this->configuration());\n>> }\n>> int CameraManager::Private::start()\n>> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp\n>> index 068ddd5a5..f7c69890c 100644\n>> --- a/src/libcamera/global_configuration.cpp\n>> +++ b/src/libcamera/global_configuration.cpp\n>> @@ -9,9 +9,11 @@\n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> @@ -116,13 +118,22 @@ GlobalConfiguration::GlobalConfiguration()\n>> */\n>> /**\n>> + * \\fn std::optional GlobalConfiguration::option(const std::string &confPath) const\n>> * \\brief Return value of the configuration option identified by \\a confPath\n>> * \\param[in] confPath Sequence of the YAML section names (excluding\n>> * `configuration') leading to the requested option separated by slashes\n>> * \\return A value if an item corresponding to \\a confPath exists in the\n>> * configuration file, no value otherwise\n>> */\n>> -std::optional GlobalConfiguration::option(const std::string &confPath) const\n>> +\n>> +/**\n>> + * \\brief Return values of the configuration option identified by \\a confPath\n>> + * \\tparam T The type of the retrieved configuration value\n>> + * \\param[in] confPath Sequence of the YAML section names (excluding\n>> + * `configuration') leading to the requested list option, separated by dots\n>> + * \\return A vector of strings (empty one if the option is not found)\n>> + */\n>> +std::vector GlobalConfiguration::listOption(const std::string &confPath) const\n>> {\n>> \tconst YamlObject *c = &configuration();\n>> \tfor (auto part : utils::split(confPath, \"/\")) {\n>> @@ -130,7 +141,7 @@ std::optional GlobalConfiguration::option(const std::string &confPa\n>> \t\tif (!*c)\n>> \t\t\treturn {};\n>> \t}\n>> -\treturn c->get();\n>> +\treturn c->getList().value_or(std::vector());\n>> }\n>> /**\n>> @@ -157,7 +168,37 @@ std::optional GlobalConfiguration::envOption(\n>> \tconst char *envValue = utils::secure_getenv(envVariable);\n>> \tif (envValue)\n>> \t\treturn std::optional{ std::string{ envValue } };\n>> -\treturn option(confPath);\n>> +\treturn option(confPath);\n>> +}\n>> +\n>> +/**\n>> + * \\brief Return values 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>> + *\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 (an empty vector is returned if nothing is found)\n>\n> I think we want to distinguish \"not present\" and \"empty\". I could imagine\n> that someone might want to inhibit some behaviour by setting the env var\n> to empty.\n\nWell, introducing new environment variables for configuration is\ndiscouraged.\n\n> `envOption()` also differentiates the two. For example, with the new\n> \"layers\" proposal, I could image that we can set the layers in a\n> configuration file, etc, as well as an environmental variable. In that\n> case doing `LIBCAMERA_LAYERS= cam ...` might be useful for testing to\n> temporarily disable all layers.\n\nHmm, sounds reasonable but I'm still not sure we want to encourage this.\nBut technically, I can change the method signature.\n\n>> + */\n>> +std::vector GlobalConfiguration::envListOption(\n>> +\tconst char *const envVariable,\n>> +\tconst std::string &confPath) const\n>> +{\n>> +\tconst char *envValue = utils::secure_getenv(envVariable);\n>> +\tif (envValue) {\n>> +\t\tauto items = utils::split(envValue, \":\");\n>> +\t\treturn std::vector(items.begin(), items.end());\n>> +\t}\n>> +\treturn listOption(confPath);\n>> }\n>> /**\n>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>> index 830750dcc..67c1b23d2 100644\n>> --- a/src/libcamera/ipa_manager.cpp\n>> +++ b/src/libcamera/ipa_manager.cpp\n>> @@ -9,6 +9,7 @@\n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> @@ -16,6 +17,7 @@\n>> #include \n>> #include \n>> +#include \"libcamera/internal/global_configuration.h\"\n>> #include \"libcamera/internal/ipa_module.h\"\n>> #include \"libcamera/internal/ipa_proxy.h\"\n>> #include \"libcamera/internal/pipeline_handler.h\"\n>> @@ -101,7 +103,7 @@ LOG_DEFINE_CATEGORY(IPAManager)\n>> * The IPAManager class is meant to only be instantiated once, by the\n>> * CameraManager.\n>> */\n>> -IPAManager::IPAManager()\n>> +IPAManager::IPAManager(const GlobalConfiguration &configuration)\n>> {\n>> #if HAVE_IPA_PUBKEY\n>> \tif (!pubKey_.isValid())\n>> @@ -111,18 +113,25 @@ IPAManager::IPAManager()\n>> \tunsigned int ipaCount = 0;\n>> \t/* User-specified paths take precedence. */\n>> -\tconst char *modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n>> -\tif (modulePaths) {\n>> -\t\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n>> -\t\t\tif (dir.empty())\n>> -\t\t\t\tcontinue;\n>> +\tconst auto modulePaths =\n>> +\t\tconfiguration.envListOption(\n>> +\t\t\t\"LIBCAMERA_IPA_MODULE_PATH\", \"ipa/module_paths\");\n>> +\tfor (const auto &dir : modulePaths) {\n>> +\t\tif (dir.empty())\n>> +\t\t\tcontinue;\n>> -\t\t\tipaCount += addDir(dir.c_str());\n>> -\t\t}\n>> +\t\tipaCount += addDir(dir.c_str());\n>> +\t}\n>> -\t\tif (!ipaCount)\n>> -\t\t\tLOG(IPAManager, Warning)\n>> -\t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n>> +\tif (!ipaCount) {\n>> +\t\tstd::string paths;\n>> +\t\tif (!modulePaths.empty()) {\n>> +\t\t\tpaths = std::accumulate(std::next(modulePaths.begin()),\n>> +\t\t\t\t\t\tmodulePaths.end(),\n>> +\t\t\t\t\t\tmodulePaths[0],\n>> +\t\t\t\t\t\t[](std::string s1, std::string s2) { return s1 + \":\" + s2; });\n>> +\t\t}\n>> +\t\tLOG(IPAManager, Warning) << \"No IPA found in '\" << paths << \"'\";\n>\n> #include \n> ...\n> LOG(IPAManager, Warning) << \"No IPA found in '\" << utils::join(modulePaths, \":\") << \"'\";\n>\n> ?\n\nOK.\n\n>> \t}\n>> \t/*\n>> @@ -276,11 +285,13 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n>> */\n>> #endif\n>> -bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n>> +bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa, const GlobalConfiguration &configuration) const\n>\n> Maybe `configuration.option(\"ipa/force_isolation\").value_or(false)` could be\n> saved into a member variable in the constructor? Then this won't need\n> the configuration parameter.\n\nYes. Assuming the `pipe' CameraManager instance is always the same that\nwas used to create the IPAManager instance.\n\n>> {\n>> #if HAVE_IPA_PUBKEY\n>> \tchar *force = utils::secure_getenv(\"LIBCAMERA_IPA_FORCE_ISOLATION\");\n>> -\tif (force && force[0] != '\\0') {\n>> +\tif ((force && force[0] != '\\0') ||\n>> +\t (!force && configuration.option(\"ipa/force_isolation\")\n>> +\t\t\t .value_or(false))) {\n>> \t\tLOG(IPAManager, Debug)\n>> \t\t\t<< \"Isolation of IPA module \" << ipa->path()\n>> \t\t\t<< \" forced through environment variable\";\n>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n>> index 9907b9615..77927b0d4 100644\n>> --- a/src/libcamera/ipa_proxy.cpp\n>> +++ b/src/libcamera/ipa_proxy.cpp\n>> @@ -14,6 +14,7 @@\n>> #include \n>> #include \n>> +#include \"libcamera/internal/global_configuration.h\"\n>> #include \"libcamera/internal/ipa_module.h\"\n>> /**\n>> @@ -71,6 +72,7 @@ IPAProxy::~IPAProxy()\n>> /**\n>> * \\brief Retrieve the absolute path to an IPA configuration file\n>> * \\param[in] name The configuration file name\n>> + * \\param[in] configuration The global configuration\n>> * \\param[in] fallbackName The name of a fallback configuration file\n>> *\n>> * This function locates the configuration file for an IPA and returns its\n>> @@ -96,6 +98,7 @@ IPAProxy::~IPAProxy()\n>> * no configuration file can be found\n>> */\n>> std::string IPAProxy::configurationFile(const std::string &name,\n>> +\t\t\t\t\tconst GlobalConfiguration &configuration,\n>> \t\t\t\t\tconst std::string &fallbackName) const\n>> {\n>> \t/*\n>> @@ -105,37 +108,37 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>> \tconst std::string ipaName = ipam_->info().name;\n>> \t/*\n>> -\t * Start with any user override through the module-specific environment\n>> -\t * variable. Use the name of the IPA module up to the first '/' to\n>> -\t * construct the variable name.\n>> +\t * Start with any user override through the module-specific configuration or\n>> +\t * environment variable. Use the name of the IPA module up to the first '/'\n>> +\t * to construct the configuration and variable names.\n>> \t */\n>> -\tstd::string ipaEnvName = ipaName.substr(0, ipaName.find('/'));\n>> +\tstd::string ipaBaseName = ipaName.substr(0, ipaName.find('/'));\n>> +\tstd::string ipaConfigName = \"ipa/ipas/\" + ipaBaseName + \"/tuning_file\";\n>\n> I feel like we're unnecessarily limiting outself wrt. rpi here. Since the IPA\n> names are \"rpi/vc4\" and \"rpi/pisp\", thus `ipaBaseName == \"rpi\"`. Which means\n> that it is impossible to specify different tuning files for the different\n> platforms. I think this is fine for an env var, but for a configuration file\n> I think this flexibility is expected.\n\nRight; so rather than trimming, let's replace the slash with another\ncharacter here?\n\n> But I don't quite see the point of overriding the tuning file like this from a\n> configuration file. So maybe leaving just the env var override is sufficient?\n\nI think the initial idea was to be able to configure everything in the\nconfiguration file (why not?). Logging is currently a noticeable\nexception, although hopefully only \"temporary\".\n\n>> +\tstd::string ipaEnvName = ipaBaseName;\n>> \tstd::transform(ipaEnvName.begin(), ipaEnvName.end(), ipaEnvName.begin(),\n>> \t\t [](unsigned char c) { return std::toupper(c); });\n>> \tipaEnvName = \"LIBCAMERA_\" + ipaEnvName + \"_TUNING_FILE\";\n>> -\tchar const *configFromEnv = utils::secure_getenv(ipaEnvName.c_str());\n>> -\tif (configFromEnv && *configFromEnv != '\\0')\n>> -\t\treturn { configFromEnv };\n>> +\tauto config = configuration.envOption(ipaEnvName.c_str(), ipaConfigName);\n>> +\tif (config)\n>> +\t\treturn { config.value() };\n>> \tstruct stat statbuf;\n>> \tint ret;\n>> \t/*\n>> -\t * Check the directory pointed to by the IPA config path environment\n>> -\t * variable next.\n>> +\t * Check the directory pointed to by the IPA config path next.\n>> \t */\n>> -\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n>> -\tif (confPaths) {\n>> -\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n>> -\t\t\tif (dir.empty())\n>> -\t\t\t\tcontinue;\n>> -\n>> -\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>> -\t\t\tret = stat(confPath.c_str(), &statbuf);\n>> -\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>> -\t\t\t\treturn confPath;\n>> -\t\t}\n>> +\tauto confPaths =\n>> +\t\tconfiguration.envListOption(\n>> +\t\t\t\"LIBCAMERA_IPA_CONFIG_PATH\", \"ipa/config_paths\");\n>> +\tfor (const auto &dir : confPaths) {\n>> +\t\tif (dir.empty())\n>> +\t\t\tcontinue;\n>> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>> +\t\tret = stat(confPath.c_str(), &statbuf);\n>> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>> +\t\t\treturn confPath;\n>> \t}\n>> \tstd::string root = utils::libcameraSourcePath();\n>> @@ -178,12 +181,13 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>> \t\t<< \"Configuration file '\" << name\n>> \t\t<< \"' not found for IPA module '\" << ipaName\n>> \t\t<< \"', falling back to '\" << fallbackName << \"'\";\n>> -\treturn configurationFile(fallbackName);\n>> +\treturn configurationFile(fallbackName, configuration);\n>> }\n>> /**\n>> * \\brief Find a valid full path for a proxy worker for a given executable name\n>> * \\param[in] file File name of proxy worker executable\n>> + * \\param[in] configuration The global configuration\n>> *\n>> * A proxy worker's executable could be found in either the global installation\n>> * directory, or in the paths specified by the environment variable\n>> @@ -195,22 +199,22 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>> * \\return The full path to the proxy worker executable, or an empty string if\n>> * no valid executable path\n>> */\n>> -std::string IPAProxy::resolvePath(const std::string &file) const\n>> +std::string IPAProxy::resolvePath(const std::string &file, const GlobalConfiguration &configuration) const\n>> {\n>> \tstd::string proxyFile = \"/\" + file;\n>> -\t/* Check env variable first. */\n>> -\tconst char *execPaths = utils::secure_getenv(\"LIBCAMERA_IPA_PROXY_PATH\");\n>> -\tif (execPaths) {\n>> -\t\tfor (const auto &dir : utils::split(execPaths, \":\")) {\n>> -\t\t\tif (dir.empty())\n>> -\t\t\t\tcontinue;\n>> -\n>> -\t\t\tstd::string proxyPath = dir;\n>> -\t\t\tproxyPath += proxyFile;\n>> -\t\t\tif (!access(proxyPath.c_str(), X_OK))\n>> -\t\t\t\treturn proxyPath;\n>> -\t\t}\n>> +\t/* Check the configuration first. */\n>> +\tconst auto execPaths =\n>> +\t\tconfiguration.envListOption(\n>> +\t\t\t\"LIBCAMERA_IPA_PROXY_PATH\", \"ipa/proxy_paths\");\n>> +\tfor (const auto &dir : execPaths) {\n>> +\t\tif (dir.empty())\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tstd::string proxyPath = dir;\n>\n> std::string proxyPath = dir + proxyFile;\n\nOK.\n\n> Regards,\n> Barnabás Pőcze\n>\n>\n>> +\t\tproxyPath += proxyFile;\n>> +\t\tif (!access(proxyPath.c_str(), X_OK))\n>> +\t\t\treturn proxyPath;\n>> \t}\n>> \t/*\n>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> index e31e3879d..723f7665b 100644\n>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> @@ -1183,8 +1183,9 @@ int IPU3CameraData::loadIPA()\n>> \t * The API tuning file is made from the sensor name. If the tuning file\n>> \t * isn't found, fall back to the 'uncalibrated' file.\n>> \t */\n>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>> \tstd::string ipaTuningFile =\n>> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n>> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>> \tret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },\n>> \t\t\t sensorInfo, sensor->controls(), &ipaControls_);\n>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> index 4acc091bd..d346fa25f 100644\n>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> @@ -388,7 +388,9 @@ int MaliC55CameraData::loadIPA()\n>> \tipa_->setSensorControls.connect(this, &MaliC55CameraData::setSensorControls);\n>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>> \tstd::string ipaTuningFile = ipa_->configurationFile(sensor_->model() + \".yaml\",\n>> +\t\t\t\t\t\t\t configuration,\n>> \t\t\t\t\t\t\t \"uncalibrated.yaml\");\n>> \t/* We need to inform the IPA of the sensor configuration */\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 675f0a749..aded4cb43 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -40,6 +40,7 @@\n>> #include \"libcamera/internal/delayed_controls.h\"\n>> #include \"libcamera/internal/device_enumerator.h\"\n>> #include \"libcamera/internal/framebuffer.h\"\n>> +#include \"libcamera/internal/global_configuration.h\"\n>> #include \"libcamera/internal/ipa_manager.h\"\n>> #include \"libcamera/internal/media_device.h\"\n>> #include \"libcamera/internal/media_pipeline.h\"\n>> @@ -383,8 +384,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n>> \tipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);\n>> \t/* The IPA tuning file is made from the sensor name. */\n>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>> \tstd::string ipaTuningFile =\n>> -\t\tipa_->configurationFile(sensor_->model() + \".yaml\", \"uncalibrated.yaml\");\n>> +\t\tipa_->configurationFile(sensor_->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>> \tIPACameraSensorInfo sensorInfo{};\n>> \tint ret = sensor_->sensorInfo(&sensorInfo);\n>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> index a316ef297..3da919ee7 100644\n>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> @@ -1157,7 +1157,8 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n>> \tstd::string model = sensor_->model();\n>> \tif (isMonoSensor(sensor_))\n>> \t\tmodel += \"_mono\";\n>> -\tstd::string configurationFile = ipa_->configurationFile(model + \".json\");\n>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>> +\tstd::string configurationFile = ipa_->configurationFile(model + \".json\", configuration);\n>> \tIPASettings settings(configurationFile, sensor_->model());\n>> \tipa::RPi::InitParams params;\n>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n>> index 07273bd2b..555d51bb6 100644\n>> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n>> @@ -494,7 +494,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>> \tdata->ipa_->paramsComputed.connect(data.get(), &VimcCameraData::paramsComputed);\n>> -\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n>> +\tconst GlobalConfiguration &configuration = manager_->_d()->configuration();\n>> +\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\", configuration);\n>> \tFlags inFlags = ipa::vimc::TestFlag::Flag2;\n>> \tFlags outFlags;\n>> \tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() },\n>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>> index 28e2a360e..3ce354111 100644\n>> --- a/src/libcamera/software_isp/software_isp.cpp\n>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>> @@ -21,6 +21,7 @@\n>> #include \n>> #include \"libcamera/internal/framebuffer.h\"\n>> +#include \"libcamera/internal/global_configuration.h\"\n>> #include \"libcamera/internal/ipa_manager.h\"\n>> #include \"libcamera/internal/software_isp/debayer_params.h\"\n>> @@ -130,8 +131,9 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n>> \t * The API tuning file is made from the sensor name. If the tuning file\n>> \t * isn't found, fall back to the 'uncalibrated' file.\n>> \t */\n>> +\tconst GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration();\n>> \tstd::string ipaTuningFile =\n>> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n>> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>> \tIPACameraSensorInfo sensorInfo{};\n>> \tint ret = sensor->sensorInfo(&sensorInfo);\n>> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n>> index b81783664..1afcbf106 100644\n>> --- a/test/ipa/ipa_interface_test.cpp\n>> +++ b/test/ipa/ipa_interface_test.cpp\n>> @@ -106,7 +106,8 @@ protected:\n>> \t\t}\n>> \t\t/* Test initialization of IPA module. */\n>> -\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\");\n>> +\t\tconst GlobalConfiguration configuration;\n>> +\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\", configuration);\n>> \t\tFlags inFlags;\n>> \t\tFlags outFlags;\n>> \t\tint ret = ipa_->init(IPASettings{ conf, \"vimc\" },\n>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>> index 9a3aadbd2..71e920eb1 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>> @@ -45,7 +45,7 @@ namespace {{ns}} {\n>> {% endfor %}\n>> {%- endif %}\n>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)\n>> +{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration)\n>> \t: IPAProxy(ipam), isolate_(isolate),\n>> \t controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n>> {\n>> @@ -54,7 +54,7 @@ namespace {{ns}} {\n>> \t\t<< ipam->path();\n>> \tif (isolate_) {\n>> -\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n>> +\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\", configuration);\n>> \t\tif (proxyWorkerPath.empty()) {\n>> \t\t\tLOG(IPAProxy, Error)\n>> \t\t\t\t<< \"Failed to get proxy worker path\";\n>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>> index a0312a7c1..057c3ab03 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>> @@ -37,7 +37,7 @@ namespace {{ns}} {\n>> class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object\n>> {\n>> public:\n>> -\t{{proxy_name}}(IPAModule *ipam, bool isolate);\n>> +\t{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration);\n>> \t~{{proxy_name}}();\n>> {% for method in interface_main.methods %}","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 669B5BDCBF\n\tfor ;\n\tFri, 27 Jun 2025 21:04:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F87368E04;\n\tFri, 27 Jun 2025 23:04:09 +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 8DA2868DE5\n\tfor ;\n\tFri, 27 Jun 2025 23:04:07 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-158-ihS3KrK0Nw6lihq0nO61Qg-1; Fri, 27 Jun 2025 17:04:04 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-450d6768d4dso1479075e9.2\n\tfor ;\n\tFri, 27 Jun 2025 14:04:04 -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-453823c463asm90964925e9.39.2025.06.27.14.04.00\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 27 Jun 2025 14:04:01 -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=\"e9YjUlXS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1751058246;\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=Q715fJ9oSiOpM9IIaYaHdy7otH5q50PYDYTkd2rmPAA=;\n\tb=e9YjUlXSHFxViOqtKtI1InKTDcw5Yq09ITcgPGMiiFq9LmYUEWHWEMbupxNeFLe56iALQl\n\t9I5VSRsucDkexHxKZv/1FPdNS4H2a6xgjWvZx1yP3Wkjk9ZkLVznrtpalG17fXiZ8tVIdF\n\t+1ZQInIVVyNUJYMambsY6V+T5E0RSuY=","X-MC-Unique":"ihS3KrK0Nw6lihq0nO61Qg-1","X-Mimecast-MFC-AGG-ID":"ihS3KrK0Nw6lihq0nO61Qg_1751058243","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1751058243; x=1751663043;\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=9EpYdeCWjDv99uk3VHk3cBoClaDOGzWvG9e4egwPNuw=;\n\tb=k0Uc/BrZYoWKhcPCUydEMjDl2vRlDHVmHJ/u593XeoFHVI/oDyKDE9uMcir6zm+fKB\n\tM4wavEst5WvyrzkZ6qkGxYUqHGbyUKIjI5Rx5396cQppYy5rJLf0F9EBt1rvfxocVQhS\n\tcygb8y7bKFrgSiELkfipK551BIo5IXmYjoqBR3Ipo8Ko5PwzaKFOh18Y04H8L/3EGWff\n\tjVkHL+fl3a9XhbVfXHTVy6+4nz7rgHCnJRV5Skn1Kh+V3lpG1hHcOq282uR40H2Vx5GL\n\tzlbExtDm6EJJAd20Zs2rbHqyWtmJL6R6RF2xwk/EP2dByqkJG+N89/NbHo9ACyUD1igC\n\tU9jg==","X-Gm-Message-State":"AOJu0YyuChZQkUTOdK2/m+DTtTg7tMDUNjhMG/eIj+fbCIsQ8ggRgAhy\n\t6Rj9nzJTCSF7F35h+/eW8VUhQVkXAq8Fo1Dp9vLkqSNnOnfjLlx6GAv3jIL3rcz5hCC2Hv2zbu4\n\tj3gi8HWZyCHs5gmYitXNT8PniAP1I9/EOmHyiHF1zJXYsregMf+sWJqgibaFpG0SgvStWhYf//N\n\tWlbdHif44=","X-Gm-Gg":"ASbGncv7/wDKYYn7bWIyQI5n6uf25MwoV97wAnDoUJMlgDH+kRGeHjs/Dz7gfzP0ZW3\n\tbz/FERSiPiZt+kUFsluaH1SR8wYCBw0OW/6sXiKbF57QuY334BmcWE1T4SiOZyRZjMHHhty6A++\n\tVPvYTFV4Hw3loG/TTVr1X3Z4R8OxKWY4kOTk6yMGYoeqE6YMoDEn+T4fUYEA8dAAsegBjeCPbn9\n\trWczWWy0tctQv2ADxG/dSuEEEkefOVmI1aFfdM9TNgVQ/7IPi0RYlDSlAB5IVf9e38OzXa/Owtn\n\tpZoEM2un3hsaa8U86Y9SCdk3tO0+m6ettYV8VnWS7WglvvdwF4m1ipV3UzTuhfoo9bheJWrGF0Q\n\t=","X-Received":["by 2002:adf:b603:0:b0:3a5:2698:f65d with SMTP id\n\tffacd0b85a97d-3a8fe5b1eb8mr4172728f8f.27.1751058242665; \n\tFri, 27 Jun 2025 14:04:02 -0700 (PDT)","by 2002:adf:b603:0:b0:3a5:2698:f65d with SMTP id\n\tffacd0b85a97d-3a8fe5b1eb8mr4172712f8f.27.1751058242005; \n\tFri, 27 Jun 2025 14:04:02 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGKaSyNmv4+9J8iNygLQe0BW17a2QARRvussrQceD9xX2AFVwb+3xGzyYW0BF3a01NQDaJsiw==","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 v11 05/12] config: Look up IPA configurables in\n\tconfiguration file","In-Reply-To":"<1791792c-8970-43e4-811e-e0dc5d08f68e@ideasonboard.com> (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Fri,\n\t27 Jun 2025 16:00:44 +0200\")","References":"<20250624083612.27230-1-mzamazal@redhat.com>\n\t<20250624083612.27230-6-mzamazal@redhat.com>\n\t<1791792c-8970-43e4-811e-e0dc5d08f68e@ideasonboard.com>","Date":"Fri, 27 Jun 2025 23:04:00 +0200","Message-ID":"<8534bkx5fj.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":"EmiHBcoDqVRdWkExTrxa3-m4Sqv6wLhH2Kw7N_ZkX4k_1751058243","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":34732,"web_url":"https://patchwork.libcamera.org/comment/34732/","msgid":"<8affaa1e-96f5-4e96-9674-1ed91d461d09@ideasonboard.com>","date":"2025-06-30T09:52:33","subject":"Re: [PATCH v11 05/12] config: Look up IPA configurables in\n\tconfiguration 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. 06. 27. 23:04 keltezéssel, Milan Zamazal írta:\n> Hi Barnabás,\n> \n> thank you for review.\n> \n> Barnabás Pőcze writes:\n> \n>> Hi\n>>\n>> 2025. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:\n>>> This patch adds configuration options for environment variables used in\n>>> the IPA proxy. Two utility functions configuration retrieval functions\n>>> are added, to retrieve lists of values.\n>>> The configuration snippet:\n>>> configuration:\n>>> ipa:\n>>> config_paths:\n>>> - config path 1\n>>> - config path 2\n>>> - ...\n>>> module_paths:\n>>> - module path 1\n>>> - module path 2\n>>> - ...\n>>> proxy_paths:\n>>> - proxy path 1\n>>> - proxy path 2\n>>> - ...\n>>> force_isolation: BOOL\n>>> ipas:\n>>> IPA_NAME:\n>>> tuning_file: TUNING FILE\n>>> Signed-off-by: Milan Zamazal \n>>> ---\n>>> .../libcamera/internal/global_configuration.h | 35 ++++++++-\n>>> include/libcamera/internal/ipa_manager.h | 8 ++-\n>>> include/libcamera/internal/ipa_proxy.h | 5 +-\n>>> src/libcamera/camera_manager.cpp | 2 +-\n>>> src/libcamera/global_configuration.cpp | 47 +++++++++++-\n>>> src/libcamera/ipa_manager.cpp | 37 ++++++----\n>>> src/libcamera/ipa_proxy.cpp | 72 ++++++++++---------\n>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n>>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +\n>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +-\n>>> .../pipeline/rpi/common/pipeline_base.cpp | 3 +-\n>>> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n>>> src/libcamera/software_isp/software_isp.cpp | 4 +-\n>>> test/ipa/ipa_interface_test.cpp | 3 +-\n>>> .../module_ipa_proxy.cpp.tmpl | 4 +-\n>>> .../module_ipa_proxy.h.tmpl | 2 +-\n>>> 16 files changed, 169 insertions(+), 65 deletions(-)\n>>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>>> index 357119628..cc436703e 100644\n>>> --- a/include/libcamera/internal/global_configuration.h\n>>> +++ b/include/libcamera/internal/global_configuration.h\n>>> @@ -11,6 +11,8 @@\n>>> #include \n>>> #include \n>>> +#include \n>>> +\n>>> #include \"libcamera/internal/yaml_parser.h\"\n>>> namespace libcamera {\n>>> @@ -24,9 +26,40 @@ public:\n>>> \tunsigned int version() const;\n>>> \tConfiguration configuration() const;\n>>> -\tstd::optional option(const std::string &confPath) const;\n>>> +\n>>> +#ifndef __DOXYGEN__\n>>> +\ttemplate>> +\t\t std::enable_if_t<\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v ||\n>>> +\t\t\t std::is_same_v> * = nullptr>\n>>\n>> Is this needed? `YamlObject::get()` does not have any constraints,\n>> so I think it can be omitted here as well?\n> \n> Yes; YamlObject::get() used to have the constraints but doesn't have\n> them any more.\n> \n>>> +#else\n>>> +\ttemplate\n>>> +#endif\n>>> +\tstd::optional option(const std::string &confPath) const\n>>> +\t{\n>>> +\t\tconst YamlObject *c = &configuration();\n>>> +\t\tfor (auto part : utils::split(confPath, \"/\")) {\n>>> +\t\t\tc = &(*c)[part];\n>>> +\t\t\tif (!*c)\n>>> +\t\t\t\treturn {};\n>>> +\t\t}\n>>> +\t\treturn c->get();\n>>> +\t}\n>>> +\n>>> +\tstd::vector listOption(const std::string &confPath) const;\n>>> \tstd::optional envOption(const char *const envVariable,\n>>> \t\t\t\t\t const std::string &confPath) const;\n>>> +\tstd::vector envListOption(\n>>> +\t\tconst char *const envVariable,\n>>> +\t\tconst std::string &confPath) const;\n>>> private:\n>>> \tbool loadFile(const std::filesystem::path &fileName);\n>>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n>>> index a0d448cf9..8ad717919 100644\n>>> --- a/include/libcamera/internal/ipa_manager.h\n>>> +++ b/include/libcamera/internal/ipa_manager.h\n>>> @@ -17,6 +17,7 @@\n>>> #include \n>>> #include \"libcamera/internal/camera_manager.h\"\n>>> +#include \"libcamera/internal/global_configuration.h\"\n>>> #include \"libcamera/internal/ipa_module.h\"\n>>> #include \"libcamera/internal/pipeline_handler.h\"\n>>> #include \"libcamera/internal/pub_key.h\"\n>>> @@ -28,7 +29,7 @@ LOG_DECLARE_CATEGORY(IPAManager)\n>>> class IPAManager\n>>> {\n>>> public:\n>>> -\tIPAManager();\n>>> +\tIPAManager(const GlobalConfiguration &configuration);\n>>> \t~IPAManager();\n>>> \ttemplate\n>>> @@ -42,7 +43,8 @@ public:\n>>> \t\tif (!m)\n>>> \t\t\treturn nullptr;\n>>> -\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m));\n>>> +\t\tconst GlobalConfiguration &configuration = cm->_d()->configuration();\n>>> +\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m, configuration), configuration);\n>>> \t\tif (!proxy->isValid()) {\n>>> \t\t\tLOG(IPAManager, Error) << \"Failed to load proxy\";\n>>> \t\t\treturn nullptr;\n>>> @@ -66,7 +68,7 @@ private:\n>>> \tIPAModule *module(PipelineHandler *pipe, uint32_t minVersion,\n>>> \t\t\t uint32_t maxVersion);\n>>> -\tbool isSignatureValid(IPAModule *ipa) const;\n>>> +\tbool isSignatureValid(IPAModule *ipa, const GlobalConfiguration &configuration) const;\n>>> \tstd::vector> modules_;\n>>> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\n>>> index 983bcc5fa..01cc5deff 100644\n>>> --- a/include/libcamera/internal/ipa_proxy.h\n>>> +++ b/include/libcamera/internal/ipa_proxy.h\n>>> @@ -11,6 +11,8 @@\n>>> #include \n>>> +#include \"libcamera/internal/global_configuration.h\"\n>>> +\n>>> namespace libcamera {\n>>> class IPAModule;\n>>> @@ -30,10 +32,11 @@ public:\n>>> \tbool isValid() const { return valid_; }\n>>> \tstd::string configurationFile(const std::string &name,\n>>> +\t\t\t\t const GlobalConfiguration &configuration,\n>>> \t\t\t\t const std::string &fallbackName = std::string()) const;\n>>> protected:\n>>> -\tstd::string resolvePath(const std::string &file) const;\n>>> +\tstd::string resolvePath(const std::string &file, const GlobalConfiguration &configuration) const;\n>>\n>> I feel like `IPAProxy` could take the `GlobalConfiguration`\n>> in the constructor?\n> \n> I.e. copy the configuration and changing std::unique_ptr\n> inside to shared_ptr? Possible, I think, and I don't have a particular\n> preference -- up to your judgement what's better.\n\nI had two options in mind:\n \n (1) Extract everything from the configuration that you need and store them\n in member variables. This would mean moving the envListOption() calls to\n the constructor.\n (2) Just store the `GlobalConfiguration::Configuration` object. Since the lifetime\n of IPAManager is tied to CameraManager, this shouldn't cause issue.\n\nBut now I see that while (2) is simpler, `GlobalConfiguration::Configuration` is just\na reference, so it does not have the `{env,}{list,}option()` functions that would\nbe needed. And (1) is a more significant change.\n\nI would really prefer if you didn't have to pass a configuration instance to every\nmethod that might need it. But I don't have a good suggestion at the moment.\n\n\n\n> \n>>> \tbool valid_;\n>>> \tProxyState state_;\n>>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>>> index f3b4ec708..c47fd3677 100644\n>>> --- a/src/libcamera/camera_manager.cpp\n>>> +++ b/src/libcamera/camera_manager.cpp\n>>> @@ -41,7 +41,7 @@ LOG_DEFINE_CATEGORY(Camera)\n>>> CameraManager::Private::Private()\n>>> \t: initialized_(false)\n>>> {\n>>> -\tipaManager_ = std::make_unique();\n>>> +\tipaManager_ = std::make_unique(this->configuration());\n>>> }\n>>> int CameraManager::Private::start()\n>>> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp\n>>> index 068ddd5a5..f7c69890c 100644\n>>> --- a/src/libcamera/global_configuration.cpp\n>>> +++ b/src/libcamera/global_configuration.cpp\n>>> @@ -9,9 +9,11 @@\n>>> #include \n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>> @@ -116,13 +118,22 @@ GlobalConfiguration::GlobalConfiguration()\n>>> */\n>>> /**\n>>> + * \\fn std::optional GlobalConfiguration::option(const std::string &confPath) const\n>>> * \\brief Return value of the configuration option identified by \\a confPath\n>>> * \\param[in] confPath Sequence of the YAML section names (excluding\n>>> * `configuration') leading to the requested option separated by slashes\n>>> * \\return A value if an item corresponding to \\a confPath exists in the\n>>> * configuration file, no value otherwise\n>>> */\n>>> -std::optional GlobalConfiguration::option(const std::string &confPath) const\n>>> +\n>>> +/**\n>>> + * \\brief Return values of the configuration option identified by \\a confPath\n>>> + * \\tparam T The type of the retrieved configuration value\n>>> + * \\param[in] confPath Sequence of the YAML section names (excluding\n>>> + * `configuration') leading to the requested list option, separated by dots\n>>> + * \\return A vector of strings (empty one if the option is not found)\n>>> + */\n>>> +std::vector GlobalConfiguration::listOption(const std::string &confPath) const\n>>> {\n>>> \tconst YamlObject *c = &configuration();\n>>> \tfor (auto part : utils::split(confPath, \"/\")) {\n>>> @@ -130,7 +141,7 @@ std::optional GlobalConfiguration::option(const std::string &confPa\n>>> \t\tif (!*c)\n>>> \t\t\treturn {};\n>>> \t}\n>>> -\treturn c->get();\n>>> +\treturn c->getList().value_or(std::vector());\n>>> }\n>>> /**\n>>> @@ -157,7 +168,37 @@ std::optional GlobalConfiguration::envOption(\n>>> \tconst char *envValue = utils::secure_getenv(envVariable);\n>>> \tif (envValue)\n>>> \t\treturn std::optional{ std::string{ envValue } };\n>>> -\treturn option(confPath);\n>>> +\treturn option(confPath);\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\brief Return values 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>>> + *\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 (an empty vector is returned if nothing is found)\n>>\n>> I think we want to distinguish \"not present\" and \"empty\". I could imagine\n>> that someone might want to inhibit some behaviour by setting the env var\n>> to empty.\n> \n> Well, introducing new environment variables for configuration is\n> discouraged.\n\nI think for some things the convenience for testing/debugging/etc. will\noutweigh the drawbacks and we will decide to add a new env var.\n\n\n> \n>> `envOption()` also differentiates the two. For example, with the new\n>> \"layers\" proposal, I could image that we can set the layers in a\n>> configuration file, etc, as well as an environmental variable. In that\n>> case doing `LIBCAMERA_LAYERS= cam ...` might be useful for testing to\n>> temporarily disable all layers.\n> \n> Hmm, sounds reasonable but I'm still not sure we want to encourage this.\n> But technically, I can change the method signature.\n\nEncourage env vars in general, or the use of empty env vars specifically?\nIn any case, I think it would be nice if `envOption()` and `envListOption`\nwere consistent wrt. empty env vars.\n\n\n> \n>>> + */\n>>> +std::vector GlobalConfiguration::envListOption(\n>>> +\tconst char *const envVariable,\n>>> +\tconst std::string &confPath) const\n>>> +{\n>>> +\tconst char *envValue = utils::secure_getenv(envVariable);\n>>> +\tif (envValue) {\n>>> +\t\tauto items = utils::split(envValue, \":\");\n>>> +\t\treturn std::vector(items.begin(), items.end());\n>>> +\t}\n>>> +\treturn listOption(confPath);\n>>> }\n>>> /**\n>>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>>> index 830750dcc..67c1b23d2 100644\n>>> --- a/src/libcamera/ipa_manager.cpp\n>>> +++ b/src/libcamera/ipa_manager.cpp\n>>> @@ -9,6 +9,7 @@\n>>> #include \n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>> @@ -16,6 +17,7 @@\n>>> #include \n>>> #include \n>>> +#include \"libcamera/internal/global_configuration.h\"\n>>> #include \"libcamera/internal/ipa_module.h\"\n>>> #include \"libcamera/internal/ipa_proxy.h\"\n>>> #include \"libcamera/internal/pipeline_handler.h\"\n>>> @@ -101,7 +103,7 @@ LOG_DEFINE_CATEGORY(IPAManager)\n>>> * The IPAManager class is meant to only be instantiated once, by the\n>>> * CameraManager.\n>>> */\n>>> -IPAManager::IPAManager()\n>>> +IPAManager::IPAManager(const GlobalConfiguration &configuration)\n>>> {\n>>> #if HAVE_IPA_PUBKEY\n>>> \tif (!pubKey_.isValid())\n>>> @@ -111,18 +113,25 @@ IPAManager::IPAManager()\n>>> \tunsigned int ipaCount = 0;\n>>> \t/* User-specified paths take precedence. */\n>>> -\tconst char *modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n>>> -\tif (modulePaths) {\n>>> -\t\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n>>> -\t\t\tif (dir.empty())\n>>> -\t\t\t\tcontinue;\n>>> +\tconst auto modulePaths =\n>>> +\t\tconfiguration.envListOption(\n>>> +\t\t\t\"LIBCAMERA_IPA_MODULE_PATH\", \"ipa/module_paths\");\n>>> +\tfor (const auto &dir : modulePaths) {\n>>> +\t\tif (dir.empty())\n>>> +\t\t\tcontinue;\n>>> -\t\t\tipaCount += addDir(dir.c_str());\n>>> -\t\t}\n>>> +\t\tipaCount += addDir(dir.c_str());\n>>> +\t}\n>>> -\t\tif (!ipaCount)\n>>> -\t\t\tLOG(IPAManager, Warning)\n>>> -\t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n>>> +\tif (!ipaCount) {\n>>> +\t\tstd::string paths;\n>>> +\t\tif (!modulePaths.empty()) {\n>>> +\t\t\tpaths = std::accumulate(std::next(modulePaths.begin()),\n>>> +\t\t\t\t\t\tmodulePaths.end(),\n>>> +\t\t\t\t\t\tmodulePaths[0],\n>>> +\t\t\t\t\t\t[](std::string s1, std::string s2) { return s1 + \":\" + s2; });\n>>> +\t\t}\n>>> +\t\tLOG(IPAManager, Warning) << \"No IPA found in '\" << paths << \"'\";\n>>\n>> #include \n>> ...\n>> LOG(IPAManager, Warning) << \"No IPA found in '\" << utils::join(modulePaths, \":\") << \"'\";\n>>\n>> ?\n> \n> OK.\n> \n>>> \t}\n>>> \t/*\n>>> @@ -276,11 +285,13 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n>>> */\n>>> #endif\n>>> -bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n>>> +bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa, const GlobalConfiguration &configuration) const\n>>\n>> Maybe `configuration.option(\"ipa/force_isolation\").value_or(false)` could be\n>> saved into a member variable in the constructor? Then this won't need\n>> the configuration parameter.\n> \n> Yes. Assuming the `pipe' CameraManager instance is always the same that\n> was used to create the IPAManager instance.\n\nIpaManager::createIPA() goes from PipelineHandler -> CameraManager -> IPAManager,\nso I am quite certain there is no room for mixup.\n\n\n> \n>>> {\n>>> #if HAVE_IPA_PUBKEY\n>>> \tchar *force = utils::secure_getenv(\"LIBCAMERA_IPA_FORCE_ISOLATION\");\n>>> -\tif (force && force[0] != '\\0') {\n>>> +\tif ((force && force[0] != '\\0') ||\n>>> +\t (!force && configuration.option(\"ipa/force_isolation\")\n>>> +\t\t\t .value_or(false))) {\n>>> \t\tLOG(IPAManager, Debug)\n>>> \t\t\t<< \"Isolation of IPA module \" << ipa->path()\n>>> \t\t\t<< \" forced through environment variable\";\n>>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n>>> index 9907b9615..77927b0d4 100644\n>>> --- a/src/libcamera/ipa_proxy.cpp\n>>> +++ b/src/libcamera/ipa_proxy.cpp\n>>> @@ -14,6 +14,7 @@\n>>> #include \n>>> #include \n>>> +#include \"libcamera/internal/global_configuration.h\"\n>>> #include \"libcamera/internal/ipa_module.h\"\n>>> /**\n>>> @@ -71,6 +72,7 @@ IPAProxy::~IPAProxy()\n>>> /**\n>>> * \\brief Retrieve the absolute path to an IPA configuration file\n>>> * \\param[in] name The configuration file name\n>>> + * \\param[in] configuration The global configuration\n>>> * \\param[in] fallbackName The name of a fallback configuration file\n>>> *\n>>> * This function locates the configuration file for an IPA and returns its\n>>> @@ -96,6 +98,7 @@ IPAProxy::~IPAProxy()\n>>> * no configuration file can be found\n>>> */\n>>> std::string IPAProxy::configurationFile(const std::string &name,\n>>> +\t\t\t\t\tconst GlobalConfiguration &configuration,\n>>> \t\t\t\t\tconst std::string &fallbackName) const\n>>> {\n>>> \t/*\n>>> @@ -105,37 +108,37 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>>> \tconst std::string ipaName = ipam_->info().name;\n>>> \t/*\n>>> -\t * Start with any user override through the module-specific environment\n>>> -\t * variable. Use the name of the IPA module up to the first '/' to\n>>> -\t * construct the variable name.\n>>> +\t * Start with any user override through the module-specific configuration or\n>>> +\t * environment variable. Use the name of the IPA module up to the first '/'\n>>> +\t * to construct the configuration and variable names.\n>>> \t */\n>>> -\tstd::string ipaEnvName = ipaName.substr(0, ipaName.find('/'));\n>>> +\tstd::string ipaBaseName = ipaName.substr(0, ipaName.find('/'));\n>>> +\tstd::string ipaConfigName = \"ipa/ipas/\" + ipaBaseName + \"/tuning_file\";\n>>\n>> I feel like we're unnecessarily limiting outself wrt. rpi here. Since the IPA\n>> names are \"rpi/vc4\" and \"rpi/pisp\", thus `ipaBaseName == \"rpi\"`. Which means\n>> that it is impossible to specify different tuning files for the different\n>> platforms. I think this is fine for an env var, but for a configuration file\n>> I think this flexibility is expected.\n> \n> Right; so rather than trimming, let's replace the slash with another\n> character here?\n\nI am not sure what the best option is. Replacing `/` with `_` or `-` seems\nlike a good candidate. But note that the env var has to keep its name, so\nthe trimming will still have to be done.\n\n\n> \n>> But I don't quite see the point of overriding the tuning file like this from a\n>> configuration file. So maybe leaving just the env var override is sufficient?\n> \n> I think the initial idea was to be able to configure everything in the\n> configuration file (why not?). Logging is currently a noticeable\n> exception, although hopefully only \"temporary\".\n\nI think `LIBCAMERA_${IPA}_TUNING_FILE` is mainly a testing feature because\nit overrides the tuning file \"globally\" for the given IPA. My first impression\nwas that it is probably not that useful to have in a configuration file. And\nI still think that, so my preference is not including it in the configuration\nfile. With that, feel free to decide, but please address the above rpi naming\nissue if you want to include it.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n>>> +\tstd::string ipaEnvName = ipaBaseName;\n>>> \tstd::transform(ipaEnvName.begin(), ipaEnvName.end(), ipaEnvName.begin(),\n>>> \t\t [](unsigned char c) { return std::toupper(c); });\n>>> \tipaEnvName = \"LIBCAMERA_\" + ipaEnvName + \"_TUNING_FILE\";\n>>> -\tchar const *configFromEnv = utils::secure_getenv(ipaEnvName.c_str());\n>>> -\tif (configFromEnv && *configFromEnv != '\\0')\n>>> -\t\treturn { configFromEnv };\n>>> +\tauto config = configuration.envOption(ipaEnvName.c_str(), ipaConfigName);\n>>> +\tif (config)\n>>> +\t\treturn { config.value() };\n>>> \tstruct stat statbuf;\n>>> \tint ret;\n>>> \t/*\n>>> -\t * Check the directory pointed to by the IPA config path environment\n>>> -\t * variable next.\n>>> +\t * Check the directory pointed to by the IPA config path next.\n>>> \t */\n>>> -\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n>>> -\tif (confPaths) {\n>>> -\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n>>> -\t\t\tif (dir.empty())\n>>> -\t\t\t\tcontinue;\n>>> -\n>>> -\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>>> -\t\t\tret = stat(confPath.c_str(), &statbuf);\n>>> -\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>>> -\t\t\t\treturn confPath;\n>>> -\t\t}\n>>> +\tauto confPaths =\n>>> +\t\tconfiguration.envListOption(\n>>> +\t\t\t\"LIBCAMERA_IPA_CONFIG_PATH\", \"ipa/config_paths\");\n>>> +\tfor (const auto &dir : confPaths) {\n>>> +\t\tif (dir.empty())\n>>> +\t\t\tcontinue;\n>>> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>>> +\t\tret = stat(confPath.c_str(), &statbuf);\n>>> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>>> +\t\t\treturn confPath;\n>>> \t}\n>>> \tstd::string root = utils::libcameraSourcePath();\n>>> @@ -178,12 +181,13 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>>> \t\t<< \"Configuration file '\" << name\n>>> \t\t<< \"' not found for IPA module '\" << ipaName\n>>> \t\t<< \"', falling back to '\" << fallbackName << \"'\";\n>>> -\treturn configurationFile(fallbackName);\n>>> +\treturn configurationFile(fallbackName, configuration);\n>>> }\n>>> /**\n>>> * \\brief Find a valid full path for a proxy worker for a given executable name\n>>> * \\param[in] file File name of proxy worker executable\n>>> + * \\param[in] configuration The global configuration\n>>> *\n>>> * A proxy worker's executable could be found in either the global installation\n>>> * directory, or in the paths specified by the environment variable\n>>> @@ -195,22 +199,22 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>>> * \\return The full path to the proxy worker executable, or an empty string if\n>>> * no valid executable path\n>>> */\n>>> -std::string IPAProxy::resolvePath(const std::string &file) const\n>>> +std::string IPAProxy::resolvePath(const std::string &file, const GlobalConfiguration &configuration) const\n>>> {\n>>> \tstd::string proxyFile = \"/\" + file;\n>>> -\t/* Check env variable first. */\n>>> -\tconst char *execPaths = utils::secure_getenv(\"LIBCAMERA_IPA_PROXY_PATH\");\n>>> -\tif (execPaths) {\n>>> -\t\tfor (const auto &dir : utils::split(execPaths, \":\")) {\n>>> -\t\t\tif (dir.empty())\n>>> -\t\t\t\tcontinue;\n>>> -\n>>> -\t\t\tstd::string proxyPath = dir;\n>>> -\t\t\tproxyPath += proxyFile;\n>>> -\t\t\tif (!access(proxyPath.c_str(), X_OK))\n>>> -\t\t\t\treturn proxyPath;\n>>> -\t\t}\n>>> +\t/* Check the configuration first. */\n>>> +\tconst auto execPaths =\n>>> +\t\tconfiguration.envListOption(\n>>> +\t\t\t\"LIBCAMERA_IPA_PROXY_PATH\", \"ipa/proxy_paths\");\n>>> +\tfor (const auto &dir : execPaths) {\n>>> +\t\tif (dir.empty())\n>>> +\t\t\tcontinue;\n>>> +\n>>> +\t\tstd::string proxyPath = dir;\n>>\n>> std::string proxyPath = dir + proxyFile;\n> \n> OK.\n> \n>> Regards,\n>> Barnabás Pőcze\n>>\n>>\n>>> +\t\tproxyPath += proxyFile;\n>>> +\t\tif (!access(proxyPath.c_str(), X_OK))\n>>> +\t\t\treturn proxyPath;\n>>> \t}\n>>> \t/*\n>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> index e31e3879d..723f7665b 100644\n>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> @@ -1183,8 +1183,9 @@ int IPU3CameraData::loadIPA()\n>>> \t * The API tuning file is made from the sensor name. If the tuning file\n>>> \t * isn't found, fall back to the 'uncalibrated' file.\n>>> \t */\n>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>> \tstd::string ipaTuningFile =\n>>> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n>>> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>>> \tret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },\n>>> \t\t\t sensorInfo, sensor->controls(), &ipaControls_);\n>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> index 4acc091bd..d346fa25f 100644\n>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> @@ -388,7 +388,9 @@ int MaliC55CameraData::loadIPA()\n>>> \tipa_->setSensorControls.connect(this, &MaliC55CameraData::setSensorControls);\n>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>> \tstd::string ipaTuningFile = ipa_->configurationFile(sensor_->model() + \".yaml\",\n>>> +\t\t\t\t\t\t\t configuration,\n>>> \t\t\t\t\t\t\t \"uncalibrated.yaml\");\n>>> \t/* We need to inform the IPA of the sensor configuration */\n>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> index 675f0a749..aded4cb43 100644\n>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> @@ -40,6 +40,7 @@\n>>> #include \"libcamera/internal/delayed_controls.h\"\n>>> #include \"libcamera/internal/device_enumerator.h\"\n>>> #include \"libcamera/internal/framebuffer.h\"\n>>> +#include \"libcamera/internal/global_configuration.h\"\n>>> #include \"libcamera/internal/ipa_manager.h\"\n>>> #include \"libcamera/internal/media_device.h\"\n>>> #include \"libcamera/internal/media_pipeline.h\"\n>>> @@ -383,8 +384,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n>>> \tipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);\n>>> \t/* The IPA tuning file is made from the sensor name. */\n>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>> \tstd::string ipaTuningFile =\n>>> -\t\tipa_->configurationFile(sensor_->model() + \".yaml\", \"uncalibrated.yaml\");\n>>> +\t\tipa_->configurationFile(sensor_->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>>> \tIPACameraSensorInfo sensorInfo{};\n>>> \tint ret = sensor_->sensorInfo(&sensorInfo);\n>>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>> index a316ef297..3da919ee7 100644\n>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>> @@ -1157,7 +1157,8 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n>>> \tstd::string model = sensor_->model();\n>>> \tif (isMonoSensor(sensor_))\n>>> \t\tmodel += \"_mono\";\n>>> -\tstd::string configurationFile = ipa_->configurationFile(model + \".json\");\n>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>> +\tstd::string configurationFile = ipa_->configurationFile(model + \".json\", configuration);\n>>> \tIPASettings settings(configurationFile, sensor_->model());\n>>> \tipa::RPi::InitParams params;\n>>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n>>> index 07273bd2b..555d51bb6 100644\n>>> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n>>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n>>> @@ -494,7 +494,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>>> \tdata->ipa_->paramsComputed.connect(data.get(), &VimcCameraData::paramsComputed);\n>>> -\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n>>> +\tconst GlobalConfiguration &configuration = manager_->_d()->configuration();\n>>> +\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\", configuration);\n>>> \tFlags inFlags = ipa::vimc::TestFlag::Flag2;\n>>> \tFlags outFlags;\n>>> \tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() },\n>>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>>> index 28e2a360e..3ce354111 100644\n>>> --- a/src/libcamera/software_isp/software_isp.cpp\n>>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>>> @@ -21,6 +21,7 @@\n>>> #include \n>>> #include \"libcamera/internal/framebuffer.h\"\n>>> +#include \"libcamera/internal/global_configuration.h\"\n>>> #include \"libcamera/internal/ipa_manager.h\"\n>>> #include \"libcamera/internal/software_isp/debayer_params.h\"\n>>> @@ -130,8 +131,9 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n>>> \t * The API tuning file is made from the sensor name. If the tuning file\n>>> \t * isn't found, fall back to the 'uncalibrated' file.\n>>> \t */\n>>> +\tconst GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration();\n>>> \tstd::string ipaTuningFile =\n>>> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n>>> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>>> \tIPACameraSensorInfo sensorInfo{};\n>>> \tint ret = sensor->sensorInfo(&sensorInfo);\n>>> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n>>> index b81783664..1afcbf106 100644\n>>> --- a/test/ipa/ipa_interface_test.cpp\n>>> +++ b/test/ipa/ipa_interface_test.cpp\n>>> @@ -106,7 +106,8 @@ protected:\n>>> \t\t}\n>>> \t\t/* Test initialization of IPA module. */\n>>> -\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\");\n>>> +\t\tconst GlobalConfiguration configuration;\n>>> +\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\", configuration);\n>>> \t\tFlags inFlags;\n>>> \t\tFlags outFlags;\n>>> \t\tint ret = ipa_->init(IPASettings{ conf, \"vimc\" },\n>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>>> index 9a3aadbd2..71e920eb1 100644\n>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>>> @@ -45,7 +45,7 @@ namespace {{ns}} {\n>>> {% endfor %}\n>>> {%- endif %}\n>>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)\n>>> +{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration)\n>>> \t: IPAProxy(ipam), isolate_(isolate),\n>>> \t controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n>>> {\n>>> @@ -54,7 +54,7 @@ namespace {{ns}} {\n>>> \t\t<< ipam->path();\n>>> \tif (isolate_) {\n>>> -\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n>>> +\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\", configuration);\n>>> \t\tif (proxyWorkerPath.empty()) {\n>>> \t\t\tLOG(IPAProxy, Error)\n>>> \t\t\t\t<< \"Failed to get proxy worker path\";\n>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>>> index a0312a7c1..057c3ab03 100644\n>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>>> @@ -37,7 +37,7 @@ namespace {{ns}} {\n>>> class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object\n>>> {\n>>> public:\n>>> -\t{{proxy_name}}(IPAModule *ipam, bool isolate);\n>>> +\t{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration);\n>>> \t~{{proxy_name}}();\n>>> {% for method in interface_main.methods %}\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 D3830BDCBF\n\tfor ;\n\tMon, 30 Jun 2025 09:52:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A865168E05;\n\tMon, 30 Jun 2025 11:52:39 +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 DE69D61529\n\tfor ;\n\tMon, 30 Jun 2025 11:52:37 +0200 (CEST)","from [192.168.33.21] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 07D5078E;\n\tMon, 30 Jun 2025 11:52:15 +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=\"SCXe+OWY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751277136;\n\tbh=cRfH1D1WJ4jVypjb4uxFhFyapUXT1s1VyOx10BYMk2c=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=SCXe+OWYzjj8u9xBcL9blmbhKwMiFuBOjUU4xMVFr6muTbMNiGTEljivc9Qqj9SdJ\n\tSs1OK3uGvK4R5ggFgqri7bt1Z9KIOy2HzPvI2c0yXIXbvxoTP+kpRUvPvzkRiVuJ4E\n\tWAG7y+NIQmzoaCErjFV0IUgqkOcieEZglawRYtI4=","Message-ID":"<8affaa1e-96f5-4e96-9674-1ed91d461d09@ideasonboard.com>","Date":"Mon, 30 Jun 2025 11:52:33 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v11 05/12] config: Look up IPA configurables in\n\tconfiguration file","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tLaurent Pinchart ","References":"<20250624083612.27230-1-mzamazal@redhat.com>\n\t<20250624083612.27230-6-mzamazal@redhat.com>\n\t<1791792c-8970-43e4-811e-e0dc5d08f68e@ideasonboard.com>\n\t<8534bkx5fj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<8534bkx5fj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","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":34748,"web_url":"https://patchwork.libcamera.org/comment/34748/","msgid":"<85tt3xc7sa.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-30T20:07:49","subject":"Re: [PATCH v11 05/12] config: Look up IPA configurables in\n\tconfiguration file","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Barnabás Pőcze writes:\n\n> Hi\n>\n> 2025. 06. 27. 23:04 keltezéssel, Milan Zamazal írta:\n>> Hi Barnabás,\n>> thank you for review.\n>> Barnabás Pőcze writes:\n>> \n>>> Hi\n>>>\n>>> 2025. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:\n>>>> This patch adds configuration options for environment variables used in\n>>>> the IPA proxy. Two utility functions configuration retrieval functions\n>>>> are added, to retrieve lists of values.\n>>>> The configuration snippet:\n>>>> configuration:\n>>>> ipa:\n>>>> config_paths:\n>>>> - config path 1\n>>>> - config path 2\n>>>> - ...\n>>>> module_paths:\n>>>> - module path 1\n>>>> - module path 2\n>>>> - ...\n>>>> proxy_paths:\n>>>> - proxy path 1\n>>>> - proxy path 2\n>>>> - ...\n>>>> force_isolation: BOOL\n>>>> ipas:\n>>>> IPA_NAME:\n>>>> tuning_file: TUNING FILE\n>>>> Signed-off-by: Milan Zamazal \n>>>> ---\n>>>> .../libcamera/internal/global_configuration.h | 35 ++++++++-\n>>>> include/libcamera/internal/ipa_manager.h | 8 ++-\n>>>> include/libcamera/internal/ipa_proxy.h | 5 +-\n>>>> src/libcamera/camera_manager.cpp | 2 +-\n>>>> src/libcamera/global_configuration.cpp | 47 +++++++++++-\n>>>> src/libcamera/ipa_manager.cpp | 37 ++++++----\n>>>> src/libcamera/ipa_proxy.cpp | 72 ++++++++++---------\n>>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n>>>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +\n>>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +-\n>>>> .../pipeline/rpi/common/pipeline_base.cpp | 3 +-\n>>>> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n>>>> src/libcamera/software_isp/software_isp.cpp | 4 +-\n>>>> test/ipa/ipa_interface_test.cpp | 3 +-\n>>>> .../module_ipa_proxy.cpp.tmpl | 4 +-\n>>>> .../module_ipa_proxy.h.tmpl | 2 +-\n>>>> 16 files changed, 169 insertions(+), 65 deletions(-)\n>>>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>>>> index 357119628..cc436703e 100644\n>>>> --- a/include/libcamera/internal/global_configuration.h\n>>>> +++ b/include/libcamera/internal/global_configuration.h\n>>>> @@ -11,6 +11,8 @@\n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> +\n>>>> #include \"libcamera/internal/yaml_parser.h\"\n>>>> namespace libcamera {\n>>>> @@ -24,9 +26,40 @@ public:\n>>>> \tunsigned int version() const;\n>>>> \tConfiguration configuration() const;\n>>>> -\tstd::optional option(const std::string &confPath) const;\n>>>> +\n>>>> +#ifndef __DOXYGEN__\n>>>> +\ttemplate>>> +\t\t std::enable_if_t<\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v ||\n>>>> +\t\t\t std::is_same_v> * = nullptr>\n>>>\n>>> Is this needed? `YamlObject::get()` does not have any constraints,\n>>> so I think it can be omitted here as well?\n>> Yes; YamlObject::get() used to have the constraints but doesn't have\n>> them any more.\n>> \n>>>> +#else\n>>>> +\ttemplate\n>>>> +#endif\n>>>> +\tstd::optional option(const std::string &confPath) const\n>>>> +\t{\n>>>> +\t\tconst YamlObject *c = &configuration();\n>>>> +\t\tfor (auto part : utils::split(confPath, \"/\")) {\n>>>> +\t\t\tc = &(*c)[part];\n>>>> +\t\t\tif (!*c)\n>>>> +\t\t\t\treturn {};\n>>>> +\t\t}\n>>>> +\t\treturn c->get();\n>>>> +\t}\n>>>> +\n>>>> +\tstd::vector listOption(const std::string &confPath) const;\n>>>> \tstd::optional envOption(const char *const envVariable,\n>>>> \t\t\t\t\t const std::string &confPath) const;\n>>>> +\tstd::vector envListOption(\n>>>> +\t\tconst char *const envVariable,\n>>>> +\t\tconst std::string &confPath) const;\n>>>> private:\n>>>> \tbool loadFile(const std::filesystem::path &fileName);\n>>>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n>>>> index a0d448cf9..8ad717919 100644\n>>>> --- a/include/libcamera/internal/ipa_manager.h\n>>>> +++ b/include/libcamera/internal/ipa_manager.h\n>>>> @@ -17,6 +17,7 @@\n>>>> #include \n>>>> #include \"libcamera/internal/camera_manager.h\"\n>>>> +#include \"libcamera/internal/global_configuration.h\"\n>>>> #include \"libcamera/internal/ipa_module.h\"\n>>>> #include \"libcamera/internal/pipeline_handler.h\"\n>>>> #include \"libcamera/internal/pub_key.h\"\n>>>> @@ -28,7 +29,7 @@ LOG_DECLARE_CATEGORY(IPAManager)\n>>>> class IPAManager\n>>>> {\n>>>> public:\n>>>> -\tIPAManager();\n>>>> +\tIPAManager(const GlobalConfiguration &configuration);\n>>>> \t~IPAManager();\n>>>> \ttemplate\n>>>> @@ -42,7 +43,8 @@ public:\n>>>> \t\tif (!m)\n>>>> \t\t\treturn nullptr;\n>>>> -\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m));\n>>>> +\t\tconst GlobalConfiguration &configuration = cm->_d()->configuration();\n>>>> +\t\tstd::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m, configuration), configuration);\n>>>> \t\tif (!proxy->isValid()) {\n>>>> \t\t\tLOG(IPAManager, Error) << \"Failed to load proxy\";\n>>>> \t\t\treturn nullptr;\n>>>> @@ -66,7 +68,7 @@ private:\n>>>> \tIPAModule *module(PipelineHandler *pipe, uint32_t minVersion,\n>>>> \t\t\t uint32_t maxVersion);\n>>>> -\tbool isSignatureValid(IPAModule *ipa) const;\n>>>> +\tbool isSignatureValid(IPAModule *ipa, const GlobalConfiguration &configuration) const;\n>>>> \tstd::vector> modules_;\n>>>> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\n>>>> index 983bcc5fa..01cc5deff 100644\n>>>> --- a/include/libcamera/internal/ipa_proxy.h\n>>>> +++ b/include/libcamera/internal/ipa_proxy.h\n>>>> @@ -11,6 +11,8 @@\n>>>> #include \n>>>> +#include \"libcamera/internal/global_configuration.h\"\n>>>> +\n>>>> namespace libcamera {\n>>>> class IPAModule;\n>>>> @@ -30,10 +32,11 @@ public:\n>>>> \tbool isValid() const { return valid_; }\n>>>> \tstd::string configurationFile(const std::string &name,\n>>>> +\t\t\t\t const GlobalConfiguration &configuration,\n>>>> \t\t\t\t const std::string &fallbackName = std::string()) const;\n>>>> protected:\n>>>> -\tstd::string resolvePath(const std::string &file) const;\n>>>> +\tstd::string resolvePath(const std::string &file, const GlobalConfiguration &configuration) const;\n>>>\n>>> I feel like `IPAProxy` could take the `GlobalConfiguration`\n>>> in the constructor?\n>> I.e. copy the configuration and changing std::unique_ptr\n>> inside to shared_ptr? Possible, I think, and I don't have a particular\n>> preference -- up to your judgement what's better.\n>\n> I had two options in mind:\n> (1) Extract everything from the configuration that you need and store them\n> in member variables. This would mean moving the envListOption() calls to\n> the constructor.\n> (2) Just store the `GlobalConfiguration::Configuration` object. Since the lifetime\n> of IPAManager is tied to CameraManager, this shouldn't cause issue.\n>\n> But now I see that while (2) is simpler, `GlobalConfiguration::Configuration` is just\n> a reference, so it does not have the `{env,}{list,}option()` functions that would\n> be needed. \n\nHmm, right.\n\n> And (1) is a more significant change.\n\nI may try.\n\n> I would really prefer if you didn't have to pass a configuration instance to every\n> method that might need it. \n\nMe too, but things like this are the price for not having a globally\naccessible configuration. It's manageable some way here but I'm really\ncurious about how to make logging non-global.\n\n> But I don't have a good suggestion at the moment.\n>\n>\n>\n>> \n>>>> \tbool valid_;\n>>>> \tProxyState state_;\n>>>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>>>> index f3b4ec708..c47fd3677 100644\n>>>> --- a/src/libcamera/camera_manager.cpp\n>>>> +++ b/src/libcamera/camera_manager.cpp\n>>>> @@ -41,7 +41,7 @@ LOG_DEFINE_CATEGORY(Camera)\n>>>> CameraManager::Private::Private()\n>>>> \t: initialized_(false)\n>>>> {\n>>>> -\tipaManager_ = std::make_unique();\n>>>> +\tipaManager_ = std::make_unique(this->configuration());\n>>>> }\n>>>> int CameraManager::Private::start()\n>>>> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp\n>>>> index 068ddd5a5..f7c69890c 100644\n>>>> --- a/src/libcamera/global_configuration.cpp\n>>>> +++ b/src/libcamera/global_configuration.cpp\n>>>> @@ -9,9 +9,11 @@\n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> @@ -116,13 +118,22 @@ GlobalConfiguration::GlobalConfiguration()\n>>>> */\n>>>> /**\n>>>> + * \\fn std::optional GlobalConfiguration::option(const std::string &confPath) const\n>>>> * \\brief Return value of the configuration option identified by \\a confPath\n>>>> * \\param[in] confPath Sequence of the YAML section names (excluding\n>>>> * `configuration') leading to the requested option separated by slashes\n>>>> * \\return A value if an item corresponding to \\a confPath exists in the\n>>>> * configuration file, no value otherwise\n>>>> */\n>>>> -std::optional GlobalConfiguration::option(const std::string &confPath) const\n>>>> +\n>>>> +/**\n>>>> + * \\brief Return values of the configuration option identified by \\a confPath\n>>>> + * \\tparam T The type of the retrieved configuration value\n>>>> + * \\param[in] confPath Sequence of the YAML section names (excluding\n>>>> + * `configuration') leading to the requested list option, separated by dots\n>>>> + * \\return A vector of strings (empty one if the option is not found)\n>>>> + */\n>>>> +std::vector GlobalConfiguration::listOption(const std::string &confPath) const\n>>>> {\n>>>> \tconst YamlObject *c = &configuration();\n>>>> \tfor (auto part : utils::split(confPath, \"/\")) {\n>>>> @@ -130,7 +141,7 @@ std::optional GlobalConfiguration::option(const std::string &confPa\n>>>> \t\tif (!*c)\n>>>> \t\t\treturn {};\n>>>> \t}\n>>>> -\treturn c->get();\n>>>> +\treturn c->getList().value_or(std::vector());\n>>>> }\n>>>> /**\n>>>> @@ -157,7 +168,37 @@ std::optional GlobalConfiguration::envOption(\n>>>> \tconst char *envValue = utils::secure_getenv(envVariable);\n>>>> \tif (envValue)\n>>>> \t\treturn std::optional{ std::string{ envValue } };\n>>>> -\treturn option(confPath);\n>>>> +\treturn option(confPath);\n>>>> +}\n>>>> +\n>>>> +/**\n>>>> + * \\brief Return values 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>>>> + *\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 (an empty vector is returned if nothing is found)\n>>>\n>>> I think we want to distinguish \"not present\" and \"empty\". I could imagine\n>>> that someone might want to inhibit some behaviour by setting the env var\n>>> to empty.\n>> Well, introducing new environment variables for configuration is\n>> discouraged.\n>\n> I think for some things the convenience for testing/debugging/etc. will\n> outweigh the drawbacks and we will decide to add a new env var.\n>\n>\n>> \n>>> `envOption()` also differentiates the two. For example, with the new\n>>> \"layers\" proposal, I could image that we can set the layers in a\n>>> configuration file, etc, as well as an environmental variable. In that\n>>> case doing `LIBCAMERA_LAYERS= cam ...` might be useful for testing to\n>>> temporarily disable all layers.\n>> Hmm, sounds reasonable but I'm still not sure we want to encourage this.\n>> But technically, I can change the method signature.\n>\n> Encourage env vars in general, or the use of empty env vars specifically?\n\nEncourage env vars in general.\n\n> In any case, I think it would be nice if `envOption()` and `envListOption`\n> were consistent wrt. empty env vars.\n\nOK, let envListOption and listOption return std::optional.\n\n>>>> + */\n>>>> +std::vector GlobalConfiguration::envListOption(\n>>>> +\tconst char *const envVariable,\n>>>> +\tconst std::string &confPath) const\n>>>> +{\n>>>> +\tconst char *envValue = utils::secure_getenv(envVariable);\n>>>> +\tif (envValue) {\n>>>> +\t\tauto items = utils::split(envValue, \":\");\n>>>> +\t\treturn std::vector(items.begin(), items.end());\n>>>> +\t}\n>>>> +\treturn listOption(confPath);\n>>>> }\n>>>> /**\n>>>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>>>> index 830750dcc..67c1b23d2 100644\n>>>> --- a/src/libcamera/ipa_manager.cpp\n>>>> +++ b/src/libcamera/ipa_manager.cpp\n>>>> @@ -9,6 +9,7 @@\n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> @@ -16,6 +17,7 @@\n>>>> #include \n>>>> #include \n>>>> +#include \"libcamera/internal/global_configuration.h\"\n>>>> #include \"libcamera/internal/ipa_module.h\"\n>>>> #include \"libcamera/internal/ipa_proxy.h\"\n>>>> #include \"libcamera/internal/pipeline_handler.h\"\n>>>> @@ -101,7 +103,7 @@ LOG_DEFINE_CATEGORY(IPAManager)\n>>>> * The IPAManager class is meant to only be instantiated once, by the\n>>>> * CameraManager.\n>>>> */\n>>>> -IPAManager::IPAManager()\n>>>> +IPAManager::IPAManager(const GlobalConfiguration &configuration)\n>>>> {\n>>>> #if HAVE_IPA_PUBKEY\n>>>> \tif (!pubKey_.isValid())\n>>>> @@ -111,18 +113,25 @@ IPAManager::IPAManager()\n>>>> \tunsigned int ipaCount = 0;\n>>>> \t/* User-specified paths take precedence. */\n>>>> -\tconst char *modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n>>>> -\tif (modulePaths) {\n>>>> -\t\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n>>>> -\t\t\tif (dir.empty())\n>>>> -\t\t\t\tcontinue;\n>>>> +\tconst auto modulePaths =\n>>>> +\t\tconfiguration.envListOption(\n>>>> +\t\t\t\"LIBCAMERA_IPA_MODULE_PATH\", \"ipa/module_paths\");\n>>>> +\tfor (const auto &dir : modulePaths) {\n>>>> +\t\tif (dir.empty())\n>>>> +\t\t\tcontinue;\n>>>> -\t\t\tipaCount += addDir(dir.c_str());\n>>>> -\t\t}\n>>>> +\t\tipaCount += addDir(dir.c_str());\n>>>> +\t}\n>>>> -\t\tif (!ipaCount)\n>>>> -\t\t\tLOG(IPAManager, Warning)\n>>>> -\t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n>>>> +\tif (!ipaCount) {\n>>>> +\t\tstd::string paths;\n>>>> +\t\tif (!modulePaths.empty()) {\n>>>> +\t\t\tpaths = std::accumulate(std::next(modulePaths.begin()),\n>>>> +\t\t\t\t\t\tmodulePaths.end(),\n>>>> +\t\t\t\t\t\tmodulePaths[0],\n>>>> +\t\t\t\t\t\t[](std::string s1, std::string s2) { return s1 + \":\" + s2; });\n>>>> +\t\t}\n>>>> +\t\tLOG(IPAManager, Warning) << \"No IPA found in '\" << paths << \"'\";\n>>>\n>>> #include \n>>> ...\n>>> LOG(IPAManager, Warning) << \"No IPA found in '\" << utils::join(modulePaths, \":\") << \"'\";\n>>>\n>>> ?\n>> OK.\n>> \n>>>> \t}\n>>>> \t/*\n>>>> @@ -276,11 +285,13 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n>>>> */\n>>>> #endif\n>>>> -bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n>>>> +bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa, const GlobalConfiguration &configuration) const\n>>>\n>>> Maybe `configuration.option(\"ipa/force_isolation\").value_or(false)` could be\n>>> saved into a member variable in the constructor? Then this won't need\n>>> the configuration parameter.\n>> Yes. Assuming the `pipe' CameraManager instance is always the same that\n>> was used to create the IPAManager instance.\n>\n> IpaManager::createIPA() goes from PipelineHandler -> CameraManager -> IPAManager,\n> so I am quite certain there is no room for mixup.\n\nOK.\n\n>>>> {\n>>>> #if HAVE_IPA_PUBKEY\n>>>> \tchar *force = utils::secure_getenv(\"LIBCAMERA_IPA_FORCE_ISOLATION\");\n>>>> -\tif (force && force[0] != '\\0') {\n>>>> +\tif ((force && force[0] != '\\0') ||\n>>>> +\t (!force && configuration.option(\"ipa/force_isolation\")\n>>>> +\t\t\t .value_or(false))) {\n>>>> \t\tLOG(IPAManager, Debug)\n>>>> \t\t\t<< \"Isolation of IPA module \" << ipa->path()\n>>>> \t\t\t<< \" forced through environment variable\";\n>>>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n>>>> index 9907b9615..77927b0d4 100644\n>>>> --- a/src/libcamera/ipa_proxy.cpp\n>>>> +++ b/src/libcamera/ipa_proxy.cpp\n>>>> @@ -14,6 +14,7 @@\n>>>> #include \n>>>> #include \n>>>> +#include \"libcamera/internal/global_configuration.h\"\n>>>> #include \"libcamera/internal/ipa_module.h\"\n>>>> /**\n>>>> @@ -71,6 +72,7 @@ IPAProxy::~IPAProxy()\n>>>> /**\n>>>> * \\brief Retrieve the absolute path to an IPA configuration file\n>>>> * \\param[in] name The configuration file name\n>>>> + * \\param[in] configuration The global configuration\n>>>> * \\param[in] fallbackName The name of a fallback configuration file\n>>>> *\n>>>> * This function locates the configuration file for an IPA and returns its\n>>>> @@ -96,6 +98,7 @@ IPAProxy::~IPAProxy()\n>>>> * no configuration file can be found\n>>>> */\n>>>> std::string IPAProxy::configurationFile(const std::string &name,\n>>>> +\t\t\t\t\tconst GlobalConfiguration &configuration,\n>>>> \t\t\t\t\tconst std::string &fallbackName) const\n>>>> {\n>>>> \t/*\n>>>> @@ -105,37 +108,37 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>>>> \tconst std::string ipaName = ipam_->info().name;\n>>>> \t/*\n>>>> -\t * Start with any user override through the module-specific environment\n>>>> -\t * variable. Use the name of the IPA module up to the first '/' to\n>>>> -\t * construct the variable name.\n>>>> +\t * Start with any user override through the module-specific configuration or\n>>>> +\t * environment variable. Use the name of the IPA module up to the first '/'\n>>>> +\t * to construct the configuration and variable names.\n>>>> \t */\n>>>> -\tstd::string ipaEnvName = ipaName.substr(0, ipaName.find('/'));\n>>>> +\tstd::string ipaBaseName = ipaName.substr(0, ipaName.find('/'));\n>>>> +\tstd::string ipaConfigName = \"ipa/ipas/\" + ipaBaseName + \"/tuning_file\";\n>>>\n>>> I feel like we're unnecessarily limiting outself wrt. rpi here. Since the IPA\n>>> names are \"rpi/vc4\" and \"rpi/pisp\", thus `ipaBaseName == \"rpi\"`. Which means\n>>> that it is impossible to specify different tuning files for the different\n>>> platforms. I think this is fine for an env var, but for a configuration file\n>>> I think this flexibility is expected.\n>> Right; so rather than trimming, let's replace the slash with another\n>> character here?\n>\n> I am not sure what the best option is. Replacing `/` with `_` or `-` seems\n> like a good candidate. But note that the env var has to keep its name, so\n> the trimming will still have to be done.\n>\n>\n>> \n>>> But I don't quite see the point of overriding the tuning file like this from a\n>>> configuration file. So maybe leaving just the env var override is sufficient?\n>> I think the initial idea was to be able to configure everything in the\n>> configuration file (why not?). Logging is currently a noticeable\n>> exception, although hopefully only \"temporary\".\n>\n> I think `LIBCAMERA_${IPA}_TUNING_FILE` is mainly a testing feature because\n> it overrides the tuning file \"globally\" for the given IPA. My first impression\n> was that it is probably not that useful to have in a configuration file. And\n> I still think that, so my preference is not including it in the configuration\n> file. With that, feel free to decide, but please address the above rpi naming\n> issue if you want to include it.\n\nOK, let's drop this to simplify the things.\n\n> Regards,\n> Barnabás Pőcze\n>\n>\n>> \n>>>> +\tstd::string ipaEnvName = ipaBaseName;\n>>>> \tstd::transform(ipaEnvName.begin(), ipaEnvName.end(), ipaEnvName.begin(),\n>>>> \t\t [](unsigned char c) { return std::toupper(c); });\n>>>> \tipaEnvName = \"LIBCAMERA_\" + ipaEnvName + \"_TUNING_FILE\";\n>>>> -\tchar const *configFromEnv = utils::secure_getenv(ipaEnvName.c_str());\n>>>> -\tif (configFromEnv && *configFromEnv != '\\0')\n>>>> -\t\treturn { configFromEnv };\n>>>> +\tauto config = configuration.envOption(ipaEnvName.c_str(), ipaConfigName);\n>>>> +\tif (config)\n>>>> +\t\treturn { config.value() };\n>>>> \tstruct stat statbuf;\n>>>> \tint ret;\n>>>> \t/*\n>>>> -\t * Check the directory pointed to by the IPA config path environment\n>>>> -\t * variable next.\n>>>> +\t * Check the directory pointed to by the IPA config path next.\n>>>> \t */\n>>>> -\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n>>>> -\tif (confPaths) {\n>>>> -\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n>>>> -\t\t\tif (dir.empty())\n>>>> -\t\t\t\tcontinue;\n>>>> -\n>>>> -\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>>>> -\t\t\tret = stat(confPath.c_str(), &statbuf);\n>>>> -\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>>>> -\t\t\t\treturn confPath;\n>>>> -\t\t}\n>>>> +\tauto confPaths =\n>>>> +\t\tconfiguration.envListOption(\n>>>> +\t\t\t\"LIBCAMERA_IPA_CONFIG_PATH\", \"ipa/config_paths\");\n>>>> +\tfor (const auto &dir : confPaths) {\n>>>> +\t\tif (dir.empty())\n>>>> +\t\t\tcontinue;\n>>>> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>>>> +\t\tret = stat(confPath.c_str(), &statbuf);\n>>>> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>>>> +\t\t\treturn confPath;\n>>>> \t}\n>>>> \tstd::string root = utils::libcameraSourcePath();\n>>>> @@ -178,12 +181,13 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>>>> \t\t<< \"Configuration file '\" << name\n>>>> \t\t<< \"' not found for IPA module '\" << ipaName\n>>>> \t\t<< \"', falling back to '\" << fallbackName << \"'\";\n>>>> -\treturn configurationFile(fallbackName);\n>>>> +\treturn configurationFile(fallbackName, configuration);\n>>>> }\n>>>> /**\n>>>> * \\brief Find a valid full path for a proxy worker for a given executable name\n>>>> * \\param[in] file File name of proxy worker executable\n>>>> + * \\param[in] configuration The global configuration\n>>>> *\n>>>> * A proxy worker's executable could be found in either the global installation\n>>>> * directory, or in the paths specified by the environment variable\n>>>> @@ -195,22 +199,22 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>>>> * \\return The full path to the proxy worker executable, or an empty string if\n>>>> * no valid executable path\n>>>> */\n>>>> -std::string IPAProxy::resolvePath(const std::string &file) const\n>>>> +std::string IPAProxy::resolvePath(const std::string &file, const GlobalConfiguration &configuration) const\n>>>> {\n>>>> \tstd::string proxyFile = \"/\" + file;\n>>>> -\t/* Check env variable first. */\n>>>> -\tconst char *execPaths = utils::secure_getenv(\"LIBCAMERA_IPA_PROXY_PATH\");\n>>>> -\tif (execPaths) {\n>>>> -\t\tfor (const auto &dir : utils::split(execPaths, \":\")) {\n>>>> -\t\t\tif (dir.empty())\n>>>> -\t\t\t\tcontinue;\n>>>> -\n>>>> -\t\t\tstd::string proxyPath = dir;\n>>>> -\t\t\tproxyPath += proxyFile;\n>>>> -\t\t\tif (!access(proxyPath.c_str(), X_OK))\n>>>> -\t\t\t\treturn proxyPath;\n>>>> -\t\t}\n>>>> +\t/* Check the configuration first. */\n>>>> +\tconst auto execPaths =\n>>>> +\t\tconfiguration.envListOption(\n>>>> +\t\t\t\"LIBCAMERA_IPA_PROXY_PATH\", \"ipa/proxy_paths\");\n>>>> +\tfor (const auto &dir : execPaths) {\n>>>> +\t\tif (dir.empty())\n>>>> +\t\t\tcontinue;\n>>>> +\n>>>> +\t\tstd::string proxyPath = dir;\n>>>\n>>> std::string proxyPath = dir + proxyFile;\n>> OK.\n\nThis actually fixes the original code but it's not worth of a separate commit.\n\n>>> Regards,\n>>> Barnabás Pőcze\n>>>\n>>>\n>>>> +\t\tproxyPath += proxyFile;\n>>>> +\t\tif (!access(proxyPath.c_str(), X_OK))\n>>>> +\t\t\treturn proxyPath;\n>>>> \t}\n>>>> \t/*\n>>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>> index e31e3879d..723f7665b 100644\n>>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>> @@ -1183,8 +1183,9 @@ int IPU3CameraData::loadIPA()\n>>>> \t * The API tuning file is made from the sensor name. If the tuning file\n>>>> \t * isn't found, fall back to the 'uncalibrated' file.\n>>>> \t */\n>>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>>> \tstd::string ipaTuningFile =\n>>>> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n>>>> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>>>> \tret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },\n>>>> \t\t\t sensorInfo, sensor->controls(), &ipaControls_);\n>>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>> index 4acc091bd..d346fa25f 100644\n>>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>> @@ -388,7 +388,9 @@ int MaliC55CameraData::loadIPA()\n>>>> \tipa_->setSensorControls.connect(this, &MaliC55CameraData::setSensorControls);\n>>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>>> \tstd::string ipaTuningFile = ipa_->configurationFile(sensor_->model() + \".yaml\",\n>>>> +\t\t\t\t\t\t\t configuration,\n>>>> \t\t\t\t\t\t\t \"uncalibrated.yaml\");\n>>>> \t/* We need to inform the IPA of the sensor configuration */\n>>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> index 675f0a749..aded4cb43 100644\n>>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> @@ -40,6 +40,7 @@\n>>>> #include \"libcamera/internal/delayed_controls.h\"\n>>>> #include \"libcamera/internal/device_enumerator.h\"\n>>>> #include \"libcamera/internal/framebuffer.h\"\n>>>> +#include \"libcamera/internal/global_configuration.h\"\n>>>> #include \"libcamera/internal/ipa_manager.h\"\n>>>> #include \"libcamera/internal/media_device.h\"\n>>>> #include \"libcamera/internal/media_pipeline.h\"\n>>>> @@ -383,8 +384,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n>>>> \tipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);\n>>>> \t/* The IPA tuning file is made from the sensor name. */\n>>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>>> \tstd::string ipaTuningFile =\n>>>> -\t\tipa_->configurationFile(sensor_->model() + \".yaml\", \"uncalibrated.yaml\");\n>>>> +\t\tipa_->configurationFile(sensor_->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>>>> \tIPACameraSensorInfo sensorInfo{};\n>>>> \tint ret = sensor_->sensorInfo(&sensorInfo);\n>>>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>>> index a316ef297..3da919ee7 100644\n>>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>>> @@ -1157,7 +1157,8 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n>>>> \tstd::string model = sensor_->model();\n>>>> \tif (isMonoSensor(sensor_))\n>>>> \t\tmodel += \"_mono\";\n>>>> -\tstd::string configurationFile = ipa_->configurationFile(model + \".json\");\n>>>> +\tconst GlobalConfiguration &configuration = pipe()->cameraManager()->_d()->configuration();\n>>>> +\tstd::string configurationFile = ipa_->configurationFile(model + \".json\", configuration);\n>>>> \tIPASettings settings(configurationFile, sensor_->model());\n>>>> \tipa::RPi::InitParams params;\n>>>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n>>>> index 07273bd2b..555d51bb6 100644\n>>>> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n>>>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n>>>> @@ -494,7 +494,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>>>> \tdata->ipa_->paramsComputed.connect(data.get(), &VimcCameraData::paramsComputed);\n>>>> -\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n>>>> +\tconst GlobalConfiguration &configuration = manager_->_d()->configuration();\n>>>> +\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\", configuration);\n>>>> \tFlags inFlags = ipa::vimc::TestFlag::Flag2;\n>>>> \tFlags outFlags;\n>>>> \tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() },\n>>>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>>>> index 28e2a360e..3ce354111 100644\n>>>> --- a/src/libcamera/software_isp/software_isp.cpp\n>>>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>>>> @@ -21,6 +21,7 @@\n>>>> #include \n>>>> #include \"libcamera/internal/framebuffer.h\"\n>>>> +#include \"libcamera/internal/global_configuration.h\"\n>>>> #include \"libcamera/internal/ipa_manager.h\"\n>>>> #include \"libcamera/internal/software_isp/debayer_params.h\"\n>>>> @@ -130,8 +131,9 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n>>>> \t * The API tuning file is made from the sensor name. If the tuning file\n>>>> \t * isn't found, fall back to the 'uncalibrated' file.\n>>>> \t */\n>>>> +\tconst GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration();\n>>>> \tstd::string ipaTuningFile =\n>>>> -\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n>>>> +\t\tipa_->configurationFile(sensor->model() + \".yaml\", configuration, \"uncalibrated.yaml\");\n>>>> \tIPACameraSensorInfo sensorInfo{};\n>>>> \tint ret = sensor->sensorInfo(&sensorInfo);\n>>>> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n>>>> index b81783664..1afcbf106 100644\n>>>> --- a/test/ipa/ipa_interface_test.cpp\n>>>> +++ b/test/ipa/ipa_interface_test.cpp\n>>>> @@ -106,7 +106,8 @@ protected:\n>>>> \t\t}\n>>>> \t\t/* Test initialization of IPA module. */\n>>>> -\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\");\n>>>> +\t\tconst GlobalConfiguration configuration;\n>>>> +\t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\", configuration);\n>>>> \t\tFlags inFlags;\n>>>> \t\tFlags outFlags;\n>>>> \t\tint ret = ipa_->init(IPASettings{ conf, \"vimc\" },\n>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>>>> index 9a3aadbd2..71e920eb1 100644\n>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>>>> @@ -45,7 +45,7 @@ namespace {{ns}} {\n>>>> {% endfor %}\n>>>> {%- endif %}\n>>>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)\n>>>> +{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration)\n>>>> \t: IPAProxy(ipam), isolate_(isolate),\n>>>> \t controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n>>>> {\n>>>> @@ -54,7 +54,7 @@ namespace {{ns}} {\n>>>> \t\t<< ipam->path();\n>>>> \tif (isolate_) {\n>>>> -\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n>>>> +\t\tconst std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\", configuration);\n>>>> \t\tif (proxyWorkerPath.empty()) {\n>>>> \t\t\tLOG(IPAProxy, Error)\n>>>> \t\t\t\t<< \"Failed to get proxy worker path\";\n>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>>>> index a0312a7c1..057c3ab03 100644\n>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>>>> @@ -37,7 +37,7 @@ namespace {{ns}} {\n>>>> class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object\n>>>> {\n>>>> public:\n>>>> -\t{{proxy_name}}(IPAModule *ipam, bool isolate);\n>>>> +\t{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration);\n>>>> \t~{{proxy_name}}();\n>>>> {% for method in interface_main.methods %}\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 4EF20C3237\n\tfor ;\n\tMon, 30 Jun 2025 20:08:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 06FAC68E04;\n\tMon, 30 Jun 2025 22:07:59 +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 8BD5761527\n\tfor ;\n\tMon, 30 Jun 2025 22:07:56 +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-220-In8FT44yO7u4O8G-U_Kgdw-1; Mon, 30 Jun 2025 16:07:53 -0400","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-453817323afso28983855e9.1\n\tfor ;\n\tMon, 30 Jun 2025 13:07:53 -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-4538f2fec5fsm121993855e9.40.2025.06.30.13.07.49\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 30 Jun 2025 13:07:50 -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=\"Moq/sUII\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1751314075;\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=CyRcHDHE63rLeyXfVG9lkTqXBXQw31iuTQnSsUbT4UQ=;\n\tb=Moq/sUIImFIzWl66Gfw6uJw1LBgaErcTkl+0cZobjw/LkarPL8X+duN4z5MCP0ePQpF+zO\n\tA2Bgjo3fAyZwHwh+/a/xWg6Xek3CnH0FxFFHBZRoddMtX3q2DPgxYpQ4PGRCnCu4vkJ2iy\n\tTi0MaoiIbFcMqgGtOc//Bn9rnZ5Em2s=","X-MC-Unique":"In8FT44yO7u4O8G-U_Kgdw-1","X-Mimecast-MFC-AGG-ID":"In8FT44yO7u4O8G-U_Kgdw_1751314073","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1751314072; x=1751918872;\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=zDcEZ7mW7Kv4zRwcQInSTpbtliuYsTbE1+zWiLQGZ/o=;\n\tb=s9+ZNUQbKprllo94X8e09bwq2hS3qtkMjJD+Yl237uKdv7CfXdLmRR4s+6JLxr1b0D\n\tdiAUivWQw7tEppKbWTGmrElwKe8wEOvC6m8ZGOYn9LfUbjDuDk7c7BCUDjyWTThFNIFx\n\tbeD2gtWwrPbvqd7Z2+ES8jYzp531wtM1XCkKYsTinrGmI6eyWYtqu4orDKZ/ZflxJGde\n\tjl+ZUJ5j7Ty9Ml74WefWQKSIXgGkVavJFj8t5b3WmvYKMmgvN4VRraUnfoU91rEaSLZE\n\tfDoRkR43Vs25/uGhb+bPQQ0dlH4K6q1dv4KNSphO49lVmssrxcJ6llvH3uYz2ElC9Sqx\n\t+V6Q==","X-Gm-Message-State":"AOJu0YxeBGeTcrfrkiX0+px2F+9XKgUIk4TtKkfRNqG0m3edPM78t/sF\n\tuY19dp1hQhLLj4Fu2RJQLGu9hS1utw0ZmB7voDiCbaEVIPS6pHRYb3bx/qtWPqJqdrXLJRd1/DD\n\tv4/gLbNM2z5Rj5UzCc7zin14s6Z72b7hXnmR4IR5qsArZp+kDaFor5BvPVKx/KB4JHuWxFcZPDU\n\tbbc8yQ7qY=","X-Gm-Gg":"ASbGncu0oDG1WxUDTQQRf6uQN5ycAWXkZjjoxeoRl3EC+QG70uvNyUk+EgzBRc0MISw\n\tJrfliDmIRGPcNWt5ph9WH5mvn2pLujcwCV/vj1FiBpQX1Lxik4JOVmIq+9ON3wTVphPZtPB941I\n\tLWtDLaUJAaq4fH40PoB+0ylC2WQYTQE1ofzeHq0m4QOwzuh6PegI561Yhtsqs8fp7kwSvmqUt1S\n\tnp4QyoQ1lu1m7FQzXjDiCiQFv2pbYgN1ZmZEAdHCaFqh+eRwvHOxBc/EqsyBKxyW5Cdc8EYmLwJ\n\t1PbuetpEd+HD2qmwTJsgpdmFWW5vIRa4w942jUHODmg2tTYvUH5PIHT1lnhppF2AE57OZxXR2gY\n\t=","X-Received":["by 2002:a05:600c:1ca0:b0:440:6a37:be0d with SMTP id\n\t5b1f17b1804b1-45390bfd05fmr151608505e9.15.1751314071780; \n\tMon, 30 Jun 2025 13:07:51 -0700 (PDT)","by 2002:a05:600c:1ca0:b0:440:6a37:be0d with SMTP id\n\t5b1f17b1804b1-45390bfd05fmr151608155e9.15.1751314071102; \n\tMon, 30 Jun 2025 13:07:51 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGT/Wc0TxgW29vfdsTC82LY9AmfFClaAXYzB8hxgJFOzEf5QE/5lJS+oG/bacGcQu6iEFdK1A==","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 v11 05/12] config: Look up IPA configurables in\n\tconfiguration file","In-Reply-To":"<8affaa1e-96f5-4e96-9674-1ed91d461d09@ideasonboard.com> (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Mon,\n\t30 Jun 2025 11:52:33 +0200\")","References":"<20250624083612.27230-1-mzamazal@redhat.com>\n\t<20250624083612.27230-6-mzamazal@redhat.com>\n\t<1791792c-8970-43e4-811e-e0dc5d08f68e@ideasonboard.com>\n\t<8534bkx5fj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<8affaa1e-96f5-4e96-9674-1ed91d461d09@ideasonboard.com>","Date":"Mon, 30 Jun 2025 22:07:49 +0200","Message-ID":"<85tt3xc7sa.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":"H5TZY9Bu0nhZSlAuhGQ_7KuD_QnV-Y_dyZmWe4AiRsQ_1751314073","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\" "}}]