| Message ID | 20250616084733.18707-6-mzamazal@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, Thank you for the patch. On Mon, Jun 16, 2025 at 10:47:23AM +0200, Milan Zamazal wrote: > Let's introduce a configuration option for rpi configuration file path. > It may be arguable whether pipeline specific configurations belong to > the global configuration file. But: > > - Having a single configuration file is generally easier for the user. > - The original configuration via environment variables can be already > considered global. > - This option points to other configuration files and it makes little > sense to add another configuration file to the chain. I'm not sure to follow you here. With this patch we'll have two configuration files, a global one, and one for the rpi pipeline handler. Isn't that exactly "adding another configuration file to the chain" ? Have you considered including the pipeline-specific configuration to the global configuration file ? CC'ing Naush and David for feedback. > The configuration snippet: > > configuration: > pipeline: > rpi: > config_file: FILENAME > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > index d8c7ca935..850a5e31b 100644 > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > @@ -20,6 +20,7 @@ > #include <libcamera/property_ids.h> > > #include "libcamera/internal/camera_lens.h" > +#include "libcamera/internal/global_configuration.h" > #include "libcamera/internal/ipa_manager.h" > #include "libcamera/internal/v4l2_subdevice.h" > > @@ -1084,7 +1085,12 @@ int CameraData::loadPipelineConfiguration() > /* Initial configuration of the platform, in case no config file is present */ > platformPipelineConfigure({}); > > - char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE"); > + const GlobalConfiguration &config = pipe()->cameraManager()->_d()->configuration(); > + std::optional<std::string> configFile = > + config.envOption("LIBCAMERA_RPI_CONFIG_FILE", "pipelines.rpi.config_file"); > + if (!configFile.has_value()) > + return 0; > + char const *configFromEnv = configFile.value().c_str(); > if (!configFromEnv || *configFromEnv == '\0') > return 0; >
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index d8c7ca935..850a5e31b 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -20,6 +20,7 @@ #include <libcamera/property_ids.h> #include "libcamera/internal/camera_lens.h" +#include "libcamera/internal/global_configuration.h" #include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/v4l2_subdevice.h" @@ -1084,7 +1085,12 @@ int CameraData::loadPipelineConfiguration() /* Initial configuration of the platform, in case no config file is present */ platformPipelineConfigure({}); - char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE"); + const GlobalConfiguration &config = pipe()->cameraManager()->_d()->configuration(); + std::optional<std::string> configFile = + config.envOption("LIBCAMERA_RPI_CONFIG_FILE", "pipelines.rpi.config_file"); + if (!configFile.has_value()) + return 0; + char const *configFromEnv = configFile.value().c_str(); if (!configFromEnv || *configFromEnv == '\0') return 0;
Let's introduce a configuration option for rpi configuration file path. It may be arguable whether pipeline specific configurations belong to the global configuration file. But: - Having a single configuration file is generally easier for the user. - The original configuration via environment variables can be already considered global. - This option points to other configuration files and it makes little sense to add another configuration file to the chain. The configuration snippet: configuration: pipeline: rpi: config_file: FILENAME Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)