From patchwork Fri Dec 6 17:11:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 22239 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 9014BBF415 for ; Fri, 6 Dec 2024 17:12:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 424AF67E24; Fri, 6 Dec 2024 18:12:19 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="OelEYaLl"; 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 088E467E24 for ; Fri, 6 Dec 2024 18:12:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733505136; 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=SJ4/Qc9xBXXgL2S2k+WsK1wnYxEZpf1H7/XHSnnGZQI=; b=OelEYaLl7RvBMeQHIQVoy+YOlte8si8bYDHS5kKAZECS8l9RUsCKAbvIU1RVmAWWLy4WTU EP1EvFA9qCivvzS9GJOCwmO1ICm3FL6GblfrnZ+ZBXxInEKCub5TxnNA7kGYV3nxKkfdIM wUwHpx4ps+q+TDQ5loiRjLPhKTg/i9E= 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-459-CNgGHhDjOkeua58cFbglWQ-1; Fri, 06 Dec 2024 12:12:11 -0500 X-MC-Unique: CNgGHhDjOkeua58cFbglWQ-1 X-Mimecast-MFC-AGG-ID: CNgGHhDjOkeua58cFbglWQ Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (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 9C74619560B6; Fri, 6 Dec 2024 17:12:10 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.224.27]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 3154A300019E; Fri, 6 Dec 2024 17:12:07 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , Kieran Bingham , Naushir Patuck , =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= , Laurent Pinchart Subject: [PATCH v6 03/16] config: Look up logging levels in the configuration file Date: Fri, 6 Dec 2024 18:11:34 +0100 Message-ID: <20241206171148.1189292-4-mzamazal@redhat.com> In-Reply-To: <20241206171148.1189292-1-mzamazal@redhat.com> References: <20241206171148.1189292-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: F2pDMuZCsyHvEgV4K2h_0TqiYgpHJHaQRNLEmKCSIgU_1733505130 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 | 12 +++++++-- src/libcamera/ipa_manager.cpp | 3 +++ 3 files changed, 40 insertions(+), 5 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 3a656b8f..0f676710 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,8 @@ #include #include +#include "libcamera/internal/global_configuration.h" + /** * \file base/log.h * \brief Logging infrastructure @@ -626,8 +629,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 cfc24d38..33ae74e8 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";