{"id":9631,"url":"https://patchwork.libcamera.org/api/1.1/patches/9631/?format=json","web_url":"https://patchwork.libcamera.org/patch/9631/","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":"<20200915142038.28757-22-paul.elder@ideasonboard.com>","date":"2020-09-15T14:20:36","name":"[libcamera-devel,21/23] libcamera: IPAManager: Make createIPA return proxy directly","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"ab67d1f5a9619a9ce9330e8379442a008a9f6885","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/1.1/people/17/?format=json","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/9631/mbox/","series":[{"id":1291,"url":"https://patchwork.libcamera.org/api/1.1/series/1291/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1291","date":"2020-09-15T14:20:16","name":"IPA isolation implementation","version":1,"mbox":"https://patchwork.libcamera.org/series/1291/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/9631/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/9631/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 19A43BF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 15 Sep 2020 14:21:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D819162E35;\n\tTue, 15 Sep 2020 16:21:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 111EE62D27\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Sep 2020 16:21:43 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 496E1FD8;\n\tTue, 15 Sep 2020 16:21:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"emUlahsz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600179702;\n\tbh=CaMNBBUB7TnmWGd3aw5JUmKT74YiY1Sgfi5UpiBtBm4=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=emUlahszO80fB0zz/WPOAp10WM6UKs5T9tSOieXFbvXhg9pUvZB7qBjtElMh3ENU8\n\txkNcWem7wiSJl4MTkY0Gp/Ivw43zTYUENHSy5ShVqsns8fj01hxDLDBai/rMegV/E2\n\t5nO+d2YquVpqvayqZkR9X2WJoJgGkCyVe3eJZV+U=","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 15 Sep 2020 23:20:36 +0900","Message-Id":"<20200915142038.28757-22-paul.elder@ideasonboard.com>","X-Mailer":"git-send-email 2.27.0","In-Reply-To":"<20200915142038.28757-1-paul.elder@ideasonboard.com>","References":"<20200915142038.28757-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH 21/23] libcamera: IPAManager: Make\n\tcreateIPA return proxy directly","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>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"Since every pipeline knows the type of the proxy that it needs, and\nsince all IPAs are to be wrapped in a proxy, IPAManager no longer needs\nto search in the factory list to fetch the proxy factory to construct a\nfactory. Instead, we define createIPA as a template function, and the\npipeline handler can declare the proxy type when it calls createIPA.\n\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\n---\n include/libcamera/internal/ipa_manager.h      | 31 ++++++++++--\n src/libcamera/ipa_manager.cpp                 | 48 +------------------\n .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n 3 files changed, 30 insertions(+), 52 deletions(-)","diff":"diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\nindex 4a143b6a..297a8a58 100644\n--- a/include/libcamera/internal/ipa_manager.h\n+++ b/include/libcamera/internal/ipa_manager.h\n@@ -14,20 +14,45 @@\n #include <libcamera/ipa/ipa_module_info.h>\n \n #include \"libcamera/internal/ipa_module.h\"\n+#include \"libcamera/internal/log.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n #include \"libcamera/internal/pub_key.h\"\n \n namespace libcamera {\n \n+LOG_DECLARE_CATEGORY(IPAManager)\n+\n class IPAManager\n {\n public:\n \tIPAManager();\n \t~IPAManager();\n \n-\tstatic std::unique_ptr<IPAProxy> createIPA(PipelineHandler *pipe,\n-\t\t\t\t\t\t   uint32_t maxVersion,\n-\t\t\t\t\t\t   uint32_t minVersion);\n+\ttemplate<class P>\n+\tstatic std::unique_ptr<P> createIPA(PipelineHandler *pipe,\n+\t\t\t\t\t    uint32_t maxVersion,\n+\t\t\t\t\t    uint32_t minVersion)\n+\t{\n+\t\tIPAModule *m = nullptr;\n+\n+\t\tfor (IPAModule *module : self_->modules_) {\n+\t\t\tif (module->match(pipe, minVersion, maxVersion)) {\n+\t\t\t\tm = module;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (!m)\n+\t\t\treturn nullptr;\n+\n+\t\tstd::unique_ptr<P> proxy = std::make_unique<P>(m, !self_->isSignatureValid(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 private:\n \tstatic IPAManager *self_;\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex 26458153..0d518443 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -245,6 +245,7 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)\n }\n \n /**\n+ * \\fn IPAManager::createIPA()\n  * \\brief Create an IPA proxy that matches a given pipeline handler\n  * \\param[in] pipe The pipeline handler that wants a matching IPA proxy\n  * \\param[in] minVersion Minimum acceptable version of IPA module\n@@ -253,53 +254,6 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)\n  * \\return A newly created IPA proxy, or nullptr if no matching IPA module is\n  * found or if the IPA proxy fails to initialize\n  */\n-std::unique_ptr<IPAProxy> IPAManager::createIPA(PipelineHandler *pipe,\n-\t\t\t\t\t\tuint32_t maxVersion,\n-\t\t\t\t\t\tuint32_t minVersion)\n-{\n-\tIPAModule *m = nullptr;\n-\n-\tfor (IPAModule *module : self_->modules_) {\n-\t\tif (module->match(pipe, minVersion, maxVersion)) {\n-\t\t\tm = module;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n-\tif (!m)\n-\t\treturn nullptr;\n-\n-\t/*\n-\t * Load and run the IPA module in a thread if it has a valid signature,\n-\t * or isolate it in a separate process otherwise.\n-\t *\n-\t * \\todo Implement a better proxy selection\n-\t */\n-\tstd::string pipeName(pipe->name());\n-\tconst char *proxyName = pipeName.replace(0, 15, \"IPAProxy\").c_str();\n-\tIPAProxyFactory *pf = nullptr;\n-\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-\t}\n-\n-\tif (!pf) {\n-\t\tLOG(IPAManager, Error) << \"Failed to get proxy factory\";\n-\t\treturn nullptr;\n-\t}\n-\n-\tstd::unique_ptr<IPAProxy> proxy =\n-\t\tpf->create(m, !self_->isSignatureValid(m));\n-\tif (!proxy->isValid()) {\n-\t\tLOG(IPAManager, Error) << \"Failed to load proxy\";\n-\t\treturn nullptr;\n-\t}\n-\n-\treturn proxy;\n-}\n \n bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n {\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 70bb6fcc..19755e8c 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -1108,8 +1108,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)\n \n int RPiCameraData::loadIPA()\n {\n-\tstd::unique_ptr<IPAProxy> ptr = IPAManager::createIPA(pipe_, 1, 1);\n-\tipa_ = std::unique_ptr<IPAProxyRPi>{static_cast<IPAProxyRPi*>(std::move(ptr).release())};\n+\tipa_ = IPAManager::createIPA<IPAProxyRPi>(pipe_, 1, 1);\n \n \tif (!ipa_)\n \t\treturn -ENOENT;\n","prefixes":["libcamera-devel","21/23"]}