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;