Patch Detail
Show a patch.
GET /api/1.1/patches/22749/?format=api
{ "id": 22749, "url": "https://patchwork.libcamera.org/api/1.1/patches/22749/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22749/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20250206161607.50738-4-mzamazal@redhat.com>", "date": "2025-02-06T16:15:54", "name": "[v7,03/15] config: Look up logging levels in the configuration file", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "dae8381c40e82b0630f7ba88dceab035bb07214b", "submitter": { "id": 177, "url": "https://patchwork.libcamera.org/api/1.1/people/177/?format=api", "name": "Milan Zamazal", "email": "mzamazal@redhat.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22749/mbox/", "series": [ { "id": 4993, "url": "https://patchwork.libcamera.org/api/1.1/series/4993/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4993", "date": "2025-02-06T16:15:51", "name": "Add global configuration file", "version": 7, "mbox": "https://patchwork.libcamera.org/series/4993/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22749/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22749/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 CF5F4C32EF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 6 Feb 2025 16:16:31 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D100685FA;\n\tThu, 6 Feb 2025 17:16:31 +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 6CB5F685EB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 6 Feb 2025 17:16:29 +0100 (CET)", "from mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com\n\t(ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-649-FdBcWcDqMwyN1a9gyHSrxA-1;\n\tThu, 06 Feb 2025 11:16:24 -0500", "from mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com\n\t(mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com\n\t[10.30.177.111])\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-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id D4FA11801A18; Thu, 6 Feb 2025 16:16:23 +0000 (UTC)", "from mzamazal-thinkpadp1gen3.tpbc.com (unknown [10.43.17.31])\n\tby mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id 073CC180035E; Thu, 6 Feb 2025 16:16:21 +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=\"hgDJNu6G\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1738858588;\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=na27U2JjcdS8Hy8OErLtqgCQfv1UqXhoIbJxOGF+Iow=;\n\tb=hgDJNu6GIkLrPSd4yF7JA5yWHlw73YIvHV5IHOLW5GsGuqdSM3EYUv8IJrLJ2i95n7mEJ8\n\tMgbTtCaPjU2UnftbbBndwfpAan6ZZ81nyCECYzYwH7uzqatFWty+i5YgVL2QIgiFh4TX02\n\trd9vjnpXUTaY5HbyjTjnPVwBcqogybQ=", "X-MC-Unique": "FdBcWcDqMwyN1a9gyHSrxA-1", "X-Mimecast-MFC-AGG-ID": "FdBcWcDqMwyN1a9gyHSrxA", "From": "Milan Zamazal <mzamazal@redhat.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Milan Zamazal <mzamazal@redhat.com>, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, Naushir Patuck\n\t<naush@raspberrypi.com>, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t<pobrn@protonmail.com>, Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>", "Subject": "[PATCH v7 03/15] config: Look up logging levels in the configuration\n\tfile", "Date": "Thu, 6 Feb 2025 17:15:54 +0100", "Message-ID": "<20250206161607.50738-4-mzamazal@redhat.com>", "In-Reply-To": "<20250206161607.50738-1-mzamazal@redhat.com>", "References": "<20250206161607.50738-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": "GzNzldlfHXr4kPFZYNlB-K6SrXsFHRbPC_cG86au3oo_1738858583", "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": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "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 <mzamazal@redhat.com>\n---\n src/libcamera/base/global_configuration.cpp | 30 ++++++++++++++++++---\n src/libcamera/base/log.cpp | 12 +++++++--\n src/libcamera/ipa_manager.cpp | 3 +++\n 3 files changed, 40 insertions(+), 5 deletions(-)", "diff": "diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\nindex 3350a26a..8d823b98 100644\n--- a/src/libcamera/base/global_configuration.cpp\n+++ b/src/libcamera/base/global_configuration.cpp\n@@ -22,8 +22,15 @@ namespace GlobalConfiguration {\n \n namespace {\n \n+LOG_DEFINE_CATEGORY(Configuration)\n+\n std::unique_ptr<YamlObject> yamlConfiguration = std::make_unique<YamlObject>();\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 bool loadFile(const std::filesystem::path &fileName)\n {\n \tFile file(fileName);\n@@ -31,12 +38,18 @@ bool 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 \tyamlConfiguration = std::move(root);\n \n \treturn true;\n@@ -83,7 +96,11 @@ Configuration get()\n /**\n * \\brief Initialize the global configuration\n *\n- * This must be called before global configuration is accessed.\n+ * This function is expected to be called only once, before the configuration is\n+ * queried for the first time.\n+ *\n+ * \\note Most notably, the function must be called before any logging, because\n+ * logging queries the configuration.\n */\n void initialize()\n {\n@@ -97,6 +114,13 @@ void 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\ndiff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\nindex 3a656b8f..0f676710 100644\n--- a/src/libcamera/base/log.cpp\n+++ b/src/libcamera/base/log.cpp\n@@ -14,6 +14,7 @@\n #include <stdio.h>\n #include <stdlib.h>\n #include <string.h>\n+#include <string>\n #include <syslog.h>\n #include <time.h>\n #include <unordered_set>\n@@ -25,6 +26,8 @@\n #include <libcamera/base/thread.h>\n #include <libcamera/base/utils.h>\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<std::string> confDebug =\n+\t\t\tGlobalConfiguration::configuration()[\"log\"][\"levels\"].get<std::string>();\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 cfc24d38..33ae74e8 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -16,6 +16,7 @@\n #include <libcamera/base/log.h>\n #include <libcamera/base/utils.h>\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": [ "v7", "03/15" ] }