[libcamera-devel,v1,01/10] ipa: raspberrypi Store the target string in the controller
diff mbox series

Message ID 20230322130612.5208-2-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi: Generalised algorithms
Related show

Commit Message

Naushir Patuck March 22, 2023, 1:06 p.m. UTC 
The target string may be used by algorithms to determine the running
hardware target.

Store the target string provided by the camera tuning files in the
controller state. Add a getTarget() member function to retrieve this
string.

Validate the correct hardware target ("bcm2835") during the IPA
initialisation phase.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/algorithm.h    | 4 ++++
 src/ipa/raspberrypi/controller/controller.cpp | 6 ++++++
 src/ipa/raspberrypi/controller/controller.h   | 4 ++++
 src/ipa/raspberrypi/raspberrypi.cpp           | 8 ++++++++
 4 files changed, 22 insertions(+)

Comments

David Plowman March 22, 2023, 1:54 p.m. UTC | #1 
Hi Naush

Thanks for the patch!

On Wed, 22 Mar 2023 at 13:06, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> The target string may be used by algorithms to determine the running
> hardware target.
>
> Store the target string provided by the camera tuning files in the
> controller state. Add a getTarget() member function to retrieve this
> string.
>
> Validate the correct hardware target ("bcm2835") during the IPA
> initialisation phase.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

LGTM.

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

> ---
>  src/ipa/raspberrypi/controller/algorithm.h    | 4 ++++
>  src/ipa/raspberrypi/controller/controller.cpp | 6 ++++++
>  src/ipa/raspberrypi/controller/controller.h   | 4 ++++
>  src/ipa/raspberrypi/raspberrypi.cpp           | 8 ++++++++
>  4 files changed, 22 insertions(+)
>
> diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h
> index 4f3275987305..7c22fbe4945c 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.h
> +++ b/src/ipa/raspberrypi/controller/algorithm.h
> @@ -41,6 +41,10 @@ public:
>         {
>                 return controller_->getGlobalMetadata();
>         }
> +       const std::string &getTarget() const
> +       {
> +               return controller_->getTarget();
> +       }
>
>  private:
>         Controller *controller_;
> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
> index e91567852d00..a6250ee140b0 100644
> --- a/src/ipa/raspberrypi/controller/controller.cpp
> +++ b/src/ipa/raspberrypi/controller/controller.cpp
> @@ -38,6 +38,7 @@ int Controller::read(char const *filename)
>
>         std::unique_ptr<YamlObject> root = YamlParser::parse(file);
>         double version = (*root)["version"].get<double>(1.0);
> +       target_ = (*root)["target"].get<std::string>("bcm2835");
>
>         if (version < 2.0) {
>                 LOG(RPiController, Warning)
> @@ -142,3 +143,8 @@ Algorithm *Controller::getAlgorithm(std::string const &name) const
>         }
>         return nullptr;
>  }
> +
> +const std::string &Controller::getTarget() const
> +{
> +       return target_;
> +}
> diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h
> index e6c950c3a509..24e02903d438 100644
> --- a/src/ipa/raspberrypi/controller/controller.h
> +++ b/src/ipa/raspberrypi/controller/controller.h
> @@ -46,6 +46,7 @@ public:
>         void process(StatisticsPtr stats, Metadata *imageMetadata);
>         Metadata &getGlobalMetadata();
>         Algorithm *getAlgorithm(std::string const &name) const;
> +       const std::string &getTarget() const;
>
>  protected:
>         int createAlgorithm(const std::string &name, const libcamera::YamlObject &params);
> @@ -53,6 +54,9 @@ protected:
>         Metadata globalMetadata_;
>         std::vector<AlgorithmPtr> algorithms_;
>         bool switchModeCalled_;
> +
> +private:
> +       std::string target_;
>  };
>
>  } /* namespace RPiController */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 1375795568e2..86359538cf67 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -269,6 +269,14 @@ int IPARPi::init(const IPASettings &settings, bool lensPresent, IPAInitResult *r
>                 return ret;
>         }
>
> +       const std::string &target = controller_.getTarget();
> +       if (target != "bcm2835") {
> +               LOG(IPARPI, Error)
> +                       << "Tuning data file target returned \"" << target << "\""
> +                       << ", expected \"bcm2835\"";
> +               return -EINVAL;
> +       }
> +
>         lensPresent_ = lensPresent;
>
>         controller_.initialise();
> --
> 2.34.1
>
Jacopo Mondi March 23, 2023, 4:01 p.m. UTC | #2 
Hi Naush

On Wed, Mar 22, 2023 at 01:06:03PM +0000, Naushir Patuck via libcamera-devel wrote:
> The target string may be used by algorithms to determine the running
> hardware target.
>
> Store the target string provided by the camera tuning files in the
> controller state. Add a getTarget() member function to retrieve this
> string.
>
> Validate the correct hardware target ("bcm2835") during the IPA
> initialisation phase.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  src/ipa/raspberrypi/controller/algorithm.h    | 4 ++++
>  src/ipa/raspberrypi/controller/controller.cpp | 6 ++++++
>  src/ipa/raspberrypi/controller/controller.h   | 4 ++++
>  src/ipa/raspberrypi/raspberrypi.cpp           | 8 ++++++++
>  4 files changed, 22 insertions(+)
>
> diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h
> index 4f3275987305..7c22fbe4945c 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.h
> +++ b/src/ipa/raspberrypi/controller/algorithm.h
> @@ -41,6 +41,10 @@ public:
>  	{
>  		return controller_->getGlobalMetadata();
>  	}
> +	const std::string &getTarget() const
> +	{
> +		return controller_->getTarget();
> +	}

Is this used ? Maybe in the next patches...

Otherwise
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
   j

>
>  private:
>  	Controller *controller_;
> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
> index e91567852d00..a6250ee140b0 100644
> --- a/src/ipa/raspberrypi/controller/controller.cpp
> +++ b/src/ipa/raspberrypi/controller/controller.cpp
> @@ -38,6 +38,7 @@ int Controller::read(char const *filename)
>
>  	std::unique_ptr<YamlObject> root = YamlParser::parse(file);
>  	double version = (*root)["version"].get<double>(1.0);
> +	target_ = (*root)["target"].get<std::string>("bcm2835");
>
>  	if (version < 2.0) {
>  		LOG(RPiController, Warning)
> @@ -142,3 +143,8 @@ Algorithm *Controller::getAlgorithm(std::string const &name) const
>  	}
>  	return nullptr;
>  }
> +
> +const std::string &Controller::getTarget() const
> +{
> +	return target_;
> +}
> diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h
> index e6c950c3a509..24e02903d438 100644
> --- a/src/ipa/raspberrypi/controller/controller.h
> +++ b/src/ipa/raspberrypi/controller/controller.h
> @@ -46,6 +46,7 @@ public:
>  	void process(StatisticsPtr stats, Metadata *imageMetadata);
>  	Metadata &getGlobalMetadata();
>  	Algorithm *getAlgorithm(std::string const &name) const;
> +	const std::string &getTarget() const;
>
>  protected:
>  	int createAlgorithm(const std::string &name, const libcamera::YamlObject &params);
> @@ -53,6 +54,9 @@ protected:
>  	Metadata globalMetadata_;
>  	std::vector<AlgorithmPtr> algorithms_;
>  	bool switchModeCalled_;
> +
> +private:
> +	std::string target_;
>  };
>
>  } /* namespace RPiController */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 1375795568e2..86359538cf67 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -269,6 +269,14 @@ int IPARPi::init(const IPASettings &settings, bool lensPresent, IPAInitResult *r
>  		return ret;
>  	}
>
> +	const std::string &target = controller_.getTarget();
> +	if (target != "bcm2835") {
> +		LOG(IPARPI, Error)
> +			<< "Tuning data file target returned \"" << target << "\""
> +			<< ", expected \"bcm2835\"";
> +		return -EINVAL;
> +	}
> +
>  	lensPresent_ = lensPresent;
>
>  	controller_.initialise();
> --
> 2.34.1
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h
index 4f3275987305..7c22fbe4945c 100644
--- a/src/ipa/raspberrypi/controller/algorithm.h
+++ b/src/ipa/raspberrypi/controller/algorithm.h
@@ -41,6 +41,10 @@  public:
 	{
 		return controller_->getGlobalMetadata();
 	}
+	const std::string &getTarget() const
+	{
+		return controller_->getTarget();
+	}
 
 private:
 	Controller *controller_;
diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
index e91567852d00..a6250ee140b0 100644
--- a/src/ipa/raspberrypi/controller/controller.cpp
+++ b/src/ipa/raspberrypi/controller/controller.cpp
@@ -38,6 +38,7 @@  int Controller::read(char const *filename)
 
 	std::unique_ptr<YamlObject> root = YamlParser::parse(file);
 	double version = (*root)["version"].get<double>(1.0);
+	target_ = (*root)["target"].get<std::string>("bcm2835");
 
 	if (version < 2.0) {
 		LOG(RPiController, Warning)
@@ -142,3 +143,8 @@  Algorithm *Controller::getAlgorithm(std::string const &name) const
 	}
 	return nullptr;
 }
+
+const std::string &Controller::getTarget() const
+{
+	return target_;
+}
diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h
index e6c950c3a509..24e02903d438 100644
--- a/src/ipa/raspberrypi/controller/controller.h
+++ b/src/ipa/raspberrypi/controller/controller.h
@@ -46,6 +46,7 @@  public:
 	void process(StatisticsPtr stats, Metadata *imageMetadata);
 	Metadata &getGlobalMetadata();
 	Algorithm *getAlgorithm(std::string const &name) const;
+	const std::string &getTarget() const;
 
 protected:
 	int createAlgorithm(const std::string &name, const libcamera::YamlObject &params);
@@ -53,6 +54,9 @@  protected:
 	Metadata globalMetadata_;
 	std::vector<AlgorithmPtr> algorithms_;
 	bool switchModeCalled_;
+
+private:
+	std::string target_;
 };
 
 } /* namespace RPiController */
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 1375795568e2..86359538cf67 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -269,6 +269,14 @@  int IPARPi::init(const IPASettings &settings, bool lensPresent, IPAInitResult *r
 		return ret;
 	}
 
+	const std::string &target = controller_.getTarget();
+	if (target != "bcm2835") {
+		LOG(IPARPI, Error)
+			<< "Tuning data file target returned \"" << target << "\""
+			<< ", expected \"bcm2835\"";
+		return -EINVAL;
+	}
+
 	lensPresent_ = lensPresent;
 
 	controller_.initialise();