From patchwork Fri Jul 11 20:12:24 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 23794 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 3A255C32C8 for ; Fri, 11 Jul 2025 20:12:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DBF2D68F2D; Fri, 11 Jul 2025 22:12:56 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="GvyOQa2r"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C0D168F27 for ; Fri, 11 Jul 2025 22:12:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1752264774; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=csKT2xOe36u+jNvnG+n4rUCOorv8GFRhnX3X0jCzfKU=; b=GvyOQa2rhYv7TtRVYJZQ7GcGcyLxAHrX9itd44TL3rrWPU0w/qF1K4OaPLtOOpFDzs1N0u sLVgaNBMz31Ql64nKvX4L52L86tw+zih9kKwcvrd4utIKY+6eswFGtGP/FrmneL8hQLAbT yvxLM7WswE6P93adrzLBv5ZvitIKDIA= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-573-jej1DBeDO1eMCMK5-PoaHw-1; Fri, 11 Jul 2025 16:12:53 -0400 X-MC-Unique: jej1DBeDO1eMCMK5-PoaHw-1 X-Mimecast-MFC-AGG-ID: jej1DBeDO1eMCMK5-PoaHw_1752264772 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 353551956080; Fri, 11 Jul 2025 20:12:52 +0000 (UTC) Received: from mzamazal-thinkpadp1gen7.tpbc.com (unknown [10.45.224.49]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 09BD619560A3; Fri, 11 Jul 2025 20:12:49 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , Kieran Bingham , =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= , Laurent Pinchart Subject: [PATCH v13 05/12] config: Look up IPA configurables in configuration file Date: Fri, 11 Jul 2025 22:12:24 +0200 Message-ID: <20250711201232.129264-6-mzamazal@redhat.com> In-Reply-To: <20250711201232.129264-1-mzamazal@redhat.com> References: <20250711201232.129264-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: JpeDU8zQL_1cXLSvsGucvzIOWkKcWE8onPMJfjO3hkY_1752264772 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" This patch adds configuration options for environment variables used in the IPA proxy. Two utility functions configuration retrieval functions are added, to retrieve lists of values. The configuration snippet: configuration: ipa: config_paths: - config path 1 - config path 2 - ... module_paths: - module path 1 - module path 2 - ... proxy_paths: - proxy path 1 - proxy path 2 - ... force_isolation: BOOL LIBCAMERA__TUNING_FILE remains configurable only via the environment variable; this is supposed to be used only for testing and debugging and it's not clear what to do about IPA names like "rpi/vc4" and "rpi/pisp" exactly. There are two ways to pass the configuration to the places where it is needed: Either to pass it as an argument to the method calls that need it, or to pass it to the class constructors and extract the needed configuration from there. This patch uses the second method as it is less polluting the code. Signed-off-by: Milan Zamazal --- .../libcamera/internal/global_configuration.h | 21 +++++++- include/libcamera/internal/ipa_manager.h | 7 ++- include/libcamera/internal/ipa_proxy.h | 8 ++- src/libcamera/camera_manager.cpp | 2 +- src/libcamera/global_configuration.cpp | 47 +++++++++++++++-- src/libcamera/ipa_manager.cpp | 39 ++++++++------ src/libcamera/ipa_proxy.cpp | 51 +++++++++---------- .../module_ipa_proxy.cpp.tmpl | 4 +- .../module_ipa_proxy.h.tmpl | 2 +- 9 files changed, 129 insertions(+), 52 deletions(-) diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h index 4a9b1efd6..eb7a4ad98 100644 --- a/include/libcamera/internal/global_configuration.h +++ b/include/libcamera/internal/global_configuration.h @@ -12,6 +12,8 @@ #include #include +#include + #include "libcamera/internal/yaml_parser.h" namespace libcamera { @@ -25,11 +27,28 @@ public: unsigned int version() const; Configuration configuration() const; - std::optional option( + + template + std::optional option( + const std::initializer_list confPath) const + { + const YamlObject *c = &configuration(); + for (auto part : confPath) { + c = &(*c)[part]; + if (!*c) + return {}; + } + return c->get(); + } + + std::optional> listOption( const std::initializer_list confPath) const; std::optional envOption( const char *const envVariable, const std::initializer_list confPath) const; + std::optional> envListOption( + const char *const envVariable, + const std::initializer_list confPath) const; private: bool loadFile(const std::filesystem::path &fileName); diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index a0d448cf9..b0b44c74a 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -17,6 +17,7 @@ #include #include "libcamera/internal/camera_manager.h" +#include "libcamera/internal/global_configuration.h" #include "libcamera/internal/ipa_module.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/pub_key.h" @@ -28,7 +29,7 @@ LOG_DECLARE_CATEGORY(IPAManager) class IPAManager { public: - IPAManager(); + IPAManager(const GlobalConfiguration &configuration); ~IPAManager(); template @@ -42,7 +43,8 @@ public: if (!m) return nullptr; - std::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m)); + const GlobalConfiguration &configuration = cm->_d()->configuration(); + std::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m), configuration); if (!proxy->isValid()) { LOG(IPAManager, Error) << "Failed to load proxy"; return nullptr; @@ -73,6 +75,7 @@ private: #if HAVE_IPA_PUBKEY static const uint8_t publicKeyData_[]; static const PubKey pubKey_; + bool forceIsolation_; #endif }; diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h index 983bcc5fa..97c22c730 100644 --- a/include/libcamera/internal/ipa_proxy.h +++ b/include/libcamera/internal/ipa_proxy.h @@ -7,10 +7,14 @@ #pragma once +#include #include +#include #include +#include "libcamera/internal/global_configuration.h" + namespace libcamera { class IPAModule; @@ -24,7 +28,7 @@ public: ProxyRunning, }; - IPAProxy(IPAModule *ipam); + IPAProxy(IPAModule *ipam, const GlobalConfiguration &configuration); ~IPAProxy(); bool isValid() const { return valid_; } @@ -40,6 +44,8 @@ protected: private: IPAModule *ipam_; + std::optional> configPaths_; + std::optional> execPaths_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index f3b4ec708..c47fd3677 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -41,7 +41,7 @@ LOG_DEFINE_CATEGORY(Camera) CameraManager::Private::Private() : initialized_(false) { - ipaManager_ = std::make_unique(); + ipaManager_ = std::make_unique(this->configuration()); } int CameraManager::Private::start() diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp index f98744200..0eab053b2 100644 --- a/src/libcamera/global_configuration.cpp +++ b/src/libcamera/global_configuration.cpp @@ -9,9 +9,11 @@ #include #include +#include #include #include #include +#include #include #include @@ -115,13 +117,22 @@ GlobalConfiguration::GlobalConfiguration() */ /** + * \fn std::optional GlobalConfiguration::option(const std::initializer_list &confPath) const * \brief Return value of the configuration option identified by \a confPath * \param[in] confPath Sequence of the YAML section names (excluding * `configuration') leading to the requested option * \return A value if an item corresponding to \a confPath exists in the * configuration file, no value otherwise */ -std::optional GlobalConfiguration::option( + +/** + * \brief Return values of the configuration option identified by \a confPath + * \tparam T The type of the retrieved configuration value + * \param[in] confPath Sequence of the YAML section names (excluding + * `configuration') leading to the requested list option, separated by dots + * \return A vector of strings or no value if not found + */ +std::optional> GlobalConfiguration::listOption( const std::initializer_list confPath) const { const YamlObject *c = &configuration(); @@ -130,7 +141,7 @@ std::optional GlobalConfiguration::option( if (!*c) return {}; } - return c->get(); + return c->getList(); } /** @@ -157,7 +168,37 @@ std::optional GlobalConfiguration::envOption( const char *envValue = utils::secure_getenv(envVariable); if (envValue) return std::optional{ std::string{ envValue } }; - return option(confPath); + return option(confPath); +} + +/** + * \brief Return values of the configuration option from a file or environment + * \param[in] envVariable Environment variable to get the value from + * \param[in] confPath The same as in GlobalConfiguration::option + * + * This helper looks first at the given environment variable and if it is + * defined (even if it is empty) then it splits its value by semicolons and + * returns the resulting list of strings. Otherwise it looks for \a confPath the + * same way as in GlobalConfiguration::option, value of which must be a list of + * strings. + * + * \note Support for using environment variables to configure libcamera behavior + * is provided here mostly for backward compatibility reasons. Introducing new + * configuration environment variables is discouraged. + * + * \return A vector of strings retrieved from the given environment option or + * configuration file or no value if not found; the vector may be empty + */ +std::optional> GlobalConfiguration::envListOption( + const char *const envVariable, + const std::initializer_list confPath) const +{ + const char *envValue = utils::secure_getenv(envVariable); + if (envValue) { + auto items = utils::split(envValue, ":"); + return std::vector(items.begin(), items.end()); + } + return listOption(confPath); } /** diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 830750dcc..000d56efa 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -9,13 +9,17 @@ #include #include +#include #include +#include #include +#include #include #include #include +#include "libcamera/internal/global_configuration.h" #include "libcamera/internal/ipa_module.h" #include "libcamera/internal/ipa_proxy.h" #include "libcamera/internal/pipeline_handler.h" @@ -101,30 +105,36 @@ LOG_DEFINE_CATEGORY(IPAManager) * The IPAManager class is meant to only be instantiated once, by the * CameraManager. */ -IPAManager::IPAManager() +IPAManager::IPAManager(const GlobalConfiguration &configuration) { #if HAVE_IPA_PUBKEY if (!pubKey_.isValid()) LOG(IPAManager, Warning) << "Public key not valid"; + + char *force = utils::secure_getenv("LIBCAMERA_IPA_FORCE_ISOLATION"); + forceIsolation_ = (force && force[0] != '\0') || + (!force && configuration.option({ "ipa", "force_isolation" }) + .value_or(false)); #endif unsigned int ipaCount = 0; /* User-specified paths take precedence. */ - const char *modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH"); - if (modulePaths) { - for (const auto &dir : utils::split(modulePaths, ":")) { - if (dir.empty()) - continue; - - ipaCount += addDir(dir.c_str()); - } + const auto modulePaths = + configuration.envListOption( + "LIBCAMERA_IPA_MODULE_PATH", { "ipa", "module_paths" }); + for (const auto &dir : modulePaths.value_or(std::vector())) { + if (dir.empty()) + continue; - if (!ipaCount) - LOG(IPAManager, Warning) - << "No IPA found in '" << modulePaths << "'"; + ipaCount += addDir(dir.c_str()); } + if (!ipaCount) + LOG(IPAManager, Warning) << "No IPA found in '" + << utils::join(modulePaths.value_or(std::vector()), ":") + << "'"; + /* * When libcamera is used before it is installed, load IPAs from the * same build directory as the libcamera library itself. @@ -279,11 +289,10 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const { #if HAVE_IPA_PUBKEY - char *force = utils::secure_getenv("LIBCAMERA_IPA_FORCE_ISOLATION"); - if (force && force[0] != '\0') { + if (forceIsolation_) { LOG(IPAManager, Debug) << "Isolation of IPA module " << ipa->path() - << " forced through environment variable"; + << " forced through configuration"; return false; } diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp index 9907b9615..468e0ddf8 100644 --- a/src/libcamera/ipa_proxy.cpp +++ b/src/libcamera/ipa_proxy.cpp @@ -7,13 +7,16 @@ #include "libcamera/internal/ipa_proxy.h" +#include #include #include #include +#include #include #include +#include "libcamera/internal/global_configuration.h" #include "libcamera/internal/ipa_module.h" /** @@ -48,10 +51,15 @@ LOG_DEFINE_CATEGORY(IPAProxy) /** * \brief Construct an IPAProxy instance * \param[in] ipam The IPA module + * \param[in] configuration The global configuration */ -IPAProxy::IPAProxy(IPAModule *ipam) +IPAProxy::IPAProxy(IPAModule *ipam, const GlobalConfiguration &configuration) : valid_(false), state_(ProxyStopped), ipam_(ipam) { + configPaths_ = configuration.envListOption("LIBCAMERA_IPA_CONFIG_PATH", + { "ipa", "config_paths" }); + execPaths_ = configuration.envListOption( + "LIBCAMERA_IPA_PROXY_PATH", { "ipa", "proxy_paths" }); } IPAProxy::~IPAProxy() @@ -122,20 +130,15 @@ std::string IPAProxy::configurationFile(const std::string &name, int ret; /* - * Check the directory pointed to by the IPA config path environment - * variable next. + * Check the directory pointed to by the IPA config path next. */ - const char *confPaths = utils::secure_getenv("LIBCAMERA_IPA_CONFIG_PATH"); - if (confPaths) { - for (const auto &dir : utils::split(confPaths, ":")) { - if (dir.empty()) - continue; - - std::string confPath = dir + "/" + ipaName + "/" + name; - ret = stat(confPath.c_str(), &statbuf); - if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG) - return confPath; - } + for (const auto &dir : configPaths_.value_or(std::vector())) { + if (dir.empty()) + continue; + std::string confPath = dir + "/" + ipaName + "/" + name; + ret = stat(confPath.c_str(), &statbuf); + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG) + return confPath; } std::string root = utils::libcameraSourcePath(); @@ -199,18 +202,14 @@ std::string IPAProxy::resolvePath(const std::string &file) const { std::string proxyFile = "/" + file; - /* Check env variable first. */ - const char *execPaths = utils::secure_getenv("LIBCAMERA_IPA_PROXY_PATH"); - if (execPaths) { - for (const auto &dir : utils::split(execPaths, ":")) { - if (dir.empty()) - continue; - - std::string proxyPath = dir; - proxyPath += proxyFile; - if (!access(proxyPath.c_str(), X_OK)) - return proxyPath; - } + /* Check the configuration first. */ + for (const auto &dir : execPaths_.value_or(std::vector())) { + if (dir.empty()) + continue; + + std::string proxyPath = dir + proxyFile; + if (!access(proxyPath.c_str(), X_OK)) + return proxyPath; } /* 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 index 9a3aadbd2..0f87e7976 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -45,8 +45,8 @@ namespace {{ns}} { {% endfor %} {%- endif %} -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate) - : IPAProxy(ipam), isolate_(isolate), +{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration) + : IPAProxy(ipam, configuration), isolate_(isolate), controlSerializer_(ControlSerializer::Role::Proxy), seq_(0) { LOG(IPAProxy, Debug) 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 index a0312a7c1..057c3ab03 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -37,7 +37,7 @@ namespace {{ns}} { class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object { public: - {{proxy_name}}(IPAModule *ipam, bool isolate); + {{proxy_name}}(IPAModule *ipam, bool isolate, const GlobalConfiguration &configuration); ~{{proxy_name}}(); {% for method in interface_main.methods %}