{"id":3936,"url":"https://patchwork.libcamera.org/api/patches/3936/?format=json","web_url":"https://patchwork.libcamera.org/patch/3936/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/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":"<20200605090106.15424-2-paul.elder@ideasonboard.com>","date":"2020-06-05T09:01:00","name":"[libcamera-devel,v2,1/7] IPAManager: make IPAManager lifetime explicitly managed","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"bff571914d9bc2cb897c2588cb0fa3eff583f0f8","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/?format=json","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/3936/mbox/","series":[{"id":956,"url":"https://patchwork.libcamera.org/api/series/956/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=956","date":"2020-06-05T09:00:59","name":"Support qv4l2 with v4l2-compat","version":2,"mbox":"https://patchwork.libcamera.org/series/956/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/3936/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/3936/checks/","tags":{},"headers":{"Return-Path":"","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 C5F2661027\n\tfor ;\n\tFri, 5 Jun 2020 11:01:17 +0200 (CEST)","from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp\n\t[118.238.243.68])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A151F27C;\n\tFri, 5 Jun 2020 11:01:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HrHlCjzR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591347677;\n\tbh=7zrH4zk88iLncnUuGh1zlgEHRYvx0Sw1lvltKXxCbvA=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=HrHlCjzR8AFfyUijcYSh2DzR+Woevg5Ffe86t5LY8k4prnFGyOjjdL9R5g8StaMZg\n\tK+grA0kBAS/AlPr13S/xJ/1XRUYmzWylTd8m0D1SybLbTUOVMcv+sZK5nwXla40Y2U\n\tLPHUHndWLjmVKi4jeEKmOkKYP0qLGhyiHQhhkXBA=","From":"Paul Elder ","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 5 Jun 2020 18:01:00 +0900","Message-Id":"<20200605090106.15424-2-paul.elder@ideasonboard.com>","X-Mailer":"git-send-email 2.20.1","In-Reply-To":"<20200605090106.15424-1-paul.elder@ideasonboard.com>","References":"<20200605090106.15424-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 1/7] IPAManager: make IPAManager\n\tlifetime explicitly managed","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":"Fri, 05 Jun 2020 09:01:18 -0000"},"content":"If any ipa_context instances are destroyed after the IPAManager is\ndestroyed, then a segfault will occur, since the modules have been\nunloaded by the IPAManager and the context function pointers have been\nfreed.\n\nFix this by making the lifetime of the IPAManager explicit, and make the\nCameraManager construct and deconstruct (automatically, via a unique\npointer) the IPAManager.\n\nAlso update the IPA interface test to do the construction and\ndeconstruction of the IPAManager, as it does not use the CameraManager.\n\nSigned-off-by: Paul Elder \n\n---\nChanges in v2: Move the IPAManager instance into\n\t CameraManager::Private from CameraManager\n---\n include/libcamera/camera_manager.h | 1 -\n include/libcamera/internal/ipa_manager.h | 6 ++++--\n src/libcamera/camera_manager.cpp | 3 +++\n src/libcamera/ipa_manager.cpp | 20 ++++++++++++++++++--\n test/ipa/ipa_interface_test.cpp | 5 +++++\n 5 files changed, 30 insertions(+), 5 deletions(-)","diff":"diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\nindex 079f848a..00658a70 100644\n--- a/include/libcamera/camera_manager.h\n+++ b/include/libcamera/camera_manager.h\n@@ -18,7 +18,6 @@ namespace libcamera {\n \n class Camera;\n class EventDispatcher;\n-\n class CameraManager : public Object\n {\n public:\ndiff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\nindex 16d74291..43f700d3 100644\n--- a/include/libcamera/internal/ipa_manager.h\n+++ b/include/libcamera/internal/ipa_manager.h\n@@ -22,6 +22,9 @@ namespace libcamera {\n class IPAManager\n {\n public:\n+\tIPAManager();\n+\t~IPAManager();\n+\n \tstatic IPAManager *instance();\n \n \tstd::unique_ptr createIPA(PipelineHandler *pipe,\n@@ -29,8 +32,7 @@ public:\n \t\t\t\t\t uint32_t minVersion);\n \n private:\n-\tIPAManager();\n-\t~IPAManager();\n+\tstatic IPAManager *self_;\n \n \tvoid parseDir(const char *libDir, unsigned int maxDepth,\n \t\t std::vector &files);\ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 849377ad..da806fa7 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -15,6 +15,7 @@\n \n #include \"libcamera/internal/device_enumerator.h\"\n #include \"libcamera/internal/event_dispatcher_poll.h\"\n+#include \"libcamera/internal/ipa_manager.h\"\n #include \"libcamera/internal/log.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n #include \"libcamera/internal/thread.h\"\n@@ -63,6 +64,8 @@ private:\n \n \tstd::vector> pipes_;\n \tstd::unique_ptr enumerator_;\n+\n+\tIPAManager ipaManager_;\n };\n \n CameraManager::Private::Private(CameraManager *cm)\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex 505cf610..abb681a3 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -93,8 +93,21 @@ LOG_DEFINE_CATEGORY(IPAManager)\n * plain C API, or to transmit the data to the isolated process through IPC.\n */\n \n+IPAManager *IPAManager::self_ = nullptr;\n+\n+/**\n+ * \\brief Construct an IPAManager instance\n+ *\n+ * The IPAManager class is meant to only be instantiated once, by the\n+ * CameraManager. Pipeline handlers shall use the instance() function to access\n+ * the IPAManager instance.\n+ */\n IPAManager::IPAManager()\n {\n+\tif (self_)\n+\t\tLOG(IPAManager, Fatal)\n+\t\t\t<< \"Multiple IPAManager objects are not allowed\";\n+\n \tunsigned int ipaCount = 0;\n \n \t/* User-specified paths take precedence. */\n@@ -134,12 +147,16 @@ IPAManager::IPAManager()\n \tif (!ipaCount)\n \t\tLOG(IPAManager, Warning)\n \t\t\t<< \"No IPA found in '\" IPA_MODULE_DIR \"'\";\n+\n+\tself_ = this;\n }\n \n IPAManager::~IPAManager()\n {\n \tfor (IPAModule *module : modules_)\n \t\tdelete module;\n+\n+\tself_ = nullptr;\n }\n \n /**\n@@ -153,8 +170,7 @@ IPAManager::~IPAManager()\n */\n IPAManager *IPAManager::instance()\n {\n-\tstatic IPAManager ipaManager;\n-\treturn &ipaManager;\n+\treturn self_;\n }\n \n /**\ndiff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\nindex 2f02af49..153493ba 100644\n--- a/test/ipa/ipa_interface_test.cpp\n+++ b/test/ipa/ipa_interface_test.cpp\n@@ -39,11 +39,15 @@ public:\n \t~IPAInterfaceTest()\n \t{\n \t\tdelete notifier_;\n+\t\tipa_.reset();\n+\t\tipaManager_.reset();\n \t}\n \n protected:\n \tint init() override\n \t{\n+\t\tipaManager_ = make_unique();\n+\n \t\t/* Create a pipeline handler for vimc. */\n \t\tstd::vector &factories =\n \t\t\tPipelineHandlerFactory::factories();\n@@ -161,6 +165,7 @@ private:\n \n \tstd::shared_ptr pipe_;\n \tstd::unique_ptr ipa_;\n+\tstd::unique_ptr ipaManager_;\n \tenum IPAOperationCode trace_;\n \tEventNotifier *notifier_;\n \tint fd_;\n","prefixes":["libcamera-devel","v2","1/7"]}