{"id":3454,"url":"https://patchwork.libcamera.org/api/1.1/patches/3454/?format=json","web_url":"https://patchwork.libcamera.org/patch/3454/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200413212700.1373247-9-niklas.soderlund@ragnatech.se>","date":"2020-04-13T21:27:00","name":"[libcamera-devel,v4,8/8] libcamera: ipa_manager: Proxy open-source IPAs to a thread","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"bfc23aabb7bd6b999a3eaed52773c9374c426ef3","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/1.1/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/3454/mbox/","series":[{"id":805,"url":"https://patchwork.libcamera.org/api/1.1/series/805/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=805","date":"2020-04-13T21:26:52","name":"libcamera: ipa_manager: Proxy open-source IPAs to a thread","version":4,"mbox":"https://patchwork.libcamera.org/series/805/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/3454/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/3454/checks/","tags":{},"headers":{"Return-Path":"","Received":["from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net\n\t[195.74.38.229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 236E662826\n\tfor ;\n\tMon, 13 Apr 2020 23:27:13 +0200 (CEST)","from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de\n\t[79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid 84ae242f-7dcd-11ea-aeed-005056917f90;\n\tMon, 13 Apr 2020 23:27:04 +0200 (CEST)"],"X-Halon-ID":"84ae242f-7dcd-11ea-aeed-005056917f90","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 13 Apr 2020 23:27:00 +0200","Message-Id":"<20200413212700.1373247-9-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.26.0","In-Reply-To":"<20200413212700.1373247-1-niklas.soderlund@ragnatech.se>","References":"<20200413212700.1373247-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v4 8/8] libcamera: ipa_manager: Proxy\n\topen-source IPAs to a thread","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 13 Apr 2020 21:27:13 -0000"},"content":"From: Laurent Pinchart \n\nWhile closed-source IPA modules will always be sandboxed, open-source\nIPA modules may be run in the main libcamera process or be sandboxed,\ndepending on platform configuration. These two models exhibit very\ndifferent timings, which require extensive testing with both\nconfigurations.\n\nWhen run into the main libcamera process, IPA modules are executed in\nthe pipeline handler thread (which is currently a global CameraManager\nthread). Time-consuming operations in the IPA may thus slow down the\npipeline handler and compromise real-time behaviour. At least some\npipeline handlers will thus likely spawn a thread to isolate the IPA,\nleading to code duplication in pipeline handlers.\n\nSolve both issues by always proxying IPA modules. For open-source IPA\nmodules that run in the libcamera process, a new IPAProxyThread class is\nadded to run the IPA in a separate thread.\n\nSigned-off-by: Laurent Pinchart \n[Niklas: Move thread start/stop of thread into start()/stop()]\nSigned-off-by: Niklas Söderlund \n---\n* Changes since v2\n- Move the thread at init() instead of start()\n- Call start() in the target thread.\n- Block frame actions in IPAProxyThread instead of leaving it up to each\n pipeline handler.\n---\n src/libcamera/ipa_interface.cpp | 3 +\n src/libcamera/ipa_manager.cpp | 48 +++----\n src/libcamera/proxy/ipa_proxy_thread.cpp | 162 +++++++++++++++++++++++\n src/libcamera/proxy/meson.build | 1 +\n 4 files changed, 187 insertions(+), 27 deletions(-)\n create mode 100644 src/libcamera/proxy/ipa_proxy_thread.cpp","diff":"diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\nindex 0b785bdfe6e6a705..890d4340e4f373c3 100644\n--- a/src/libcamera/ipa_interface.cpp\n+++ b/src/libcamera/ipa_interface.cpp\n@@ -528,6 +528,9 @@ namespace libcamera {\n * \\a data.operation field, as defined by the IPA protocol, and the rest of the\n * \\a data is interpreted accordingly. The pipeline handler shall queue the\n * action and execute it as appropriate.\n+ *\n+ * The signal is only emitted when the IPA is running, that is after start() and\n+ * before stop() have been called.\n */\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex bcaae3564ea14e07..6d23f470506561b8 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -12,7 +12,6 @@\n #include \n #include \n \n-#include \"ipa_context_wrapper.h\"\n #include \"ipa_module.h\"\n #include \"ipa_proxy.h\"\n #include \"log.h\"\n@@ -271,40 +270,35 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe,\n \tif (!m)\n \t\treturn nullptr;\n \n-\tif (!m->isOpenSource()) {\n-\t\tIPAProxyFactory *pf = nullptr;\n-\t\tstd::vector &factories = IPAProxyFactory::factories();\n+\t/*\n+\t * Load and run the IPA module in a thread if it is open-source, or\n+\t * isolate it in a separate process otherwise.\n+\t *\n+\t * \\todo Implement a better proxy selection\n+\t */\n+\tconst char *proxyName = m->isOpenSource()\n+\t\t\t ? \"IPAProxyThread\" : \"IPAProxyLinux\";\n+\tIPAProxyFactory *pf = nullptr;\n \n-\t\tfor (IPAProxyFactory *factory : factories) {\n-\t\t\t/* TODO: Better matching */\n-\t\t\tif (!strcmp(factory->name().c_str(), \"IPAProxyLinux\")) {\n-\t\t\t\tpf = factory;\n-\t\t\t\tbreak;\n-\t\t\t}\n+\tfor (IPAProxyFactory *factory : IPAProxyFactory::factories()) {\n+\t\tif (!strcmp(factory->name().c_str(), proxyName)) {\n+\t\t\tpf = factory;\n+\t\t\tbreak;\n \t\t}\n-\n-\t\tif (!pf) {\n-\t\t\tLOG(IPAManager, Error) << \"Failed to get proxy factory\";\n-\t\t\treturn nullptr;\n-\t\t}\n-\n-\t\tstd::unique_ptr proxy = pf->create(m);\n-\t\tif (!proxy->isValid()) {\n-\t\t\tLOG(IPAManager, Error) << \"Failed to load proxy\";\n-\t\t\treturn nullptr;\n-\t\t}\n-\n-\t\treturn proxy;\n \t}\n \n-\tif (!m->load())\n+\tif (!pf) {\n+\t\tLOG(IPAManager, Error) << \"Failed to get proxy factory\";\n \t\treturn nullptr;\n+\t}\n \n-\tstruct ipa_context *ctx = m->createContext();\n-\tif (!ctx)\n+\tstd::unique_ptr proxy = pf->create(m);\n+\tif (!proxy->isValid()) {\n+\t\tLOG(IPAManager, Error) << \"Failed to load proxy\";\n \t\treturn nullptr;\n+\t}\n \n-\treturn std::make_unique(ctx);\n+\treturn proxy;\n }\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\nnew file mode 100644\nindex 0000000000000000..368ccca1cf60b8ec\n--- /dev/null\n+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n@@ -0,0 +1,162 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2020, Google Inc.\n+ *\n+ * ipa_proxy_thread.cpp - Proxy running an Image Processing Algorithm in a thread\n+ */\n+\n+#include \n+\n+#include \n+#include \n+\n+#include \"ipa_context_wrapper.h\"\n+#include \"ipa_module.h\"\n+#include \"ipa_proxy.h\"\n+#include \"log.h\"\n+#include \"thread.h\"\n+\n+namespace libcamera {\n+\n+LOG_DECLARE_CATEGORY(IPAProxy)\n+\n+class IPAProxyThread : public IPAProxy, public Object\n+{\n+public:\n+\tIPAProxyThread(IPAModule *ipam);\n+\n+\tint init() override;\n+\tint start() override;\n+\tvoid stop() override;\n+\n+\tvoid configure(const std::map &streamConfig,\n+\t\t const std::map &entityControls) override;\n+\tvoid mapBuffers(const std::vector &buffers) override;\n+\tvoid unmapBuffers(const std::vector &ids) override;\n+\tvoid processEvent(const IPAOperationData &event) override;\n+\n+private:\n+\tvoid queueFrameAction(unsigned int frame, const IPAOperationData &data);\n+\n+\t/* Helper class to invoke processEvent() in another thread. */\n+\tclass ThreadProxy : public Object\n+\t{\n+\tpublic:\n+\t\tvoid setIPA(IPAInterface *ipa)\n+\t\t{\n+\t\t\tipa_ = ipa;\n+\t\t}\n+\n+\t\tint start()\n+\t\t{\n+\t\t\treturn ipa_->start();\n+\t\t}\n+\n+\t\tvoid stop()\n+\t\t{\n+\t\t\tipa_->stop();\n+\t\t}\n+\n+\t\tvoid processEvent(const IPAOperationData &event)\n+\t\t{\n+\t\t\tipa_->processEvent(event);\n+\t\t}\n+\n+\tprivate:\n+\t\tIPAInterface *ipa_;\n+\t};\n+\n+\tbool running_;\n+\tThread thread_;\n+\tThreadProxy proxy_;\n+\tstd::unique_ptr ipa_;\n+};\n+\n+IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n+\t: running_(false)\n+{\n+\tif (!ipam->load())\n+\t\treturn;\n+\n+\tstruct ipa_context *ctx = ipam->createContext();\n+\tif (!ctx) {\n+\t\tLOG(IPAProxy, Error)\n+\t\t\t<< \"Failed to create IPA context for \" << ipam->path();\n+\t\treturn;\n+\t}\n+\n+\tipa_ = std::make_unique(ctx);\n+\tproxy_.setIPA(ipa_.get());\n+\n+\t/*\n+\t * Proxy the queueFrameAction signal to dispatch it in the caller's\n+\t * thread.\n+\t */\n+\tipa_->queueFrameAction.connect(this, &IPAProxyThread::queueFrameAction);\n+\n+\tvalid_ = true;\n+}\n+\n+int IPAProxyThread::init()\n+{\n+\tint ret = ipa_->init();\n+\tif (ret)\n+\t\treturn ret;\n+\n+\tproxy_.moveToThread(&thread_);\n+\n+\treturn 0;\n+}\n+\n+int IPAProxyThread::start()\n+{\n+\trunning_ = true;\n+\tthread_.start();\n+\n+\treturn proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking);\n+}\n+\n+void IPAProxyThread::stop()\n+{\n+\trunning_ = false;\n+\n+\tproxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking);\n+\n+\tthread_.exit();\n+\tthread_.wait();\n+}\n+\n+void IPAProxyThread::configure(const std::map &streamConfig,\n+\t\t\t const std::map &entityControls)\n+{\n+\tipa_->configure(streamConfig, entityControls);\n+}\n+\n+void IPAProxyThread::mapBuffers(const std::vector &buffers)\n+{\n+\tipa_->mapBuffers(buffers);\n+}\n+\n+void IPAProxyThread::unmapBuffers(const std::vector &ids)\n+{\n+\tipa_->unmapBuffers(ids);\n+}\n+\n+void IPAProxyThread::processEvent(const IPAOperationData &event)\n+{\n+\tif (!running_)\n+\t\treturn;\n+\n+\t/* Dispatch the processEvent() call to the thread. */\n+\tproxy_.invokeMethod(&ThreadProxy::processEvent, ConnectionTypeQueued,\n+\t\t\t event);\n+}\n+\n+void IPAProxyThread::queueFrameAction(unsigned int frame, const IPAOperationData &data)\n+{\n+\tIPAInterface::queueFrameAction.emit(frame, data);\n+}\n+\n+REGISTER_IPA_PROXY(IPAProxyThread)\n+\n+} /* namespace libcamera */\ndiff --git a/src/libcamera/proxy/meson.build b/src/libcamera/proxy/meson.build\nindex efc1132302176f63..6c00d5f30ad2e5f0 100644\n--- a/src/libcamera/proxy/meson.build\n+++ b/src/libcamera/proxy/meson.build\n@@ -1,3 +1,4 @@\n libcamera_sources += files([\n 'ipa_proxy_linux.cpp',\n+ 'ipa_proxy_thread.cpp',\n ])\n","prefixes":["libcamera-devel","v4","8/8"]}