From patchwork Tue Jun 16 19:45:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 4064 Return-Path: Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 01F0261167 for ; Tue, 16 Jun 2020 21:45:34 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="VED8T0Gc"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc:content-type: content-transfer-encoding; s=s1; bh=yx6dU7ckU6ShtvwUOxgtaiD4W7+suRpe2YocURNhkf8=; b=VED8T0GcnyhgnatFQj4aUQSxS47llaeCDxpWcTkAWm/kdJztg9J6rbEJyYyresCP0vWz dl7IMswKa/Ru5Z/i5+RcJ1Y8uNVhQe7VmS796k1Yw8uCvRVwQ3Mno/xID4wCV2y2RlgHwj wjakyLSTACzXrH8o4j5eSvpY3787WrOG4= Received: by filterdrecv-p3mdw1-6f5df8956d-4d2bg with SMTP id filterdrecv-p3mdw1-6f5df8956d-4d2bg-21-5EE9215D-2A 2020-06-16 19:45:33.47457207 +0000 UTC m=+1121502.324567696 Received: from mail.uajain.com (unknown) by ismtpd0004p1maa1.sendgrid.net (SG) with ESMTP id _BnG-p6kSZuoZoFr7UImzA Tue, 16 Jun 2020 19:45:33.074 +0000 (UTC) From: Umang Jain Date: Tue, 16 Jun 2020 19:45:33 +0000 (UTC) Message-Id: <20200616194523.23268-2-email@uajain.com> In-Reply-To: <20200616194523.23268-1-email@uajain.com> References: <20200616194523.23268-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPc9vLW9W8H4bP+KZ0PPVtun2GUUTio/M2EKpbAqho5duHtKP8eDtaspFrCVhJ3PR4vH4mRMd6r9cvmCvIQ41w0V5s6xB89i6oWBMoIuY7NpmIkHD8QkD8HoD7yLL+opDFU9zWt6CvP0UsGm++XnnIalUSTSuKsRfmnw79WvGN0AY/sQXGanPBxpsGKAHATyFM45p9lFjfFgueV2J9k7IurlA== To: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v5 1/6] libcamera: CameraManager: Drop the vector of created PipelineHandlers 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-List-Received-Date: Tue, 16 Jun 2020 19:45:35 -0000 The pipes_ vector was initially used to store pipeline handlers instances with the CameraManager when it cannot be referenced from anywhere else. It was used to retrieve cameras and deleting pipeline handlers when stopping the camera manager. In f3695e9b09ce ("libcamera: camera_manager: Register cameras with the camera manager"), cameras started to get registered directly with camera manager and in 5b02e03199b7 ("libcamera: camera: Associate cameras with their pipeline handler") pipeline handler started to get stored in a std::shared_ptr<> with each camera starting to hold a strong reference to its associated pipeline-handler. At this point, both the camera manager and the camera held a strong reference to the pipeline handler. Since the additional reference held by the camera manager gets released only on cleanup(), this lurking reference held on pipeline handler, did not allow it to get destroyed the even when cameras instances have been destroyed. This situation of having a pipeline handler instance around without having a camera, may lead to problems (one of them explained below) especially when the camera manager is still running. It was noticed that, there was a dangling driver directory issue (tested for UVC camera - in /sys/bus/usb/drivers/uvcvideo) on 'unbind' → 'bind' operation while the CameraManager is running. The directories were still kept around even after 'unbind' because of the lurking reference of pipeline handler holding onto them. That reference would clear if and only if the CameraManager is stopped and then only directories were getting removed in the above stated path. Rather than writing a fix to release the pipeline handlers' reference from camera manager on camera disconnection, it is decided to eliminate the pipes_ vector from CameraManager moving forwards. There is no point in holding a reference to it from camera manager's point-of-view at this stage. It also helps us to fix the issue as explained above. Now that the pipeline handler instances are referenced via cameras only, it can happen that the destruction of last camera instance may result into destruction of pipeline handler itself. Such a possibility exists PipelineHandler::disconnect(), where the pipeline handler itself can get destroyed while removing the camera. This is acceptable as long as we make sure that there is no access of pipeline handler's members later on in the code-path. Address this situation and also add a detailed comment about it. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart --- src/libcamera/camera_manager.cpp | 8 ++------ src/libcamera/pipeline_handler.cpp | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 576856a..dbdc78e 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -63,7 +63,6 @@ private: bool initialized_; int status_; - std::vector> pipes_; std::unique_ptr enumerator_; IPAManager ipaManager_; @@ -144,7 +143,6 @@ int CameraManager::Private::init() LOG(Camera, Debug) << "Pipeline handler \"" << factory->name() << "\" matched"; - pipes_.push_back(std::move(pipe)); } } @@ -158,11 +156,9 @@ void CameraManager::Private::cleanup() /* TODO: unregister hot-plug callback here */ /* - * Release all references to cameras and pipeline handlers to ensure - * they all get destroyed before the device enumerator deletes the - * media devices. + * Release all references to cameras to ensure they all get destroyed + * before the device enumerator deletes the media devices. */ - pipes_.clear(); cameras_.clear(); enumerator_.reset(nullptr); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index a0f6b0f..c457be9 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -559,7 +559,21 @@ void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) */ void PipelineHandler::disconnect() { - for (std::weak_ptr ptr : cameras_) { + /* + * Each camera holds a reference to its associated pipeline handler + * instance. Hence, when the last camera is dropped, the pipeline + * handler will get destroyed by the last manager_->removeCamera(camera) + * call in the loop below. + * + * This is acceptable as long as we make sure that the code path does not + * access any member of the (already destroyed)pipeline handler instance + * afterwards. Therefore, we move the cameras_ vector to a local temporary + * container to avoid accessing freed memory later i.e. to explicitly run + * cameras_.clear(). + */ + std::vector> cameras{ std::move(cameras_) }; + + for (std::weak_ptr ptr : cameras) { std::shared_ptr camera = ptr.lock(); if (!camera) continue; @@ -567,8 +581,6 @@ void PipelineHandler::disconnect() camera->disconnect(); manager_->removeCamera(camera.get()); } - - cameras_.clear(); } /** From patchwork Tue Jun 16 19:45:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 4066 Return-Path: Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F9DE603C4 for ; Tue, 16 Jun 2020 21:45:36 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="GerWgxdu"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc: content-transfer-encoding:content-type; s=s1; bh=FikAzZVC5wWvR5l9nNnuXN41hQ1xpsTkcWITVMaV9w8=; b=GerWgxduibsIj9F3tbfZDEIXqcl+BuRVysv8F1VBaHCtXCgxwwqpRIsO3MLhjCfdXH9n W7b0arLfGisO5g2/L3ez/hWRc4Cqhm2ZaExUiE0zL/pm0rvvoEUaUY9ghT33hSCCi/OsqR Qpp5PtvBLC6DLbi7ThbApgD70NDtC6V7k= Received: by filter0088p3las1.sendgrid.net with SMTP id filter0088p3las1-13542-5EE9215E-EB 2020-06-16 19:45:34.897993716 +0000 UTC m=+696962.422127552 Received: from mail.uajain.com (unknown) by ismtpd0007p1maa1.sendgrid.net (SG) with ESMTP id UUHnVglXTjO3BhjCr1MdVg Tue, 16 Jun 2020 19:45:34.523 +0000 (UTC) From: Umang Jain Date: Tue, 16 Jun 2020 19:45:34 +0000 (UTC) Message-Id: <20200616194523.23268-3-email@uajain.com> In-Reply-To: <20200616194523.23268-1-email@uajain.com> References: <20200616194523.23268-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPclq38gSC/txiORZpK15NbTiNN4t34vkHbngDjoVHubyaJU1FuzYgc2u/UQviNeN62Dkfco7jq6QVug6u9j3fxs8okeTO/ce1eJ7rurDyE5Ai3aobw0oNnF61atrYFR6MAd4MRqqec6qNNBOZtbe4JSDW/ftj6oPfP7EmQZhX2x8q+sAmAyMiK+00DNu/VWf29 To: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v5 2/6] libcamera: camera_manager: Refactor pipelines creation into separate function 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-List-Received-Date: Tue, 16 Jun 2020 19:45:37 -0000 This commit introduces no functional changes. Split pipelines creation code into a separate function, so that the function can be re-used for upcoming hotplug functionality in subsequent commits. Also, fixup correct tag for \todo. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart --- src/libcamera/camera_manager.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index dbdc78e..de6d5d8 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -55,6 +55,7 @@ protected: private: int init(); + void createPipelineHandlers(); void cleanup(); CameraManager *cm_; @@ -123,12 +124,20 @@ int CameraManager::Private::init() if (!enumerator_ || enumerator_->enumerate()) return -ENODEV; + createPipelineHandlers(); + + return 0; +} + +void CameraManager::Private::createPipelineHandlers() +{ /* - * TODO: Try to read handlers and order from configuration + * \todo Try to read handlers and order from configuration * file and only fallback on all handlers if there is no * configuration file. */ - std::vector &factories = PipelineHandlerFactory::factories(); + std::vector &factories = + PipelineHandlerFactory::factories(); for (PipelineHandlerFactory *factory : factories) { /* @@ -146,14 +155,12 @@ int CameraManager::Private::init() } } - /* TODO: register hot-plug callback here */ - - return 0; + /* \todo Register hot-plug callback here */ } void CameraManager::Private::cleanup() { - /* TODO: unregister hot-plug callback here */ + /* \todo Unregister hot-plug callback here */ /* * Release all references to cameras to ensure they all get destroyed From patchwork Tue Jun 16 19:45:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 4067 Return-Path: Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FE32603C4 for ; Tue, 16 Jun 2020 21:45:37 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="homrzrVG"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc: content-transfer-encoding:content-type; s=s1; bh=HRIMNLA7LvCcjxKSj7EXJEBLwqsw622/8tZpTpXIiKM=; b=homrzrVG/mTYqN/OmKYyECq7uVt5vEiNFrn+W1sch0AZjSw2r+SKH45QvssyrFeA7TCq CsOzXN8fVnGhAwC45bIB/lWwAPubJvMSvRcxX3DRMTeEF99OMRAZOqSgjvtXfSrKNauEM+ rluRWLw/2STQ7IV5ueU7qQJ0+qYXnPD6s= Received: by filterdrecv-p3mdw1-6f5df8956d-25bh4 with SMTP id filterdrecv-p3mdw1-6f5df8956d-25bh4-19-5EE9215F-8E 2020-06-16 19:45:35.945609367 +0000 UTC m=+1121503.543881372 Received: from mail.uajain.com (unknown) by ismtpd0006p1maa1.sendgrid.net (SG) with ESMTP id XTwDqPMGS7G7f7OK3HfI6Q Tue, 16 Jun 2020 19:45:35.587 +0000 (UTC) From: Umang Jain Date: Tue, 16 Jun 2020 19:45:36 +0000 (UTC) Message-Id: <20200616194523.23268-4-email@uajain.com> In-Reply-To: <20200616194523.23268-1-email@uajain.com> References: <20200616194523.23268-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPclKwCK9CCZVFbQyNjpl2ex/ZeUGECgJ3/kvYxMJpVbVxe/Y/bRHUQZ5dpXE55Cw1MNGLVQWvLoPv8rSaooLH+nGSNa6CPFgOK7Yjxd+EUHlr644WwKjA53nU4vA/woL4axWn10SvFqkEmXfz2XqV4gVeyU/sYXFE8anC/rJtZfRYPOS+ihC5P5NwkWV0LuZWctpXcKf4PkCsyKTp8QzMbEA== To: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v5 3/6] libcamera: device_enumerator: Emit a signal when new devices are added 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-List-Received-Date: Tue, 16 Jun 2020 19:45:38 -0000 Emit a signal whenever new MediaDevices are added to the DeviceEnumerator. This will allow CameraManager to be notified about the new devices and it can re-emumerate all the devices currently present on the system. Device enumeration by the CameraManger is an expensive operation hence, we want one signal emission per 'x' milliseconds to notify multiple devices additions as a single batch, by the DeviceEnumerator. Add a \todo to investigate the support for that. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- include/libcamera/internal/device_enumerator.h | 4 ++++ src/libcamera/camera_manager.cpp | 4 ++-- src/libcamera/device_enumerator.cpp | 13 +++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/include/libcamera/internal/device_enumerator.h b/include/libcamera/internal/device_enumerator.h index 25a3630..a985040 100644 --- a/include/libcamera/internal/device_enumerator.h +++ b/include/libcamera/internal/device_enumerator.h @@ -13,6 +13,8 @@ #include +#include + namespace libcamera { class MediaDevice; @@ -43,6 +45,8 @@ public: std::shared_ptr search(const DeviceMatch &dm); + Signal<> devicesAdded; + protected: std::unique_ptr createDevice(const std::string &deviceNode); void addDevice(std::unique_ptr &&media); diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index de6d5d8..3082797 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -155,12 +155,12 @@ void CameraManager::Private::createPipelineHandlers() } } - /* \todo Register hot-plug callback here */ + enumerator_->devicesAdded.connect(this, &Private::createPipelineHandlers); } void CameraManager::Private::cleanup() { - /* \todo Unregister hot-plug callback here */ + enumerator_->devicesAdded.disconnect(this, &Private::createPipelineHandlers); /* * Release all references to cameras to ensure they all get destroyed diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index e21a2a7..647974b 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -227,6 +227,16 @@ std::unique_ptr DeviceEnumerator::createDevice(const std::string &d return media; } +/** +* \var DeviceEnumerator::devicesAdded +* \brief Notify of new media devices being found +* +* This signal is emitted when the device enumerator finds new media devices in +* the system. It may be emitted for every newly detected device, or once for +* multiple devices, at the discretion of the device enumerator. Not all device +* enumerator types may support dynamic detection of new devices. +*/ + /** * \brief Add a media device to the enumerator * \param[in] media media device instance to add @@ -242,6 +252,9 @@ void DeviceEnumerator::addDevice(std::unique_ptr &&media) << "Added device " << media->deviceNode() << ": " << media->driver(); devices_.push_back(std::move(media)); + + /* \todo To batch multiple additions, emit with a small delay here. */ + devicesAdded.emit(); } /** From patchwork Tue Jun 16 19:45:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 4068 Return-Path: Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D20761FD1 for ; Tue, 16 Jun 2020 21:45:38 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="SyAqNdY9"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc: content-transfer-encoding:content-type; s=s1; bh=5fZuCOlOjehHfZWcTYNUUbIdzczRLNu4hqtWf5b/40I=; b=SyAqNdY9OY32QEiTB2nGK/c1x6drnHt1PAXw6mEz6cj4Q5Tr13Hxrb/HNT4Mfhj8U9tI PYjuOeoJEjZHflyUeCW1HiKsP4UMccAUP5swzDI52rQEFbc4YLw0QXo+PNWJjdK1ohaQJw Ez9WdccvJ8vknpURcm/CrFQWT9CDA+FB4= Received: by filter0072p3las1.sendgrid.net with SMTP id filter0072p3las1-4518-5EE92160-12C 2020-06-16 19:45:37.016591458 +0000 UTC m=+696974.774711833 Received: from mail.uajain.com (unknown) by ismtpd0001p1maa1.sendgrid.net (SG) with ESMTP id tZfbyfNcSXSoVVk-cYFJtA Tue, 16 Jun 2020 19:45:36.593 +0000 (UTC) From: Umang Jain Date: Tue, 16 Jun 2020 19:45:37 +0000 (UTC) Message-Id: <20200616194523.23268-5-email@uajain.com> In-Reply-To: <20200616194523.23268-1-email@uajain.com> References: <20200616194523.23268-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPc+BcW/nTZaHvUFD9w8u96rn6USOorH4S8WjFE0sjV2vKQO7b8kniI5IXhu4VpoOlqj8gzD9PuY9u3jLT3eFNtYfcCulN+55dMj3C9Mklmc0hz7NDK7iA0hvNOmj2g1DCZG5BL2HJ2ny4ypNUj81ruSarfc5as1h1ppZBuf+PfMh+VNyo5f5YFizKpT723kOjp To: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v5 4/6] libcamera: camera_manager: Introduce signals when a camera is added or removed 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-List-Received-Date: Tue, 16 Jun 2020 19:45:39 -0000 Emit 'cameraAdded' and 'cameraRemoved' from CameraManager to enable hotplug and hot-unplug support in application like QCam. To avoid use-after-free race between the CameraManager and the application, emit the 'cameraRemoved' with the shared_ptr version of . This requires to change the function signature of CameraManager::removeCamera() API. Also, until now, CameraManager::Private::addCamera() transfers the entire ownership of camera shared_ptr to CameraManager using std::move(). This patch changes the signature of Private::addCamera to accept pass-by-value camera parameter. It is done to make it clear from the caller point of view that the pointer within the caller will still be valid after this function returns. With this change in, we can emit the camera pointer via 'cameraAdded' signal without hitting a segfault. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart --- include/libcamera/camera_manager.h | 6 ++++- src/libcamera/camera_manager.cpp | 38 ++++++++++++++++++++++++++---- src/libcamera/pipeline_handler.cpp | 2 +- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 95dc636..9eb2b6f 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -13,6 +13,7 @@ #include #include +#include namespace libcamera { @@ -36,13 +37,16 @@ public: void addCamera(std::shared_ptr camera, const std::vector &devnums); - void removeCamera(Camera *camera); + void removeCamera(std::shared_ptr camera); static const std::string &version() { return version_; } void setEventDispatcher(std::unique_ptr dispatcher); EventDispatcher *eventDispatcher(); + Signal> cameraAdded; + Signal> cameraRemoved; + private: static const std::string version_; static CameraManager *self_; diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 3082797..f60491d 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -36,7 +36,7 @@ public: Private(CameraManager *cm); int start(); - void addCamera(std::shared_ptr &camera, + void addCamera(std::shared_ptr camera, const std::vector &devnums); void removeCamera(Camera *camera); @@ -171,7 +171,7 @@ void CameraManager::Private::cleanup() enumerator_.reset(nullptr); } -void CameraManager::Private::addCamera(std::shared_ptr &camera, +void CameraManager::Private::addCamera(std::shared_ptr camera, const std::vector &devnums) { MutexLocker locker(mutex_); @@ -374,6 +374,34 @@ std::shared_ptr CameraManager::get(dev_t devnum) return iter->second.lock(); } +/** + * \var CameraManager::cameraAdded + * \brief Notify of a new camera added to the system + * + * This signal is emitted when a new camera is detected and successfully handled + * by the camera manager. The notification occurs alike for cameras detected + * when the manager is started with start() or when new cameras are later + * connected to the system. When the signal is emitted the new camera is already + * available from the list of cameras(). + * + * The signal is emitted from the CameraManager thread. Applications shall + * minimize the time spent in the signal handler and shall in particular not + * perform any blocking operation. + */ + +/** + * \var CameraManager::cameraRemoved + * \brief Notify of a new camera removed from the system + * + * This signal is emitted when a camera is removed from the system. When the + * signal is emitted the camera is not available from the list of cameras() + * anymore. + * + * The signal is emitted from the CameraManager thread. Applications shall + * minimize the time spent in the signal handler and shall in particular not + * perform any blocking operation. + */ + /** * \brief Add a camera to the camera manager * \param[in] camera The camera to be added @@ -394,6 +422,7 @@ void CameraManager::addCamera(std::shared_ptr camera, ASSERT(Thread::current() == p_.get()); p_->addCamera(camera, devnums); + cameraAdded.emit(camera); } /** @@ -406,11 +435,12 @@ void CameraManager::addCamera(std::shared_ptr camera, * * \context This function shall be called from the CameraManager thread. */ -void CameraManager::removeCamera(Camera *camera) +void CameraManager::removeCamera(std::shared_ptr camera) { ASSERT(Thread::current() == p_.get()); - p_->removeCamera(camera); + p_->removeCamera(camera.get()); + cameraRemoved.emit(camera); } /** diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index c457be9..0e13790 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -579,7 +579,7 @@ void PipelineHandler::disconnect() continue; camera->disconnect(); - manager_->removeCamera(camera.get()); + manager_->removeCamera(camera); } } From patchwork Tue Jun 16 19:45:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 4069 Return-Path: Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CB66D61F24 for ; Tue, 16 Jun 2020 21:45:39 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="BUwHJwKf"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc: content-transfer-encoding:content-type; s=s1; bh=a09DxRVCdx2Juyi1912x8Ji3Vtfs0ty9OppLXC61jBU=; b=BUwHJwKf7bZwXvvE9ULEI3dqiG6hE/iU3VI0xYjXAFpuDBZ7rCEChxJBOK6TyY/fE1s3 GNsopYyY6jzxvBE7pVDLMsU6jwYe0KxK/A2VFEhiZhojcGbTnaVjYLp6q4WTunwaKj/Uj0 N7EG13km5YyhSSIQmloJsJI4j6Ysie7aI= Received: by filter0104p3las1.sendgrid.net with SMTP id filter0104p3las1-31629-5EE92161-132 2020-06-16 19:45:38.316073928 +0000 UTC m=+696655.239765817 Received: from mail.uajain.com (unknown) by ismtpd0001p1maa1.sendgrid.net (SG) with ESMTP id Nedy3N-4RE-Rf1Vu5vXEXA Tue, 16 Jun 2020 19:45:37.797 +0000 (UTC) From: Umang Jain Date: Tue, 16 Jun 2020 19:45:38 +0000 (UTC) Message-Id: <20200616194523.23268-6-email@uajain.com> In-Reply-To: <20200616194523.23268-1-email@uajain.com> References: <20200616194523.23268-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcHIuSzkf5/M5SksOT1pntG8LZHPMsqXV31uFoZfh8NyQ1mvi2JGiTrcfWSqIMI8j0KquW2MH6aIEnkoN7gUAHjmyCKl4og9A4/KNXnVW+11FuaP4xODobxRLrV0REhw/D2+W2JFRAsgQ7CAvTUcS2jJUcOPtyF1cfOi0fF0MjCxp0LfPp75ju9BR6uSLHqau2 To: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v5 5/6] qcam: main_window: Introduce initial hotplug support 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-List-Received-Date: Tue, 16 Jun 2020 19:45:40 -0000 Hook up various QCam UI bits with hotplug support introduced in previous commits. This looks good-enough as first steps to see how the hotplugging functionality is turning out to be from application point-of-view. One can still think of few edge case nuances not yet covered under this implementation especially around having only one camera in the system and hotplugging/hot-unplugging it. Hence, those are intentionally kept out of scope for now. It might require some thinking on how to handle it on application level having additional time on hand. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/qcam/main_window.cpp | 76 ++++++++++++++++++++++++++++++++++++++++ src/qcam/main_window.h | 6 ++++ 2 files changed, 82 insertions(+) diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 2960259..7bc1360 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -59,6 +59,36 @@ public: } }; +/** + * \brief Custom QEvent to signal hotplug or unplug + */ +class HotplugEvent : public QEvent +{ +public: + enum PlugEvent { + HotPlug, + HotUnplug + }; + + HotplugEvent(std::shared_ptr camera, PlugEvent event) + : QEvent(type()), camera_(std::move(camera)), plugEvent_(event) + { + } + + static Type type() + { + static int type = QEvent::registerEventType(); + return static_cast(type); + } + + PlugEvent hotplugEvent() const { return plugEvent_; } + Camera *camera() const { return camera_.get(); } + +private: + std::shared_ptr camera_; + PlugEvent plugEvent_; +}; + MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) : saveRaw_(nullptr), options_(options), cm_(cm), allocator_(nullptr), isCapturing_(false), captureRaw_(false) @@ -81,6 +111,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) setCentralWidget(viewfinder_); adjustSize(); + /* Hotplug/unplug support */ + cm_->cameraAdded.connect(this, &MainWindow::addCamera); + cm_->cameraRemoved.connect(this, &MainWindow::removeCamera); + /* Open the camera and start capture. */ ret = openCamera(); if (ret < 0) { @@ -105,6 +139,9 @@ bool MainWindow::event(QEvent *e) if (e->type() == CaptureEvent::type()) { processCapture(); return true; + } else if (e->type() == HotplugEvent::type()) { + processHotplug(static_cast(e)); + return true; } return QMainWindow::event(e); @@ -535,6 +572,45 @@ void MainWindow::stopCapture() setWindowTitle(title_); } +/* ----------------------------------------------------------------------------- + * Camera hotplugging support + */ + +void MainWindow::processHotplug(HotplugEvent *e) +{ + Camera *camera = e->camera(); + HotplugEvent::PlugEvent event = e->hotplugEvent(); + + if (event == HotplugEvent::HotPlug) { + cameraCombo_->addItem(QString::fromStdString(camera->name())); + } else if (event == HotplugEvent::HotUnplug) { + /* Check if the currently-streaming camera is removed. */ + if (camera == camera_.get()) { + toggleCapture(false); + cameraCombo_->setCurrentIndex(0); + } + + int camIndex = cameraCombo_->findText(QString::fromStdString(camera->name())); + cameraCombo_->removeItem(camIndex); + } +} + +void MainWindow::addCamera(std::shared_ptr camera) +{ + qInfo() << "Adding new camera:" << camera->name().c_str(); + QCoreApplication::postEvent(this, + new HotplugEvent(std::move(camera), + HotplugEvent::HotPlug)); +} + +void MainWindow::removeCamera(std::shared_ptr camera) +{ + qInfo() << "Removing camera:" << camera->name().c_str(); + QCoreApplication::postEvent(this, + new HotplugEvent(std::move(camera), + HotplugEvent::HotUnplug)); +} + /* ----------------------------------------------------------------------------- * Image Save */ diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 59fa2d9..4606fe4 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -32,6 +32,8 @@ using namespace libcamera; class QAction; class QComboBox; +class HotplugEvent; + enum { OptCamera = 'c', OptHelp = 'h', @@ -87,8 +89,12 @@ private: int startCapture(); void stopCapture(); + void addCamera(std::shared_ptr camera); + void removeCamera(std::shared_ptr camera); + void requestComplete(Request *request); void processCapture(); + void processHotplug(HotplugEvent *e); void processViewfinder(FrameBuffer *buffer); /* UI elements */ From patchwork Tue Jun 16 19:45:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 4070 Return-Path: Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BE0D761F24 for ; Tue, 16 Jun 2020 21:45:40 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="hmXbvOGd"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc: content-transfer-encoding:content-type; s=s1; bh=lCwG2FumzuGu3eT7GFXMRl8Auk2TG9RkFgA5H2Mq8L0=; b=hmXbvOGdmLYr/ZnpJ6FRPkY6v/gvqJstUdkHzDZYGZRHB0s7e53Cbev035pzdwcb+14+ BUwY0rNsW8JA5/fYzTJXGKxx9ClqOJBs7U8dTSb2RMbxln4Ek27TG0yyIVAuB8e0uMSl4o 9OH7hLnSSfd4s/IVxWQzqopr0Fwv4Ij+M= Received: by filterdrecv-p3mdw1-6f5df8956d-w9s4g with SMTP id filterdrecv-p3mdw1-6f5df8956d-w9s4g-18-5EE92163-16 2020-06-16 19:45:39.386303073 +0000 UTC m=+1121510.399594273 Received: from mail.uajain.com (unknown) by ismtpd0005p1maa1.sendgrid.net (SG) with ESMTP id QyotJ4fZRr-pQjWE0Cl0yw Tue, 16 Jun 2020 19:45:39.001 +0000 (UTC) From: Umang Jain Date: Tue, 16 Jun 2020 19:45:39 +0000 (UTC) Message-Id: <20200616194523.23268-7-email@uajain.com> In-Reply-To: <20200616194523.23268-1-email@uajain.com> References: <20200616194523.23268-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcaThFi/Xwo9wPYVdHfbcDxgDhEYNc+7SkskSbO1EHFmIdV6ojG+6PuxutfExZlOvl4kTrL4qFnq5eCS54j8tF04jqQRrY1+pfmLUtMAin6uKCH0BMzEEoFSmw0kl7IbDAOUEh2K/XQijoa/iZfsBL26jtIY3Bc4dln3BQiUPM79vaP4fzoGgIC4PfS08NTVsaqjI+oyqQyWliBlsPBwR+hA== To: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v5 6/6] tests: Introduce hotplug hot-unplug unit test 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-List-Received-Date: Tue, 16 Jun 2020 19:45:41 -0000 This test checks the code-paths for camera's hotplugged and unplugged support. It is based on bind/unbind of a UVC device from sysfs. Hence, this test requires root permissions to run and should have at least one already bound UVC device present in the system. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart --- test/hotplug-cameras.cpp | 128 +++++++++++++++++++++++++++++++++++++++ test/meson.build | 1 + 2 files changed, 129 insertions(+) create mode 100644 test/hotplug-cameras.cpp diff --git a/test/hotplug-cameras.cpp b/test/hotplug-cameras.cpp new file mode 100644 index 0000000..6a94535 --- /dev/null +++ b/test/hotplug-cameras.cpp @@ -0,0 +1,128 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020, Umang Jain + * + * hotplug-cameras.cpp - Test cameraAdded/cameraRemoved signals in CameraManager + */ + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "libcamera/internal/file.h" +#include "libcamera/internal/thread.h" + +#include "test.h" + +using namespace libcamera; + +class HotplugTest : public Test +{ +protected: + void cameraAddedHandler(std::shared_ptr cam) + { + cameraAdded_ = true; + } + + void cameraRemovedHandler(std::shared_ptr cam) + { + cameraRemoved_ = true; + } + + int init() + { + if (!File::exists("/sys/module/uvcvideo")) { + std::cout << "uvcvideo driver is not loaded, skipping" << std::endl; + return TestSkip; + } + + if (geteuid() != 0) { + std::cout << "This test requires root permissions, skipping" << std::endl; + return TestSkip; + } + + cm_ = new CameraManager(); + if (cm_->start()) { + std::cout << "Failed to start camera manager" << std::endl; + return TestFail; + } + + cameraAdded_ = false; + cameraRemoved_ = false; + + cm_->cameraAdded.connect(this, &HotplugTest::cameraAddedHandler); + cm_->cameraRemoved.connect(this, &HotplugTest::cameraRemovedHandler); + + return 0; + } + + int run() + { + DIR *dir; + struct dirent *dirent; + std::string uvcDeviceDir; + + dir = opendir(uvcDriverDir_.c_str()); + /* Find a UVC device directory, which we can bind/unbind. */ + while ((dirent = readdir(dir)) != nullptr) { + if (!File::exists(uvcDriverDir_ + dirent->d_name + "/video4linux")) + continue; + + uvcDeviceDir = dirent->d_name; + break; + } + closedir(dir); + + /* If no UVC device found, skip the test. */ + if (uvcDeviceDir.empty()) + return TestSkip; + + /* Unbind a camera and process events. */ + std::ofstream(uvcDriverDir_ + "unbind", std::ios::binary) + << uvcDeviceDir; + Timer timer; + timer.start(1000); + while (timer.isRunning() && !cameraRemoved_) + Thread::current()->eventDispatcher()->processEvents(); + if (!cameraRemoved_) { + std::cout << "Camera unplug not detected" << std::endl; + return TestFail; + } + + /* Bind the camera again and process events. */ + std::ofstream(uvcDriverDir_ + "bind", std::ios::binary) + << uvcDeviceDir; + timer.start(1000); + while (timer.isRunning() && !cameraAdded_) + Thread::current()->eventDispatcher()->processEvents(); + if (!cameraAdded_) { + std::cout << "Camera plug not detected" << std::endl; + return TestFail; + } + + return TestPass; + } + + void cleanup() + { + cm_->stop(); + delete cm_; + } + +private: + CameraManager *cm_; + static const std::string uvcDriverDir_; + bool cameraRemoved_; + bool cameraAdded_; +}; + +const std::string HotplugTest::uvcDriverDir_ = "/sys/bus/usb/drivers/uvcvideo/"; + +TEST_REGISTER(HotplugTest) diff --git a/test/meson.build b/test/meson.build index bd7da14..a868813 100644 --- a/test/meson.build +++ b/test/meson.build @@ -30,6 +30,7 @@ internal_tests = [ ['event-thread', 'event-thread.cpp'], ['file', 'file.cpp'], ['file-descriptor', 'file-descriptor.cpp'], + ['hotplug-cameras', 'hotplug-cameras.cpp'], ['message', 'message.cpp'], ['object', 'object.cpp'], ['object-invoke', 'object-invoke.cpp'],