[libcamera-devel,3/5] libcamera: ipa: Pass a set of controls and return results from ipa::start()
diff mbox series

Message ID 20201002093309.109180-4-naush@raspberrypi.com
State Superseded
Headers show
Series
  • RFQ: Pass controls on camera:start()
Related show

Commit Message

Naushir Patuck Oct. 2, 2020, 9:33 a.m. UTC 
This change allows controls passed into pipeline_hanlder::start to be
forwarded onto ipa::start(). We also add a return channel if the
pipeline handle must action any of these controls, e.g. setting the
analogue gain or shutter speed in the sensor device.

todo: The IPA interface wrapper needs a translation for passing
IPAOperationData into start() and configure()

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 include/libcamera/internal/ipa_context_wrapper.h   |  3 ++-
 include/libcamera/ipa/ipa_interface.h              |  3 ++-
 src/ipa/libipa/ipa_interface_wrapper.cpp           |  4 +++-
 src/ipa/raspberrypi/raspberrypi.cpp                |  3 ++-
 src/ipa/rkisp1/rkisp1.cpp                          |  3 ++-
 src/ipa/vimc/vimc.cpp                              |  6 ++++--
 src/libcamera/ipa_context_wrapper.cpp              |  6 ++++--
 src/libcamera/ipa_interface.cpp                    |  7 +++++++
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  4 +++-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp           |  5 +++--
 src/libcamera/pipeline/vimc/vimc.cpp               |  3 ++-
 src/libcamera/proxy/ipa_proxy_linux.cpp            |  3 ++-
 src/libcamera/proxy/ipa_proxy_thread.cpp           | 13 ++++++++-----
 test/ipa/ipa_interface_test.cpp                    |  3 ++-
 test/ipa/ipa_wrappers_test.cpp                     |  5 +++--
 15 files changed, 49 insertions(+), 22 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h
index 8f767e84..1878a615 100644
--- a/include/libcamera/internal/ipa_context_wrapper.h
+++ b/include/libcamera/internal/ipa_context_wrapper.h
@@ -20,7 +20,8 @@  public:
 	~IPAContextWrapper();
 
 	int init(const IPASettings &settings) override;
-	int start() override;
+	int start(const IPAOperationData &ipaConfig,
+		  IPAOperationData *result) override;
 	void stop() override;
 	void configure(const CameraSensorInfo &sensorInfo,
 		       const std::map<unsigned int, IPAStream> &streamConfig,
diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
index 5016ec25..ee7b2e87 100644
--- a/include/libcamera/ipa/ipa_interface.h
+++ b/include/libcamera/ipa/ipa_interface.h
@@ -153,7 +153,8 @@  public:
 	virtual ~IPAInterface() {}
 
 	virtual int init(const IPASettings &settings) = 0;
-	virtual int start() = 0;
+	virtual int start(const IPAOperationData &ipaConfig,
+			  IPAOperationData *result) = 0;
 	virtual void stop() = 0;
 
 	virtual void configure(const CameraSensorInfo &sensorInfo,
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index cee532e3..fb2da9d3 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -95,7 +95,9 @@  int IPAInterfaceWrapper::start(struct ipa_context *_ctx)
 {
 	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
 
-	return ctx->ipa_->start();
+	/* \todo Translate the ipaConfig and result. */
+	IPAOperationData ipaConfig;
+	return ctx->ipa_->start(ipaConfig, nullptr);
 }
 
 void IPAInterfaceWrapper::stop(struct ipa_context *_ctx)
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index b0c7d1c1..9610229a 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -77,7 +77,8 @@  public:
 	}
 
 	int init(const IPASettings &settings) override;
-	int start() override { return 0; }
+	int start([[maybe_unused]] const IPAOperationData &ipaConfig,
+		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
 	void stop() override {}
 
 	void configure(const CameraSensorInfo &sensorInfo,
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 07d7f1b2..0b8a9096 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -37,7 +37,8 @@  public:
 	{
 		return 0;
 	}
-	int start() override { return 0; }
+	int start([[maybe_unused]] const IPAOperationData &ipaConfig,
+		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
 	void stop() override {}
 
 	void configure(const CameraSensorInfo &info,
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index ef257762..07951c17 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -34,7 +34,8 @@  public:
 
 	int init(const IPASettings &settings) override;
 
-	int start() override;
+	int start(const IPAOperationData &ipaConfig,
+		  IPAOperationData *result) override;
 	void stop() override;
 
 	void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
@@ -82,7 +83,8 @@  int IPAVimc::init(const IPASettings &settings)
 	return 0;
 }
 
-int IPAVimc::start()
+int IPAVimc::start([[maybe_unused]] const IPAOperationData &ipaConfig,
+		   [[maybe_unused]] IPAOperationData *result)
 {
 	trace(IPAOperationStart);
 
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index 231300ce..de96a606 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -86,14 +86,16 @@  int IPAContextWrapper::init(const IPASettings &settings)
 	return 0;
 }
 
-int IPAContextWrapper::start()
+int IPAContextWrapper::start(const IPAOperationData &ipaConfig,
+			     IPAOperationData *result)
 {
 	if (intf_)
-		return intf_->start();
+		return intf_->start(ipaConfig, result);
 
 	if (!ctx_)
 		return 0;
 
+	/* \todo Translate the ipaConfig and reponse */
 	return ctx_->ops->start(ctx_);
 }
 
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index 23fc56d7..282c3c0f 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -536,10 +536,17 @@  namespace libcamera {
 /**
  * \fn IPAInterface::start()
  * \brief Start the IPA
+ * \param[in] ipaConfig Pipeline-handler-specific configuration data
+ * \param[out] result Pipeline-handler-specific configuration result
  *
  * This method informs the IPA module that the camera is about to be started.
  * The IPA module shall prepare any resources it needs to operate.
  *
+ * The \a ipaConfig and \a result parameters carry custom data passed by the
+ * pipeline handler to the IPA and back. The pipeline handler may set the \a
+ * result parameter to null if the IPA protocol doesn't need to pass a result
+ * back through the configure() function.
+ *
  * \return 0 on success or a negative error code otherwise
  */
 
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 307ed9a2..2e0b0dcf 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -733,7 +733,9 @@  int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
 	}
 
 	/* Start the IPA. */
-	ret = data->ipa_->start();
+	IPAOperationData ipaConfig, result;
+	ipaConfig.controls.emplace_back(*controls);
+	ret = data->ipa_->start(ipaConfig, &result);
 	if (ret) {
 		LOG(RPI, Error)
 			<< "Failed to start IPA for " << camera->id();
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 15af7e5c..872bfc07 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -832,7 +832,8 @@  int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
 	if (ret)
 		return ret;
 
-	ret = data->ipa_->start();
+	IPAOperationData ipaConfig = {};
+	ret = data->ipa_->start(ipaConfig, nullptr);
 	if (ret) {
 		freeBuffers(camera);
 		LOG(RkISP1, Error)
@@ -911,7 +912,7 @@  int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
 	std::map<unsigned int, const ControlInfoMap &> entityControls;
 	entityControls.emplace(0, data->sensor_->controls());
 
-	IPAOperationData ipaConfig;
+	ipaConfig = {};
 	data->ipa_->configure(sensorInfo, streamConfig, entityControls,
 			      ipaConfig, nullptr);
 
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 3b868e4b..8eb1998d 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -322,7 +322,8 @@  int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con
 	if (ret < 0)
 		return ret;
 
-	ret = data->ipa_->start();
+	IPAOperationData ipaConfig = {};
+	ret = data->ipa_->start(ipaConfig, nullptr);
 	if (ret) {
 		data->video_->releaseBuffers();
 		return ret;
diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
index b78a0e45..619976e5 100644
--- a/src/libcamera/proxy/ipa_proxy_linux.cpp
+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
@@ -30,7 +30,8 @@  public:
 	{
 		return 0;
 	}
-	int start() override { return 0; }
+	int start([[maybe_unused]] const IPAOperationData &ipaConfig,
+		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
 	void stop() override {}
 	void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
 		       [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
index eead2883..9a81b6e7 100644
--- a/src/libcamera/proxy/ipa_proxy_thread.cpp
+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
@@ -26,7 +26,8 @@  public:
 	IPAProxyThread(IPAModule *ipam);
 
 	int init(const IPASettings &settings) override;
-	int start() override;
+	int start(const IPAOperationData &ipaConfig,
+		  IPAOperationData *result) override;
 	void stop() override;
 
 	void configure(const CameraSensorInfo &sensorInfo,
@@ -50,9 +51,9 @@  private:
 			ipa_ = ipa;
 		}
 
-		int start()
+		int start(const IPAOperationData &ipaConfig, IPAOperationData *result)
 		{
-			return ipa_->start();
+			return ipa_->start(ipaConfig, result);
 		}
 
 		void stop()
@@ -111,12 +112,14 @@  int IPAProxyThread::init(const IPASettings &settings)
 	return 0;
 }
 
-int IPAProxyThread::start()
+int IPAProxyThread::start(const IPAOperationData &ipaConfig,
+			  IPAOperationData *result)
 {
 	running_ = true;
 	thread_.start();
 
-	return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking);
+	return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking,
+				   ipaConfig, result);
 }
 
 void IPAProxyThread::stop()
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index 1bc93a63..a7bfcce8 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -120,7 +120,8 @@  protected:
 		}
 
 		/* Test start of IPA module. */
-		ipa_->start();
+		IPAOperationData ipaConfig;
+		ipa_->start(ipaConfig, nullptr);
 		timer.start(1000);
 		while (timer.isRunning() && trace_ != IPAOperationStart)
 			dispatcher->processEvents();
diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
index 59d991cb..7b8ae77b 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -56,7 +56,8 @@  public:
 		return 0;
 	}
 
-	int start() override
+	int start([[maybe_unused]] const IPAOperationData &ipaConfig,
+		  [[maybe_unused]] IPAOperationData *result) override
 	{
 		report(Op_start, TestPass);
 		return 0;
@@ -376,7 +377,7 @@  protected:
 			return TestFail;
 		}
 
-		ret = INVOKE(start);
+		ret = INVOKE(start, ipaConfig, nullptr);
 		if (ret == TestFail) {
 			cerr << "Failed to run start()";
 			return TestFail;