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

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Jun 22 05:54:24 CEST 2020


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 at raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at 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_;
>  }

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list