From patchwork Wed Apr 30 12:14:36 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 23311 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 9983ABE08B for ; Wed, 30 Apr 2025 12:15:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 481A568B2D; Wed, 30 Apr 2025 14:15:13 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="BcGG48bE"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 76F8868B26 for ; Wed, 30 Apr 2025 14:15:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1746015309; 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=MZahhlnFn2YyYQybrHFbq9BB4Ijfz/5yM6URBkHdkfM=; b=BcGG48bEFKwyo3nqgJ27fdgMNWPgYk/+tWBUve2cuaBzsKlqc/nXy5ygAfctxw2smtOwLL v0/cQlyiqWflTvrVYtkOjJicuMSSkJaFWHPR7xeHkDkeJ0YcODS1vIQ/9nXCUcZJUYhU6N l1KkOSG15Mk8VTwpdBKnL+eE11cSSgk= Received: from mx-prod-mc-04.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-58-vveXiH1cPR2wWgNMO13rtg-1; Wed, 30 Apr 2025 08:15:06 -0400 X-MC-Unique: vveXiH1cPR2wWgNMO13rtg-1 X-Mimecast-MFC-AGG-ID: vveXiH1cPR2wWgNMO13rtg_1746015305 Received: from mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.111]) (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-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id DCB1D19560BB; Wed, 30 Apr 2025 12:15:04 +0000 (UTC) Received: from mzamazal-thinkpadp1gen7.tpbc.com (unknown [10.44.32.120]) by mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id C3F3418001D5; Wed, 30 Apr 2025 12:15:02 +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 v8 03/15] config: Look up logging levels in the configuration file Date: Wed, 30 Apr 2025 14:14:36 +0200 Message-ID: <20250430121449.39910-4-mzamazal@redhat.com> In-Reply-To: <20250430121449.39910-1-mzamazal@redhat.com> References: <20250430121449.39910-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.111 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: vudk1kCO3aKJ_fxD6vuLiLQ_XxvzqVLKVmqChtEFZos_1746015305 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" In addition to LIBCAMERA_LOG_LEVELS environment variable, expose the same configuration in the configuration file. Because logging asks for configuration and configuration loading uses logging, extra care is needed. We must prevent calling any log function before configuration is loaded. Otherwise the log function would ask for configuration, it would load the configuration files and would try to log the results. Which can lead to various breakages, even quite subtle; this has been demonstrated in camera_reconfigure test when trying to make the configuration initialization more transparent, as a static variable instance with automatic initialization rather than a pointer with manual initialization() invocation. But we must log at least errors when reading the configuration. Especially, we cannot prevent logging in configuration initialization entirely because the YAML parser may log errors. But in case of an error during configuration initialization, the configuration cannot be read anyway and the statically created initial global configuration instance serves the purpose of providing some, empty configuration. Then the errors from the YAML parser can be logged. We cannot use the same mechanism for other logging in configuration initialization because then logging would read and use the empty configuration, which is not a valid configuration this time, for the whole run of libcamera. The configuration must be initialized, which is done by calling initialize() method in IPAManager constructor. A more suitable place would be CameraManager constructor but it would be too late there, the IPAManager constructor is called earlier. The configuration snippet for logging looks as follows: configuration: log: levels: LIBCAMERA_LOG_LEVELS Signed-off-by: Milan Zamazal --- src/libcamera/base/global_configuration.cpp | 30 ++++++++++++++++++--- src/libcamera/base/log.cpp | 28 ++++++++++--------- src/libcamera/ipa_manager.cpp | 3 +++ 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp index 3350a26a..8d823b98 100644 --- a/src/libcamera/base/global_configuration.cpp +++ b/src/libcamera/base/global_configuration.cpp @@ -22,8 +22,15 @@ namespace GlobalConfiguration { namespace { +LOG_DEFINE_CATEGORY(Configuration) + std::unique_ptr yamlConfiguration = std::make_unique(); +/* + * Care is needed here to not log anything before the configuration is + * loaded otherwise the logger would be initialized with empty configuration. + */ + bool loadFile(const std::filesystem::path &fileName) { File file(fileName); @@ -31,12 +38,18 @@ bool loadFile(const std::filesystem::path &fileName) return false; } - if (!file.open(File::OpenModeFlag::ReadOnly)) + if (!file.open(File::OpenModeFlag::ReadOnly)) { + LOG(Configuration, Warning) + << "Failed to open configuration file " << fileName; return true; + } auto root = YamlParser::parse(file); - if (!root) + if (!root) { + LOG(Configuration, Warning) + << "Failed to parse configuration file " << fileName; return true; + } yamlConfiguration = std::move(root); return true; @@ -83,7 +96,11 @@ Configuration get() /** * \brief Initialize the global configuration * - * This must be called before global configuration is accessed. + * This function is expected to be called only once, before the configuration is + * queried for the first time. + * + * \note Most notably, the function must be called before any logging, because + * logging queries the configuration. */ void initialize() { @@ -97,6 +114,13 @@ void initialize() * The configuration file is a YAML file and the configuration itself is stored * under `configuration' top-level item. * + * Example configuration file content: + * \code{.yaml} + * configuration: + * log: + * levels: 'IPAManager:DEBUG' + * \endcode + * * The configuration file is looked up in user's home directory first and if it * is not found then in system-wide configuration directories. If multiple * configuration files exist then only the first one found is used and no diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 8bf3e1da..4b287af0 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -5,8 +5,6 @@ * Logging infrastructure */ -#include - #include #include #include @@ -16,18 +14,22 @@ #include #include #include +#include #include #include #include #include -#include - #include +#include #include #include #include +#include + +#include "libcamera/internal/global_configuration.h" + /** * \file base/log.h * \brief Logging infrastructure @@ -235,8 +237,7 @@ void LogOutput::write(const LogMessage &msg) switch (target_) { case LoggingTargetSyslog: - str = std::string(log_severity_name(severity)) + " " - + msg.category().name() + " " + msg.fileInfo() + " "; + str = std::string(log_severity_name(severity)) + " " + msg.category().name() + " " + msg.fileInfo() + " "; if (!msg.prefix().empty()) str += msg.prefix() + ": "; str += msg.msg(); @@ -244,11 +245,7 @@ void LogOutput::write(const LogMessage &msg) break; case LoggingTargetStream: case LoggingTargetFile: - str = "[" + utils::time_point_to_string(msg.timestamp()) + "] [" - + std::to_string(Thread::currentId()) + "] " - + severityColor + log_severity_name(severity) + " " - + categoryColor + msg.category().name() + " " - + fileColor + msg.fileInfo() + " "; + str = "[" + utils::time_point_to_string(msg.timestamp()) + "] [" + std::to_string(Thread::currentId()) + "] " + severityColor + log_severity_name(severity) + " " + categoryColor + msg.category().name() + " " + fileColor + msg.fileInfo() + " "; if (!msg.prefix().empty()) str += prefixColor + msg.prefix() + ": "; str += resetColor + msg.msg(); @@ -628,8 +625,13 @@ void Logger::parseLogFile() void Logger::parseLogLevels() { const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS"); - if (!debug) - return; + if (!debug) { + const std::optional confDebug = + GlobalConfiguration::configuration()["log"]["levels"].get(); + if (!confDebug.has_value()) + return; + debug = confDebug.value().c_str(); + } for (const char *pair = debug; *debug != '\0'; pair = debug) { const char *comma = strchrnul(debug, ','); diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 830750dc..7b4b2946 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -16,6 +16,7 @@ #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" @@ -103,6 +104,8 @@ LOG_DEFINE_CATEGORY(IPAManager) */ IPAManager::IPAManager() { + GlobalConfiguration::initialize(); + #if HAVE_IPA_PUBKEY if (!pubKey_.isValid()) LOG(IPAManager, Warning) << "Public key not valid";