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

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

Commit Message

Milan Zamazal July 2, 2025, 1:10 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     | 59 +++++++++++--------
 .../pipeline/rpi/common/pipeline_base.h       |  5 +-
 src/libcamera/pipeline/rpi/pisp/pisp.cpp      | 29 ++++-----
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        | 28 ++++-----
 4 files changed, 67 insertions(+), 54 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..0e31066cf 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;
 
@@ -1089,37 +1091,41 @@  int CameraData::loadPipelineConfiguration()
 	};
 
 	/* Initial configuration of the platform, in case no config file is present */
-	platformPipelineConfigure({});
+	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 (!file.open(File::OpenModeFlag::ReadOnly)) {
-		LOG(RPI, Warning) << "Failed to open configuration file '" << filename << "'"
-				  << ", using defaults";
-		return 0;
-	}
+	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;
+		}
 
-	LOG(RPI, Info) << "Using configuration file '" << filename << "'";
+		LOG(RPI, Info) << "Using configuration file '" << filename << "'";
 
-	std::unique_ptr<YamlObject> root = YamlParser::parse(file);
-	if (!root) {
-		LOG(RPI, Warning) << "Failed to parse configuration file, using defaults";
-		return 0;
-	}
+		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;
+		}
 	}
 
-	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 +1141,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, expectedTarget);
 }
 
 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..0badce293 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
@@ -95,8 +95,11 @@  public:
 	virtual V4L2VideoDevice::Formats ispFormats() const = 0;
 	virtual V4L2VideoDevice::Formats rawFormats() const = 0;
 	virtual V4L2VideoDevice *frontendDevice() = 0;
+	virtual std::string target() const = 0;
 
-	virtual int platformPipelineConfigure(const std::unique_ptr<YamlObject> &root) = 0;
+	virtual int platformPipelineConfigure(
+		const YamlObject &phConfig,
+		const std::optional<std::string> &target) = 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 2df91bacf..dcb973722 100644
--- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
+++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
@@ -740,10 +740,17 @@  public:
 		return cfe_[Cfe::Output0].dev();
 	}
 
+	std::string target() const override
+	{
+		return "pisp";
+	}
+
 	CameraConfiguration::Status
 	platformValidate(RPi::RPiCameraConfiguration *rpiConfig) const override;
 
-	int platformPipelineConfigure(const std::unique_ptr<YamlObject> &root) override;
+	int platformPipelineConfigure(
+		const YamlObject &phConfig,
+		const std::optional<std::string> &expectedTarget) override;
 
 	void platformStart() override;
 	void platformStop() override;
@@ -1331,7 +1338,9 @@  PiSPCameraData::platformValidate(RPi::RPiCameraConfiguration *rpiConfig) const
 	return status;
 }
 
-int PiSPCameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject> &root)
+int PiSPCameraData::platformPipelineConfigure(
+	const YamlObject &phConfig,
+	const std::optional<std::string> &expectedTarget)
 {
 	config_ = {
 		.numCfeConfigStatsBuffers = 12,
@@ -1340,23 +1349,15 @@  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)");
+	if (expectedTarget != "pisp") {
+		LOG(RPI, Error) << "Unexpected target reported: expected \"" << target() << "\", got "
+				<< (expectedTarget ? expectedTarget->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 e99a7edf8..e00221a3a 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();
 	}
 
+	std::string target() const override
+	{
+		return "bcm2835";
+	}
+
 	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,
+				      const std::optional<std::string> &expectedTarget) override;
 
 	void platformStart() override;
 	void platformStop() override;
@@ -493,30 +499,24 @@  CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig
 	return status;
 }
 
-int Vc4CameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject> &root)
+int Vc4CameraData::platformPipelineConfigure(
+	const YamlObject &phConfig,
+	const std::optional<std::string> &expectedTarget)
 {
 	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)");
+	if (expectedTarget != target()) {
+		LOG(RPI, Error) << "Unexpected target reported: expected \"" << target() << "\", got "
+				<< (expectedTarget ? expectedTarget->c_str() : "(unknown)");
 		return -EINVAL;
 	}
 
-	const YamlObject &phConfig = (*root)["pipeline_handler"];
 	config_.minUnicamBuffers =
 		phConfig["min_unicam_buffers"].get<unsigned int>(config_.minUnicamBuffers);
 	config_.minTotalUnicamBuffers =