From patchwork Sat Sep 30 15:28:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19099 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 A3CF2BD808 for ; Sat, 30 Sep 2023 15:28:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D6A3762963; Sat, 30 Sep 2023 17:28:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1696087700; bh=eCrfEneJ67wjNS40pgH/lCo3wuYkviRMOtiUcxOMPGE=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=yN9o50frjQwUJFhXOdjRGa1gkkzF3yt01PvdeSUGNdM56eshZWc7AdKOR2eINmRL0 7Y+jKvjjrfb7TL+B3bA88bNUqnLGilgQBur3jwAgYXzYsSgg027EnddXldUaTTea6P 1A1HDs53S40ZMCMMlD/xorFvMz4WfNcpnsBQNkloA3Qi+9wrz64iZIgIZ4YVqVf3I5 veKUWn7PmPjejwRLi030ZuIjy/DBIVUhYn8ibn10S+bBmvXc63GjGh+vXnQPpTsymf Nd6P9gnl99kbNJthDmblOBCmmBe3pR2u1sBOOb3sZRseWrJz5KJDMjjkfrMuY7hnay gdUsazeRPNnkA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3EB9D61DE1 for ; Sat, 30 Sep 2023 17:28:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vP7caORS"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (lfbn-idf1-1-343-200.w86-195.abo.wanadoo.fr [86.195.61.200]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8B445497 for ; Sat, 30 Sep 2023 17:26:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1696087595; bh=eCrfEneJ67wjNS40pgH/lCo3wuYkviRMOtiUcxOMPGE=; h=From:To:Subject:Date:From; b=vP7caORS5wnPywOp2r1zUpjCPIGuwj+Eetb3rUb/7SllFKboFM86JxTalBrZv1WnF zbaa336j1m6xe2GIJWIBWBxbJyhIcDcGhAm4MigW3o8kyHv61H6c9I8FhDBjwVOa// Sh8yntT5SeFYIZa7+EuNXDVSHeO6RQVzimRD1azM= 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 Subject: [libcamera-devel] [PATCH] libcamera: ipa_manager: Remove singleton requirement 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart 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 --- include/libcamera/internal/camera_manager.h | 6 ++++-- 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, 32 insertions(+), 23 deletions(-) diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h index 33ebe0699fdf..a0eec42fac7b 100644 --- a/include/libcamera/internal/camera_manager.h +++ b/include/libcamera/internal/camera_manager.h @@ -19,13 +19,13 @@ #include #include -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/process.h" namespace libcamera { class Camera; class DeviceEnumerator; +class IPAManager; class CameraManager::Private : public Extensible::Private, public Thread { @@ -38,6 +38,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; @@ -61,7 +63,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 bf823563c91c..23db6b8a0221 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 c96944f4ecc4..bbfae557e023 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 355f3adad68f..909a3cd70117 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() @@ -220,6 +222,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 7a4515d90d7b..be1bb704c6cd 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 9c74c6cfda70..15783fdc7447 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 051ef96e7ed2..af52eed72ec8 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -19,11 +19,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" @@ -43,20 +43,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() == "PipelineHandlerVimc") { - pipe_ = factory->create(nullptr); + pipe_ = factory->create(cameraManager_.get()); break; } } @@ -170,11 +170,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_;