[PATCH v5 04/15] config: Look up logging levels in the configuration file

[Milan Zamazal]

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 <mzamazal at redhat.com>
---
 .../libcamera/internal/global_configuration.h |  1 +
 src/libcamera/base/global_configuration.cpp   | 35 +++++++++++++++++--
 src/libcamera/base/log.cpp                    | 12 +++++--
 src/libcamera/ipa_manager.cpp                 |  3 ++
 4 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h
index 628d85cb8..8058fd8e2 100644
--- a/include/libcamera/internal/global_configuration.h
+++ b/include/libcamera/internal/global_configuration.h
@@ -35,6 +35,7 @@ private:
 
 	bool loadFile(const std::filesystem::path &fileName);
 	void load();
+
 	static Configuration get();
 };
 
diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp
index 7dcf97265..c6f4ebd7b 100644
--- a/src/libcamera/base/global_configuration.cpp
+++ b/src/libcamera/base/global_configuration.cpp
@@ -19,11 +19,19 @@
 
 namespace libcamera {
 
+LOG_DEFINE_CATEGORY(Configuration)
+
 std::unique_ptr<GlobalConfiguration> GlobalConfiguration::instance_ =
 	std::make_unique<GlobalConfiguration>();
 
 /**
- * \brief Do not create GlobalConfiguration instance directly, use initialize()
+ * \brief Initialize the global configuration
+ *
+ * This method is expected to be called only once, before the configuration is
+ * queried for the first time.
+ *
+ * \note Most notably, the method must be called before any logging, because
+ * logging queries the configuration.
  */
 void GlobalConfiguration::initialize()
 {
@@ -40,6 +48,13 @@ void GlobalConfiguration::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
@@ -57,6 +72,9 @@ void GlobalConfiguration::initialize()
  * the underlying type.
  */
 
+/**
+ * \brief Do not create GlobalConfiguration instance directly, use initialize()
+ */
 GlobalConfiguration::GlobalConfiguration()
 	: configuration_(std::make_unique<YamlObject>())
 {
@@ -68,6 +86,11 @@ const std::vector<std::filesystem::path>
 		std::filesystem::path("/etc/libcamera/configuration.yaml"),
 	};
 
+/*
+ * Care is needed here to not log anything before the configuration is
+ * loaded otherwise the logger would be initialized with empty configuration.
+ */
+
 void GlobalConfiguration::load()
 {
 	std::filesystem::path userConfigurationDirectory;
@@ -100,12 +123,18 @@ bool GlobalConfiguration::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;
+	}
 	configuration_ = std::move(root);
 
 	return true;
diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
index 3a656b8f0..0f6767107 100644
--- a/src/libcamera/base/log.cpp
+++ b/src/libcamera/base/log.cpp
@@ -14,6 +14,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <string>
 #include <syslog.h>
 #include <time.h>
 #include <unordered_set>
@@ -25,6 +26,8 @@
 #include <libcamera/base/thread.h>
 #include <libcamera/base/utils.h>
 
+#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<std::string> confDebug =
+			GlobalConfiguration::configuration()["log"]["levels"].get<std::string>();
+		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 cfc24d389..33ae74e8f 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -16,6 +16,7 @@
 #include <libcamera/base/log.h>
 #include <libcamera/base/utils.h>
 
+#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";
-- 
2.44.1