[PATCH v5 03/15] config: Introduce global runtime configuration

[Barnabás Pőcze]

Hi


2024. október 1., kedd 12:27 keltezéssel, Milan Zamazal <mzamazal at redhat.com> írta:

> Currently, libcamera can be configured in runtime using several
> environment variables.  With introducing more and more variables, this
> mechanism reaches its limits.  It would be simpler and more flexible if
> it was possible to configure libcamera in a single file.
> 
> For example, there have been a request for defining pipeline precedence
> in runtime.  We want to compile in multiple pipelines, in order to have
> them accessible within single packages in distributions.  And then being
> able to select among the pipelines manually as needed based on the
> particular hardware or operating system environment.  Having the
> configuration file then allows easy switching between hardware, GPU or
> CPU IPAs.  Another possible use case is tuning image output, especially
> with software ISP, to user liking.  For example, some users may prefer
> higher contrast without the need to use the corresponding knobs, if
> present at all, in every application.  The configuration file can also
> be used to enable or disable experimental features and avoid the need to
> track local patches changing configuration options hard-wired in the
> code when working on new features.
> 
> This patch introduces basic support for configuration files.
> GlobalConfiguration class reads, stores and accesses the configuration.
> Its instance is stored as a private singleton accessed using a static
> method of the class.  There is currently no reason to have more than one
> instance.
> 
> libcamera configuration can be specified using a system-wide
> configuration file or a user configuration file.  The user configuration
> file takes precedence if present.  There is currently no way to merge
> multiple configuration files, the one found is used as the only
> configuration file.  If no configuration file is present, nothing
> changes to the current libcamera behavior (except for some log
> messages related to configuration file lookup).
> 
> The configuration file is a YAML file.  We already have a mechanism for
> handling YAML configuration files in libcamera and the given
> infrastructure can be reused for the purpose.  However, the
> configuration type is abstracted to make contingent future change of the
> underlying class easier while retaining (most of) the original API.
> 
> The configuration is versioned.  This has currently no particular
> meaning but is likely to have its purpose in future, especially once
> configuration validation is introduced.
> 
> The configuration YAML file looks as follows:
> 
>   ---
>   version: 1
>   configuration:
>     WHATEVER CONFIGURATION NEEDED
> 
> There is no logging about reading the configuration file and contingent
> errors.  This is on purpose because logging will query configuration,
> which can lead to various problems when done during configuration
> initialization.  Reporting the errors will be added later.
> 
> The global configuration is intended to be used as a hidden singleton,
> with static class methods around it providing the configuration
> information.  The constructor must be still public so that unique_ptr
> can be used.
> 
> A complication arises from the fact that when the configuration is
> loaded, logging may be called and logging will ask for the
> configuration.  This is error-prone and may lead to subtle problems.
> For this reason, the global configuration is instantiated to a pointer,
> with an empty configuration initially.  The real configuration will be
> created through initialize() method.  It will be clearer how it helps in
> the followup patch introducing logging configuration.
> 
> Logging is also the most notable component from base that uses global
> configuration.  In order to be able to do it, the global configuration
> must be put to base.
> 
> This patch introduces just the basic idea.  Actually using the
> configuration in the corresponding places (everything what is currently
> configurable via environment variables should be configurable in the
> file configuration) and other enhancements are implemented in the
> followup patches.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  .../libcamera/internal/global_configuration.h |  41 +++++
>  include/libcamera/internal/meson.build        |   1 +
>  src/libcamera/base/global_configuration.cpp   | 154 ++++++++++++++++++
>  src/libcamera/base/meson.build                |   1 +
>  4 files changed, 197 insertions(+)
>  create mode 100644 include/libcamera/internal/global_configuration.h
>  create mode 100644 src/libcamera/base/global_configuration.cpp
> 
> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h
> new file mode 100644
> index 000000000..628d85cb8
> --- /dev/null
> +++ b/include/libcamera/internal/global_configuration.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024 Red Hat, inc.
> + *
> + * global_configuration.h - Global configuration handling
> + */
> +
> +#pragma once
> +
> +#include <filesystem>
> +#include <vector>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +namespace libcamera {
> +
> +class GlobalConfiguration
> +{
> +public:
> +	using Configuration = const YamlObject &;
> +
> +	/* The constructor must be public to be able to use unique_ptr. */

If one does

  std::unique_ptr<T>(new T)

then the constructor does not need to be public.


> +	GlobalConfiguration();
> +	static void initialize();
> +
> +	static unsigned int version();
> +	static Configuration configuration();
> +
> +private:
> +	static const std::vector<std::filesystem::path> globalConfigurationFiles;
> +
> +	static std::unique_ptr<GlobalConfiguration> instance_;
> +
> +	std::unique_ptr<YamlObject> configuration_;
> +
> +	bool loadFile(const std::filesystem::path &fileName);
> +	void load();
> +	static Configuration get();
> +};
> +
> +} /* namespace libcamera */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 1c5eef9ca..6fc6406d7 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -21,6 +21,7 @@ libcamera_internal_headers = files([
>      'dma_buf_allocator.h',
>      'formats.h',
>      'framebuffer.h',
> +    'global_configuration.h',
>      'ipa_data_serializer.h',
>      'ipa_manager.h',
>      'ipa_module.h',
> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp
> new file mode 100644
> index 000000000..7dcf97265
> --- /dev/null
> +++ b/src/libcamera/base/global_configuration.cpp
> @@ -0,0 +1,154 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024 Red Hat, inc.
> + *
> + * global_configuration.cpp - Global configuration handling
> + */
> +
> +#include "libcamera/internal/global_configuration.h"
> +
> +#include <filesystem>
> +#include <memory>
> +#include <sys/types.h>
> +
> +#include <libcamera/base/file.h>
> +#include <libcamera/base/log.h>
> +#include <libcamera/base/utils.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +namespace libcamera {
> +
> +std::unique_ptr<GlobalConfiguration> GlobalConfiguration::instance_ =
> +	std::make_unique<GlobalConfiguration>();

Why is a unique_ptr needed here?


> +
> +/**
> + * \brief Do not create GlobalConfiguration instance directly, use initialize()
> + */
> +void GlobalConfiguration::initialize()
> +{
> +	std::unique_ptr<GlobalConfiguration> instance =
> +		std::make_unique<GlobalConfiguration>();
> +	instance->load();
> +	instance_ = std::move(instance);
> +}
> +
> +/**
> + * \class GlobalConfiguration
> + * \brief Support for global libcamera configuration
> + *
> + * The configuration file is a YAML file and the configuration itself is stored
> + * under `configuration' top-level item.
> + *
> + * 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
> + * configuration merging is performed.
> + *
> + * The class is used as a private singleton and the configuration can be
> + * accessed using GlobalConfiguration::configuration().
> + */
> +
> +/**
> + * \typedef GlobalConfiguration::Configuration
> + * \brief Type representing global libcamera configuration
> + *
> + * All code outside GlobalConfiguration must use this type declaration and not
> + * the underlying type.
> + */
> +
> +GlobalConfiguration::GlobalConfiguration()
> +	: configuration_(std::make_unique<YamlObject>())
> +{
> +}
> +
> +const std::vector<std::filesystem::path>
> +	GlobalConfiguration::globalConfigurationFiles = {
> +		std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
> +		std::filesystem::path("/etc/libcamera/configuration.yaml"),
> +	};

I think the above array can go into an anonymous namespace as there does not
appear to be used anywhere else.


> +
> +void GlobalConfiguration::load()
> +{
> +	std::filesystem::path userConfigurationDirectory;
> +	char *xdgConfigHome = utils::secure_getenv("XDG_CONFIG_HOME");
> +	if (xdgConfigHome) {
> +		userConfigurationDirectory = xdgConfigHome;
> +	} else {
> +		const char *home = utils::secure_getenv("HOME");
> +		if (home)
> +			userConfigurationDirectory =
> +				std::filesystem::path(home) / ".config";
> +	}
> +
> +	if (!userConfigurationDirectory.empty()) {
> +		std::filesystem::path user_configuration_file =
> +			userConfigurationDirectory / "libcamera" / "configuration.yaml";
> +		if (loadFile(user_configuration_file))
> +			return;
> +	}
> +
> +	for (auto path : globalConfigurationFiles)

const auto &path


> +		if (loadFile(path))
> +			return;
> +}
> +
> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
> +{
> +	File file(fileName);
> +	if (!file.exists()) {
> +		return false;
> +	}
> +
> +	if (!file.open(File::OpenModeFlag::ReadOnly))
> +		return true;
> +
> +	auto root = YamlParser::parse(file);
> +	if (!root)
> +		return true;
> +	configuration_ = std::move(root);
> +
> +	return true;
> +}
> +
> +GlobalConfiguration::Configuration GlobalConfiguration::get()
> +{
> +	return *instance_->configuration_;
> +}
> +
> +/**
> + * \brief Return configuration version
> + *
> + * The version is (optionally) declared in the configuration file in the
> + * top-level section `version', alongside `configuration'. This has currently no
> + * real use but may be needed in future if configuration incompatibilities
> + * occur.
> + *
> + * \return Configuration version as declared in the configuration file or 0 if
> + * no version is declared there
> + */
> +unsigned int GlobalConfiguration::version()
> +{
> +	return get()["version"].get<unsigned int>().value_or(0);
> +}
> +
> +/**
> + * \brief Return libcamera global configuration
> + *
> + * This returns the whole configuration stored in the top-level section
> + * `configuration' of the YAML configuration file.
> + *
> + * The requested part of the configuration can be accessed using \a YamlObject
> + * methods.
> + *
> + * \note \a YamlObject type itself shouldn't be used in type declarations to
> + * avoid trouble if we decide to change the underlying data objects in future.
> + *
> + * \return The whole configuration section
> + */
> +GlobalConfiguration::Configuration GlobalConfiguration::configuration()
> +{
> +	return get()["configuration"];
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> index 94843eb95..4c0032845 100644
> --- a/src/libcamera/base/meson.build
> +++ b/src/libcamera/base/meson.build
> @@ -16,6 +16,7 @@ libcamera_base_internal_sources = files([
>      'event_dispatcher_poll.cpp',
>      'event_notifier.cpp',
>      'file.cpp',
> +    'global_configuration.cpp',
>      'log.cpp',
>      'memfd.cpp',
>      'message.cpp',
> --
> 2.44.1
> 
> 

Inspired by pipewire, I think the following two would be quite useful:
  1. `LIBCAMERA_CONFIG_DIR` environmental variable, which denotes a (list of) directory(s)
     that is searched first.
  2. `LIBCAMERA_CONFIG_NAME` environmental variable, which denotes
      a) the path of the configuration file to load if it is an absolute path; or
      b) the path of the configuration file to load, relative to the configuration
         directories

  (1) would make it easy to integrate with `meson devenv`
  (2) would make it easy to switch between configurations, which is not too easy
      to do if the filename is hard-coded


Regards,
Barnabás Pőcze