{"id":21451,"url":"https://patchwork.libcamera.org/api/patches/21451/?format=json","web_url":"https://patchwork.libcamera.org/patch/21451/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20241001102810.479285-5-mzamazal@redhat.com>","date":"2024-10-01T10:27:56","name":"[v5,04/15] config: Look up logging levels in the configuration file","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"01f5c06bccb81db3b977139b4b51f18b76483085","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/?format=json","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/21451/mbox/","series":[{"id":4644,"url":"https://patchwork.libcamera.org/api/series/4644/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4644","date":"2024-10-01T10:27:52","name":"Add global configuration file","version":5,"mbox":"https://patchwork.libcamera.org/series/4644/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/21451/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/21451/checks/","tags":{},"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 D48DDBD80A\n\tfor ;\n\tTue, 1 Oct 2024 10:28:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A7086352B;\n\tTue, 1 Oct 2024 12:28:36 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E278C6352C\n\tfor ;\n\tTue, 1 Oct 2024 12:28:29 +0200 (CEST)","from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com\n\t(ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-257-VW3pk_K-PDKJ8vrivBIMlQ-1;\n\tTue, 01 Oct 2024 06:28:25 -0400","from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (unknown\n\t[10.30.177.40])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id E442B192DE2C; Tue, 1 Oct 2024 10:28:24 +0000 (UTC)","from nuthatch.brq.redhat.com (unknown [10.43.17.37])\n\tby mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id A694B1956054; Tue, 1 Oct 2024 10:28:23 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"gEQQ9M17\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1727778509;\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=sHNHnO/rAThzWzruTpKDdGLqUkZDPYTMt4wIO/bqGd0=;\n\tb=gEQQ9M17u8TN74My6UuVA5nl/aAhcDIZP1YhT2PGA5qzMNLIeIfD6uB29A+CiYWUNVkcQz\n\tarJvfF4e694DkgU9eYqtc+Xnv8Ro+DxelE5OaYNEZ9ILgK0udTAEZcjlk+Aqcwz3MQ6v4x\n\tMnlCrTaIYYRehbGO7xGkqlesg65sU3Q=","X-MC-Unique":"VW3pk_K-PDKJ8vrivBIMlQ-1","From":"Milan Zamazal ","To":"libcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ,\n\tKieran Bingham ,\n\tNaushir Patuck ","Subject":"[PATCH v5 04/15] config: Look up logging levels in the configuration\n\tfile","Date":"Tue, 1 Oct 2024 12:27:56 +0200","Message-ID":"<20241001102810.479285-5-mzamazal@redhat.com>","In-Reply-To":"<20241001102810.479285-1-mzamazal@redhat.com>","References":"<20241001102810.479285-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-Originator":"redhat.com","Content-Transfer-Encoding":"8bit","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":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"In addition to LIBCAMERA_LOG_LEVELS environment variable, expose the\nsame configuration in the configuration file.\n\nBecause logging asks for configuration and configuration loading uses\nlogging, extra care is needed. We must prevent calling any log function\nbefore configuration is loaded. Otherwise the log function would ask\nfor configuration, it would load the configuration files and would try\nto log the results. Which can lead to various breakages, even quite\nsubtle; this has been demonstrated in camera_reconfigure test when\ntrying to make the configuration initialization more transparent, as a\nstatic variable instance with automatic initialization rather than a\npointer with manual initialization() invocation.\n\nBut we must log at least errors when reading the configuration.\nEspecially, we cannot prevent logging in configuration initialization\nentirely because the YAML parser may log errors. But in case of an\nerror during configuration initialization, the configuration cannot be\nread anyway and the statically created initial global configuration\ninstance serves the purpose of providing some, empty configuration.\nThen the errors from the YAML parser can be logged.\n\nWe cannot use the same mechanism for other logging in configuration\ninitialization because then logging would read and use the empty\nconfiguration, which is not a valid configuration this time, for the\nwhole run of libcamera.\n\nThe configuration must be initialized, which is done by calling\ninitialize() method in IPAManager constructor. A more suitable place\nwould be CameraManager constructor but it would be too late there, the\nIPAManager constructor is called earlier.\n\nThe configuration snippet for logging looks as follows:\n\n configuration:\n log:\n levels: LIBCAMERA_LOG_LEVELS\n\nSigned-off-by: Milan Zamazal \n---\n .../libcamera/internal/global_configuration.h | 1 +\n src/libcamera/base/global_configuration.cpp | 35 +++++++++++++++++--\n src/libcamera/base/log.cpp | 12 +++++--\n src/libcamera/ipa_manager.cpp | 3 ++\n 4 files changed, 46 insertions(+), 5 deletions(-)","diff":"diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\nindex 628d85cb8..8058fd8e2 100644\n--- a/include/libcamera/internal/global_configuration.h\n+++ b/include/libcamera/internal/global_configuration.h\n@@ -35,6 +35,7 @@ private:\n \n \tbool loadFile(const std::filesystem::path &fileName);\n \tvoid load();\n+\n \tstatic Configuration get();\n };\n \ndiff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\nindex 7dcf97265..c6f4ebd7b 100644\n--- a/src/libcamera/base/global_configuration.cpp\n+++ b/src/libcamera/base/global_configuration.cpp\n@@ -19,11 +19,19 @@\n \n namespace libcamera {\n \n+LOG_DEFINE_CATEGORY(Configuration)\n+\n std::unique_ptr GlobalConfiguration::instance_ =\n \tstd::make_unique();\n \n /**\n- * \\brief Do not create GlobalConfiguration instance directly, use initialize()\n+ * \\brief Initialize the global configuration\n+ *\n+ * This method is expected to be called only once, before the configuration is\n+ * queried for the first time.\n+ *\n+ * \\note Most notably, the method must be called before any logging, because\n+ * logging queries the configuration.\n */\n void GlobalConfiguration::initialize()\n {\n@@ -40,6 +48,13 @@ void GlobalConfiguration::initialize()\n * The configuration file is a YAML file and the configuration itself is stored\n * under `configuration' top-level item.\n *\n+ * Example configuration file content:\n+ * \\code{.yaml}\n+ * configuration:\n+ * log:\n+ * levels: 'IPAManager:DEBUG'\n+ * \\endcode\n+ *\n * The configuration file is looked up in user's home directory first and if it\n * is not found then in system-wide configuration directories. If multiple\n * configuration files exist then only the first one found is used and no\n@@ -57,6 +72,9 @@ void GlobalConfiguration::initialize()\n * the underlying type.\n */\n \n+/**\n+ * \\brief Do not create GlobalConfiguration instance directly, use initialize()\n+ */\n GlobalConfiguration::GlobalConfiguration()\n \t: configuration_(std::make_unique())\n {\n@@ -68,6 +86,11 @@ const std::vector\n \t\tstd::filesystem::path(\"/etc/libcamera/configuration.yaml\"),\n \t};\n \n+/*\n+ * Care is needed here to not log anything before the configuration is\n+ * loaded otherwise the logger would be initialized with empty configuration.\n+ */\n+\n void GlobalConfiguration::load()\n {\n \tstd::filesystem::path userConfigurationDirectory;\n@@ -100,12 +123,18 @@ bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n \t\treturn false;\n \t}\n \n-\tif (!file.open(File::OpenModeFlag::ReadOnly))\n+\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n+\t\tLOG(Configuration, Warning)\n+\t\t\t<< \"Failed to open configuration file \" << fileName;\n \t\treturn true;\n+\t}\n \n \tauto root = YamlParser::parse(file);\n-\tif (!root)\n+\tif (!root) {\n+\t\tLOG(Configuration, Warning)\n+\t\t\t<< \"Failed to parse configuration file \" << fileName;\n \t\treturn true;\n+\t}\n \tconfiguration_ = std::move(root);\n \n \treturn true;\ndiff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\nindex 3a656b8f0..0f6767107 100644\n--- a/src/libcamera/base/log.cpp\n+++ b/src/libcamera/base/log.cpp\n@@ -14,6 +14,7 @@\n #include \n #include \n #include \n+#include \n #include \n #include \n #include \n@@ -25,6 +26,8 @@\n #include \n #include \n \n+#include \"libcamera/internal/global_configuration.h\"\n+\n /**\n * \\file base/log.h\n * \\brief Logging infrastructure\n@@ -626,8 +629,13 @@ void Logger::parseLogFile()\n void Logger::parseLogLevels()\n {\n \tconst char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\");\n-\tif (!debug)\n-\t\treturn;\n+\tif (!debug) {\n+\t\tconst std::optional confDebug =\n+\t\t\tGlobalConfiguration::configuration()[\"log\"][\"levels\"].get();\n+\t\tif (!confDebug.has_value())\n+\t\t\treturn;\n+\t\tdebug = confDebug.value().c_str();\n+\t}\n \n \tfor (const char *pair = debug; *debug != '\\0'; pair = debug) {\n \t\tconst char *comma = strchrnul(debug, ',');\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex cfc24d389..33ae74e8f 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -16,6 +16,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@@ -103,6 +104,8 @@ LOG_DEFINE_CATEGORY(IPAManager)\n */\n IPAManager::IPAManager()\n {\n+\tGlobalConfiguration::initialize();\n+\n #if HAVE_IPA_PUBKEY\n \tif (!pubKey_.isValid())\n \t\tLOG(IPAManager, Warning) << \"Public key not valid\";\n","prefixes":["v5","04/15"]}