From 0219dc71b3c8eea8833f9a8214fac672b54613c2 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Fri, 20 Mar 2020 03:16:18 +0200
Subject: libcamera: ipa_manager: Proxy open-source IPAs to a thread
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 src/libcamera/ipa_manager.cpp | 48 +++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

(limited to 'src/libcamera/ipa_manager.cpp')

diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index bcaae356..6d23f470 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();
-
-		for (IPAProxyFactory *factory : factories) {
-			/* TODO: Better matching */
-			if (!strcmp(factory->name().c_str(), "IPAProxyLinux")) {
-				pf = factory;
-				break;
-			}
-		}
-
-		if (!pf) {
-			LOG(IPAManager, Error) << "Failed to get proxy factory";
-			return nullptr;
-		}
+	/*
+	 * 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;
 
-		std::unique_ptr<IPAProxy> proxy = pf->create(m);
-		if (!proxy->isValid()) {
-			LOG(IPAManager, Error) << "Failed to load proxy";
-			return nullptr;
+	for (IPAProxyFactory *factory : IPAProxyFactory::factories()) {
+		if (!strcmp(factory->name().c_str(), proxyName)) {
+			pf = factory;
+			break;
 		}
-
-		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 */
-- 
cgit v1.2.1