[libcamera-devel] [PATCH v8 06/12] libcamera: IPAProxy, IPAManager: Switch to one-proxy-per-pipeline scheme

Paul Elder paul.elder at ideasonboard.com
Sat Feb 13 05:22:19 CET 2021


IPAProxy is changed in two major ways:
- Every pipeline has its own proxy, to support each pipeline's IPA
  interface
- IPAProxy implementations always encapsulate IPA modules, and switch
  internally for isolation or threaded

The IPAProxy registration mechanism is removed, as each pipeline will
have its own proxy, so the pipeline can pass the specialized class name
of the IPAProxy to the IPAManager for construction.

IPAManager is changed accordingly to support these changes:
- createIPA is a template function that takes an IPAProxy class, and
  always returns an IPAProxy
- IPAManager no longer decides on isolation, and simply creates an
  IPAProxy instance while passing the isolation flag

Consequently, the old IPAProxy classes (IPAProxyThread and
IPAProxyLinux) are removed. The IPAInterfaceTest is updated to use
the new IPAManager interface, and to construct a ProcessManager as no
single global instance is created anymore.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>

---
No change in v8

No change in v7

Changes in v6:
- remove include vimc.h
- use the ipa::vimc namespace
- use the new start() IPA API for vimc

Squashed in v5

Changes in v4
- rename libcamera_generated_headers to libcamera_generated_ipa_headers

Changes in v3:
- declare ProcessManager
- add libcamera_generated_headers as dependency to meson
  - otherwise test might build before the generated IPA headers and
    #include will fail

Changes in v2:
- document isolate argument
- remove documentation

---

This is a combination of 7 commits:

---

libcamera: IPAProxy: Add isolate parameter to create()

Since IPAProxy implementations now always encapsulate IPA modules, add a
parameter to create() to signal if the proxy should isolate the IPA or not.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---

libcamera: IPAManager: Fetch IPAProxy corresponding to pipeline

Now that each pipeline handler has its own IPAProxy implementation, make
the IPAManager fetch the IPAProxy based on the pipeline handler name.
Also, since the IPAProxy is used regardless of isolation or no
isolation, remove the isolation check from the proxy selection.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

---

libcamera: IPAManager: add isolation flag to proxy creation

When the IPA proxy is created, it needs to know whether to isolate or
not. Feed the flag at creation of the IPA proxy.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---

libcamera: IPAManager: Make createIPA return proxy directly

Since every pipeline knows the type of the proxy that it needs, and
since all IPAs are to be wrapped in a proxy, IPAManager no longer needs
to search in the factory list to fetch the proxy factory to construct a
factory. Instead, we define createIPA as a template function, and the
pipeline handler can declare the proxy type when it calls createIPA.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---

libcamera: IPAProxy: Remove registration mechanism

Implementations of IPA proxies use a registration mechanism to register
themselves with the main IPA proxy factory. This registration declares
static objects, causing a risk of things being constructed before the
proper libcamera facilities are ready. Since each pipeline handler has
its own IPA proxy and knows the type, it isn't necessary to have a proxy
factory. Remove it to alleviate the risk of early construction.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---

libcamera: proxy: Remove IPAProxyLinux and IPAProxyThread

We have now changed the proxy from per-IPC mechanism to per-pipeline.
The per-IPC mechanism proxies are thus no longer needed; remove them.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---

tests: ipa_interface_test: Update to use new createIPA

Update the IPA interface test to use the new createIPA function from
IPAManager. Also create an instance of ProcessManager, as no single
global instance is created automatically anymore. Update meson.build to
to depend on the generated IPA interface headers.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 Documentation/Doxyfile.in                     |   1 -
 include/libcamera/internal/ipa_manager.h      |  31 +++-
 include/libcamera/internal/ipa_proxy.h        |  29 ---
 src/libcamera/ipa_manager.cpp                 |  47 +----
 src/libcamera/ipa_proxy.cpp                   |  91 ---------
 src/libcamera/proxy/ipa_proxy_linux.cpp       | 104 -----------
 src/libcamera/proxy/ipa_proxy_thread.cpp      | 175 ------------------
 src/libcamera/proxy/meson.build               |   5 -
 .../proxy/worker/ipa_proxy_linux_worker.cpp   |  90 ---------
 src/libcamera/proxy/worker/meson.build        |   4 -
 test/ipa/ipa_interface_test.cpp               |  44 ++---
 test/ipa/meson.build                          |   2 +-
 12 files changed, 53 insertions(+), 570 deletions(-)
 delete mode 100644 src/libcamera/proxy/ipa_proxy_linux.cpp
 delete mode 100644 src/libcamera/proxy/ipa_proxy_thread.cpp
 delete mode 100644 src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp

diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
index e0de6c6d..af006724 100644
--- a/Documentation/Doxyfile.in
+++ b/Documentation/Doxyfile.in
@@ -842,7 +842,6 @@ EXCLUDE                = @TOP_SRCDIR@/include/libcamera/span.h \
 			 @TOP_SRCDIR@/src/libcamera/device_enumerator_udev.cpp \
 			 @TOP_SRCDIR@/src/libcamera/ipc_pipe_unixsocket.cpp \
 			 @TOP_SRCDIR@/src/libcamera/pipeline/ \
-			 @TOP_SRCDIR@/src/libcamera/proxy/ \
 			 @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
 			 @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
 			 @TOP_BUILDDIR@/include/libcamera/ipa/ \
diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
index 4a143b6a..e904a2be 100644
--- a/include/libcamera/internal/ipa_manager.h
+++ b/include/libcamera/internal/ipa_manager.h
@@ -14,20 +14,45 @@
 #include <libcamera/ipa/ipa_module_info.h>
 
 #include "libcamera/internal/ipa_module.h"
+#include "libcamera/internal/log.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/pub_key.h"
 
 namespace libcamera {
 
+LOG_DECLARE_CATEGORY(IPAManager)
+
 class IPAManager
 {
 public:
 	IPAManager();
 	~IPAManager();
 
-	static std::unique_ptr<IPAProxy> createIPA(PipelineHandler *pipe,
-						   uint32_t maxVersion,
-						   uint32_t minVersion);
+	template<typename T>
+	static std::unique_ptr<T> createIPA(PipelineHandler *pipe,
+					    uint32_t maxVersion,
+					    uint32_t minVersion)
+	{
+		IPAModule *m = nullptr;
+
+		for (IPAModule *module : self_->modules_) {
+			if (module->match(pipe, minVersion, maxVersion)) {
+				m = module;
+				break;
+			}
+		}
+
+		if (!m)
+			return nullptr;
+
+		std::unique_ptr<T> proxy = std::make_unique<T>(m, !self_->isSignatureValid(m));
+		if (!proxy->isValid()) {
+			LOG(IPAManager, Error) << "Failed to load proxy";
+			return nullptr;
+		}
+
+		return proxy;
+	}
 
 private:
 	static IPAManager *self_;
diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h
index 59a5b841..f651a3ae 100644
--- a/include/libcamera/internal/ipa_proxy.h
+++ b/include/libcamera/internal/ipa_proxy.h
@@ -36,35 +36,6 @@ private:
 	IPAModule *ipam_;
 };
 
-class IPAProxyFactory
-{
-public:
-	IPAProxyFactory(const char *name);
-	virtual ~IPAProxyFactory() = default;
-
-	virtual std::unique_ptr<IPAProxy> create(IPAModule *ipam) = 0;
-
-	const std::string &name() const { return name_; }
-
-	static void registerType(IPAProxyFactory *factory);
-	static std::vector<IPAProxyFactory *> &factories();
-
-private:
-	std::string name_;
-};
-
-#define REGISTER_IPA_PROXY(proxy)			\
-class proxy##Factory final : public IPAProxyFactory	\
-{							\
-public:							\
-	proxy##Factory() : IPAProxyFactory(#proxy) {}	\
-	std::unique_ptr<IPAProxy> create(IPAModule *ipam)	\
-	{						\
-		return std::make_unique<proxy>(ipam);	\
-	}						\
-};							\
-static proxy##Factory global_##proxy##Factory;
-
 } /* namespace libcamera */
 
 #endif /* __LIBCAMERA_INTERNAL_IPA_PROXY_H__ */
diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index ad05b9c9..93d02d94 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -245,6 +245,7 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)
 }
 
 /**
+ * \fn IPAManager::createIPA()
  * \brief Create an IPA proxy that matches a given pipeline handler
  * \param[in] pipe The pipeline handler that wants a matching IPA proxy
  * \param[in] minVersion Minimum acceptable version of IPA module
@@ -253,52 +254,6 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)
  * \return A newly created IPA proxy, or nullptr if no matching IPA module is
  * found or if the IPA proxy fails to initialize
  */
-std::unique_ptr<IPAProxy> IPAManager::createIPA(PipelineHandler *pipe,
-						uint32_t maxVersion,
-						uint32_t minVersion)
-{
-	IPAModule *m = nullptr;
-
-	for (IPAModule *module : self_->modules_) {
-		if (module->match(pipe, minVersion, maxVersion)) {
-			m = module;
-			break;
-		}
-	}
-
-	if (!m)
-		return nullptr;
-
-	/*
-	 * Load and run the IPA module in a thread if it has a valid signature,
-	 * or isolate it in a separate process otherwise.
-	 *
-	 * \todo Implement a better proxy selection
-	 */
-	const char *proxyName = self_->isSignatureValid(m)
-			      ? "IPAProxyThread" : "IPAProxyLinux";
-	IPAProxyFactory *pf = nullptr;
-
-	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;
-}
 
 bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const
 {
diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
index 23be24ad..29c0e9e0 100644
--- a/src/libcamera/ipa_proxy.cpp
+++ b/src/libcamera/ipa_proxy.cpp
@@ -30,17 +30,11 @@ LOG_DEFINE_CATEGORY(IPAProxy)
  * \brief IPA Proxy
  *
  * Isolate IPA into separate process.
- *
- * Every subclass of proxy shall be registered with libcamera using
- * the REGISTER_IPA_PROXY() macro.
  */
 
 /**
  * \brief Construct an IPAProxy instance
  * \param[in] ipam The IPA module
- *
- * IPAProxy instances shall be constructed through the IPAProxyFactory::create()
- * method implemented by the respective factories.
  */
 IPAProxy::IPAProxy(IPAModule *ipam)
 	: valid_(false), ipam_(ipam)
@@ -219,89 +213,4 @@ std::string IPAProxy::resolvePath(const std::string &file) const
  * construction.
  */
 
-/**
- * \class IPAProxyFactory
- * \brief Registration of IPAProxy classes and creation of instances
- *
- * To facilitate discovery and instantiation of IPAProxy classes, the
- * IPAProxyFactory class maintains a registry of IPAProxy classes. Each
- * IPAProxy subclass shall register itself using the REGISTER_IPA_PROXY()
- * macro, which will create a corresponding instance of a IPAProxyFactory
- * subclass and register it with the static list of factories.
- */
-
-/**
- * \brief Construct a IPAProxy factory
- * \param[in] name Name of the IPAProxy class
- *
- * Creating an instance of the factory registers is with the global list of
- * factories, accessible through the factories() function.
- *
- * The factory \a name is used for debugging and IPAProxy matching purposes
- * and shall be unique.
- */
-IPAProxyFactory::IPAProxyFactory(const char *name)
-	: name_(name)
-{
-	registerType(this);
-}
-
-/**
- * \fn IPAProxyFactory::create()
- * \brief Create an instance of the IPAProxy corresponding to the factory
- * \param[in] ipam The IPA module
- *
- * This virtual function is implemented by the REGISTER_IPA_PROXY() macro.
- * It creates a IPAProxy instance that isolates an IPA interface designated
- * by the IPA module \a ipam.
- *
- * \return A pointer to a newly constructed instance of the IPAProxy subclass
- * corresponding to the factory
- */
-
-/**
- * \fn IPAProxyFactory::name()
- * \brief Retrieve the factory name
- * \return The factory name
- */
-
-/**
- * \brief Add a IPAProxy class to the registry
- * \param[in] factory Factory to use to construct the IPAProxy
- *
- * The caller is responsible to guarantee the uniqueness of the IPAProxy name.
- */
-void IPAProxyFactory::registerType(IPAProxyFactory *factory)
-{
-	std::vector<IPAProxyFactory *> &factories = IPAProxyFactory::factories();
-
-	factories.push_back(factory);
-
-	LOG(IPAProxy, Debug)
-		<< "Registered proxy \"" << factory->name() << "\"";
-}
-
-/**
- * \brief Retrieve the list of all IPAProxy factories
- *
- * The static factories map is defined inside the function to ensure it gets
- * initialized on first use, without any dependency on link order.
- *
- * \return The list of pipeline handler factories
- */
-std::vector<IPAProxyFactory *> &IPAProxyFactory::factories()
-{
-	static std::vector<IPAProxyFactory *> factories;
-	return factories;
-}
-
-/**
- * \def REGISTER_IPA_PROXY
- * \brief Register a IPAProxy with the IPAProxy factory
- * \param[in] proxy Class name of IPAProxy derived class to register
- *
- * Register a proxy subclass with the factory and make it available to
- * isolate IPA modules.
- */
-
 } /* namespace libcamera */
diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
deleted file mode 100644
index ea6f3e5e..00000000
--- a/src/libcamera/proxy/ipa_proxy_linux.cpp
+++ /dev/null
@@ -1,104 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2019, Google Inc.
- *
- * ipa_proxy_linux.cpp - Default Image Processing Algorithm proxy for Linux
- */
-
-#include <vector>
-
-#include <libcamera/ipa/ipa_interface.h>
-#include <libcamera/ipa/ipa_module_info.h>
-
-#include "libcamera/internal/ipa_module.h"
-#include "libcamera/internal/ipa_proxy.h"
-#include "libcamera/internal/ipc_unixsocket.h"
-#include "libcamera/internal/log.h"
-#include "libcamera/internal/process.h"
-
-namespace libcamera {
-
-LOG_DECLARE_CATEGORY(IPAProxy)
-
-class IPAProxyLinux : public IPAProxy
-{
-public:
-	IPAProxyLinux(IPAModule *ipam);
-	~IPAProxyLinux();
-
-	int init([[maybe_unused]] const IPASettings &settings) override
-	{
-		return 0;
-	}
-	int start([[maybe_unused]] const IPAOperationData &data,
-		  [[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,
-		       [[maybe_unused]] const std::map<unsigned int, const ControlInfoMap &> &entityControls,
-		       [[maybe_unused]] const IPAOperationData &ipaConfig,
-		       [[maybe_unused]] IPAOperationData *result) override {}
-	void mapBuffers([[maybe_unused]] const std::vector<IPABuffer> &buffers) override {}
-	void unmapBuffers([[maybe_unused]] const std::vector<unsigned int> &ids) override {}
-	void processEvent([[maybe_unused]] const IPAOperationData &event) override {}
-
-private:
-	void readyRead(IPCUnixSocket *ipc);
-
-	Process *proc_;
-
-	IPCUnixSocket *socket_;
-};
-
-IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)
-	: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)
-{
-	LOG(IPAProxy, Debug)
-		<< "initializing dummy proxy: loading IPA from "
-		<< ipam->path();
-
-	std::vector<int> fds;
-	std::vector<std::string> args;
-	args.push_back(ipam->path());
-	const std::string path = resolvePath("ipa_proxy_linux");
-	if (path.empty()) {
-		LOG(IPAProxy, Error)
-			<< "Failed to get proxy worker path";
-		return;
-	}
-
-	socket_ = new IPCUnixSocket();
-	int fd = socket_->create();
-	if (fd < 0) {
-		LOG(IPAProxy, Error)
-			<< "Failed to create socket";
-		return;
-	}
-	socket_->readyRead.connect(this, &IPAProxyLinux::readyRead);
-	args.push_back(std::to_string(fd));
-	fds.push_back(fd);
-
-	proc_ = new Process();
-	int ret = proc_->start(path, args, fds);
-	if (ret) {
-		LOG(IPAProxy, Error)
-			<< "Failed to start proxy worker process";
-		return;
-	}
-
-	valid_ = true;
-}
-
-IPAProxyLinux::~IPAProxyLinux()
-{
-	delete proc_;
-	delete socket_;
-}
-
-void IPAProxyLinux::readyRead([[maybe_unused]] IPCUnixSocket *ipc)
-{
-}
-
-REGISTER_IPA_PROXY(IPAProxyLinux)
-
-} /* namespace libcamera */
diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
deleted file mode 100644
index a5fda2c8..00000000
--- a/src/libcamera/proxy/ipa_proxy_thread.cpp
+++ /dev/null
@@ -1,175 +0,0 @@
-/* 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 <libcamera/ipa/ipa_interface.h>
-#include <libcamera/ipa/ipa_module_info.h>
-
-#include "libcamera/internal/ipa_context_wrapper.h"
-#include "libcamera/internal/ipa_module.h"
-#include "libcamera/internal/ipa_proxy.h"
-#include "libcamera/internal/log.h"
-#include "libcamera/internal/thread.h"
-
-namespace libcamera {
-
-LOG_DECLARE_CATEGORY(IPAProxy)
-
-class IPAProxyThread : public IPAProxy, public Object
-{
-public:
-	IPAProxyThread(IPAModule *ipam);
-
-	int init(const IPASettings &settings) override;
-	int start(const IPAOperationData &data,
-		  IPAOperationData *result) override;
-	void stop() override;
-
-	void configure(const CameraSensorInfo &sensorInfo,
-		       const std::map<unsigned int, IPAStream> &streamConfig,
-		       const std::map<unsigned int, const ControlInfoMap &> &entityControls,
-		       const IPAOperationData &ipaConfig,
-		       IPAOperationData *result) 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(const IPAOperationData &data, IPAOperationData *result)
-		{
-			return ipa_->start(data, result);
-		}
-
-		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)
-	: IPAProxy(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(const IPASettings &settings)
-{
-	int ret = ipa_->init(settings);
-	if (ret)
-		return ret;
-
-	proxy_.moveToThread(&thread_);
-
-	return 0;
-}
-
-int IPAProxyThread::start(const IPAOperationData &data,
-			  IPAOperationData *result)
-{
-	running_ = true;
-	thread_.start();
-
-	return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking,
-				   data, result);
-}
-
-void IPAProxyThread::stop()
-{
-	if (!running_)
-		return;
-
-	running_ = false;
-
-	proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking);
-
-	thread_.exit();
-	thread_.wait();
-}
-
-void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,
-			       const std::map<unsigned int, IPAStream> &streamConfig,
-			       const std::map<unsigned int, const ControlInfoMap &> &entityControls,
-			       const IPAOperationData &ipaConfig,
-			       IPAOperationData *result)
-{
-	ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,
-			result);
-}
-
-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 5c4365e7..00ae5a8f 100644
--- a/src/libcamera/proxy/meson.build
+++ b/src/libcamera/proxy/meson.build
@@ -1,10 +1,5 @@
 # SPDX-License-Identifier: CC0-1.0
 
-libcamera_sources += files([
-    'ipa_proxy_linux.cpp',
-    'ipa_proxy_thread.cpp',
-])
-
 # generate {pipeline}_ipa_proxy.cpp
 foreach mojom : ipa_mojoms
     proxy = custom_target(mojom['name'] + '_proxy_cpp',
diff --git a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp
deleted file mode 100644
index bdbac988..00000000
--- a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp
+++ /dev/null
@@ -1,90 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2019, Google Inc.
- *
- * ipa_proxy_linux_worker.cpp - Default Image Processing Algorithm proxy worker for Linux
- */
-
-#include <iostream>
-#include <sys/types.h>
-#include <unistd.h>
-
-#include <libcamera/ipa/ipa_interface.h>
-#include <libcamera/logging.h>
-
-#include "libcamera/internal/event_dispatcher.h"
-#include "libcamera/internal/ipa_module.h"
-#include "libcamera/internal/ipc_unixsocket.h"
-#include "libcamera/internal/log.h"
-#include "libcamera/internal/thread.h"
-
-using namespace libcamera;
-
-LOG_DEFINE_CATEGORY(IPAProxyLinuxWorker)
-
-void readyRead(IPCUnixSocket *ipc)
-{
-	IPCUnixSocket::Payload message;
-	int ret;
-
-	ret = ipc->receive(&message);
-	if (ret) {
-		LOG(IPAProxyLinuxWorker, Error)
-			<< "Receive message failed: " << ret;
-		return;
-	}
-
-	LOG(IPAProxyLinuxWorker, Debug) << "Received a message!";
-}
-
-int main(int argc, char **argv)
-{
-	/* Uncomment this for debugging. */
-#if 0
-	std::string logPath = "/tmp/libcamera.worker." +
-			      std::to_string(getpid()) + ".log";
-	logSetFile(logPath.c_str());
-#endif
-
-	if (argc < 3) {
-		LOG(IPAProxyLinuxWorker, Debug)
-			<< "Tried to start worker with no args";
-		return EXIT_FAILURE;
-	}
-
-	int fd = std::stoi(argv[2]);
-	LOG(IPAProxyLinuxWorker, Debug)
-		<< "Starting worker for IPA module " << argv[1]
-		<< " with IPC fd = " << fd;
-
-	std::unique_ptr<IPAModule> ipam = std::make_unique<IPAModule>(argv[1]);
-	if (!ipam->isValid() || !ipam->load()) {
-		LOG(IPAProxyLinuxWorker, Error)
-			<< "IPAModule " << argv[1] << " should be valid but isn't";
-		return EXIT_FAILURE;
-	}
-
-	IPCUnixSocket socket;
-	if (socket.bind(fd) < 0) {
-		LOG(IPAProxyLinuxWorker, Error) << "IPC socket binding failed";
-		return EXIT_FAILURE;
-	}
-	socket.readyRead.connect(&readyRead);
-
-	struct ipa_context *ipac = ipam->createContext();
-	if (!ipac) {
-		LOG(IPAProxyLinuxWorker, Error) << "Failed to create IPA context";
-		return EXIT_FAILURE;
-	}
-
-	LOG(IPAProxyLinuxWorker, Debug) << "Proxy worker successfully started";
-
-	/* \todo upgrade listening loop */
-	EventDispatcher *dispatcher = Thread::current()->eventDispatcher();
-	while (1)
-		dispatcher->processEvents();
-
-	ipac->ops->destroy(ipac);
-
-	return 0;
-}
diff --git a/src/libcamera/proxy/worker/meson.build b/src/libcamera/proxy/worker/meson.build
index 0fd26fd8..3796103e 100644
--- a/src/libcamera/proxy/worker/meson.build
+++ b/src/libcamera/proxy/worker/meson.build
@@ -1,9 +1,5 @@
 # SPDX-License-Identifier: CC0-1.0
 
-ipa_proxy_sources = [
-    ['ipa_proxy_linux', 'ipa_proxy_linux_worker.cpp']
-]
-
 proxy_install_dir = join_paths(get_option('libexecdir'), 'libcamera')
 
 # generate {pipeline}_ipa_proxy_worker.cpp
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index dbb6727f..4b88806f 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -12,7 +12,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include <libcamera/ipa/vimc.h>
+#include <libcamera/ipa/vimc_ipa_proxy.h>
 
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/event_dispatcher.h"
@@ -20,6 +20,7 @@
 #include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/ipa_module.h"
 #include "libcamera/internal/pipeline_handler.h"
+#include "libcamera/internal/process.h"
 #include "libcamera/internal/thread.h"
 #include "libcamera/internal/timer.h"
 
@@ -32,7 +33,7 @@ class IPAInterfaceTest : public Test, public Object
 {
 public:
 	IPAInterfaceTest()
-		: trace_(IPAOperationNone), notifier_(nullptr), fd_(-1)
+		: trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1)
 	{
 	}
 
@@ -64,22 +65,22 @@ protected:
 		}
 
 		/* Create and open the communication FIFO. */
-		int ret = mkfifo(VIMC_IPA_FIFO_PATH, S_IRUSR | S_IWUSR);
+		int ret = mkfifo(ipa::vimc::VimcIPAFIFOPath.c_str(), S_IRUSR | S_IWUSR);
 		if (ret) {
 			ret = errno;
 			cerr << "Failed to create IPA test FIFO at '"
-			     << VIMC_IPA_FIFO_PATH << "': " << strerror(ret)
+			     << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret)
 			     << endl;
 			return TestFail;
 		}
 
-		ret = open(VIMC_IPA_FIFO_PATH, O_RDONLY | O_NONBLOCK);
+		ret = open(ipa::vimc::VimcIPAFIFOPath.c_str(), O_RDONLY | O_NONBLOCK);
 		if (ret < 0) {
 			ret = errno;
 			cerr << "Failed to open IPA test FIFO at '"
-			     << VIMC_IPA_FIFO_PATH << "': " << strerror(ret)
+			     << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret)
 			     << endl;
-			unlink(VIMC_IPA_FIFO_PATH);
+			unlink(ipa::vimc::VimcIPAFIFOPath.c_str());
 			return TestFail;
 		}
 		fd_ = ret;
@@ -95,7 +96,7 @@ protected:
 		EventDispatcher *dispatcher = thread()->eventDispatcher();
 		Timer timer;
 
-		ipa_ = IPAManager::createIPA(pipe_.get(), 0, 0);
+		ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(pipe_.get(), 0, 0);
 		if (!ipa_) {
 			cerr << "Failed to create VIMC IPA interface" << endl;
 			return TestFail;
@@ -110,23 +111,22 @@ protected:
 		}
 
 		timer.start(1000);
-		while (timer.isRunning() && trace_ != IPAOperationInit)
+		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationInit)
 			dispatcher->processEvents();
 
-		if (trace_ != IPAOperationInit) {
+		if (trace_ != ipa::vimc::IPAOperationInit) {
 			cerr << "Failed to test IPA initialization sequence"
 			     << endl;
 			return TestFail;
 		}
 
 		/* Test start of IPA module. */
-		IPAOperationData data = {};
-		ipa_->start(data, nullptr);
+		ipa_->start();
 		timer.start(1000);
-		while (timer.isRunning() && trace_ != IPAOperationStart)
+		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStart)
 			dispatcher->processEvents();
 
-		if (trace_ != IPAOperationStart) {
+		if (trace_ != ipa::vimc::IPAOperationStart) {
 			cerr << "Failed to test IPA start sequence" << endl;
 			return TestFail;
 		}
@@ -134,10 +134,10 @@ protected:
 		/* Test stop of IPA module. */
 		ipa_->stop();
 		timer.start(1000);
-		while (timer.isRunning() && trace_ != IPAOperationStop)
+		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStop)
 			dispatcher->processEvents();
 
-		if (trace_ != IPAOperationStop) {
+		if (trace_ != ipa::vimc::IPAOperationStop) {
 			cerr << "Failed to test IPA stop sequence" << endl;
 			return TestFail;
 		}
@@ -148,7 +148,7 @@ protected:
 	void cleanup() override
 	{
 		close(fd_);
-		unlink(VIMC_IPA_FIFO_PATH);
+		unlink(ipa::vimc::VimcIPAFIFOPath.c_str());
 	}
 
 private:
@@ -158,16 +158,18 @@ private:
 		if (s < 0) {
 			int ret = errno;
 			cerr << "Failed to read from IPA test FIFO at '"
-			     << VIMC_IPA_FIFO_PATH << "': " << strerror(ret)
+			     << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret)
 			     << endl;
-			trace_ = IPAOperationNone;
+			trace_ = ipa::vimc::IPAOperationNone;
 		}
 	}
 
+	ProcessManager processManager_;
+
 	std::shared_ptr<PipelineHandler> pipe_;
-	std::unique_ptr<IPAProxy> ipa_;
+	std::unique_ptr<ipa::vimc::IPAProxyVimc> ipa_;
 	std::unique_ptr<IPAManager> ipaManager_;
-	enum IPAOperationCode trace_;
+	enum ipa::vimc::IPAOperationCode trace_;
 	EventNotifier *notifier_;
 	int fd_;
 };
diff --git a/test/ipa/meson.build b/test/ipa/meson.build
index e4f0818a..e8a041b5 100644
--- a/test/ipa/meson.build
+++ b/test/ipa/meson.build
@@ -6,7 +6,7 @@ ipa_test = [
 ]
 
 foreach t : ipa_test
-    exe = executable(t[0], t[1],
+    exe = executable(t[0], [t[1], libcamera_generated_ipa_headers],
                      dependencies : libcamera_dep,
                      link_with : [libipa, test_libraries],
                      include_directories : [libipa_includes, test_includes_internal])
-- 
2.27.0



More information about the libcamera-devel mailing list