From patchwork Thu May 11 22:58:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 18623 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 881FBBD16B for ; Thu, 11 May 2023 22:58:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 81CC8633C2; Fri, 12 May 2023 00:58:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1683845922; bh=3ExVC91FmGtc9FklNc+yXz0Wp0KSHyqL3SC0ExEt0S0=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=FcIHktMStt7CQzE5/aNl0rmPwlCUSecT1RQZWgNlW3int7cVNVE/QfAu7V9CC/mxq PLqQYvpmtNMLjhjOer78eTtgcUmtw+ruETbFAjfoqGkp4RHPjPWxFdZF+0S5xdaVHl B1eJ6mm5uHrW53I14nPA/mJSNomVdi/92gyOvfkU+PcEdOJXVf169dhpC9rsklukSD DX+1fo9g2VQmZO6f8+dahawcYFf9fZ9U4Wb16T1Y+gpyjNK8CKoNkJCYSYbmneePcY wyHDVbMCSHHTaUNwVQc4ieh5Wqx1pL+z1VAwMus1cwtZYmSvsqT8I+OxDHvQZXaq04 ro4AzCjxlUX3Q== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 60D83633B6 for ; Fri, 12 May 2023 00:58:39 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="L2bc/HEl"; dkim-atps=neutral Received: from Monstersaurus.local (aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net [82.37.23.78]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E28B8CF9; Fri, 12 May 2023 00:58:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1683845911; bh=3ExVC91FmGtc9FklNc+yXz0Wp0KSHyqL3SC0ExEt0S0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=L2bc/HElpoDIzZz0BVWvei0qsC3EAEpC8W8mWCTE3XQNYOoUlRCUNdDhdKJjWNqop SJtmdi5PGS/KBkRCn4QxKNI9NJ3Hn/7UJsxVNibGucZT//XkuEt0TBkWUfXCIexbsr rVxZrjJoEyOUIfcaJWHeREy/wllCeBCHe0U9N3v4= To: libcamera devel Date: Thu, 11 May 2023 23:58:31 +0100 Message-Id: <20230511225833.361699-3-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230511225833.361699-1-kieran.bingham@ideasonboard.com> References: <20230511225833.361699-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_manager: Move {add, remove}Camera to internal 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: Kieran Bingham via libcamera-devel From: Kieran Bingham Reply-To: Kieran Bingham Cc: George Kiagiadakis , Robert Mader , Wim Taymans Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The CameraManager exposes addCamera and removeCamera as public API calls, while they should never be called from an application. These calls are only expected to be used by PipelineHandlers to update the CameraManager that a new Camera has been created and allow the Camera Manager to expose it to applications. Remove the public calls and update the private implementations such that they can be used directly by the PipelineHandler through the internal CameraManager::Private provided by the Extensible class. Signed-off-by: Kieran Bingham Reviewed-by: Jacopo Mondi --- include/libcamera/camera_manager.h | 4 - include/libcamera/internal/camera_manager.h | 2 +- src/libcamera/camera_manager.cpp | 87 +++++++++------------ src/libcamera/pipeline_handler.cpp | 6 +- 4 files changed, 43 insertions(+), 56 deletions(-) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 4b1fb7568e83..9767acc42c89 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -34,10 +34,6 @@ public: std::shared_ptr get(const std::string &id); std::shared_ptr get(dev_t devnum); - void addCamera(std::shared_ptr camera, - const std::vector &devnums); - void removeCamera(std::shared_ptr camera); - static const std::string &version() { return version_; } Signal> cameraAdded; diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h index 4bba3ac2eb29..8386e90e0f60 100644 --- a/include/libcamera/internal/camera_manager.h +++ b/include/libcamera/internal/camera_manager.h @@ -31,7 +31,7 @@ public: int start(); void addCamera(std::shared_ptr camera, const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_); - void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_); + void removeCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_); /* * This mutex protects diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index b95284ba5a80..34f09a4d181b 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -148,9 +148,25 @@ void CameraManager::Private::cleanup() enumerator_.reset(nullptr); } +/** + * \brief Add a camera to the camera manager + * \param[in] camera The camera to be added + * \param[in] devnums The device numbers to associate with \a camera + * + * This function is called by pipeline handlers to register the cameras they + * handle with the camera manager. Registered cameras are immediately made + * available to the system. + * + * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes + * to Camera instances. + * + * \context This function shall be called from the CameraManager thread. + */ void CameraManager::Private::addCamera(std::shared_ptr camera, const std::vector &devnums) { + ASSERT(Thread::current() == this); + MutexLocker locker(mutex_); for (const std::shared_ptr &c : cameras_) { @@ -167,15 +183,31 @@ void CameraManager::Private::addCamera(std::shared_ptr camera, unsigned int index = cameras_.size() - 1; for (dev_t devnum : devnums) camerasByDevnum_[devnum] = cameras_[index]; + + /* Report the addition to the public signal */ + CameraManager *const o = LIBCAMERA_O_PTR(); + o->cameraAdded.emit(cameras_[index]); } -void CameraManager::Private::removeCamera(Camera *camera) +/** + * \brief Remove a camera from the camera manager + * \param[in] camera The camera to be removed + * + * This function is called by pipeline handlers to unregister cameras from the + * camera manager. Unregistered cameras won't be reported anymore by the + * cameras() and get() calls, but references may still exist in applications. + * + * \context This function shall be called from the CameraManager thread. + */ +void CameraManager::Private::removeCamera(std::shared_ptr camera) { + ASSERT(Thread::current() == this); + MutexLocker locker(mutex_); auto iter = std::find_if(cameras_.begin(), cameras_.end(), [camera](std::shared_ptr &c) { - return c.get() == camera; + return c.get() == camera.get(); }); if (iter == cameras_.end()) return; @@ -185,12 +217,16 @@ void CameraManager::Private::removeCamera(Camera *camera) auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(), [camera](const std::pair> &p) { - return p.second.lock().get() == camera; + return p.second.lock().get() == camera.get(); }); if (iter_d != camerasByDevnum_.end()) camerasByDevnum_.erase(iter_d); cameras_.erase(iter); + + /* Report the removal to the public signal */ + CameraManager *const o = LIBCAMERA_O_PTR(); + o->cameraRemoved.emit(camera); } /** @@ -383,51 +419,6 @@ std::shared_ptr CameraManager::get(dev_t devnum) * perform any blocking operation. */ -/** - * \brief Add a camera to the camera manager - * \param[in] camera The camera to be added - * \param[in] devnums The device numbers to associate with \a camera - * - * This function is called by pipeline handlers to register the cameras they - * handle with the camera manager. Registered cameras are immediately made - * available to the system. - * - * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes - * to Camera instances. - * - * \context This function shall be called from the CameraManager thread. - */ -void CameraManager::addCamera(std::shared_ptr camera, - const std::vector &devnums) -{ - Private *const d = _d(); - - ASSERT(Thread::current() == d); - - d->addCamera(camera, devnums); - cameraAdded.emit(camera); -} - -/** - * \brief Remove a camera from the camera manager - * \param[in] camera The camera to be removed - * - * This function is called by pipeline handlers to unregister cameras from the - * camera manager. Unregistered cameras won't be reported anymore by the - * cameras() and get() calls, but references may still exist in applications. - * - * \context This function shall be called from the CameraManager thread. - */ -void CameraManager::removeCamera(std::shared_ptr camera) -{ - Private *const d = _d(); - - ASSERT(Thread::current() == d); - - d->removeCamera(camera.get()); - cameraRemoved.emit(camera); -} - /** * \fn const std::string &CameraManager::version() * \brief Retrieve the libcamera version string diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index f72613b8e515..49092ea88a58 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -16,10 +16,10 @@ #include #include -#include #include #include "libcamera/internal/camera.h" +#include "libcamera/internal/camera_manager.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/framebuffer.h" #include "libcamera/internal/media_device.h" @@ -624,7 +624,7 @@ void PipelineHandler::registerCamera(std::shared_ptr camera) } } - manager_->addCamera(std::move(camera), devnums); + manager_->_d()->addCamera(std::move(camera), devnums); } /** @@ -691,7 +691,7 @@ void PipelineHandler::disconnect() continue; camera->disconnect(); - manager_->removeCamera(camera); + manager_->_d()->removeCamera(camera); } }