[libcamera-devel] [PATCH v6 5/8] ipa: raspberrypi: Introduce version 2.0 format for the camera tuning file

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Mon, Jul 18, 2022 at 09:15:59AM +0100, Naushir Patuck via libcamera-devel wrote:
> The existing tuning file format (version 1.0) requires the controller algorithms
> to run in the same order as listed in the JSON structure. The JSON specification
> does not mandate any such ordering, but the Boost JSON parser would maintain
> this order.
> 
> In order to remove this reliance on the parser to provide ordering, introduce a
> new version 2.0 format for the camera tuning file. In this version, the
> algorithms are specified in a top level list node ("algorithms"), which does
> require strict ordering of the elements.
> 
> A "version" node is added to distinguish between the version 1.0 and 2.0
> formats. The absence of the "version" node implies version 1.0.
> 
> A "target" node is also added to specify the target platform for this
> configuration.
> 
> Update the controller to support either version of the tuning file by looking
> at the version node. CreateAlgorithm member function to now load and configure
> each algorithm. Additionally, make CreateAlgorithm a private member, it does not
> get called externally.
> 
> If a version 1.0 format tuning file is used, throw a warning message indicating
> it will be soon deprecated.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/ipa/raspberrypi/controller/controller.cpp | 43 +++++++++++++------
>  src/ipa/raspberrypi/controller/controller.hpp |  4 +-
>  2 files changed, 34 insertions(+), 13 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
> index 67d650ef0c1b..b22a17f18f8e 100644
> --- a/src/ipa/raspberrypi/controller/controller.cpp
> +++ b/src/ipa/raspberrypi/controller/controller.cpp
> @@ -42,22 +42,41 @@ void Controller::Read(char const *filename)
>  	}
>  
>  	std::unique_ptr<YamlObject> root = YamlParser::parse(file);
> -
> -	for (auto const &[key, value] : root->asDict()) {
> -		Algorithm *algo = CreateAlgorithm(key.c_str());
> -		if (algo) {
> -			algo->Read(value);
> -			algorithms_.push_back(AlgorithmPtr(algo));
> -		} else
> -			LOG(RPiController, Warning)
> -				<< "No algorithm found for \"" << key << "\"";
> +	double version = (*root)["version"].get<double>(1.0);
> +	if (version < 2.0) {
> +		LOG(RPiController, Warning)
> +			<< "This format of the tuning file will be deprecated soon!"
> +			<< " Please use the convert_tuning.py utility to update to version 2.0.";
> +		for (auto const &[key, value] : root->asDict())
> +			CreateAlgorithm(key, value);
> +	} else if (version < 3.0) {
> +		if (!root->contains("algorithms")) {
> +			LOG(RPiController, Fatal)
> +				<< "Tuning file " << filename
> +				<< " does not have an \"algorithms\" list!";
> +			return;
> +		}
> +		for (auto const &rootAlgo : (*root)["algorithms"].asList())
> +			for (auto const &[key, value] : rootAlgo.asDict())
> +				CreateAlgorithm(key, value);
> +	} else {
> +		LOG(RPiController, Fatal)
> +			<< "Unrecognised version " << version
> +			<< " for the tuning file " << filename;
>  	}
>  }
>  
> -Algorithm *Controller::CreateAlgorithm(char const *name)
> +void Controller::CreateAlgorithm(const std::string &name, const YamlObject &params)
>  {
> -	auto it = GetAlgorithms().find(std::string(name));
> -	return it != GetAlgorithms().end() ? (*it->second)(this) : nullptr;
> +	auto it = GetAlgorithms().find(name);
> +	if (it == GetAlgorithms().end()) {
> +		LOG(RPiController, Warning)
> +			<< "No algorithm found for \"" << name << "\"";
> +		return;
> +	}
> +	Algorithm *algo = (*it->second)(this);
> +	algo->Read(params);
> +	algorithms_.push_back(AlgorithmPtr(algo));
>  }
>  
>  void Controller::Initialise()
> diff --git a/src/ipa/raspberrypi/controller/controller.hpp b/src/ipa/raspberrypi/controller/controller.hpp
> index 3b50ae770d11..9091deac5feb 100644
> --- a/src/ipa/raspberrypi/controller/controller.hpp
> +++ b/src/ipa/raspberrypi/controller/controller.hpp
> @@ -15,6 +15,8 @@
>  
>  #include <linux/bcm2835-isp.h>
>  
> +#include "libcamera/internal/yaml_parser.h"
> +
>  #include "camera_mode.h"
>  #include "device_status.h"
>  #include "metadata.hpp"
> @@ -36,7 +38,6 @@ public:
>  	Controller();
>  	Controller(char const *json_filename);
>  	~Controller();
> -	Algorithm *CreateAlgorithm(char const *name);
>  	void Read(char const *filename);
>  	void Initialise();
>  	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);
> @@ -46,6 +47,7 @@ public:
>  	Algorithm *GetAlgorithm(std::string const &name) const;
>  
>  protected:
> +	void CreateAlgorithm(const std::string &name, const libcamera::YamlObject &params);
>  	Metadata global_metadata_;
>  	std::vector<AlgorithmPtr> algorithms_;
>  	bool switch_mode_called_;

-- 
Regards,

Laurent Pinchart