[libcamera-devel,v2,1/2] libcamera: raspberrypi: allow SwitchMode method to return camera settings

Message ID 20200618111236.26897-2-david.plowman@raspberrypi.com
State Accepted
Commit ff291b3c15ba250fb05236a774771fdf54554ff7
Headers show
Series
  • Raspberry pi IPAs: update sensor exposure/gain when camera mode changes
Related show

Commit Message

David Plowman June 18, 2020, 11:12 a.m. UTC
This commit adds a Metadata parameter to the SwitchMode method
enabling it to return camera and other settings to the caller
(usually the configure method, just after the camera mode has been
selected).

In future this will allow the Raspberry Pi IPAs to take those settings
(such as exposure and analogue gain) and program them directly into
the camera or ISP before the camera is even started.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/algorithm.cpp   | 3 ++-
 src/ipa/raspberrypi/controller/algorithm.hpp   | 2 +-
 src/ipa/raspberrypi/controller/controller.cpp  | 4 ++--
 src/ipa/raspberrypi/controller/controller.hpp  | 2 +-
 src/ipa/raspberrypi/controller/rpi/alsc.cpp    | 4 +++-
 src/ipa/raspberrypi/controller/rpi/alsc.hpp    | 2 +-
 src/ipa/raspberrypi/controller/rpi/noise.cpp   | 4 +++-
 src/ipa/raspberrypi/controller/rpi/noise.hpp   | 2 +-
 src/ipa/raspberrypi/controller/rpi/sharpen.cpp | 4 +++-
 src/ipa/raspberrypi/controller/rpi/sharpen.hpp | 2 +-
 src/ipa/raspberrypi/raspberrypi.cpp            | 3 ++-
 11 files changed, 20 insertions(+), 12 deletions(-)

Comments

Laurent Pinchart June 22, 2020, 3:54 a.m. UTC | #1
Hi David,

Thank you for the patch.

On Thu, Jun 18, 2020 at 12:12:35PM +0100, David Plowman wrote:
> This commit adds a Metadata parameter to the SwitchMode method
> enabling it to return camera and other settings to the caller
> (usually the configure method, just after the camera mode has been
> selected).
> 
> In future this will allow the Raspberry Pi IPAs to take those settings
> (such as exposure and analogue gain) and program them directly into
> the camera or ISP before the camera is even started.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

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

> ---
>  src/ipa/raspberrypi/controller/algorithm.cpp   | 3 ++-
>  src/ipa/raspberrypi/controller/algorithm.hpp   | 2 +-
>  src/ipa/raspberrypi/controller/controller.cpp  | 4 ++--
>  src/ipa/raspberrypi/controller/controller.hpp  | 2 +-
>  src/ipa/raspberrypi/controller/rpi/alsc.cpp    | 4 +++-
>  src/ipa/raspberrypi/controller/rpi/alsc.hpp    | 2 +-
>  src/ipa/raspberrypi/controller/rpi/noise.cpp   | 4 +++-
>  src/ipa/raspberrypi/controller/rpi/noise.hpp   | 2 +-
>  src/ipa/raspberrypi/controller/rpi/sharpen.cpp | 4 +++-
>  src/ipa/raspberrypi/controller/rpi/sharpen.hpp | 2 +-
>  src/ipa/raspberrypi/raspberrypi.cpp            | 3 ++-
>  11 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp b/src/ipa/raspberrypi/controller/algorithm.cpp
> index 9bd3df8..55cb201 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.cpp
> +++ b/src/ipa/raspberrypi/controller/algorithm.cpp
> @@ -16,9 +16,10 @@ void Algorithm::Read(boost::property_tree::ptree const &params)
>  
>  void Algorithm::Initialise() {}
>  
> -void Algorithm::SwitchMode(CameraMode const &camera_mode)
> +void Algorithm::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
>  {
>  	(void)camera_mode;
> +	(void)metadata;
>  }
>  
>  void Algorithm::Prepare(Metadata *image_metadata)
> diff --git a/src/ipa/raspberrypi/controller/algorithm.hpp b/src/ipa/raspberrypi/controller/algorithm.hpp
> index b82c184..187c50c 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.hpp
> +++ b/src/ipa/raspberrypi/controller/algorithm.hpp
> @@ -37,7 +37,7 @@ public:
>  	virtual void Resume() { paused_ = false; }
>  	virtual void Read(boost::property_tree::ptree const &params);
>  	virtual void Initialise();
> -	virtual void SwitchMode(CameraMode const &camera_mode);
> +	virtual void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);
>  	virtual void Prepare(Metadata *image_metadata);
>  	virtual void Process(StatisticsPtr &stats, Metadata *image_metadata);
>  	Metadata &GetGlobalMetadata() const
> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
> index 20dd4c7..7c4b04f 100644
> --- a/src/ipa/raspberrypi/controller/controller.cpp
> +++ b/src/ipa/raspberrypi/controller/controller.cpp
> @@ -56,11 +56,11 @@ void Controller::Initialise()
>  	RPI_LOG("Controller finished");
>  }
>  
> -void Controller::SwitchMode(CameraMode const &camera_mode)
> +void Controller::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
>  {
>  	RPI_LOG("Controller starting");
>  	for (auto &algo : algorithms_)
> -		algo->SwitchMode(camera_mode);
> +		algo->SwitchMode(camera_mode, metadata);
>  	switch_mode_called_ = true;
>  	RPI_LOG("Controller finished");
>  }
> diff --git a/src/ipa/raspberrypi/controller/controller.hpp b/src/ipa/raspberrypi/controller/controller.hpp
> index d685386..6ba9412 100644
> --- a/src/ipa/raspberrypi/controller/controller.hpp
> +++ b/src/ipa/raspberrypi/controller/controller.hpp
> @@ -39,7 +39,7 @@ public:
>  	Algorithm *CreateAlgorithm(char const *name);
>  	void Read(char const *filename);
>  	void Initialise();
> -	void SwitchMode(CameraMode const &camera_mode);
> +	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);
>  	void Prepare(Metadata *image_metadata);
>  	void Process(StatisticsPtr stats, Metadata *image_metadata);
>  	Metadata &GetGlobalMetadata();
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> index 821a0ca..76e2f04 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> @@ -173,8 +173,10 @@ void Alsc::Initialise()
>  		lambda_r_[i] = lambda_b_[i] = 1.0;
>  }
>  
> -void Alsc::SwitchMode(CameraMode const &camera_mode)
> +void Alsc::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
>  {
> +	(void)metadata;
> +
>  	// There's a bit of a question what we should do if the "crop" of the
>  	// camera mode has changed.  Any calculation currently in flight would
>  	// not be useful to the new mode, so arguably we should abort it, and
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp
> index c8ed3d2..3806257 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp
> @@ -50,7 +50,7 @@ public:
>  	~Alsc();
>  	char const *Name() const override;
>  	void Initialise() override;
> -	void SwitchMode(CameraMode const &camera_mode) override;
> +	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
>  	void Read(boost::property_tree::ptree const &params) override;
>  	void Prepare(Metadata *image_metadata) override;
>  	void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> index 2209d79..2cafde3 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> @@ -27,8 +27,10 @@ char const *Noise::Name() const
>  	return NAME;
>  }
>  
> -void Noise::SwitchMode(CameraMode const &camera_mode)
> +void Noise::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
>  {
> +	(void)metadata;
> +
>  	// For example, we would expect a 2x2 binned mode to have a "noise
>  	// factor" of sqrt(2x2) = 2. (can't be less than one, right?)
>  	mode_factor_ = std::max(1.0, camera_mode.noise_factor);
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.hpp b/src/ipa/raspberrypi/controller/rpi/noise.hpp
> index 51d46a3..25bf188 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.hpp
> @@ -18,7 +18,7 @@ class Noise : public Algorithm
>  public:
>  	Noise(Controller *controller);
>  	char const *Name() const override;
> -	void SwitchMode(CameraMode const &camera_mode) override;
> +	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
>  	void Read(boost::property_tree::ptree const &params) override;
>  	void Prepare(Metadata *image_metadata) override;
>  
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> index 1f07bb6..086952f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> @@ -26,8 +26,10 @@ char const *Sharpen::Name() const
>  	return NAME;
>  }
>  
> -void Sharpen::SwitchMode(CameraMode const &camera_mode)
> +void Sharpen::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
>  {
> +	(void)metadata;
> +
>  	// can't be less than one, right?
>  	mode_factor_ = std::max(1.0, camera_mode.noise_factor);
>  }
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> index 3b0d680..f871aa6 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> @@ -18,7 +18,7 @@ class Sharpen : public Algorithm
>  public:
>  	Sharpen(Controller *controller);
>  	char const *Name() const override;
> -	void SwitchMode(CameraMode const &camera_mode) override;
> +	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
>  	void Read(boost::property_tree::ptree const &params) override;
>  	void Prepare(Metadata *image_metadata) override;
>  
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 9669f21..d6fd3df 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -267,7 +267,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		queueFrameAction.emit(0, op);
>  	}
>  
> -	controller_.SwitchMode(mode_);
> +	RPi::Metadata metadata;
> +	controller_.SwitchMode(mode_, &metadata);
>  
>  	lastMode_ = mode_;
>  }

Patch

diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp b/src/ipa/raspberrypi/controller/algorithm.cpp
index 9bd3df8..55cb201 100644
--- a/src/ipa/raspberrypi/controller/algorithm.cpp
+++ b/src/ipa/raspberrypi/controller/algorithm.cpp
@@ -16,9 +16,10 @@  void Algorithm::Read(boost::property_tree::ptree const &params)
 
 void Algorithm::Initialise() {}
 
-void Algorithm::SwitchMode(CameraMode const &camera_mode)
+void Algorithm::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
 {
 	(void)camera_mode;
+	(void)metadata;
 }
 
 void Algorithm::Prepare(Metadata *image_metadata)
diff --git a/src/ipa/raspberrypi/controller/algorithm.hpp b/src/ipa/raspberrypi/controller/algorithm.hpp
index b82c184..187c50c 100644
--- a/src/ipa/raspberrypi/controller/algorithm.hpp
+++ b/src/ipa/raspberrypi/controller/algorithm.hpp
@@ -37,7 +37,7 @@  public:
 	virtual void Resume() { paused_ = false; }
 	virtual void Read(boost::property_tree::ptree const &params);
 	virtual void Initialise();
-	virtual void SwitchMode(CameraMode const &camera_mode);
+	virtual void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);
 	virtual void Prepare(Metadata *image_metadata);
 	virtual void Process(StatisticsPtr &stats, Metadata *image_metadata);
 	Metadata &GetGlobalMetadata() const
diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
index 20dd4c7..7c4b04f 100644
--- a/src/ipa/raspberrypi/controller/controller.cpp
+++ b/src/ipa/raspberrypi/controller/controller.cpp
@@ -56,11 +56,11 @@  void Controller::Initialise()
 	RPI_LOG("Controller finished");
 }
 
-void Controller::SwitchMode(CameraMode const &camera_mode)
+void Controller::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
 {
 	RPI_LOG("Controller starting");
 	for (auto &algo : algorithms_)
-		algo->SwitchMode(camera_mode);
+		algo->SwitchMode(camera_mode, metadata);
 	switch_mode_called_ = true;
 	RPI_LOG("Controller finished");
 }
diff --git a/src/ipa/raspberrypi/controller/controller.hpp b/src/ipa/raspberrypi/controller/controller.hpp
index d685386..6ba9412 100644
--- a/src/ipa/raspberrypi/controller/controller.hpp
+++ b/src/ipa/raspberrypi/controller/controller.hpp
@@ -39,7 +39,7 @@  public:
 	Algorithm *CreateAlgorithm(char const *name);
 	void Read(char const *filename);
 	void Initialise();
-	void SwitchMode(CameraMode const &camera_mode);
+	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);
 	void Prepare(Metadata *image_metadata);
 	void Process(StatisticsPtr stats, Metadata *image_metadata);
 	Metadata &GetGlobalMetadata();
diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
index 821a0ca..76e2f04 100644
--- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
@@ -173,8 +173,10 @@  void Alsc::Initialise()
 		lambda_r_[i] = lambda_b_[i] = 1.0;
 }
 
-void Alsc::SwitchMode(CameraMode const &camera_mode)
+void Alsc::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
 {
+	(void)metadata;
+
 	// There's a bit of a question what we should do if the "crop" of the
 	// camera mode has changed.  Any calculation currently in flight would
 	// not be useful to the new mode, so arguably we should abort it, and
diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp
index c8ed3d2..3806257 100644
--- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp
+++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp
@@ -50,7 +50,7 @@  public:
 	~Alsc();
 	char const *Name() const override;
 	void Initialise() override;
-	void SwitchMode(CameraMode const &camera_mode) override;
+	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
 	void Read(boost::property_tree::ptree const &params) override;
 	void Prepare(Metadata *image_metadata) override;
 	void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp b/src/ipa/raspberrypi/controller/rpi/noise.cpp
index 2209d79..2cafde3 100644
--- a/src/ipa/raspberrypi/controller/rpi/noise.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp
@@ -27,8 +27,10 @@  char const *Noise::Name() const
 	return NAME;
 }
 
-void Noise::SwitchMode(CameraMode const &camera_mode)
+void Noise::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
 {
+	(void)metadata;
+
 	// For example, we would expect a 2x2 binned mode to have a "noise
 	// factor" of sqrt(2x2) = 2. (can't be less than one, right?)
 	mode_factor_ = std::max(1.0, camera_mode.noise_factor);
diff --git a/src/ipa/raspberrypi/controller/rpi/noise.hpp b/src/ipa/raspberrypi/controller/rpi/noise.hpp
index 51d46a3..25bf188 100644
--- a/src/ipa/raspberrypi/controller/rpi/noise.hpp
+++ b/src/ipa/raspberrypi/controller/rpi/noise.hpp
@@ -18,7 +18,7 @@  class Noise : public Algorithm
 public:
 	Noise(Controller *controller);
 	char const *Name() const override;
-	void SwitchMode(CameraMode const &camera_mode) override;
+	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
 	void Read(boost::property_tree::ptree const &params) override;
 	void Prepare(Metadata *image_metadata) override;
 
diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
index 1f07bb6..086952f 100644
--- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
@@ -26,8 +26,10 @@  char const *Sharpen::Name() const
 	return NAME;
 }
 
-void Sharpen::SwitchMode(CameraMode const &camera_mode)
+void Sharpen::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
 {
+	(void)metadata;
+
 	// can't be less than one, right?
 	mode_factor_ = std::max(1.0, camera_mode.noise_factor);
 }
diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
index 3b0d680..f871aa6 100644
--- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
+++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
@@ -18,7 +18,7 @@  class Sharpen : public Algorithm
 public:
 	Sharpen(Controller *controller);
 	char const *Name() const override;
-	void SwitchMode(CameraMode const &camera_mode) override;
+	void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
 	void Read(boost::property_tree::ptree const &params) override;
 	void Prepare(Metadata *image_metadata) override;
 
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 9669f21..d6fd3df 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -267,7 +267,8 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		queueFrameAction.emit(0, op);
 	}
 
-	controller_.SwitchMode(mode_);
+	RPi::Metadata metadata;
+	controller_.SwitchMode(mode_, &metadata);
 
 	lastMode_ = mode_;
 }