Patch Detail
Show a patch.
GET /api/patches/17808/?format=api
{ "id": 17808, "url": "https://patchwork.libcamera.org/api/patches/17808/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17808/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20221116001724.3938045-3-kieran.bingham@ideasonboard.com>", "date": "2022-11-16T00:17:24", "name": "[libcamera-devel,2/2,DNI/RFC] pipeline: uvcvideo: Only open devices upon acquire", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "ed3cf1ab0ee35466cd4933c28a7aee0fe24a4fe2", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/17808/mbox/", "series": [ { "id": 3614, "url": "https://patchwork.libcamera.org/api/series/3614/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3614", "date": "2022-11-16T00:17:22", "name": "Support resource acquisition at 'acquire()'", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3614/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/17808/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/17808/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 12C35BD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 16 Nov 2022 00:17:33 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C975663096;\n\tWed, 16 Nov 2022 01:17:32 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F9D863086\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 16 Nov 2022 01:17:29 +0100 (CET)", "from Monstersaurus.local\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C131BB8;\n\tWed, 16 Nov 2022 01:17:29 +0100 (CET)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668557852;\n\tbh=ztzyRAZ6r7Bm6M3fFokOgT81rMXnWm5oK0rzeQ11Of0=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=tpgRp6ikJd4z+f3OO6pdcOYFADEPXQR0zyNx8o+XpBzyrQ1FY1ZtBhRt9ossiL0oo\n\tKlbYnx/614G0cLxHGCB5D+gjxJOX3hpnuqJebf5ReiHGL3xeaW/E3boy1+Bvly37S1\n\tJRzthrGqQX/4Eh9I/o2wgGmp8e+oQeCcBJ74/XH/hBx0V5cSh8bOKpNEmS+anMWmUV\n\t9AnLVvEeg2A3xrUzRvkeR6cXXrQOo50VuLcIBv4lCPGgCznMqujfW0ikS10Kxz0yXQ\n\toM48i2DAdDDPpN9risob7VkVpD6oszv35KtQD/HA5YlbczUzoNVFnAqkWV1plgaq9s\n\t2/jtcTo0+EB9g==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668557849;\n\tbh=ztzyRAZ6r7Bm6M3fFokOgT81rMXnWm5oK0rzeQ11Of0=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=U2ahTUESX98vOgcQEmUTiHLtv0RD68xz1HyNUyEkeWUHtMro/68HncxS8bHj7zZSa\n\tHf7jNbETAXNXaqGukONFkkJsII06uYT4GvX3jlOAVQS8ICm0ElpG+YBeKngFY5beP4\n\tX12Zo/yVef6Owc08rscKwir0h96p1K0GxeR5pXLc=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"U2ahTUES\"; dkim-atps=neutral", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "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", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 2/2] [DNI/RFC] pipeline: uvcvideo: Only\n\topen devices upon acquire", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "From": "Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Keeping UVC Video device nodes open will maintain an increased use count\nfor that device and ensure that it remains powered by the USB subsystem.\n\nWhile newer kernels may improve and reduce the power consumption of UVC\ncamera devices with open but non-streaming video nodes, ensure that our\nUVC pipeline handler does not keep the video node open when it is not\ndirectly acquired by an application.\n\nThis allows camera daemons such as Pipewire to be able to maintain a\nlist of available Camera devices, without acquiring resources when\nbefore an application has requested the camera directly.\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n---\nMarked as DNI/RFC for two big reasons:\n\n - 1: We might simply not want to do this, and want to leave it to the\n kernel to maintain better power consumption. Though keeping video\n nodes open when not actually in use or needed does still seem wasteful\n of a file descriptor resource.\n\n - 2: This doesn't work yet. Closing the device during init() and then\n reopening immediately works, but if I postpone reopening the device\n until the acquire() call, I can see that buffers are queued to the video\n device, but no completion event are ever received.\n - It's not yet clear if this is a fault in V4L2 or if the\n thread/event mechanism in libcamera is failing.\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 55 ++++++++++++++++----\n 1 file changed, 44 insertions(+), 11 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\nindex 277465b72164..01cda2b12bc1 100644\n--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n@@ -53,7 +53,7 @@ public:\n \tstd::map<PixelFormat, std::vector<SizeRange>> formats_;\n \n private:\n-\tbool generateId();\n+\tbool generateId(V4L2VideoDevice *video);\n \n \tstd::string id_;\n };\n@@ -74,6 +74,9 @@ class PipelineHandlerUVC : public PipelineHandler\n public:\n \tPipelineHandlerUVC(CameraManager *manager);\n \n+\tbool acquireDevice(Camera *camera);\n+\tvoid releaseDevice(Camera *camera);\n+\n \tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n@@ -178,6 +181,28 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n {\n }\n \n+bool PipelineHandlerUVC::acquireDevice(Camera *camera)\n+{\n+\tUVCCameraData *data = cameraData(camera);\n+\n+\tint ret = data->video_->open();\n+\tif (ret)\n+\t\treturn false;\n+\n+\tdata->video_->bufferReady.connect(data, &UVCCameraData::bufferReady);\n+\n+\treturn true;\n+}\n+\n+void PipelineHandlerUVC::releaseDevice(Camera *camera)\n+{\n+\tUVCCameraData *data = cameraData(camera);\n+\n+\tdata->video_->bufferReady.disconnect();\n+\n+\tdata->video_->close();\n+}\n+\n std::unique_ptr<CameraConfiguration>\n PipelineHandlerUVC::generateConfiguration(Camera *camera,\n \tconst StreamRoles &roles)\n@@ -426,16 +451,15 @@ int UVCCameraData::init(MediaDevice *media)\n \t\treturn -ENODEV;\n \t}\n \n+\tstd::unique_ptr<V4L2VideoDevice> video = std::make_unique<V4L2VideoDevice>(*entity);\n+\n \t/* Create and open the video device. */\n-\tvideo_ = std::make_unique<V4L2VideoDevice>(*entity);\n-\tret = video_->open();\n+\tret = video->open();\n \tif (ret)\n \t\treturn ret;\n \n-\tvideo_->bufferReady.connect(this, &UVCCameraData::bufferReady);\n-\n \t/* Generate the camera ID. */\n-\tif (!generateId()) {\n+\tif (!generateId(video.get())) {\n \t\tLOG(UVC, Error) << \"Failed to generate camera ID\";\n \t\treturn -EINVAL;\n \t}\n@@ -445,7 +469,7 @@ int UVCCameraData::init(MediaDevice *media)\n \t * resolution from the largest size it advertises.\n \t */\n \tSize resolution;\n-\tfor (const auto &format : video_->formats()) {\n+\tfor (const auto &format : video->formats()) {\n \t\tPixelFormat pixelFormat = format.first.toPixelFormat();\n \t\tif (!pixelFormat.isValid())\n \t\t\tcontinue;\n@@ -485,7 +509,7 @@ int UVCCameraData::init(MediaDevice *media)\n \t * the _UPC.\n \t */\n \tproperties::LocationEnum location = properties::CameraLocationExternal;\n-\tstd::ifstream file(video_->devicePath() + \"/../removable\");\n+\tstd::ifstream file(video->devicePath() + \"/../removable\");\n \tif (file.is_open()) {\n \t\tstd::string value;\n \t\tstd::getline(file, value);\n@@ -503,7 +527,7 @@ int UVCCameraData::init(MediaDevice *media)\n \t/* Initialise the supported controls. */\n \tControlInfoMap::Map ctrls;\n \n-\tfor (const auto &ctrl : video_->controls()) {\n+\tfor (const auto &ctrl : video->controls()) {\n \t\tuint32_t cid = ctrl.first->id();\n \t\tconst ControlInfo &info = ctrl.second;\n \n@@ -512,12 +536,21 @@ int UVCCameraData::init(MediaDevice *media)\n \n \tcontrolInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);\n \n+\t/*\n+\t * Close the camera until it is acquired to prevent holding on to\n+\t * resources that we do not own, or keeping devices powered when not in\n+\t * use.\n+\t */\n+\tvideo->close();\n+\n+\tvideo_ = std::move(video);\n+\n \treturn 0;\n }\n \n-bool UVCCameraData::generateId()\n+bool UVCCameraData::generateId(V4L2VideoDevice *video)\n {\n-\tconst std::string path = video_->devicePath();\n+\tconst std::string path = video->devicePath();\n \n \t/* Create a controller ID from first device described in firmware. */\n \tstd::string controllerId;\n", "prefixes": [ "libcamera-devel", "2/2", "DNI/RFC" ] }