From patchwork Wed Nov 16 00:17:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 17807 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 9E5F1C3285 for ; Wed, 16 Nov 2022 00:17:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6EB786309B; Wed, 16 Nov 2022 01:17:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1668557850; bh=9WogkLEpfbW6noMfaI5JtIgQ8SHgfPUEVfe9+Nv+/cs=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=QwgW6wQ/AiF4cU9qUHU4hCW4MQbMLuqdUYiFHJXUXpkjdJuBN68fy3IBcAF3nkaPa N3SRyIDyJHJTMXZTkMePY2dBOmulCARy/4hYqBNjj1E7q3DkzK7gLKBDH4MSn5n/4Q zuTy5PJdnxUknFXpLy5rWaRznO+8IksoVrN043C2IfTI/SGK1obA2UrPAt1kUDXGY3 8XV5lYZBuXrT+7asj3+lp0dVaHS5HDJaPvqtE8UXTyfBrE9JyPVf2mr3MjaySfTO7m PT6oRe0Q0f0DQ7k0y1+t8G0WoNPlWP5Pgswy7KBty2hTzuI2rlPKfYKdXnvRps1FRF 12tZlCOqoWx1A== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 577A563087 for ; Wed, 16 Nov 2022 01:17:29 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Ub88ZsVo"; dkim-atps=neutral Received: from Monstersaurus.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D5AFFAF4; Wed, 16 Nov 2022 01:17:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1668557849; bh=9WogkLEpfbW6noMfaI5JtIgQ8SHgfPUEVfe9+Nv+/cs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Ub88ZsVoU3WqaezXQxalSdbgMuZZvwBAkv+bCUGX6Pl6NArCSKkXN4t4hEin3GgFr 7uPhiaAVUwog687Y3WUkoJdAzm3MdkVgoJJDw4s16q8IsRK0dhE3pYAWboQCr4SVo/ rVlu1KnLoT8qjZNe8w73PLu+msg7w8Hlt9tgpIgc= To: libcamera devel Date: Wed, 16 Nov 2022 00:17:23 +0000 Message-Id: <20221116001724.3938045-2-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221116001724.3938045-1-kieran.bingham@ideasonboard.com> References: <20221116001724.3938045-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] libcamera: pipeline: Add an acquireDevice method 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 Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Cameras may need to acquire resources such as video device nodes before they can be configured. Acquisition of these resources can prevent power saving operations from being handled for instance in the event that a camera daemon such as Pipewire keeps unacquired devices held at runtime. Provide a method to allow pipeline handlers to implement resource acquisition operations during the acquire phase of the Camera state machine. Any resources acquired during acquireDevice() must be released as part of the corresponding releaseDevice() implementation. Signed-off-by: Kieran Bingham --- include/libcamera/internal/pipeline_handler.h | 3 ++- src/libcamera/camera.cpp | 2 +- src/libcamera/pipeline_handler.cpp | 23 ++++++++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index ec4f662d7399..483553b0d027 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -45,7 +45,7 @@ public: MediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator, const DeviceMatch &dm); - bool acquire(); + bool acquire(Camera *camera); void release(Camera *camera); virtual std::unique_ptr generateConfiguration(Camera *camera, @@ -74,6 +74,7 @@ protected: virtual int queueRequestDevice(Camera *camera, Request *request) = 0; virtual void stopDevice(Camera *camera) = 0; + virtual bool acquireDevice(Camera *camera); virtual void releaseDevice(Camera *camera); CameraManager *manager_; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 2d947a442bff..4e0a6584a673 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -835,7 +835,7 @@ int Camera::acquire() if (ret < 0) return ret == -EACCES ? -EBUSY : ret; - if (!d->pipe_->acquire()) { + if (!d->pipe_->acquire(this)) { LOG(Camera, Info) << "Pipeline handler in use by another process"; return -EBUSY; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index cfade4908118..4303a5893a4d 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -143,6 +143,7 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, /** * \brief Acquire exclusive access to the pipeline handler for the process + * \param[in] camera The camera which is being acquired * * This function locks all the media devices used by the pipeline to ensure * that no other process can access them concurrently. @@ -155,13 +156,16 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, * Pipeline handlers shall not call this function directly as the Camera class * handles access internally. * + * Any resources acquired during an overridden acquireDevice() must be released + * by a corresponding override of releaseDevice(). + * * \context This function is \threadsafe. * * \return True if the pipeline handler was acquired, false if another process * has already acquired it * \sa release() */ -bool PipelineHandler::acquire() +bool PipelineHandler::acquire(Camera *camera) { MutexLocker locker(lock_); @@ -177,10 +181,27 @@ bool PipelineHandler::acquire() } } + if (!acquireDevice(camera)) { + unlockMediaDevices(); + return false; + } + ++useCount_; return true; } +/** + * \brief Acquire resources associated with this camera + * \param[in] camera The camera for which to acquire resources + * + * Pipeline handlers may override this in order to perform resource acquisition + * operations when a camera is acquired, such as opening device nodes. + */ +bool PipelineHandler::acquireDevice([[maybe_unused]] Camera *camera) +{ + return true; +} + /** * \brief Release exclusive access to the pipeline handler * \param[in] camera The camera for which to release data From patchwork Wed Nov 16 00:17:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 17808 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 12C35BD16B for ; Wed, 16 Nov 2022 00:17:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C975663096; Wed, 16 Nov 2022 01:17:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1668557852; bh=ztzyRAZ6r7Bm6M3fFokOgT81rMXnWm5oK0rzeQ11Of0=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=tpgRp6ikJd4z+f3OO6pdcOYFADEPXQR0zyNx8o+XpBzyrQ1FY1ZtBhRt9ossiL0oo KlbYnx/614G0cLxHGCB5D+gjxJOX3hpnuqJebf5ReiHGL3xeaW/E3boy1+Bvly37S1 JRzthrGqQX/4Eh9I/o2wgGmp8e+oQeCcBJ74/XH/hBx0V5cSh8bOKpNEmS+anMWmUV 9AnLVvEeg2A3xrUzRvkeR6cXXrQOo50VuLcIBv4lCPGgCznMqujfW0ikS10Kxz0yXQ oM48i2DAdDDPpN9risob7VkVpD6oszv35KtQD/HA5YlbczUzoNVFnAqkWV1plgaq9s 2/jtcTo0+EB9g== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F9D863086 for ; Wed, 16 Nov 2022 01:17:29 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="U2ahTUES"; dkim-atps=neutral Received: from Monstersaurus.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C131BB8; Wed, 16 Nov 2022 01:17:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1668557849; bh=ztzyRAZ6r7Bm6M3fFokOgT81rMXnWm5oK0rzeQ11Of0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=U2ahTUESX98vOgcQEmUTiHLtv0RD68xz1HyNUyEkeWUHtMro/68HncxS8bHj7zZSa Hf7jNbETAXNXaqGukONFkkJsII06uYT4GvX3jlOAVQS8ICm0ElpG+YBeKngFY5beP4 X12Zo/yVef6Owc08rscKwir0h96p1K0GxeR5pXLc= To: libcamera devel Date: Wed, 16 Nov 2022 00:17:24 +0000 Message-Id: <20221116001724.3938045-3-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221116001724.3938045-1-kieran.bingham@ideasonboard.com> References: <20221116001724.3938045-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/2] [DNI/RFC] pipeline: uvcvideo: Only open devices upon acquire 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 Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Keeping UVC Video device nodes open will maintain an increased use count for that device and ensure that it remains powered by the USB subsystem. While newer kernels may improve and reduce the power consumption of UVC camera devices with open but non-streaming video nodes, ensure that our UVC pipeline handler does not keep the video node open when it is not directly acquired by an application. This allows camera daemons such as Pipewire to be able to maintain a list of available Camera devices, without acquiring resources when before an application has requested the camera directly. Signed-off-by: Kieran Bingham --- Marked as DNI/RFC for two big reasons: - 1: We might simply not want to do this, and want to leave it to the kernel to maintain better power consumption. Though keeping video nodes open when not actually in use or needed does still seem wasteful of a file descriptor resource. - 2: This doesn't work yet. Closing the device during init() and then reopening immediately works, but if I postpone reopening the device until the acquire() call, I can see that buffers are queued to the video device, but no completion event are ever received. - It's not yet clear if this is a fault in V4L2 or if the thread/event mechanism in libcamera is failing. Signed-off-by: Kieran Bingham --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 55 ++++++++++++++++---- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 277465b72164..01cda2b12bc1 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -53,7 +53,7 @@ public: std::map> formats_; private: - bool generateId(); + bool generateId(V4L2VideoDevice *video); std::string id_; }; @@ -74,6 +74,9 @@ class PipelineHandlerUVC : public PipelineHandler public: PipelineHandlerUVC(CameraManager *manager); + bool acquireDevice(Camera *camera); + void releaseDevice(Camera *camera); + std::unique_ptr generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; @@ -178,6 +181,28 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) { } +bool PipelineHandlerUVC::acquireDevice(Camera *camera) +{ + UVCCameraData *data = cameraData(camera); + + int ret = data->video_->open(); + if (ret) + return false; + + data->video_->bufferReady.connect(data, &UVCCameraData::bufferReady); + + return true; +} + +void PipelineHandlerUVC::releaseDevice(Camera *camera) +{ + UVCCameraData *data = cameraData(camera); + + data->video_->bufferReady.disconnect(); + + data->video_->close(); +} + std::unique_ptr PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) @@ -426,16 +451,15 @@ int UVCCameraData::init(MediaDevice *media) return -ENODEV; } + std::unique_ptr video = std::make_unique(*entity); + /* Create and open the video device. */ - video_ = std::make_unique(*entity); - ret = video_->open(); + ret = video->open(); if (ret) return ret; - video_->bufferReady.connect(this, &UVCCameraData::bufferReady); - /* Generate the camera ID. */ - if (!generateId()) { + if (!generateId(video.get())) { LOG(UVC, Error) << "Failed to generate camera ID"; return -EINVAL; } @@ -445,7 +469,7 @@ int UVCCameraData::init(MediaDevice *media) * resolution from the largest size it advertises. */ Size resolution; - for (const auto &format : video_->formats()) { + for (const auto &format : video->formats()) { PixelFormat pixelFormat = format.first.toPixelFormat(); if (!pixelFormat.isValid()) continue; @@ -485,7 +509,7 @@ int UVCCameraData::init(MediaDevice *media) * the _UPC. */ properties::LocationEnum location = properties::CameraLocationExternal; - std::ifstream file(video_->devicePath() + "/../removable"); + std::ifstream file(video->devicePath() + "/../removable"); if (file.is_open()) { std::string value; std::getline(file, value); @@ -503,7 +527,7 @@ int UVCCameraData::init(MediaDevice *media) /* Initialise the supported controls. */ ControlInfoMap::Map ctrls; - for (const auto &ctrl : video_->controls()) { + for (const auto &ctrl : video->controls()) { uint32_t cid = ctrl.first->id(); const ControlInfo &info = ctrl.second; @@ -512,12 +536,21 @@ int UVCCameraData::init(MediaDevice *media) controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls); + /* + * Close the camera until it is acquired to prevent holding on to + * resources that we do not own, or keeping devices powered when not in + * use. + */ + video->close(); + + video_ = std::move(video); + return 0; } -bool UVCCameraData::generateId() +bool UVCCameraData::generateId(V4L2VideoDevice *video) { - const std::string path = video_->devicePath(); + const std::string path = video->devicePath(); /* Create a controller ID from first device described in firmware. */ std::string controllerId;