Patch Detail
Show a patch.
GET /api/patches/3207/?format=api
{ "id": 3207, "url": "https://patchwork.libcamera.org/api/patches/3207/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3207/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200320011618.13331-1-laurent.pinchart@ideasonboard.com>", "date": "2020-03-20T01:16:18", "name": "[libcamera-devel,RFC/PATCH] libcamera: ipa_manager: Proxy open-source IPAs to a thread", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "3e1c2b37e09ed61cddfe0d407b4cdef490fa0d67", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/3207/mbox/", "series": [ { "id": 748, "url": "https://patchwork.libcamera.org/api/series/748/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=748", "date": "2020-03-20T01:16:18", "name": "[libcamera-devel,RFC/PATCH] libcamera: ipa_manager: Proxy open-source IPAs to a thread", "version": 1, "mbox": "https://patchwork.libcamera.org/series/748/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3207/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3207/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3399960418\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Mar 2020 02:16:28 +0100 (CET)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A1FD1504;\n\tFri, 20 Mar 2020 02:16:27 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584666987;\n\tbh=49z6+biT92jCRffmc71y0N3Az02QAKrvE96PLNSO4LM=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=CDLfVj142D0gSSnhkcLvXRZR7Cp3z76pUlwWFm9tAzavStJ5QFn05Mp+gT7i/e8nb\n\tepX4tcSkqbsLAQxWEQNrB/ypwwGm4d6dUQwyhnnFMN/OMlAf/6qA87cJRnamsgVyYT\n\tIrtWSA5C39FkIz2ccg4LEiGtQYLZPRM/LdP+Go1Q=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 20 Mar 2020 03:16:18 +0200", "Message-Id": "<20200320011618.13331-1-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [RFC/PATCH] 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": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Fri, 20 Mar 2020 01:16:28 -0000" }, "content": "While 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 <laurent.pinchart@ideasonboard.com>\n---\nHello,\n\nThis patch is currently untested, I'm working on expanding the VIMC IPA\nto provide unit tests. I've however decided to send it already to get\nfeedback on the idea.\n\n src/libcamera/ipa_manager.cpp | 48 ++++-----\n src/libcamera/proxy/ipa_proxy_thread.cpp | 130 +++++++++++++++++++++++\n src/libcamera/proxy/meson.build | 1 +\n 3 files changed, 152 insertions(+), 27 deletions(-)\n create mode 100644 src/libcamera/proxy/ipa_proxy_thread.cpp", "diff": "diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex bcaae3564ea1..6d23f4705065 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -12,7 +12,6 @@\n #include <string.h>\n #include <sys/types.h>\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<IPAInterface> 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<IPAProxyFactory *> &factories = IPAProxyFactory::factories();\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-\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+\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\tstd::unique_ptr<IPAProxy> 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+\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\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<IPAProxy> 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<IPAContextWrapper>(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 000000000000..22006b3c98ee\n--- /dev/null\n+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n@@ -0,0 +1,130 @@\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 <memory>\n+\n+#include <ipa/ipa_interface.h>\n+#include <ipa/ipa_module_info.h>\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+\t~IPAProxyThread();\n+\n+\tint init() override;\n+\tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n+\t\t const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;\n+\tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n+\tvoid unmapBuffers(const std::vector<unsigned int> &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\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+\tThread thread_;\n+\tThreadProxy proxy_;\n+\tstd::unique_ptr<IPAInterface> ipa_;\n+};\n+\n+IPAProxyThread::IPAProxyThread(IPAModule *ipam)\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<IPAContextWrapper>(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+IPAProxyThread::~IPAProxyThread()\n+{\n+\tthread_.exit();\n+\tthread_.wait();\n+}\n+\n+int IPAProxyThread::init()\n+{\n+\tthread_.start();\n+\tproxy_.moveToThread(&thread_);\n+\n+\treturn ipa_->init();\n+}\n+\n+void IPAProxyThread::configure(const std::map<unsigned int, IPAStream> &streamConfig,\n+\t\t\t const std::map<unsigned int, const ControlInfoMap &> &entityControls)\n+{\n+\tipa_->configure(streamConfig, entityControls);\n+}\n+\n+void IPAProxyThread::mapBuffers(const std::vector<IPABuffer> &buffers)\n+{\n+\tipa_->mapBuffers(buffers);\n+}\n+\n+void IPAProxyThread::unmapBuffers(const std::vector<unsigned int> &ids)\n+{\n+\tipa_->unmapBuffers(ids);\n+}\n+\n+void IPAProxyThread::processEvent(const IPAOperationData &event)\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 efc113230217..6c00d5f30ad2 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", "RFC/PATCH" ] }