{"id":19099,"url":"https://patchwork.libcamera.org/api/1.1/patches/19099/?format=json","web_url":"https://patchwork.libcamera.org/patch/19099/","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":"<20230930152829.21947-1-laurent.pinchart@ideasonboard.com>","date":"2023-09-30T15:28:29","name":"[libcamera-devel] libcamera: ipa_manager: Remove singleton requirement","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"378e109bd2fc71d339fdf86f36d382f8854468d1","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19099/mbox/","series":[{"id":4044,"url":"https://patchwork.libcamera.org/api/1.1/series/4044/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4044","date":"2023-09-30T15:28:29","name":"[libcamera-devel] libcamera: ipa_manager: Remove singleton requirement","version":1,"mbox":"https://patchwork.libcamera.org/series/4044/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19099/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19099/checks/","tags":{},"headers":{"Return-Path":"","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 A3CF2BD808\n\tfor ;\n\tSat, 30 Sep 2023 15:28:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6A3762963;\n\tSat, 30 Sep 2023 17:28:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3EB9D61DE1\n\tfor ;\n\tSat, 30 Sep 2023 17:28:19 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(lfbn-idf1-1-343-200.w86-195.abo.wanadoo.fr [86.195.61.200])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8B445497\n\tfor ;\n\tSat, 30 Sep 2023 17:26:35 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1696087700;\n\tbh=eCrfEneJ67wjNS40pgH/lCo3wuYkviRMOtiUcxOMPGE=;\n\th=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post:\n\tList-Help:List-Subscribe:From:Reply-To:From;\n\tb=yN9o50frjQwUJFhXOdjRGa1gkkzF3yt01PvdeSUGNdM56eshZWc7AdKOR2eINmRL0\n\t7Y+jKvjjrfb7TL+B3bA88bNUqnLGilgQBur3jwAgYXzYsSgg027EnddXldUaTTea6P\n\t1A1HDs53S40ZMCMMlD/xorFvMz4WfNcpnsBQNkloA3Qi+9wrz64iZIgIZ4YVqVf3I5\n\tveKUWn7PmPjejwRLi030ZuIjy/DBIVUhYn8ibn10S+bBmvXc63GjGh+vXnQPpTsymf\n\tNd6P9gnl99kbNJthDmblOBCmmBe3pR2u1sBOOb3sZRseWrJz5KJDMjjkfrMuY7hnay\n\tgdUsazeRPNnkA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1696087595;\n\tbh=eCrfEneJ67wjNS40pgH/lCo3wuYkviRMOtiUcxOMPGE=;\n\th=From:To:Subject:Date:From;\n\tb=vP7caORS5wnPywOp2r1zUpjCPIGuwj+Eetb3rUb/7SllFKboFM86JxTalBrZv1WnF\n\tzbaa336j1m6xe2GIJWIBWBxbJyhIcDcGhAm4MigW3o8kyHv61H6c9I8FhDBjwVOa//\n\tSh8yntT5SeFYIZa7+EuNXDVSHeO6RQVzimRD1azM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vP7caORS\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Sat, 30 Sep 2023 18:28:29 +0300","Message-ID":"<20230930152829.21947-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.41.0","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH] libcamera: ipa_manager: Remove singleton\n\trequirement","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The IPAManager class implements a singleton pattern due to the need of\naccessing the instance in a static member function. The function now\ntakes a pointer to a PipelineHandler, which we can use to access the\nCameraManager, and from there, the IPAManager.\n\nAdd accessors to the internal API to expose the CameraManager from the\nPipelineHandler, and the IPAManager from the CameraManager. This\nrequires allocating the IPAManager dynamically to avoid a loop in\nincludes. Use those accessors to replace the IPAManager singleton.\n\nUpdate the IPA interface unit test to instantiate a CameraManager\ninstead of an IPAManager and ProcessManager, to reflect the new way that\nthe IPAManager is accessed.\n\nSigned-off-by: Laurent Pinchart \n---\n include/libcamera/internal/camera_manager.h | 6 ++++--\n include/libcamera/internal/ipa_manager.h | 9 +++++----\n include/libcamera/internal/pipeline_handler.h | 2 ++\n src/libcamera/camera_manager.cpp | 9 +++++++++\n src/libcamera/ipa_manager.cpp | 10 ----------\n src/libcamera/pipeline_handler.cpp | 7 +++++++\n test/ipa/ipa_interface_test.cpp | 12 +++++-------\n 7 files changed, 32 insertions(+), 23 deletions(-)","diff":"diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\nindex 33ebe0699fdf..a0eec42fac7b 100644\n--- a/include/libcamera/internal/camera_manager.h\n+++ b/include/libcamera/internal/camera_manager.h\n@@ -19,13 +19,13 @@\n #include \n #include \n \n-#include \"libcamera/internal/ipa_manager.h\"\n #include \"libcamera/internal/process.h\"\n \n namespace libcamera {\n \n class Camera;\n class DeviceEnumerator;\n+class IPAManager;\n \n class CameraManager::Private : public Extensible::Private, public Thread\n {\n@@ -38,6 +38,8 @@ public:\n \tvoid addCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n \tvoid removeCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n \n+\tIPAManager *ipaManager() const { return ipaManager_.get(); }\n+\n protected:\n \tvoid run() override;\n \n@@ -61,7 +63,7 @@ private:\n \n \tstd::unique_ptr enumerator_;\n \n-\tIPAManager ipaManager_;\n+\tstd::unique_ptr ipaManager_;\n \tProcessManager processManager_;\n };\n \ndiff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\nindex bf823563c91c..23db6b8a0221 100644\n--- a/include/libcamera/internal/ipa_manager.h\n+++ b/include/libcamera/internal/ipa_manager.h\n@@ -15,6 +15,7 @@\n #include \n #include \n \n+#include \"libcamera/internal/camera_manager.h\"\n #include \"libcamera/internal/ipa_module.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n #include \"libcamera/internal/pub_key.h\"\n@@ -34,11 +35,13 @@ public:\n \t\t\t\t\t uint32_t minVersion,\n \t\t\t\t\t uint32_t maxVersion)\n \t{\n-\t\tIPAModule *m = self_->module(pipe, minVersion, maxVersion);\n+\t\tCameraManager *cm = pipe->cameraManager();\n+\t\tIPAManager *self = cm->_d()->ipaManager();\n+\t\tIPAModule *m = self->module(pipe, minVersion, maxVersion);\n \t\tif (!m)\n \t\t\treturn nullptr;\n \n-\t\tstd::unique_ptr proxy = std::make_unique(m, !self_->isSignatureValid(m));\n+\t\tstd::unique_ptr proxy = std::make_unique(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@@ -55,8 +58,6 @@ public:\n #endif\n \n private:\n-\tstatic IPAManager *self_;\n-\n \tvoid parseDir(const char *libDir, unsigned int maxDepth,\n \t\t std::vector &files);\n \tunsigned int addDir(const char *libDir, unsigned int maxDepth = 0);\ndiff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\nindex c96944f4ecc4..bbfae557e023 100644\n--- a/include/libcamera/internal/pipeline_handler.h\n+++ b/include/libcamera/internal/pipeline_handler.h\n@@ -70,6 +70,8 @@ public:\n \n \tconst char *name() const { return name_; }\n \n+\tCameraManager *cameraManager() const { return manager_; }\n+\n protected:\n \tvoid registerCamera(std::shared_ptr camera);\n \tvoid hotplugMediaDevice(MediaDevice *media);\ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 355f3adad68f..909a3cd70117 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/camera.h\"\n #include \"libcamera/internal/device_enumerator.h\"\n+#include \"libcamera/internal/ipa_manager.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n \n /**\n@@ -37,6 +38,7 @@ LOG_DEFINE_CATEGORY(Camera)\n CameraManager::Private::Private()\n \t: initialized_(false)\n {\n+\tipaManager_ = std::make_unique();\n }\n \n int CameraManager::Private::start()\n@@ -220,6 +222,13 @@ void CameraManager::Private::removeCamera(std::shared_ptr camera)\n \to->cameraRemoved.emit(camera);\n }\n \n+/**\n+ * \\fn CameraManager::Private::ipaManager() const\n+ * \\brief Retrieve the IPAManager\n+ * \\context This function is \\threadsafe.\n+ * \\return The IPAManager for this CameraManager\n+ */\n+\n /**\n * \\class CameraManager\n * \\brief Provide access and manage all cameras in the system\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex 7a4515d90d7b..be1bb704c6cd 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -95,8 +95,6 @@ LOG_DEFINE_CATEGORY(IPAManager)\n * IPC.\n */\n \n-IPAManager *IPAManager::self_ = nullptr;\n-\n /**\n * \\brief Construct an IPAManager instance\n *\n@@ -105,10 +103,6 @@ IPAManager *IPAManager::self_ = nullptr;\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 #if HAVE_IPA_PUBKEY\n \tif (!pubKey_.isValid())\n \t\tLOG(IPAManager, Warning) << \"Public key not valid\";\n@@ -153,16 +147,12 @@ 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 /**\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 9c74c6cfda70..15783fdc7447 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -719,6 +719,13 @@ void PipelineHandler::disconnect()\n * \\return The pipeline handler name\n */\n \n+/**\n+ * \\fn PipelineHandler::cameraManager() const\n+ * \\brief Retrieve the CameraManager that this pipeline handler belongs to\n+ * \\context This function is \\threadsafe.\n+ * \\return The CameraManager for this pipeline handler\n+ */\n+\n /**\n * \\class PipelineHandlerFactoryBase\n * \\brief Base class for pipeline handler factories\ndiff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\nindex 051ef96e7ed2..af52eed72ec8 100644\n--- a/test/ipa/ipa_interface_test.cpp\n+++ b/test/ipa/ipa_interface_test.cpp\n@@ -19,11 +19,11 @@\n #include \n #include \n \n+#include \"libcamera/internal/camera_manager.h\"\n #include \"libcamera/internal/device_enumerator.h\"\n #include \"libcamera/internal/ipa_manager.h\"\n #include \"libcamera/internal/ipa_module.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n-#include \"libcamera/internal/process.h\"\n \n #include \"test.h\"\n \n@@ -43,20 +43,20 @@ public:\n \t{\n \t\tdelete notifier_;\n \t\tipa_.reset();\n-\t\tipaManager_.reset();\n+\t\tcameraManager_.reset();\n \t}\n \n protected:\n \tint init() override\n \t{\n-\t\tipaManager_ = make_unique();\n+\t\tcameraManager_ = make_unique();\n \n \t\t/* Create a pipeline handler for vimc. */\n \t\tconst std::vector &factories =\n \t\t\tPipelineHandlerFactoryBase::factories();\n \t\tfor (const PipelineHandlerFactoryBase *factory : factories) {\n \t\t\tif (factory->name() == \"PipelineHandlerVimc\") {\n-\t\t\t\tpipe_ = factory->create(nullptr);\n+\t\t\t\tpipe_ = factory->create(cameraManager_.get());\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\n@@ -170,11 +170,9 @@ private:\n \t\t}\n \t}\n \n-\tProcessManager processManager_;\n-\n \tstd::shared_ptr pipe_;\n \tstd::unique_ptr ipa_;\n-\tstd::unique_ptr ipaManager_;\n+\tstd::unique_ptr cameraManager_;\n \tenum ipa::vimc::IPAOperationCode trace_;\n \tEventNotifier *notifier_;\n \tint fd_;\n","prefixes":["libcamera-devel"]}