[v13,04/12] config: Look up rpi configuration in the configuration file
diff mbox series

Message ID 20250711201232.129264-5-mzamazal@redhat.com
State New
Headers show
Series
  • Add global configuration file
Related show

Commit Message

Milan Zamazal July 11, 2025, 8:12 p.m. UTC
Let's make rpi configuration available in the global configuration file.
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 rpi configuration is currently placed in a separate file, specified
by LIBCAMERA_RPI_CONFIG_FILE environment variable.  Let's support the
content of the file in the global configuration file.  `version' field
is omitted as the global configuration file has its own version and the
required version in the original file has been 1.0 anyway.

The configuration snippet:

  configuration:
    pipeline:
      rpi:
        bcm2835:
          pipeline_handler:
            ...
        pisp:
          pipeline_handler:
            ...

To not break current user setups, LIBCAMERA_RPI_CONFIG_FILE environment
variable is still supported and works as before, pointing to a separate
configuration file.  Just the duplicate check for the configuration file
version in platformPipelineConfigure methods is removed.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp     | 62 ++++++++++++-------
 .../pipeline/rpi/common/pipeline_base.h       |  3 +-
 src/libcamera/pipeline/rpi/pisp/pisp.cpp      | 26 +++-----
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        | 26 +++-----
 4 files changed, 59 insertions(+), 58 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index eafe94427..0eea242de 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -20,8 +20,10 @@ 
 #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"
+#include "libcamera/internal/yaml_parser.h"
 
 using namespace std::chrono_literals;
 
@@ -1091,35 +1093,46 @@  int CameraData::loadPipelineConfiguration()
 	/* Initial configuration of the platform, in case no config file is present */
 	platformPipelineConfigure({});
 
+	std::unique_ptr<YamlObject> root;
 	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
-	if (!configFromEnv || *configFromEnv == '\0')
-		return 0;
-
-	std::string filename = std::string(configFromEnv);
-	File file(filename);
+	if (configFromEnv && *configFromEnv != '\0') {
+		std::string filename = std::string(configFromEnv);
+		File file(filename);
+
+		if (!file.open(File::OpenModeFlag::ReadOnly)) {
+			LOG(RPI, Warning) << "Failed to open configuration file '" << filename << "'"
+					  << ", using defaults";
+			return 0;
+		}
 
-	if (!file.open(File::OpenModeFlag::ReadOnly)) {
-		LOG(RPI, Warning) << "Failed to open configuration file '" << filename << "'"
-				  << ", using defaults";
-		return 0;
-	}
+		LOG(RPI, Info) << "Using configuration file '" << filename << "'";
 
-	LOG(RPI, Info) << "Using configuration file '" << filename << "'";
+		root = YamlParser::parse(file);
+		if (!root) {
+			LOG(RPI, Warning) << "Failed to parse configuration file, using defaults";
+			return 0;
+		}
 
-	std::unique_ptr<YamlObject> root = YamlParser::parse(file);
-	if (!root) {
-		LOG(RPI, Warning) << "Failed to parse configuration file, using defaults";
-		return 0;
-	}
+		std::optional<double> ver = (*root)["version"].get<double>();
+		if (!ver || *ver != 1.0) {
+			LOG(RPI, Warning) << "Unexpected configuration file version reported: "
+					  << *ver;
+			return 0;
+		}
 
-	std::optional<double> ver = (*root)["version"].get<double>();
-	if (!ver || *ver != 1.0) {
-		LOG(RPI, Warning) << "Unexpected configuration file version reported: "
-				  << *ver;
-		return 0;
+		std::optional<std::string> t = (*root)["target"].get<std::string>();
+		if (t != target()) {
+			LOG(RPI, Error) << "Unexpected target reported: expected \""
+					<< target() << "\", got " << (t ? t->c_str() : "(unknown)");
+			return -EINVAL;
+		}
 	}
 
-	const YamlObject &phConfig = (*root)["pipeline_handler"];
+	GlobalConfiguration::Configuration config =
+		pipe()->cameraManager()->_d()->configuration().configuration()["pipeline"]["rpi"];
+	const YamlObject &phConfig = (root
+					      ? (*root)["pipeline_handler"]
+					      : config[target()]["pipeline_handler"]);
 
 	if (phConfig.contains("disable_startup_frame_drops"))
 		LOG(RPI, Warning)
@@ -1135,7 +1148,10 @@  int CameraData::loadPipelineConfiguration()
 		frontendDevice()->setDequeueTimeout(config_.cameraTimeoutValue * 1ms);
 	}
 
-	return platformPipelineConfigure(root);
+	std::optional<std::string> expectedTarget = (root
+							     ? (*root)["target"].get<std::string>()
+							     : target());
+	return platformPipelineConfigure(phConfig);
 }
 
 int CameraData::loadIPA(ipa::RPi::InitResult *result)
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
index 4bce4ec4f..ffbede798 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
@@ -95,8 +95,9 @@  public:
 	virtual V4L2VideoDevice::Formats ispFormats() const = 0;
 	virtual V4L2VideoDevice::Formats rawFormats() const = 0;
 	virtual V4L2VideoDevice *frontendDevice() = 0;
+	virtual const std::string &target() const = 0;
 
-	virtual int platformPipelineConfigure(const std::unique_ptr<YamlObject> &root) = 0;
+	virtual int platformPipelineConfigure(const YamlObject &phConfig) = 0;
 
 	std::unique_ptr<ipa::RPi::IPAProxyRPi> ipa_;
 
diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
index 92b9070c1..f66dbd9c1 100644
--- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
+++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
@@ -740,10 +740,16 @@  public:
 		return cfe_[Cfe::Output0].dev();
 	}
 
+	const std::string &target() const override
+	{
+		static const std::string target = "pisp";
+		return target;
+	}
+
 	CameraConfiguration::Status
 	platformValidate(RPi::RPiCameraConfiguration *rpiConfig) const override;
 
-	int platformPipelineConfigure(const std::unique_ptr<YamlObject> &root) override;
+	int platformPipelineConfigure(const YamlObject &phConfig) override;
 
 	void platformStart() override;
 	void platformStop() override;
@@ -1331,7 +1337,7 @@  PiSPCameraData::platformValidate(RPi::RPiCameraConfiguration *rpiConfig) const
 	return status;
 }
 
-int PiSPCameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject> &root)
+int PiSPCameraData::platformPipelineConfigure(const YamlObject &phConfig)
 {
 	config_ = {
 		.numCfeConfigStatsBuffers = 12,
@@ -1340,23 +1346,9 @@  int PiSPCameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject>
 		.disableHdr = false,
 	};
 
-	if (!root)
+	if (!phConfig)
 		return 0;
 
-	std::optional<double> ver = (*root)["version"].get<double>();
-	if (!ver || *ver != 1.0) {
-		LOG(RPI, Error) << "Unexpected configuration file version reported";
-		return -EINVAL;
-	}
-
-	std::optional<std::string> target = (*root)["target"].get<std::string>();
-	if (target != "pisp") {
-		LOG(RPI, Error) << "Unexpected target reported: expected \"pisp\", got "
-				<< (target ? target->c_str() : "(unknown)");
-		return -EINVAL;
-	}
-
-	const YamlObject &phConfig = (*root)["pipeline_handler"];
 	config_.numCfeConfigStatsBuffers =
 		phConfig["num_cfe_config_stats_buffers"].get<unsigned int>(config_.numCfeConfigStatsBuffers);
 	config_.numCfeConfigQueue =
diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
index 5cadef527..5fde67ac0 100644
--- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
+++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
@@ -61,13 +61,19 @@  public:
 		return unicam_[Unicam::Image].dev();
 	}
 
+	const std::string &target() const override
+	{
+		static const std::string target = "bcm2835";
+		return target;
+	}
+
 	void platformFreeBuffers() override
 	{
 	}
 
 	CameraConfiguration::Status platformValidate(RPi::RPiCameraConfiguration *rpiConfig) const override;
 
-	int platformPipelineConfigure(const std::unique_ptr<YamlObject> &root) override;
+	int platformPipelineConfigure(const YamlObject &phConfig) override;
 
 	void platformStart() override;
 	void platformStop() override;
@@ -493,30 +499,16 @@  CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig
 	return status;
 }
 
-int Vc4CameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject> &root)
+int Vc4CameraData::platformPipelineConfigure(const YamlObject &phConfig)
 {
 	config_ = {
 		.minUnicamBuffers = 2,
 		.minTotalUnicamBuffers = 4,
 	};
 
-	if (!root)
+	if (!phConfig)
 		return 0;
 
-	std::optional<double> ver = (*root)["version"].get<double>();
-	if (!ver || *ver != 1.0) {
-		LOG(RPI, Error) << "Unexpected configuration file version reported";
-		return -EINVAL;
-	}
-
-	std::optional<std::string> target = (*root)["target"].get<std::string>();
-	if (target != "bcm2835") {
-		LOG(RPI, Error) << "Unexpected target reported: expected \"bcm2835\", got "
-				<< (target ? target->c_str() : "(unknown)");
-		return -EINVAL;
-	}
-
-	const YamlObject &phConfig = (*root)["pipeline_handler"];
 	config_.minUnicamBuffers =
 		phConfig["min_unicam_buffers"].get<unsigned int>(config_.minUnicamBuffers);
 	config_.minTotalUnicamBuffers =