[libcamera-devel] [PATCH v1 2/2] DEMO: raspberrypi: Use custom parameters to init()

[paul.elder at ideasonboard.com]

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 at 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)