[libcamera-devel,v1,2/2] DEMO: raspberrypi: Use custom parameters to init()
diff mbox series

Message ID 20210304084743.11721-1-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-devel,v1,1/2] utils: ipc: Support custom parameters to init()
Related show

Commit Message

Paul Elder March 4, 2021, 8:47 a.m. UTC 
This is just a demo to show custom parameters to init() with the
raspberrypi IPA interface.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/ipa/raspberrypi.mojom            |  3 ++-
 src/ipa/raspberrypi/raspberrypi.cpp                | 12 +++++++++---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  6 +++++-
 3 files changed, 16 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart March 4, 2021, 8:59 a.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Thu, Mar 04, 2021 at 05:47:43PM +0900, Paul Elder wrote:
> This is just a demo to show custom parameters to init() with the
> raspberrypi IPA interface.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  include/libcamera/ipa/raspberrypi.mojom            |  3 ++-
>  src/ipa/raspberrypi/raspberrypi.cpp                | 12 +++++++++---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  6 +++++-
>  3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index f733a2cd..b8944227 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -51,7 +51,8 @@ struct StartControls {
>  };
>  
>  interface IPARPiInterface {
> -	init(IPASettings settings) => (int32 ret);
> +	init(IPASettings settings, string sensorName)
> +		=> (int32 ret, bool metadataSupport);
>  	start(StartControls controls) => (StartControls result);
>  	stop();
>  
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 6348d071..6a9aba6f 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -79,7 +79,8 @@ public:
>  			munmap(lsTable_, ipa::RPi::MaxLsGridSize);
>  	}
>  
> -	int init(const IPASettings &settings) override;
> +	void init(const IPASettings &settings, const std::string &sensorName,
> +		  int *ret, bool *metadataSupport) override;

Would it make sense to handle the case where the first output parameter
is an int32 to return it directly from the function ?

	int init(const IPASettings &settings, const std::string &sensorName,
		 bool *metadataSupport) override;

Would it be doable without too much effort on the IPC generator side ?

>  	void start(const ipa::RPi::StartControls &data,
>  		   ipa::RPi::StartControls *result) override;
>  	void stop() override {}
> @@ -164,10 +165,15 @@ private:
>  	double maxFrameDuration_;
>  };
>  
> -int IPARPi::init(const IPASettings &settings)
> +void IPARPi::init(const IPASettings &settings, const std::string &sensorName,
> +		  int *ret, bool *metadataSupport)
>  {
> +	LOG(IPARPI, Debug) << "sensor name is " << sensorName;
> +
>  	tuningFile_ = settings.configurationFile;
> -	return 0;
> +
> +	*metadataSupport = true;
> +	*ret = 0;
>  }
>  
>  void IPARPi::start(const ipa::RPi::StartControls &data,
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index db91f1b5..a1c90028 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1194,7 +1194,11 @@ int RPiCameraData::loadIPA()
>  
>  	IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"));
>  
> -	return ipa_->init(settings);
> +	int ret;
> +	bool metadataSupport;
> +	ipa_->init(settings, "sensor name", &ret, &metadataSupport);
> +	LOG(RPI, Debug) << "metadata support " << (metadataSupport ? "yes" : "no");
> +	return ret;
>  }
>  
>  int RPiCameraData::configureIPA(const CameraConfiguration *config)
Paul Elder March 4, 2021, 9:02 a.m. UTC | #2 
Hi Laurent,

On Thu, Mar 04, 2021 at 10:59:46AM +0200, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Thu, Mar 04, 2021 at 05:47:43PM +0900, Paul Elder wrote:
> > This is just a demo to show custom parameters to init() with the
> > raspberrypi IPA interface.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  include/libcamera/ipa/raspberrypi.mojom            |  3 ++-
> >  src/ipa/raspberrypi/raspberrypi.cpp                | 12 +++++++++---
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  6 +++++-
> >  3 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> > index f733a2cd..b8944227 100644
> > --- a/include/libcamera/ipa/raspberrypi.mojom
> > +++ b/include/libcamera/ipa/raspberrypi.mojom
> > @@ -51,7 +51,8 @@ struct StartControls {
> >  };
> >  
> >  interface IPARPiInterface {
> > -	init(IPASettings settings) => (int32 ret);
> > +	init(IPASettings settings, string sensorName)
> > +		=> (int32 ret, bool metadataSupport);
> >  	start(StartControls controls) => (StartControls result);
> >  	stop();
> >  
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 6348d071..6a9aba6f 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -79,7 +79,8 @@ public:
> >  			munmap(lsTable_, ipa::RPi::MaxLsGridSize);
> >  	}
> >  
> > -	int init(const IPASettings &settings) override;
> > +	void init(const IPASettings &settings, const std::string &sensorName,
> > +		  int *ret, bool *metadataSupport) override;
> 
> Would it make sense to handle the case where the first output parameter
> is an int32 to return it directly from the function ?

tbh, I think it would...

> 	int init(const IPASettings &settings, const std::string &sensorName,
> 		 bool *metadataSupport) override;
> 
> Would it be doable without too much effort on the IPC generator side ?

Without "too much" effort, sure :)

I'll put it higher up on my todo list.


Paul

> >  	void start(const ipa::RPi::StartControls &data,
> >  		   ipa::RPi::StartControls *result) override;
> >  	void stop() override {}
> > @@ -164,10 +165,15 @@ private:
> >  	double maxFrameDuration_;
> >  };
> >  
> > -int IPARPi::init(const IPASettings &settings)
> > +void IPARPi::init(const IPASettings &settings, const std::string &sensorName,
> > +		  int *ret, bool *metadataSupport)
> >  {
> > +	LOG(IPARPI, Debug) << "sensor name is " << sensorName;
> > +
> >  	tuningFile_ = settings.configurationFile;
> > -	return 0;
> > +
> > +	*metadataSupport = true;
> > +	*ret = 0;
> >  }
> >  
> >  void IPARPi::start(const ipa::RPi::StartControls &data,
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index db91f1b5..a1c90028 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1194,7 +1194,11 @@ int RPiCameraData::loadIPA()
> >  
> >  	IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"));
> >  
> > -	return ipa_->init(settings);
> > +	int ret;
> > +	bool metadataSupport;
> > +	ipa_->init(settings, "sensor name", &ret, &metadataSupport);
> > +	LOG(RPI, Debug) << "metadata support " << (metadataSupport ? "yes" : "no");
> > +	return ret;
> >  }
> >  
> >  int RPiCameraData::configureIPA(const CameraConfiguration *config)

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
index f733a2cd..b8944227 100644
--- a/include/libcamera/ipa/raspberrypi.mojom
+++ b/include/libcamera/ipa/raspberrypi.mojom
@@ -51,7 +51,8 @@  struct StartControls {
 };
 
 interface IPARPiInterface {
-	init(IPASettings settings) => (int32 ret);
+	init(IPASettings settings, string sensorName)
+		=> (int32 ret, bool metadataSupport);
 	start(StartControls controls) => (StartControls result);
 	stop();
 
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 6348d071..6a9aba6f 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -79,7 +79,8 @@  public:
 			munmap(lsTable_, ipa::RPi::MaxLsGridSize);
 	}
 
-	int init(const IPASettings &settings) override;
+	void init(const IPASettings &settings, const std::string &sensorName,
+		  int *ret, bool *metadataSupport) override;
 	void start(const ipa::RPi::StartControls &data,
 		   ipa::RPi::StartControls *result) override;
 	void stop() override {}
@@ -164,10 +165,15 @@  private:
 	double maxFrameDuration_;
 };
 
-int IPARPi::init(const IPASettings &settings)
+void IPARPi::init(const IPASettings &settings, const std::string &sensorName,
+		  int *ret, bool *metadataSupport)
 {
+	LOG(IPARPI, Debug) << "sensor name is " << sensorName;
+
 	tuningFile_ = settings.configurationFile;
-	return 0;
+
+	*metadataSupport = true;
+	*ret = 0;
 }
 
 void IPARPi::start(const ipa::RPi::StartControls &data,
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index db91f1b5..a1c90028 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1194,7 +1194,11 @@  int RPiCameraData::loadIPA()
 
 	IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"));
 
-	return ipa_->init(settings);
+	int ret;
+	bool metadataSupport;
+	ipa_->init(settings, "sensor name", &ret, &metadataSupport);
+	LOG(RPI, Debug) << "metadata support " << (metadataSupport ? "yes" : "no");
+	return ret;
 }
 
 int RPiCameraData::configureIPA(const CameraConfiguration *config)