Patch Detail
Show a patch.
GET /api/1.1/patches/16445/?format=api
{ "id": 16445, "url": "https://patchwork.libcamera.org/api/1.1/patches/16445/?format=api", "web_url": "https://patchwork.libcamera.org/patch/16445/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20220630000251.31295-9-laurent.pinchart@ideasonboard.com>", "date": "2022-06-30T00:02:47", "name": "[libcamera-devel,v2,08/12] gstreamer: Use dedicated lock for request queues", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "cf7454d84266bc422ca980fa8d17cd5268b6f119", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/16445/mbox/", "series": [ { "id": 3234, "url": "https://patchwork.libcamera.org/api/1.1/series/3234/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3234", "date": "2022-06-30T00:02:39", "name": "gstreamer: Queue multiple requests", "version": 2, "mbox": "https://patchwork.libcamera.org/series/3234/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/16445/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/16445/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 EEA99BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 30 Jun 2022 00:03:32 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AF0A65655;\n\tThu, 30 Jun 2022 02:03:32 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5DA965634\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 30 Jun 2022 02:03:18 +0200 (CEST)", "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0DD4659D;\n\tThu, 30 Jun 2022 02:03:17 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656547412;\n\tbh=nHf6gwYLwDJnSJ8B2Zk8XcZvNZ2LWnCgfGqPfNJFrMo=;\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:Cc:\n\tFrom;\n\tb=0GvNVMdmCNwfH3DV0RA2XYPR2ZlbHEEk2OQ0v5+XDnJQymVuNkFRcFDox5aoFMQe/\n\t3pIP2yC1NZqF+PliA13gS6mkOfknW6QduPscad2Y4H5uA5vX46lsLWJo0RgLRUM9se\n\t4ZhEV5VvHrj9dZfnz8tXzjaQmd5yboZl08Kgjtgn3YzfILGt+JSk7SDgQ1u0gkCz83\n\tPZPR/rKF3klsaXcItI3Gd9KEkJ7uBZRd8vpUwp7Py63ZtaPb0KvPG3F47ZMk9a3LXO\n\tIhwPiL+EBtsHSsL58+rJlMO5BBuBtf17QBQ897bWCeCiSyoOxtdKcv7jPyaW5hKXL6\n\t5rcFmiKolgpEg==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656547398;\n\tbh=nHf6gwYLwDJnSJ8B2Zk8XcZvNZ2LWnCgfGqPfNJFrMo=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=tPc9oeaciepF73lXkLZ4Hb/7ZW+J64Qt/bjPPc3VfbEa1/f15yjlyJbd9m0R+vkzM\n\tbAQP9F138TMcIrB7jgcxGVhh58MmrAq/xcfB6YkV0Z5bVC2QfAzluwkRHVAMHapC9g\n\ty6Pygz5WDK67XVEakpaW11HM8xuWC0vM26PI4X7g=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"tPc9oeac\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 30 Jun 2022 03:02:47 +0300", "Message-Id": "<20220630000251.31295-9-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.35.1", "In-Reply-To": "<20220630000251.31295-1-laurent.pinchart@ideasonboard.com>", "References": "<20220630000251.31295-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 08/12] gstreamer: Use dedicated lock\n\tfor request queues", "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": "Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "Cc": "Nicolas Dufresne <nicolas.dufresne@collabora.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Add a new lock to the GstLibcameraSrcState class to protect the queued\nand completed requests queues. This replaces the GstObject lock, and\nminimizes the lock contention between the request completion handler and\nthe task run handler as the former must run as fast as possible.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n---\nChanges since v1:\n\n- Add comment about lock_\n---\n src/gstreamer/gstlibcamerasrc.cpp | 44 +++++++++++++++++++++++--------\n 1 file changed, 33 insertions(+), 11 deletions(-)", "diff": "diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\nindex 8c1cd7017d98..6f9a03c515d2 100644\n--- a/src/gstreamer/gstlibcamerasrc.cpp\n+++ b/src/gstreamer/gstlibcamerasrc.cpp\n@@ -32,6 +32,8 @@\n #include <queue>\n #include <vector>\n \n+#include <libcamera/base/mutex.h>\n+\n #include <libcamera/camera.h>\n #include <libcamera/camera_manager.h>\n #include <libcamera/control_ids.h>\n@@ -111,8 +113,18 @@ struct GstLibcameraSrcState {\n \tstd::shared_ptr<Camera> cam_;\n \tstd::unique_ptr<CameraConfiguration> config_;\n \tstd::vector<GstPad *> srcpads_;\n-\tstd::queue<std::unique_ptr<RequestWrap>> queuedRequests_;\n-\tstd::queue<std::unique_ptr<RequestWrap>> completedRequests_;\n+\n+\t/*\n+\t * Contention on this lock_ must be minimized, as it has to be taken in\n+\t * the realtime-sensitive requestCompleted() handler to protect\n+\t * queuedRequests_ and completedRequests_.\n+\t */\n+\tMutex lock_;\n+\tstd::queue<std::unique_ptr<RequestWrap>> queuedRequests_\n+\t\tLIBCAMERA_TSA_GUARDED_BY(lock_);\n+\tstd::queue<std::unique_ptr<RequestWrap>> completedRequests_\n+\t\tLIBCAMERA_TSA_GUARDED_BY(lock_);\n+\n \tguint group_id_;\n \n \tvoid requestCompleted(Request *request);\n@@ -155,12 +167,15 @@ GstStaticPadTemplate request_src_template = {\n void\n GstLibcameraSrcState::requestCompleted(Request *request)\n {\n-\tGLibLocker lock(GST_OBJECT(src_));\n-\n \tGST_DEBUG_OBJECT(src_, \"buffers are ready\");\n \n-\tstd::unique_ptr<RequestWrap> wrap = std::move(queuedRequests_.front());\n-\tqueuedRequests_.pop();\n+\tstd::unique_ptr<RequestWrap> wrap;\n+\n+\t{\n+\t\tMutexLocker locker(lock_);\n+\t\twrap = std::move(queuedRequests_.front());\n+\t\tqueuedRequests_.pop();\n+\t}\n \n \tg_return_if_fail(wrap->request_.get() == request);\n \n@@ -183,7 +198,10 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n \t\twrap->latency_ = sys_now - timestamp;\n \t}\n \n-\tcompletedRequests_.push(std::move(wrap));\n+\t{\n+\t\tMutexLocker locker(lock_);\n+\t\tcompletedRequests_.push(std::move(wrap));\n+\t}\n \n \tgst_task_resume(src_->task);\n }\n@@ -289,16 +307,17 @@ gst_libcamera_src_task_run(gpointer user_data)\n \t}\n \n \tif (wrap) {\n-\t\tGLibLocker lock(GST_OBJECT(self));\n \t\tGST_TRACE_OBJECT(self, \"Requesting buffers\");\n \t\tstate->cam_->queueRequest(wrap->request_.get());\n+\n+\t\tMutexLocker locker(state->lock_);\n \t\tstate->queuedRequests_.push(std::move(wrap));\n \n \t\t/* The RequestWrap will be deleted in the completion handler. */\n \t}\n \n \t{\n-\t\tGLibLocker lock(GST_OBJECT(self));\n+\t\tMutexLocker locker(state->lock_);\n \n \t\tif (!state->completedRequests_.empty()) {\n \t\t\twrap = std::move(state->completedRequests_.front());\n@@ -358,7 +377,7 @@ gst_libcamera_src_task_run(gpointer user_data)\n \t\tbool do_pause;\n \n \t\t{\n-\t\t\tGLibLocker lock(GST_OBJECT(self));\n+\t\t\tMutexLocker locker(state->lock_);\n \t\t\tdo_pause = state->completedRequests_.empty();\n \t\t}\n \n@@ -513,7 +532,10 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,\n \n \tstate->cam_->stop();\n \n-\tstate->completedRequests_ = {};\n+\t{\n+\t\tMutexLocker locker(state->lock_);\n+\t\tstate->completedRequests_ = {};\n+\t}\n \n \tfor (GstPad *srcpad : state->srcpads_)\n \t\tgst_libcamera_pad_set_pool(srcpad, nullptr);\n", "prefixes": [ "libcamera-devel", "v2", "08/12" ] }