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

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

Commit Message

Milan Zamazal July 29, 2025, 7:31 a.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:
    pipelines:
      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       |  3 +-
 src/libcamera/pipeline/rpi/pisp/pisp.cpp      | 26 +++-----
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        | 26 +++-----
 4 files changed, 56 insertions(+), 58 deletions(-)

Comments

Paul Elder Sept. 8, 2025, 10:25 a.m. UTC | #1
Hi Milan,

Thanks for the patch.

Quoting Milan Zamazal (2025-07-29 16:31:52)
> 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:
>     pipelines:
>       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>

The commit title should be s/config:/pipeline: rpi:/ , as this touches rpi code
and not global_configuration code.

> ---
>  .../pipeline/rpi/common/pipeline_base.cpp     | 59 +++++++++++--------
>  .../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, 56 insertions(+), 58 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 563df198e..1effa85cd 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()["pipelines"]["rpi"];

Don't we need to check if configuration()["pipelines"] and configuration()["pipelines"]["rpi"] exist?

> +       const YamlObject &phConfig = (root
> +                                             ? (*root)["pipeline_handler"]
> +                                             : config[target()]["pipeline_handler"]);

Same here, for config[target()] and config[target()]["pipeline_handler"] ?

It looks like the raspberry pi config takes precedence. I suppose that's fine,
since it will be removed. Maybe we should add a warning message that it is
deprecated and should be removed or moved to the main configuration file.


The rest looks good.


Thanks,

Paul

>  
>         if (phConfig.contains("disable_startup_frame_drops"))
>                 LOG(RPI, Warning)
> @@ -1135,7 +1148,7 @@ int CameraData::loadPipelineConfiguration()
>                 frontendDevice()->setDequeueTimeout(config_.cameraTimeoutValue * 1ms);
>         }
>  
> -       return platformPipelineConfigure(root);
> +       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 082724c5a..0fb7c3fe8 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 99d43bd0a..71c425373 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 =
> -- 
> 2.50.1
>
Milan Zamazal Sept. 8, 2025, 3:07 p.m. UTC | #2
Hi Paul,

thank you for review.

Paul Elder <paul.elder@ideasonboard.com> writes:

> Hi Milan,
>
> Thanks for the patch.
>
> Quoting Milan Zamazal (2025-07-29 16:31:52)
>> 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:
>>     pipelines:
>>       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>
>
> The commit title should be s/config:/pipeline: rpi:/ , as this touches rpi code
> and not global_configuration code.

OK.

>> ---
>>  .../pipeline/rpi/common/pipeline_base.cpp     | 59 +++++++++++--------
>>  .../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, 56 insertions(+), 58 deletions(-)
>> 
>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> index 563df198e..1effa85cd 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()["pipelines"]["rpi"];
>
> Don't we need to check if configuration()["pipelines"] and configuration()["pipelines"]["rpi"] exist?

YamlObject::operator[] returns an empty YamlObject if the given key
doesn't exist, so it should be all fine.

>> +       const YamlObject &phConfig = (root
>> +                                             ? (*root)["pipeline_handler"]
>> +                                             : config[target()]["pipeline_handler"]);
>
> Same here, for config[target()] and config[target()]["pipeline_handler"] ?

Again, YamlObject should do the right thing as well as
YamlObject::get<>() with a default value, used later in the code.

> It looks like the raspberry pi config takes precedence.

Yes.

> I suppose that's fine, since it will be removed. Maybe we should add a
> warning message that it is deprecated and should be removed or moved
> to the main configuration file.

I don't plan the removal at the moment.  If it is intended to be done in
the future, I can add the warning.

> The rest looks good.
>
>
> Thanks,
>
> Paul
>
>>  
>>         if (phConfig.contains("disable_startup_frame_drops"))
>>                 LOG(RPI, Warning)
>> @@ -1135,7 +1148,7 @@ int CameraData::loadPipelineConfiguration()
>>                 frontendDevice()->setDequeueTimeout(config_.cameraTimeoutValue * 1ms);
>>         }
>>  
>> -       return platformPipelineConfigure(root);
>> +       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 082724c5a..0fb7c3fe8 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 99d43bd0a..71c425373 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 =
>> -- 
>> 2.50.1
>>
Paul Elder Sept. 9, 2025, 1:57 p.m. UTC | #3
Hi Milan,

Quoting Milan Zamazal (2025-09-09 00:07:56)
> Hi Paul,
> 
> thank you for review.
> 
> Paul Elder <paul.elder@ideasonboard.com> writes:
> 
> > Hi Milan,
> >
> > Thanks for the patch.
> >
> > Quoting Milan Zamazal (2025-07-29 16:31:52)
> >> 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:
> >>     pipelines:
> >>       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>
> >
> > The commit title should be s/config:/pipeline: rpi:/ , as this touches rpi code
> > and not global_configuration code.
> 
> OK.
> 
> >> ---
> >>  .../pipeline/rpi/common/pipeline_base.cpp     | 59 +++++++++++--------
> >>  .../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, 56 insertions(+), 58 deletions(-)
> >> 
> >> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> >> index 563df198e..1effa85cd 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()["pipelines"]["rpi"];
> >
> > Don't we need to check if configuration()["pipelines"] and configuration()["pipelines"]["rpi"] exist?
> 
> YamlObject::operator[] returns an empty YamlObject if the given key
> doesn't exist, so it should be all fine.

Oh ok I see what you mean. That's neat.

> 
> >> +       const YamlObject &phConfig = (root
> >> +                                             ? (*root)["pipeline_handler"]
> >> +                                             : config[target()]["pipeline_handler"]);
> >
> > Same here, for config[target()] and config[target()]["pipeline_handler"] ?
> 
> Again, YamlObject should do the right thing as well as
> YamlObject::get<>() with a default value, used later in the code.
> 
> > It looks like the raspberry pi config takes precedence.
> 
> Yes.
> 
> > I suppose that's fine, since it will be removed. Maybe we should add a
> > warning message that it is deprecated and should be removed or moved
> > to the main configuration file.
> 
> I don't plan the removal at the moment.  If it is intended to be done in
> the future, I can add the warning.

Ok.

> 
> > The rest looks good.

Then with the commit title fix,

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> >
> >
> > Thanks,
> >
> > Paul
> >
> >>  
> >>         if (phConfig.contains("disable_startup_frame_drops"))
> >>                 LOG(RPI, Warning)
> >> @@ -1135,7 +1148,7 @@ int CameraData::loadPipelineConfiguration()
> >>                 frontendDevice()->setDequeueTimeout(config_.cameraTimeoutValue * 1ms);
> >>         }
> >>  
> >> -       return platformPipelineConfigure(root);
> >> +       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 082724c5a..0fb7c3fe8 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 99d43bd0a..71c425373 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 =
> >> -- 
> >> 2.50.1
> >>
>

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 563df198e..1effa85cd 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()["pipelines"]["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,7 @@  int CameraData::loadPipelineConfiguration()
 		frontendDevice()->setDequeueTimeout(config_.cameraTimeoutValue * 1ms);
 	}
 
-	return platformPipelineConfigure(root);
+	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 082724c5a..0fb7c3fe8 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 99d43bd0a..71c425373 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 =