{"id":19804,"url":"https://patchwork.libcamera.org/api/1.1/patches/19804/?format=json","web_url":"https://patchwork.libcamera.org/patch/19804/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20240326112419.503286-4-mzamazal@redhat.com>","date":"2024-03-26T11:24:06","name":"[RFC,03/11] config: Look up logging levels in the configuration file","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"5e832558ecb412b539365545db95722db08c4b85","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/1.1/people/177/?format=json","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19804/mbox/","series":[{"id":4241,"url":"https://patchwork.libcamera.org/api/1.1/series/4241/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4241","date":"2024-03-26T11:24:03","name":"Add global configuration file","version":1,"mbox":"https://patchwork.libcamera.org/series/4241/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19804/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19804/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 E1697C32C1\n\tfor ;\n\tTue, 26 Mar 2024 11:26:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D68A6331B;\n\tTue, 26 Mar 2024 12:26:22 +0100 (CET)","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 0604063037\n\tfor ;\n\tTue, 26 Mar 2024 12:26:19 +0100 (CET)","from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-375-DnJ2tQljP36u1mbAjoJS3A-1;\n\tTue, 26 Mar 2024 07:26:17 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1])\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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id 012543C0D7B5\n\tfor ;\n\tTue, 26 Mar 2024 11:26:17 +0000 (UTC)","from nuthatch.brq.redhat.com (unknown [10.43.17.39])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 83DB53C22;\n\tTue, 26 Mar 2024 11:26:16 +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=\"H4fS+A8J\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1711452379;\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=tGxlXOyKjix+rj0eDrpR96gXNd4Z7usawprtl1OpE9w=;\n\tb=H4fS+A8J63+/6lxGfii9viGuMEcAFwhS7rFXHCz07ph+CFHwzKkrsnV7yKKRsEZeUkTNAh\n\twGkAj+UJC4iULUNdh++7ohn1GxxI4Kdyt7Tjr8/jAz2LsJQf6HbEldJmjgnmFCT8dRoEG0\n\ttwH55mujD0BRmwwK9rSqXwIGg1XYVx4=","X-MC-Unique":"DnJ2tQljP36u1mbAjoJS3A-1","From":"Milan Zamazal ","To":"libcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ","Subject":"[RFC PATCH 03/11] config: Look up logging levels in the\n\tconfiguration file","Date":"Tue, 26 Mar 2024 12:24:06 +0100","Message-ID":"<20240326112419.503286-4-mzamazal@redhat.com>","In-Reply-To":"<20240326112419.503286-1-mzamazal@redhat.com>","References":"<20240326112419.503286-1-mzamazal@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.4.1 on 10.11.54.1","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 same\nconfiguration in the configuration file.\n\nBecause logging asks for configuration and configuration loading uses logging,\nextra care is needed. We must prevent calling any log function before\nconfiguration is logged. Otherwise the log function would ask for\nconfiguration, it would load the configuration files and would try to log the\nresults, which would cause mutex deadlock due to calling logging inside logging.\nNot logging in configuration is not an option, at least without introducing\ncomplicated mechanism such as delayed logging, because we need to log at least\nerrors when reading the configuration.\n\nWe must also log in configuration loading only after the configuration is loaded\nand available (or there is an error in loading the configuration). Otherwise\nlogging would see an empty configuration and wouldn't be configured as asked.\n\nPreventing logging before configuration is loaded is ensured by asking for\nconfiguration in CameraManager constructor and in IPAManager constructor (which\nis mysteriously loaded from CameraManager constructor before its body is\nexecuted). This seems to be sufficient at least for software ISP.\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 src/libcamera/base/global_configuration.cpp | 11 ++++++++++-\n src/libcamera/base/log.cpp | 12 ++++++++++--\n src/libcamera/camera_manager.cpp | 9 +++++++++\n src/libcamera/ipa_manager.cpp | 12 ++++++++++++\n 4 files changed, 41 insertions(+), 3 deletions(-)","diff":"diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\nindex 72c3ce26..e9c44057 100644\n--- a/src/libcamera/base/global_configuration.cpp\n+++ b/src/libcamera/base/global_configuration.cpp\n@@ -41,6 +41,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@@ -71,10 +76,15 @@ void GlobalConfiguration::load()\n \n bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n {\n+\t/*\n+\t * initialized_ must be set to true before any logging is called!\n+\t */\n+\n \tFile file(fileName);\n \tif (!file.exists()) {\n \t\treturn false;\n \t}\n+\tinitialized_ = true;\n \n \tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n \t\tLOG(Configuration, Warning)\n@@ -89,7 +99,6 @@ bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n \t\treturn true;\n \t}\n \tconfiguration_ = std::move(root);\n-\tinitialized_ = true;\n \tLOG(Configuration, Info) << \"Configuration file \" << fileName << \"loaded\";\n \n \treturn true;\ndiff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\nindex c8045ef7..0564306d 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/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 355f3ada..cff8e80c 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -15,6 +15,7 @@\n \n #include \"libcamera/internal/camera.h\"\n #include \"libcamera/internal/device_enumerator.h\"\n+#include \"libcamera/internal/global_configuration.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n \n /**\n@@ -37,6 +38,14 @@ LOG_DEFINE_CATEGORY(Camera)\n CameraManager::Private::Private()\n \t: initialized_(false)\n {\n+\t/*\n+\t * This is needed to initialize global configuration\n+\t * before any logging is called. Actually, IPAManager\n+\t * constructor is called before this, so the configuration\n+\t * call is superfluous here, but the initialization belongs\n+\t * here.\n+\t */\n+\tGlobalConfiguration::configuration();\n }\n \n int CameraManager::Private::start()\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex 7a4515d9..5104b930 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@@ -105,6 +106,17 @@ IPAManager *IPAManager::self_ = nullptr;\n */\n IPAManager::IPAManager()\n {\n+\t/*\n+\t * Make sure configuration is loaded early.\n+\t * Otherwise there would be problems with mutual\n+\t * configuration-logging dependencies.\n+\t *\n+\t * This is needed to be here in addition to CameraManager constructor\n+\t * because logging is called before CameraManager constructor body is\n+\t * executed.\n+\t */\n+\tGlobalConfiguration::configuration();\n+\n \tif (self_)\n \t\tLOG(IPAManager, Fatal)\n \t\t\t<< \"Multiple IPAManager objects are not allowed\";\n","prefixes":["RFC","03/11"]}