From patchwork Sat Aug 3 21:28:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 20752 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2CD76BDC71 for ; Sat, 3 Aug 2024 21:28:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 854EF6337D; Sat, 3 Aug 2024 23:28:57 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="keSfTXVL"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A64746336E for ; Sat, 3 Aug 2024 23:28:55 +0200 (CEST) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3EBC4512 for ; Sat, 3 Aug 2024 23:28:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1722720485; bh=4gqut8XLD4X2Cj0To3tM+qeALQ04DXOIHlUSQB2/9k8=; h=From:To:Subject:Date:From; b=keSfTXVL64TmqpbrWcpYEDPy6higlcGARP7R61bg1Sad8W61K1sb8jW80oR6P/8Oo o4KBqByBMjObF7tlVrQrjD5SpG5RsFcpvXepAP9cDUrWuteGGK3vcGQW2IC9BoQs0P cgEZjC8nNc81fZ8KVM9PimDUkuuXhhNE+ZddCoqA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH] libcamera: ipa_manager: Remove singleton requirement Date: Sun, 4 Aug 2024 00:28:32 +0300 Message-ID: <20240803212832.22766-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.44.2 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The IPAManager class implements a singleton pattern due to the need of accessing the instance in a static member function. The function now takes a pointer to a PipelineHandler, which we can use to access the CameraManager, and from there, the IPAManager. Add accessors to the internal API to expose the CameraManager from the PipelineHandler, and the IPAManager from the CameraManager. This requires allocating the IPAManager dynamically to avoid a loop in includes. Use those accessors to replace the IPAManager singleton. Update the IPA interface unit test to instantiate a CameraManager instead of an IPAManager and ProcessManager, to reflect the new way that the IPAManager is accessed. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- include/libcamera/internal/camera_manager.h | 7 +++++-- include/libcamera/internal/ipa_manager.h | 9 +++++---- include/libcamera/internal/pipeline_handler.h | 2 ++ src/libcamera/camera_manager.cpp | 9 +++++++++ src/libcamera/ipa_manager.cpp | 10 ---------- src/libcamera/pipeline_handler.cpp | 7 +++++++ test/ipa/ipa_interface_test.cpp | 12 +++++------- 7 files changed, 33 insertions(+), 23 deletions(-) base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5 diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h index af9ed60a0353..e098cb69aa43 100644 --- a/include/libcamera/internal/camera_manager.h +++ b/include/libcamera/internal/camera_manager.h @@ -19,13 +19,14 @@ #include #include -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/process.h" namespace libcamera { class Camera; class DeviceEnumerator; +class IPAManager; +class PipelineHandlerFactoryBase; class CameraManager::Private : public Extensible::Private, public Thread { @@ -38,6 +39,8 @@ public: void addCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_); void removeCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_); + IPAManager *ipaManager() const { return ipaManager_.get(); } + protected: void run() override; @@ -62,7 +65,7 @@ private: std::unique_ptr enumerator_; - IPAManager ipaManager_; + std::unique_ptr ipaManager_; ProcessManager processManager_; }; diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index c6f74e11c434..16dede0c5a7e 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -15,6 +15,7 @@ #include #include +#include "libcamera/internal/camera_manager.h" #include "libcamera/internal/ipa_module.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/pub_key.h" @@ -34,11 +35,13 @@ public: uint32_t minVersion, uint32_t maxVersion) { - IPAModule *m = self_->module(pipe, minVersion, maxVersion); + CameraManager *cm = pipe->cameraManager(); + IPAManager *self = cm->_d()->ipaManager(); + IPAModule *m = self->module(pipe, minVersion, maxVersion); if (!m) return nullptr; - std::unique_ptr proxy = std::make_unique(m, !self_->isSignatureValid(m)); + std::unique_ptr proxy = std::make_unique(m, !self->isSignatureValid(m)); if (!proxy->isValid()) { LOG(IPAManager, Error) << "Failed to load proxy"; return nullptr; @@ -55,8 +58,6 @@ public: #endif private: - static IPAManager *self_; - void parseDir(const char *libDir, unsigned int maxDepth, std::vector &files); unsigned int addDir(const char *libDir, unsigned int maxDepth = 0); diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 746a34f8e7bd..cad5812f640c 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -70,6 +70,8 @@ public: const char *name() const { return name_; } + CameraManager *cameraManager() const { return manager_; } + protected: void registerCamera(std::shared_ptr camera); void hotplugMediaDevice(MediaDevice *media); diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 95a9e3264526..31760a8680cc 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -15,6 +15,7 @@ #include "libcamera/internal/camera.h" #include "libcamera/internal/device_enumerator.h" +#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/pipeline_handler.h" /** @@ -37,6 +38,7 @@ LOG_DEFINE_CATEGORY(Camera) CameraManager::Private::Private() : initialized_(false) { + ipaManager_ = std::make_unique(); } int CameraManager::Private::start() @@ -249,6 +251,13 @@ void CameraManager::Private::removeCamera(std::shared_ptr camera) o->cameraRemoved.emit(camera); } +/** + * \fn CameraManager::Private::ipaManager() const + * \brief Retrieve the IPAManager + * \context This function is \threadsafe. + * \return The IPAManager for this CameraManager + */ + /** * \class CameraManager * \brief Provide access and manage all cameras in the system diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index f4e0b6339f08..cfc24d389c4f 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -95,8 +95,6 @@ LOG_DEFINE_CATEGORY(IPAManager) * IPC. */ -IPAManager *IPAManager::self_ = nullptr; - /** * \brief Construct an IPAManager instance * @@ -105,10 +103,6 @@ IPAManager *IPAManager::self_ = nullptr; */ IPAManager::IPAManager() { - if (self_) - LOG(IPAManager, Fatal) - << "Multiple IPAManager objects are not allowed"; - #if HAVE_IPA_PUBKEY if (!pubKey_.isValid()) LOG(IPAManager, Warning) << "Public key not valid"; @@ -153,16 +147,12 @@ IPAManager::IPAManager() if (!ipaCount) LOG(IPAManager, Warning) << "No IPA found in '" IPA_MODULE_DIR "'"; - - self_ = this; } IPAManager::~IPAManager() { for (IPAModule *module : modules_) delete module; - - self_ = nullptr; } /** diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 5ea2ca780b63..5a6de685b292 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -719,6 +719,13 @@ void PipelineHandler::disconnect() * \return The pipeline handler name */ +/** + * \fn PipelineHandler::cameraManager() const + * \brief Retrieve the CameraManager that this pipeline handler belongs to + * \context This function is \threadsafe. + * \return The CameraManager for this pipeline handler + */ + /** * \class PipelineHandlerFactoryBase * \brief Base class for pipeline handler factories diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index e840f6ab17c4..b81783664977 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -20,11 +20,11 @@ #include #include +#include "libcamera/internal/camera_manager.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/ipa_module.h" #include "libcamera/internal/pipeline_handler.h" -#include "libcamera/internal/process.h" #include "test.h" @@ -44,20 +44,20 @@ public: { delete notifier_; ipa_.reset(); - ipaManager_.reset(); + cameraManager_.reset(); } protected: int init() override { - ipaManager_ = make_unique(); + cameraManager_ = make_unique(); /* Create a pipeline handler for vimc. */ const std::vector &factories = PipelineHandlerFactoryBase::factories(); for (const PipelineHandlerFactoryBase *factory : factories) { if (factory->name() == "vimc") { - pipe_ = factory->create(nullptr); + pipe_ = factory->create(cameraManager_.get()); break; } } @@ -171,11 +171,9 @@ private: } } - ProcessManager processManager_; - std::shared_ptr pipe_; std::unique_ptr ipa_; - std::unique_ptr ipaManager_; + std::unique_ptr cameraManager_; enum ipa::vimc::IPAOperationCode trace_; EventNotifier *notifier_; int fd_;