[v16,01/12] config: Introduce global runtime configuration

24012 diff mbox series

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 was a request to define 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.  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.
GlobalConfiguration instances are meant to be stored to and accessed
from instances of another class, preferably from a single 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

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@redhat.com>
---
 .../libcamera/internal/global_configuration.h |  34 +++++
 include/libcamera/internal/meson.build        |   1 +
 src/libcamera/global_configuration.cpp        | 144 ++++++++++++++++++
 src/libcamera/meson.build                     |   1 +
 4 files changed, 180 insertions(+)
 create mode 100644 include/libcamera/internal/global_configuration.h
 create mode 100644 src/libcamera/global_configuration.cpp

Hi

2025. 07. 29. 9:31 keltezéssel, Milan Zamazal í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 was a request to define 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.  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.
> GlobalConfiguration instances are meant to be stored to and accessed
> from instances of another class, preferably from a single 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.

Should `version == 1` be enforced maybe?


> 
> The configuration YAML file looks as follows:
> 
>    ---
>    version: 1
>    configuration:
>      WHATEVER CONFIGURATION NEEDED
> 
> 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@redhat.com>
> ---
>   .../libcamera/internal/global_configuration.h |  34 +++++
>   include/libcamera/internal/meson.build        |   1 +
>   src/libcamera/global_configuration.cpp        | 144 ++++++++++++++++++
>   src/libcamera/meson.build                     |   1 +
>   4 files changed, 180 insertions(+)
>   create mode 100644 include/libcamera/internal/global_configuration.h
>   create mode 100644 src/libcamera/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..f695498c4
> --- /dev/null
> +++ b/include/libcamera/internal/global_configuration.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024-2025 Red Hat, inc.
> + *
> + * Global configuration handling
> + */
> +
> +#pragma once
> +
> +#include <filesystem>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +namespace libcamera {
> +
> +class GlobalConfiguration
> +{
> +public:
> +	using Configuration = const YamlObject &;
> +
> +	GlobalConfiguration();
> +
> +	unsigned int version() const;
> +	Configuration configuration() const;
> +
> +private:
> +	bool loadFile(const std::filesystem::path &fileName);
> +	void load();
> +
> +	std::unique_ptr<YamlObject> yamlConfiguration_ =
> +		std::make_unique<YamlObject>();
> +};
> +
> +} /* namespace libcamera */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 5c80a28c4..45c299f6a 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -23,6 +23,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/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> new file mode 100644
> index 000000000..50756b859
> --- /dev/null
> +++ b/src/libcamera/global_configuration.cpp
> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024-2025 Red Hat, inc.
> + *
> + * Global configuration handling
> + */
> +
> +#include "libcamera/internal/global_configuration.h"
> +
> +#include <filesystem>
> +#include <string_view>
> +#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 {
> +
> +namespace {
> +const std::vector<std::filesystem::path> globalConfigurationFiles = {
> +	std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
> +	std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
> +};
> +}
> +
> +LOG_DEFINE_CATEGORY(Configuration)
> +
> +/**
> + * \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.
> + */
> +
> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
> +{
> +	File file(fileName);
> +	if (!file.open(File::OpenModeFlag::ReadOnly)) {
> +		if (file.error() == -ENOENT)
> +			return false;
> +
> +		LOG(Configuration, Error)
> +			<< "Failed to open configuration file " << fileName;
> +		return true;
> +	}
> +
> +	std::unique_ptr<YamlObject> configuration = YamlParser::parse(file);
> +	if (!configuration) {
> +		LOG(Configuration, Error)
> +			<< "Failed to parse configuration file " << fileName;
> +		return true;
> +	}
> +
> +	yamlConfiguration_ = std::move(configuration);
> +	return true;
> +}
> +
> +void GlobalConfiguration::load()
> +{
> +	std::filesystem::path userConfigurationDirectory;
> +	const 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 (const auto &path : globalConfigurationFiles) {
> +		if (loadFile(path))
> +			return;
> +	}
> +}
> +
> +/**
> + * \brief Initialize the global configuration
> + */
> +GlobalConfiguration::GlobalConfiguration()
> +{
> +	load();
> +}
> +
> +/**
> + * \typedef GlobalConfiguration::Configuration
> + * \brief Type representing global libcamera configuration
> + *
> + * All code outside GlobalConfiguration must use this type declaration and not
> + * the underlying type.
> + */
> +
> +/**
> + * \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() const
> +{
> +	return (*yamlConfiguration_)["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() const
> +{
> +	return (*yamlConfiguration_)["configuration"];
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index de1eb99b2..275cf5a4b 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -31,6 +31,7 @@ libcamera_internal_sources = files([
>       'device_enumerator_sysfs.cpp',
>       'dma_buf_allocator.cpp',
>       'formats.cpp',
> +    'global_configuration.cpp',
>       'ipa_controls.cpp',
>       'ipa_data_serializer.cpp',
>       'ipa_interface.cpp',

Hi Barnabás,

Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:

> Hi
>
> 2025. 07. 29. 9:31 keltezéssel, Milan Zamazal í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 was a request to define 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.  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.
>> GlobalConfiguration instances are meant to be stored to and accessed
>> from instances of another class, preferably from a single 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.
>
> Should `version == 1` be enforced maybe?

I think so, it shouldn't attempt to process other versions.  I'll add it
in v17.

>> The configuration YAML file looks as follows:
>>    ---
>>    version: 1
>>    configuration:
>>      WHATEVER CONFIGURATION NEEDED
>> 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@redhat.com>
>> ---
>>   .../libcamera/internal/global_configuration.h |  34 +++++
>>   include/libcamera/internal/meson.build        |   1 +
>>   src/libcamera/global_configuration.cpp        | 144 ++++++++++++++++++
>>   src/libcamera/meson.build                     |   1 +
>>   4 files changed, 180 insertions(+)
>>   create mode 100644 include/libcamera/internal/global_configuration.h
>>   create mode 100644 src/libcamera/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..f695498c4
>> --- /dev/null
>> +++ b/include/libcamera/internal/global_configuration.h
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024-2025 Red Hat, inc.
>> + *
>> + * Global configuration handling
>> + */
>> +
>> +#pragma once
>> +
>> +#include <filesystem>
>> +
>> +#include "libcamera/internal/yaml_parser.h"
>> +
>> +namespace libcamera {
>> +
>> +class GlobalConfiguration
>> +{
>> +public:
>> +	using Configuration = const YamlObject &;
>> +
>> +	GlobalConfiguration();
>> +
>> +	unsigned int version() const;
>> +	Configuration configuration() const;
>> +
>> +private:
>> +	bool loadFile(const std::filesystem::path &fileName);
>> +	void load();
>> +
>> +	std::unique_ptr<YamlObject> yamlConfiguration_ =
>> +		std::make_unique<YamlObject>();
>> +};
>> +
>> +} /* namespace libcamera */
>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
>> index 5c80a28c4..45c299f6a 100644
>> --- a/include/libcamera/internal/meson.build
>> +++ b/include/libcamera/internal/meson.build
>> @@ -23,6 +23,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/global_configuration.cpp b/src/libcamera/global_configuration.cpp
>> new file mode 100644
>> index 000000000..50756b859
>> --- /dev/null
>> +++ b/src/libcamera/global_configuration.cpp
>> @@ -0,0 +1,144 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024-2025 Red Hat, inc.
>> + *
>> + * Global configuration handling
>> + */
>> +
>> +#include "libcamera/internal/global_configuration.h"
>> +
>> +#include <filesystem>
>> +#include <string_view>
>> +#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 {
>> +
>> +namespace {
>> +const std::vector<std::filesystem::path> globalConfigurationFiles = {
>> +	std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
>> +	std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
>> +};
>> +}
>> +
>> +LOG_DEFINE_CATEGORY(Configuration)
>> +
>> +/**
>> + * \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.
>> + */
>> +
>> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>> +{
>> +	File file(fileName);
>> +	if (!file.open(File::OpenModeFlag::ReadOnly)) {
>> +		if (file.error() == -ENOENT)
>> +			return false;
>> +
>> +		LOG(Configuration, Error)
>> +			<< "Failed to open configuration file " << fileName;
>> +		return true;
>> +	}
>> +
>> +	std::unique_ptr<YamlObject> configuration = YamlParser::parse(file);
>> +	if (!configuration) {
>> +		LOG(Configuration, Error)
>> +			<< "Failed to parse configuration file " << fileName;
>> +		return true;
>> +	}
>> +
>> +	yamlConfiguration_ = std::move(configuration);
>> +	return true;
>> +}
>> +
>> +void GlobalConfiguration::load()
>> +{
>> +	std::filesystem::path userConfigurationDirectory;
>> +	const 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 (const auto &path : globalConfigurationFiles) {
>> +		if (loadFile(path))
>> +			return;
>> +	}
>> +}
>> +
>> +/**
>> + * \brief Initialize the global configuration
>> + */
>> +GlobalConfiguration::GlobalConfiguration()
>> +{
>> +	load();
>> +}
>> +
>> +/**
>> + * \typedef GlobalConfiguration::Configuration
>> + * \brief Type representing global libcamera configuration
>> + *
>> + * All code outside GlobalConfiguration must use this type declaration and not
>> + * the underlying type.
>> + */
>> +
>> +/**
>> + * \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() const
>> +{
>> +	return (*yamlConfiguration_)["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() const
>> +{
>> +	return (*yamlConfiguration_)["configuration"];
>> +}
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index de1eb99b2..275cf5a4b 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -31,6 +31,7 @@ libcamera_internal_sources = files([
>>       'device_enumerator_sysfs.cpp',
>>       'dma_buf_allocator.cpp',
>>       'formats.cpp',
>> +    'global_configuration.cpp',
>>       'ipa_controls.cpp',
>>       'ipa_data_serializer.cpp',
>>       'ipa_interface.cpp',

Hi Milan,

Thanks for the patch.

Quoting Milan Zamazal (2025-07-29 16:31:49)
> 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 was a request to define 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.  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.
> GlobalConfiguration instances are meant to be stored to and accessed
> from instances of another class, preferably from a single instance.

afaiu the intention is s/another class/other classes/ ? Although that doesn't
change my confusion...

This wording here is a bit confusing to me. I understand that the
GlobalConfiguration class reads the configuration files and saves its
information, but why would the GlobalConfiguration class itself access the
configuration? I'd have expected other objects to use the GlobalConfiguration
class (or instance?) as an interface for accessing the configuration.

The second sentence implies that other classes have the ability to modify the
configuration. Is this intended? Also are there going to be multiple instances
of GlobalConfiguration, or one global one? I'd have expected a global one given
the name.

> 
> 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
> 
> 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@redhat.com>
> ---
>  .../libcamera/internal/global_configuration.h |  34 +++++
>  include/libcamera/internal/meson.build        |   1 +
>  src/libcamera/global_configuration.cpp        | 144 ++++++++++++++++++
>  src/libcamera/meson.build                     |   1 +
>  4 files changed, 180 insertions(+)
>  create mode 100644 include/libcamera/internal/global_configuration.h
>  create mode 100644 src/libcamera/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..f695498c4
> --- /dev/null
> +++ b/include/libcamera/internal/global_configuration.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024-2025 Red Hat, inc.
> + *
> + * Global configuration handling
> + */
> +
> +#pragma once
> +
> +#include <filesystem>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +namespace libcamera {
> +
> +class GlobalConfiguration
> +{
> +public:
> +       using Configuration = const YamlObject &;
> +
> +       GlobalConfiguration();
> +
> +       unsigned int version() const;
> +       Configuration configuration() const;

Should this return const? I don't see why other components of libcamera should
be able to modify the configuration. Unless we expect some components to
overwrite invalid/outdated configuration options for version mismatches. Is
this what is expected?

> +
> +private:
> +       bool loadFile(const std::filesystem::path &fileName);
> +       void load();
> +
> +       std::unique_ptr<YamlObject> yamlConfiguration_ =
> +               std::make_unique<YamlObject>();
> +};
> +
> +} /* namespace libcamera */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 5c80a28c4..45c299f6a 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -23,6 +23,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/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> new file mode 100644
> index 000000000..50756b859
> --- /dev/null
> +++ b/src/libcamera/global_configuration.cpp
> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024-2025 Red Hat, inc.
> + *
> + * Global configuration handling
> + */
> +
> +#include "libcamera/internal/global_configuration.h"
> +
> +#include <filesystem>
> +#include <string_view>
> +#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 {
> +
> +namespace {
> +const std::vector<std::filesystem::path> globalConfigurationFiles = {
> +       std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
> +       std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
> +};
> +}

Is there a reason why these are in a separate namespace?

> +
> +LOG_DEFINE_CATEGORY(Configuration)
> +
> +/**
> + * \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.
> + */
> +
> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
> +{
> +       File file(fileName);
> +       if (!file.open(File::OpenModeFlag::ReadOnly)) {
> +               if (file.error() == -ENOENT)
> +                       return false;
> +
> +               LOG(Configuration, Error)
> +                       << "Failed to open configuration file " << fileName;
> +               return true;

Returning true here stops load() below from trying the other files. Is this
what we want? Although maybe it is what we want, so that an error in our $HOME
config doesn't unexpectedly cause the system-wide one to be used instead.

...on the other hand this does prevent us from loading the other system-wide
config if the first system-wide one has a problem with opening/parsing.

Perhaps the ENOENT check should be split out into load()? There is
File::exists().

> +       }
> +
> +       std::unique_ptr<YamlObject> configuration = YamlParser::parse(file);
> +       if (!configuration) {
> +               LOG(Configuration, Error)
> +                       << "Failed to parse configuration file " << fileName;
> +               return true;

Same here.

> +       }
> +
> +       yamlConfiguration_ = std::move(configuration);
> +       return true;
> +}
> +
> +void GlobalConfiguration::load()
> +{
> +       std::filesystem::path userConfigurationDirectory;
> +       const 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 (const auto &path : globalConfigurationFiles) {
> +               if (loadFile(path))
> +                       return;
> +       }
> +}
> +
> +/**
> + * \brief Initialize the global configuration
> + */
> +GlobalConfiguration::GlobalConfiguration()
> +{
> +       load();
> +}
> +
> +/**
> + * \typedef GlobalConfiguration::Configuration
> + * \brief Type representing global libcamera configuration
> + *
> + * All code outside GlobalConfiguration must use this type declaration and not
> + * the underlying type.
> + */
> +
> +/**
> + * \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() const
> +{
> +       return (*yamlConfiguration_)["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.

Ah, that's why we need to use GlobalConfiguration::Configuration instead of
YamlObject.


Thanks,

Paul

> + *
> + * \return The whole configuration section
> + */
> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const
> +{
> +       return (*yamlConfiguration_)["configuration"];
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index de1eb99b2..275cf5a4b 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -31,6 +31,7 @@ libcamera_internal_sources = files([
>      'device_enumerator_sysfs.cpp',
>      'dma_buf_allocator.cpp',
>      'formats.cpp',
> +    'global_configuration.cpp',
>      'ipa_controls.cpp',
>      'ipa_data_serializer.cpp',
>      'ipa_interface.cpp',
> -- 
> 2.50.1
>

Hi

2025. 09. 08. 12:24 keltezéssel, Paul Elder írta:
> Hi Milan,
> 
> Thanks for the patch.
> 
> Quoting Milan Zamazal (2025-07-29 16:31:49)
>> 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 was a request to define 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.  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.
>> GlobalConfiguration instances are meant to be stored to and accessed
>> from instances of another class, preferably from a single instance.
> 
> afaiu the intention is s/another class/other classes/ ? Although that doesn't
> change my confusion...
> 
> This wording here is a bit confusing to me. I understand that the
> GlobalConfiguration class reads the configuration files and saves its
> information, but why would the GlobalConfiguration class itself access the
> configuration? I'd have expected other objects to use the GlobalConfiguration
> class (or instance?) as an interface for accessing the configuration.
> 
> The second sentence implies that other classes have the ability to modify the
> configuration. Is this intended? Also are there going to be multiple instances
> of GlobalConfiguration, or one global one? I'd have expected a global one given
> the name.

Initially there was one, but it was decided to move it into the `CameraManager`
to avoid more singletons.


> 
>>
>> 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
>>
>> 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@redhat.com>
>> ---
>>   .../libcamera/internal/global_configuration.h |  34 +++++
>>   include/libcamera/internal/meson.build        |   1 +
>>   src/libcamera/global_configuration.cpp        | 144 ++++++++++++++++++
>>   src/libcamera/meson.build                     |   1 +
>>   4 files changed, 180 insertions(+)
>>   create mode 100644 include/libcamera/internal/global_configuration.h
>>   create mode 100644 src/libcamera/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..f695498c4
>> --- /dev/null
>> +++ b/include/libcamera/internal/global_configuration.h
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024-2025 Red Hat, inc.
>> + *
>> + * Global configuration handling
>> + */
>> +
>> +#pragma once
>> +
>> +#include <filesystem>
>> +
>> +#include "libcamera/internal/yaml_parser.h"
>> +
>> +namespace libcamera {
>> +
>> +class GlobalConfiguration
>> +{
>> +public:
>> +       using Configuration = const YamlObject &;
>> +
>> +       GlobalConfiguration();
>> +
>> +       unsigned int version() const;
>> +       Configuration configuration() const;
> 
> Should this return const? I don't see why other components of libcamera should
> be able to modify the configuration. Unless we expect some components to
> overwrite invalid/outdated configuration options for version mismatches. Is
> this what is expected?

Configuration is an alias for `const YamlObject&`, so not intended to be modified.


> 
>> +
>> +private:
>> +       bool loadFile(const std::filesystem::path &fileName);
>> +       void load();
>> +
>> +       std::unique_ptr<YamlObject> yamlConfiguration_ =
>> +               std::make_unique<YamlObject>();
>> +};
>> +
>> +} /* namespace libcamera */
>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
>> index 5c80a28c4..45c299f6a 100644
>> --- a/include/libcamera/internal/meson.build
>> +++ b/include/libcamera/internal/meson.build
>> @@ -23,6 +23,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/global_configuration.cpp b/src/libcamera/global_configuration.cpp
>> new file mode 100644
>> index 000000000..50756b859
>> --- /dev/null
>> +++ b/src/libcamera/global_configuration.cpp
>> @@ -0,0 +1,144 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024-2025 Red Hat, inc.
>> + *
>> + * Global configuration handling
>> + */
>> +
>> +#include "libcamera/internal/global_configuration.h"
>> +
>> +#include <filesystem>
>> +#include <string_view>
>> +#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 {
>> +
>> +namespace {
>> +const std::vector<std::filesystem::path> globalConfigurationFiles = {
>> +       std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
>> +       std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
>> +};
>> +}
> 
> Is there a reason why these are in a separate namespace?

To give it internal linkage. In any case this variable is removed in one
of the later patches, I believe.


> 
>> +
>> +LOG_DEFINE_CATEGORY(Configuration)
>> +
>> +/**
>> + * \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.
>> + */
>> +
>> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>> +{
>> +       File file(fileName);
>> +       if (!file.open(File::OpenModeFlag::ReadOnly)) {
>> +               if (file.error() == -ENOENT)
>> +                       return false;
>> +
>> +               LOG(Configuration, Error)
>> +                       << "Failed to open configuration file " << fileName;
>> +               return true;
> 
> Returning true here stops load() below from trying the other files. Is this
> what we want? Although maybe it is what we want, so that an error in our $HOME
> config doesn't unexpectedly cause the system-wide one to be used instead.

That is the intention. I think any error (including yaml parsing errors) other than ENOENT
should trigger something very visible. Falling back to another configuration file
(which has a smaller priority) is probably not desirable.


> 
> ...on the other hand this does prevent us from loading the other system-wide
> config if the first system-wide one has a problem with opening/parsing.
> 
> Perhaps the ENOENT check should be split out into load()? There is
> File::exists().

This is done like this to avoid TOCTOU issues. IMO `File::exists()` should be
removed ideally.


Regards,
Barnabás Pőcze


> 
>> +       }
>> +
>> +       std::unique_ptr<YamlObject> configuration = YamlParser::parse(file);
>> +       if (!configuration) {
>> +               LOG(Configuration, Error)
>> +                       << "Failed to parse configuration file " << fileName;
>> +               return true;
> 
> Same here.
> 
>> +       }
>> +
>> +       yamlConfiguration_ = std::move(configuration);
>> +       return true;
>> +}
>> +
>> +void GlobalConfiguration::load()
>> +{
>> +       std::filesystem::path userConfigurationDirectory;
>> +       const 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 (const auto &path : globalConfigurationFiles) {
>> +               if (loadFile(path))
>> +                       return;
>> +       }
>> +}
>> +
>> +/**
>> + * \brief Initialize the global configuration
>> + */
>> +GlobalConfiguration::GlobalConfiguration()
>> +{
>> +       load();
>> +}
>> +
>> +/**
>> + * \typedef GlobalConfiguration::Configuration
>> + * \brief Type representing global libcamera configuration
>> + *
>> + * All code outside GlobalConfiguration must use this type declaration and not
>> + * the underlying type.
>> + */
>> +
>> +/**
>> + * \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() const
>> +{
>> +       return (*yamlConfiguration_)["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.
> 
> Ah, that's why we need to use GlobalConfiguration::Configuration instead of
> YamlObject.
> 
> 
> Thanks,
> 
> Paul
> 
>> + *
>> + * \return The whole configuration section
>> + */
>> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const
>> +{
>> +       return (*yamlConfiguration_)["configuration"];
>> +}
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index de1eb99b2..275cf5a4b 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -31,6 +31,7 @@ libcamera_internal_sources = files([
>>       'device_enumerator_sysfs.cpp',
>>       'dma_buf_allocator.cpp',
>>       'formats.cpp',
>> +    'global_configuration.cpp',
>>       'ipa_controls.cpp',
>>       'ipa_data_serializer.cpp',
>>       'ipa_interface.cpp',
>> -- 
>> 2.50.1
>>

Hi Paul,

thank you for review.

Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:

> Hi
>
> 2025. 09. 08. 12:24 keltezéssel, Paul Elder írta:
>> Hi Milan,
>> Thanks for the patch.
>> Quoting Milan Zamazal (2025-07-29 16:31:49)
>>> 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 was a request to define 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.  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.
>>> GlobalConfiguration instances are meant to be stored to and accessed
>>> from instances of another class, preferably from a single instance.
>> afaiu the intention is s/another class/other classes/ ? Although that doesn't
>> change my confusion...

The intention is to not create ad hoc GlobalConfiguration instances but
just one or few of them in appropriate places.

>> This wording here is a bit confusing to me. I understand that the
>> GlobalConfiguration class reads the configuration files and saves its
>> information, but why would the GlobalConfiguration class itself access the
>> configuration? I'd have expected other objects to use the GlobalConfiguration
>> class (or instance?)

Instance.

>> as an interface for accessing the configuration.

Yes.  I'll try to improve the wording.

The rest has already been answered by Barnabás.  I'll add some source
code comments to avoid the confusions.

>> The second sentence implies that other classes have the ability to modify the
>> configuration. Is this intended? Also are there going to be multiple instances
>> of GlobalConfiguration, or one global one? I'd have expected a global one given
>> the name.
>
> Initially there was one, but it was decided to move it into the `CameraManager`
> to avoid more singletons.
>
>
>> 
>>>
>>> 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
>>>
>>> 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@redhat.com>
>>> ---
>>>   .../libcamera/internal/global_configuration.h |  34 +++++
>>>   include/libcamera/internal/meson.build        |   1 +
>>>   src/libcamera/global_configuration.cpp        | 144 ++++++++++++++++++
>>>   src/libcamera/meson.build                     |   1 +
>>>   4 files changed, 180 insertions(+)
>>>   create mode 100644 include/libcamera/internal/global_configuration.h
>>>   create mode 100644 src/libcamera/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..f695498c4
>>> --- /dev/null
>>> +++ b/include/libcamera/internal/global_configuration.h
>>> @@ -0,0 +1,34 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2024-2025 Red Hat, inc.
>>> + *
>>> + * Global configuration handling
>>> + */
>>> +
>>> +#pragma once
>>> +
>>> +#include <filesystem>
>>> +
>>> +#include "libcamera/internal/yaml_parser.h"
>>> +
>>> +namespace libcamera {
>>> +
>>> +class GlobalConfiguration
>>> +{
>>> +public:
>>> +       using Configuration = const YamlObject &;
>>> +
>>> +       GlobalConfiguration();
>>> +
>>> +       unsigned int version() const;
>>> +       Configuration configuration() const;
>> Should this return const? I don't see why other components of libcamera should
>> be able to modify the configuration. Unless we expect some components to
>> overwrite invalid/outdated configuration options for version mismatches. Is
>> this what is expected?
>
> Configuration is an alias for `const YamlObject&`, so not intended to be modified.
>
>
>> 
>>> +
>>> +private:
>>> +       bool loadFile(const std::filesystem::path &fileName);
>>> +       void load();
>>> +
>>> +       std::unique_ptr<YamlObject> yamlConfiguration_ =
>>> +               std::make_unique<YamlObject>();
>>> +};
>>> +
>>> +} /* namespace libcamera */
>>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
>>> index 5c80a28c4..45c299f6a 100644
>>> --- a/include/libcamera/internal/meson.build
>>> +++ b/include/libcamera/internal/meson.build
>>> @@ -23,6 +23,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/global_configuration.cpp b/src/libcamera/global_configuration.cpp
>>> new file mode 100644
>>> index 000000000..50756b859
>>> --- /dev/null
>>> +++ b/src/libcamera/global_configuration.cpp
>>> @@ -0,0 +1,144 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2024-2025 Red Hat, inc.
>>> + *
>>> + * Global configuration handling
>>> + */
>>> +
>>> +#include "libcamera/internal/global_configuration.h"
>>> +
>>> +#include <filesystem>
>>> +#include <string_view>
>>> +#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 {
>>> +
>>> +namespace {
>>> +const std::vector<std::filesystem::path> globalConfigurationFiles = {
>>> +       std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
>>> +       std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
>>> +};
>>> +}
>> Is there a reason why these are in a separate namespace?
>
> To give it internal linkage. In any case this variable is removed in one
> of the later patches, I believe.
>
>
>> 
>>> +
>>> +LOG_DEFINE_CATEGORY(Configuration)
>>> +
>>> +/**
>>> + * \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.
>>> + */
>>> +
>>> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>>> +{
>>> +       File file(fileName);
>>> +       if (!file.open(File::OpenModeFlag::ReadOnly)) {
>>> +               if (file.error() == -ENOENT)
>>> +                       return false;
>>> +
>>> +               LOG(Configuration, Error)
>>> +                       << "Failed to open configuration file " << fileName;
>>> +               return true;
>> Returning true here stops load() below from trying the other files. Is this
>> what we want? Although maybe it is what we want, so that an error in our $HOME
>> config doesn't unexpectedly cause the system-wide one to be used instead.
>
> That is the intention. I think any error (including yaml parsing errors) other than ENOENT
> should trigger something very visible. Falling back to another configuration file
> (which has a smaller priority) is probably not desirable.
>
>
>> ...on the other hand this does prevent us from loading the other system-wide
>> config if the first system-wide one has a problem with opening/parsing.
>> Perhaps the ENOENT check should be split out into load()? There is
>> File::exists().
>
> This is done like this to avoid TOCTOU issues. IMO `File::exists()` should be
> removed ideally.
>
>
> Regards,
> Barnabás Pőcze
>
>
>> 
>>> +       }
>>> +
>>> +       std::unique_ptr<YamlObject> configuration = YamlParser::parse(file);
>>> +       if (!configuration) {
>>> +               LOG(Configuration, Error)
>>> +                       << "Failed to parse configuration file " << fileName;
>>> +               return true;
>> Same here.
>> 
>>> +       }
>>> +
>>> +       yamlConfiguration_ = std::move(configuration);
>>> +       return true;
>>> +}
>>> +
>>> +void GlobalConfiguration::load()
>>> +{
>>> +       std::filesystem::path userConfigurationDirectory;
>>> +       const 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 (const auto &path : globalConfigurationFiles) {
>>> +               if (loadFile(path))
>>> +                       return;
>>> +       }
>>> +}
>>> +
>>> +/**
>>> + * \brief Initialize the global configuration
>>> + */
>>> +GlobalConfiguration::GlobalConfiguration()
>>> +{
>>> +       load();
>>> +}
>>> +
>>> +/**
>>> + * \typedef GlobalConfiguration::Configuration
>>> + * \brief Type representing global libcamera configuration
>>> + *
>>> + * All code outside GlobalConfiguration must use this type declaration and not
>>> + * the underlying type.
>>> + */
>>> +
>>> +/**
>>> + * \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() const
>>> +{
>>> +       return (*yamlConfiguration_)["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.
>> Ah, that's why we need to use GlobalConfiguration::Configuration instead of
>> YamlObject.
>> Thanks,
>> Paul
>> 
>>> + *
>>> + * \return The whole configuration section
>>> + */
>>> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const
>>> +{
>>> +       return (*yamlConfiguration_)["configuration"];
>>> +}
>>> +
>>> +} /* namespace libcamera */
>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>>> index de1eb99b2..275cf5a4b 100644
>>> --- a/src/libcamera/meson.build
>>> +++ b/src/libcamera/meson.build
>>> @@ -31,6 +31,7 @@ libcamera_internal_sources = files([
>>>       'device_enumerator_sysfs.cpp',
>>>       'dma_buf_allocator.cpp',
>>>       'formats.cpp',
>>> +    'global_configuration.cpp',
>>>       'ipa_controls.cpp',
>>>       'ipa_data_serializer.cpp',
>>>       'ipa_interface.cpp',
>>> -- 2.50.1
>>>

Quoting Milan Zamazal (2025-09-08 22:42:32)
> Hi Paul,
> 
> thank you for review.
> 
> Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:
> 
> > Hi
> >
> > 2025. 09. 08. 12:24 keltezéssel, Paul Elder írta:
> >> Hi Milan,
> >> Thanks for the patch.
> >> Quoting Milan Zamazal (2025-07-29 16:31:49)
> >>> 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 was a request to define 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.  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.
> >>> GlobalConfiguration instances are meant to be stored to and accessed
> >>> from instances of another class, preferably from a single instance.
> >> afaiu the intention is s/another class/other classes/ ? Although that doesn't
> >> change my confusion...
> 
> The intention is to not create ad hoc GlobalConfiguration instances but
> just one or few of them in appropriate places.

Ok. As I continued review I could see more... so we have one in the
CameraManager and it gets passed to whoever needs it.

> 
> >> This wording here is a bit confusing to me. I understand that the
> >> GlobalConfiguration class reads the configuration files and saves its
> >> information, but why would the GlobalConfiguration class itself access the
> >> configuration? I'd have expected other objects to use the GlobalConfiguration
> >> class (or instance?)
> 
> Instance.
> 
> >> as an interface for accessing the configuration.
> 
> Yes.  I'll try to improve the wording.

Ok after review I understand a bit more.

I think, starting from "GlobalConfiguration instances" above, maybe something
like "Only one instance of GlobalConfiguration is meant to be created and owned
by the CameraManager, and this instance is passed to whatever other classes
that need it."

> 
> The rest has already been answered by Barnabás.  I'll add some source
> code comments to avoid the confusions.
> 
> >> The second sentence implies that other classes have the ability to modify the

This is cleared up too.

> >> configuration. Is this intended? Also are there going to be multiple instances
> >> of GlobalConfiguration, or one global one? I'd have expected a global one given
> >> the name.
> >
> > Initially there was one, but it was decided to move it into the `CameraManager`
> > to avoid more singletons.

Ok I see.

> >
> >
> >> 
> >>>
> >>> 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
> >>>
> >>> 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@redhat.com>
> >>> ---
> >>>   .../libcamera/internal/global_configuration.h |  34 +++++
> >>>   include/libcamera/internal/meson.build        |   1 +
> >>>   src/libcamera/global_configuration.cpp        | 144 ++++++++++++++++++
> >>>   src/libcamera/meson.build                     |   1 +
> >>>   4 files changed, 180 insertions(+)
> >>>   create mode 100644 include/libcamera/internal/global_configuration.h
> >>>   create mode 100644 src/libcamera/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..f695498c4
> >>> --- /dev/null
> >>> +++ b/include/libcamera/internal/global_configuration.h
> >>> @@ -0,0 +1,34 @@
> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >>> +/*
> >>> + * Copyright (C) 2024-2025 Red Hat, inc.
> >>> + *
> >>> + * Global configuration handling
> >>> + */
> >>> +
> >>> +#pragma once
> >>> +
> >>> +#include <filesystem>
> >>> +
> >>> +#include "libcamera/internal/yaml_parser.h"
> >>> +
> >>> +namespace libcamera {
> >>> +
> >>> +class GlobalConfiguration
> >>> +{
> >>> +public:
> >>> +       using Configuration = const YamlObject &;
> >>> +
> >>> +       GlobalConfiguration();
> >>> +
> >>> +       unsigned int version() const;
> >>> +       Configuration configuration() const;
> >> Should this return const? I don't see why other components of libcamera should
> >> be able to modify the configuration. Unless we expect some components to
> >> overwrite invalid/outdated configuration options for version mismatches. Is
> >> this what is expected?
> >
> > Configuration is an alias for `const YamlObject&`, so not intended to be modified.

Right, I missed that.

> >
> >
> >> 
> >>> +
> >>> +private:
> >>> +       bool loadFile(const std::filesystem::path &fileName);
> >>> +       void load();
> >>> +
> >>> +       std::unique_ptr<YamlObject> yamlConfiguration_ =
> >>> +               std::make_unique<YamlObject>();
> >>> +};
> >>> +
> >>> +} /* namespace libcamera */
> >>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> >>> index 5c80a28c4..45c299f6a 100644
> >>> --- a/include/libcamera/internal/meson.build
> >>> +++ b/include/libcamera/internal/meson.build
> >>> @@ -23,6 +23,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/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> >>> new file mode 100644
> >>> index 000000000..50756b859
> >>> --- /dev/null
> >>> +++ b/src/libcamera/global_configuration.cpp
> >>> @@ -0,0 +1,144 @@
> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >>> +/*
> >>> + * Copyright (C) 2024-2025 Red Hat, inc.
> >>> + *
> >>> + * Global configuration handling
> >>> + */
> >>> +
> >>> +#include "libcamera/internal/global_configuration.h"
> >>> +
> >>> +#include <filesystem>
> >>> +#include <string_view>
> >>> +#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 {
> >>> +
> >>> +namespace {
> >>> +const std::vector<std::filesystem::path> globalConfigurationFiles = {
> >>> +       std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
> >>> +       std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
> >>> +};
> >>> +}
> >> Is there a reason why these are in a separate namespace?
> >
> > To give it internal linkage. In any case this variable is removed in one
> > of the later patches, I believe.

Ah ok I see. It is indeed removed later.

> >
> >
> >> 
> >>> +
> >>> +LOG_DEFINE_CATEGORY(Configuration)
> >>> +
> >>> +/**
> >>> + * \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.
> >>> + */
> >>> +
> >>> +bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
> >>> +{
> >>> +       File file(fileName);
> >>> +       if (!file.open(File::OpenModeFlag::ReadOnly)) {
> >>> +               if (file.error() == -ENOENT)
> >>> +                       return false;
> >>> +
> >>> +               LOG(Configuration, Error)
> >>> +                       << "Failed to open configuration file " << fileName;
> >>> +               return true;
> >> Returning true here stops load() below from trying the other files. Is this
> >> what we want? Although maybe it is what we want, so that an error in our $HOME
> >> config doesn't unexpectedly cause the system-wide one to be used instead.
> >
> > That is the intention. I think any error (including yaml parsing errors) other than ENOENT
> > should trigger something very visible. Falling back to another configuration file
> > (which has a smaller priority) is probably not desirable.

Ok.

> >
> >
> >> ...on the other hand this does prevent us from loading the other system-wide
> >> config if the first system-wide one has a problem with opening/parsing.
> >> Perhaps the ENOENT check should be split out into load()? There is
> >> File::exists().
> >
> > This is done like this to avoid TOCTOU issues. IMO `File::exists()` should be
> > removed ideally.

Good point.


Thanks,

Paul

> >
> >
> > Regards,
> > Barnabás Pőcze
> >
> >
> >> 
> >>> +       }
> >>> +
> >>> +       std::unique_ptr<YamlObject> configuration = YamlParser::parse(file);
> >>> +       if (!configuration) {
> >>> +               LOG(Configuration, Error)
> >>> +                       << "Failed to parse configuration file " << fileName;
> >>> +               return true;
> >> Same here.
> >> 
> >>> +       }
> >>> +
> >>> +       yamlConfiguration_ = std::move(configuration);
> >>> +       return true;
> >>> +}
> >>> +
> >>> +void GlobalConfiguration::load()
> >>> +{
> >>> +       std::filesystem::path userConfigurationDirectory;
> >>> +       const 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 (const auto &path : globalConfigurationFiles) {
> >>> +               if (loadFile(path))
> >>> +                       return;
> >>> +       }
> >>> +}
> >>> +
> >>> +/**
> >>> + * \brief Initialize the global configuration
> >>> + */
> >>> +GlobalConfiguration::GlobalConfiguration()
> >>> +{
> >>> +       load();
> >>> +}
> >>> +
> >>> +/**
> >>> + * \typedef GlobalConfiguration::Configuration
> >>> + * \brief Type representing global libcamera configuration
> >>> + *
> >>> + * All code outside GlobalConfiguration must use this type declaration and not
> >>> + * the underlying type.
> >>> + */
> >>> +
> >>> +/**
> >>> + * \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() const
> >>> +{
> >>> +       return (*yamlConfiguration_)["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.
> >> Ah, that's why we need to use GlobalConfiguration::Configuration instead of
> >> YamlObject.
> >> Thanks,
> >> Paul
> >> 
> >>> + *
> >>> + * \return The whole configuration section
> >>> + */
> >>> +GlobalConfiguration::Configuration GlobalConfiguration::configuration() const
> >>> +{
> >>> +       return (*yamlConfiguration_)["configuration"];
> >>> +}
> >>> +
> >>> +} /* namespace libcamera */
> >>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> >>> index de1eb99b2..275cf5a4b 100644
> >>> --- a/src/libcamera/meson.build
> >>> +++ b/src/libcamera/meson.build
> >>> @@ -31,6 +31,7 @@ libcamera_internal_sources = files([
> >>>       'device_enumerator_sysfs.cpp',
> >>>       'dma_buf_allocator.cpp',
> >>>       'formats.cpp',
> >>> +    'global_configuration.cpp',
> >>>       'ipa_controls.cpp',
> >>>       'ipa_data_serializer.cpp',
> >>>       'ipa_interface.cpp',
> >>> -- 2.50.1
> >>>
>