[libcamera-devel,v3,7/7] libcamera: ipa_manager: Proxy open-source IPAs to a thread

Message ID 20200411001911.1143155-8-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: ipa_manager: Proxy open-source IPAs to a thread
Related show

Commit Message

Niklas Söderlund April 11, 2020, 12:19 a.m. UTC
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

While closed-source IPA modules will always be sandboxed, open-source
IPA modules may be run in the main libcamera process or be sandboxed,
depending on platform configuration. These two models exhibit very
different timings, which require extensive testing with both
configurations.

When run into the main libcamera process, IPA modules are executed in
the pipeline handler thread (which is currently a global CameraManager
thread). Time-consuming operations in the IPA may thus slow down the
pipeline handler and compromise real-time behaviour. At least some
pipeline handlers will thus likely spawn a thread to isolate the IPA,
leading to code duplication in pipeline handlers.

Solve both issues by always proxying IPA modules. For open-source IPA
modules that run in the libcamera process, a new IPAProxyThread class is
added to run the IPA in a separate thread.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
[Niklas: Move thread start/stop of thread into start()/stop()]
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
* Changes since v2
- Move the thread at init() instead of start()
- Call start() in the target thread.
- Block frame actions in IPAProxyThread instead of leaving it up to each
  pipeline handler.
---
 src/libcamera/ipa_interface.cpp          |   3 +
 src/libcamera/ipa_manager.cpp            |  48 +++----
 src/libcamera/proxy/ipa_proxy_thread.cpp | 162 +++++++++++++++++++++++
 src/libcamera/proxy/meson.build          |   1 +
 4 files changed, 187 insertions(+), 27 deletions(-)
 create mode 100644 src/libcamera/proxy/ipa_proxy_thread.cpp

Patch

diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index d8d57fdb2ae59d2e..8eaa1480e6dfdc9c 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -528,6 +528,9 @@  namespace libcamera {
  * \a data.operation field, as defined by the IPA protocol, and the rest of the
  * \a data is interpreted accordingly. The pipeline handler shall queue the
  * action and execute it as appropriate.
+ *
+ * The signal is only emitted when the IPA is running, that is after start() and
+ * before stop() have been called.
  */
 
 } /* namespace libcamera */
diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index bcaae3564ea14e07..6d23f470506561b8 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -12,7 +12,6 @@ 
 #include <string.h>
 #include <sys/types.h>
 
-#include "ipa_context_wrapper.h"
 #include "ipa_module.h"
 #include "ipa_proxy.h"
 #include "log.h"
@@ -271,40 +270,35 @@  std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
 	if (!m)
 		return nullptr;
 
-	if (!m->isOpenSource()) {
-		IPAProxyFactory *pf = nullptr;
-		std::vector<IPAProxyFactory *> &factories = IPAProxyFactory::factories();
+	/*
+	 * Load and run the IPA module in a thread if it is open-source, or
+	 * isolate it in a separate process otherwise.
+	 *
+	 * \todo Implement a better proxy selection
+	 */
+	const char *proxyName = m->isOpenSource()
+			      ? "IPAProxyThread" : "IPAProxyLinux";
+	IPAProxyFactory *pf = nullptr;
 
-		for (IPAProxyFactory *factory : factories) {
-			/* TODO: Better matching */
-			if (!strcmp(factory->name().c_str(), "IPAProxyLinux")) {
-				pf = factory;
-				break;
-			}
+	for (IPAProxyFactory *factory : IPAProxyFactory::factories()) {
+		if (!strcmp(factory->name().c_str(), proxyName)) {
+			pf = factory;
+			break;
 		}
-
-		if (!pf) {
-			LOG(IPAManager, Error) << "Failed to get proxy factory";
-			return nullptr;
-		}
-
-		std::unique_ptr<IPAProxy> proxy = pf->create(m);
-		if (!proxy->isValid()) {
-			LOG(IPAManager, Error) << "Failed to load proxy";
-			return nullptr;
-		}
-
-		return proxy;
 	}
 
-	if (!m->load())
+	if (!pf) {
+		LOG(IPAManager, Error) << "Failed to get proxy factory";
 		return nullptr;
+	}
 
-	struct ipa_context *ctx = m->createContext();
-	if (!ctx)
+	std::unique_ptr<IPAProxy> proxy = pf->create(m);
+	if (!proxy->isValid()) {
+		LOG(IPAManager, Error) << "Failed to load proxy";
 		return nullptr;
+	}
 
-	return std::make_unique<IPAContextWrapper>(ctx);
+	return proxy;
 }
 
 } /* namespace libcamera */
diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
new file mode 100644
index 0000000000000000..368ccca1cf60b8ec
--- /dev/null
+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
@@ -0,0 +1,162 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2020, Google Inc.
+ *
+ * ipa_proxy_thread.cpp - Proxy running an Image Processing Algorithm in a thread
+ */
+
+#include <memory>
+
+#include <ipa/ipa_interface.h>
+#include <ipa/ipa_module_info.h>
+
+#include "ipa_context_wrapper.h"
+#include "ipa_module.h"
+#include "ipa_proxy.h"
+#include "log.h"
+#include "thread.h"
+
+namespace libcamera {
+
+LOG_DECLARE_CATEGORY(IPAProxy)
+
+class IPAProxyThread : public IPAProxy, public Object
+{
+public:
+	IPAProxyThread(IPAModule *ipam);
+
+	int init() override;
+	int start() override;
+	void stop() override;
+
+	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;
+	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
+	void unmapBuffers(const std::vector<unsigned int> &ids) override;
+	void processEvent(const IPAOperationData &event) override;
+
+private:
+	void queueFrameAction(unsigned int frame, const IPAOperationData &data);
+
+	/* Helper class to invoke processEvent() in another thread. */
+	class ThreadProxy : public Object
+	{
+	public:
+		void setIPA(IPAInterface *ipa)
+		{
+			ipa_ = ipa;
+		}
+
+		int start()
+		{
+			return ipa_->start();
+		}
+
+		void stop()
+		{
+			ipa_->stop();
+		}
+
+		void processEvent(const IPAOperationData &event)
+		{
+			ipa_->processEvent(event);
+		}
+
+	private:
+		IPAInterface *ipa_;
+	};
+
+	bool running_;
+	Thread thread_;
+	ThreadProxy proxy_;
+	std::unique_ptr<IPAInterface> ipa_;
+};
+
+IPAProxyThread::IPAProxyThread(IPAModule *ipam)
+	: running_(false)
+{
+	if (!ipam->load())
+		return;
+
+	struct ipa_context *ctx = ipam->createContext();
+	if (!ctx) {
+		LOG(IPAProxy, Error)
+			<< "Failed to create IPA context for " << ipam->path();
+		return;
+	}
+
+	ipa_ = std::make_unique<IPAContextWrapper>(ctx);
+	proxy_.setIPA(ipa_.get());
+
+	/*
+	 * Proxy the queueFrameAction signal to dispatch it in the caller's
+	 * thread.
+	 */
+	ipa_->queueFrameAction.connect(this, &IPAProxyThread::queueFrameAction);
+
+	valid_ = true;
+}
+
+int IPAProxyThread::init()
+{
+	int ret = ipa_->init();
+	if (ret)
+		return ret;
+
+	proxy_.moveToThread(&thread_);
+
+	return 0;
+}
+
+int IPAProxyThread::start()
+{
+	running_ = true;
+	thread_.start();
+
+	return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking);
+}
+
+void IPAProxyThread::stop()
+{
+	running_ = false;
+
+	proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking);
+
+	thread_.exit();
+	thread_.wait();
+}
+
+void IPAProxyThread::configure(const std::map<unsigned int, IPAStream> &streamConfig,
+			       const std::map<unsigned int, const ControlInfoMap &> &entityControls)
+{
+	ipa_->configure(streamConfig, entityControls);
+}
+
+void IPAProxyThread::mapBuffers(const std::vector<IPABuffer> &buffers)
+{
+	ipa_->mapBuffers(buffers);
+}
+
+void IPAProxyThread::unmapBuffers(const std::vector<unsigned int> &ids)
+{
+	ipa_->unmapBuffers(ids);
+}
+
+void IPAProxyThread::processEvent(const IPAOperationData &event)
+{
+	if (!running_)
+		return;
+
+	/* Dispatch the processEvent() call to the thread. */
+	proxy_.invokeMethod(&ThreadProxy::processEvent, ConnectionTypeQueued,
+			    event);
+}
+
+void IPAProxyThread::queueFrameAction(unsigned int frame, const IPAOperationData &data)
+{
+	IPAInterface::queueFrameAction.emit(frame, data);
+}
+
+REGISTER_IPA_PROXY(IPAProxyThread)
+
+} /* namespace libcamera */
diff --git a/src/libcamera/proxy/meson.build b/src/libcamera/proxy/meson.build
index efc1132302176f63..6c00d5f30ad2e5f0 100644
--- a/src/libcamera/proxy/meson.build
+++ b/src/libcamera/proxy/meson.build
@@ -1,3 +1,4 @@ 
 libcamera_sources += files([
     'ipa_proxy_linux.cpp',
+    'ipa_proxy_thread.cpp',
 ])