Patch Detail
Show a patch.
GET /api/patches/20213/?format=api
{ "id": 20213, "url": "https://patchwork.libcamera.org/api/patches/20213/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20213/", "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": "<20240605194120.152960-1-nicolas@ndufresne.ca>", "date": "2024-06-05T19:41:20", "name": "[v1] gstreamer: pool: Replace GstAtomicQueue with deque and mutex", "commit_ref": "04f1f2033724f038ab1152e8135292770a33f97a", "pull_url": null, "state": "accepted", "archived": false, "hash": "4e14065ebfb527511ba4469dc6ef9cd53d3ec7d2", "submitter": { "id": 30, "url": "https://patchwork.libcamera.org/api/people/30/?format=api", "name": "Nicolas Dufresne", "email": "nicolas@ndufresne.ca" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/20213/mbox/", "series": [ { "id": 4366, "url": "https://patchwork.libcamera.org/api/series/4366/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4366", "date": "2024-06-05T19:41:20", "name": "[v1] gstreamer: pool: Replace GstAtomicQueue with deque and mutex", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4366/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/20213/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20213/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 1F198BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 5 Jun 2024 19:41:32 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CEE2165445;\n\tWed, 5 Jun 2024 21:41:30 +0200 (CEST)", "from madrid.collaboradmins.com (madrid.collaboradmins.com\n\t[IPv6:2a00:1098:ed:100::25])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0CC3E634CD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 5 Jun 2024 21:41:29 +0200 (CEST)", "from nicolas-tpx395.lan (cola.collaboradmins.com [195.201.22.229])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby madrid.collaboradmins.com (Postfix) with ESMTPSA id 3E43337821B7; \n\tWed, 5 Jun 2024 19:41:28 +0000 (UTC)" ], "From": "Nicolas Dufresne <nicolas@ndufresne.ca>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Nicolas Dufresne <nicolas.dufresne@collabora.com>", "Subject": "[PATCH v1] gstreamer: pool: Replace GstAtomicQueue with deque and\n\tmutex", "Date": "Wed, 5 Jun 2024 15:41:20 -0400", "Message-ID": "<20240605194120.152960-1-nicolas@ndufresne.ca>", "X-Mailer": "git-send-email 2.45.1", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n\nThe GstAtomicQueue only supports 2 threads, one pushing, and one popping. We\npop and push on error cases and we may have multiple threads downstream\nreturning buffer (using tee), which breaks this assumption.\n\nOn top of which, the release function, that notify when the queue goes from\nempty to not-empty rely on a racy empty check. The downstream thread that\ndoes this check if effectively concurrent with our thread calling acquire().\n\nFix this by replacing the GstAtomicQueue with a std::deque, and protect access\nto that using the object lock.\n\nBug: https://bugs.libcamera.org/show_bug.cgi?id=201\nSigned-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n---\n src/gstreamer/gstlibcamerapool.cpp | 40 +++++++++++++++++++++++-------\n 1 file changed, 31 insertions(+), 9 deletions(-)", "diff": "diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\nindex 9661c67a..0b1a5689 100644\n--- a/src/gstreamer/gstlibcamerapool.cpp\n+++ b/src/gstreamer/gstlibcamerapool.cpp\n@@ -8,6 +8,7 @@\n \n #include \"gstlibcamerapool.h\"\n \n+#include <deque>\n #include <libcamera/stream.h>\n \n #include \"gstlibcamera-utils.h\"\n@@ -24,24 +25,41 @@ static guint signals[N_SIGNALS];\n struct _GstLibcameraPool {\n \tGstBufferPool parent;\n \n-\tGstAtomicQueue *queue;\n+\tstd::deque<GstBuffer *> *queue;\n \tGstLibcameraAllocator *allocator;\n \tStream *stream;\n };\n \n G_DEFINE_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_TYPE_BUFFER_POOL)\n \n+static GstBuffer *\n+gst_libcamera_pool_pop_buffer(GstLibcameraPool *self)\n+{\n+\tGLibLocker lock(GST_OBJECT(self));\n+\tGstBuffer *buf;\n+\n+\tif (self->queue->empty())\n+\t\treturn nullptr;\n+\n+\tbuf = self->queue->front();\n+\tself->queue->pop_front();\n+\n+\treturn buf;\n+}\n+\n static GstFlowReturn\n gst_libcamera_pool_acquire_buffer(GstBufferPool *pool, GstBuffer **buffer,\n \t\t\t\t [[maybe_unused]] GstBufferPoolAcquireParams *params)\n {\n \tGstLibcameraPool *self = GST_LIBCAMERA_POOL(pool);\n-\tGstBuffer *buf = GST_BUFFER(gst_atomic_queue_pop(self->queue));\n+\tGstBuffer *buf = gst_libcamera_pool_pop_buffer(self);\n+\n \tif (!buf)\n \t\treturn GST_FLOW_ERROR;\n \n \tif (!gst_libcamera_allocator_prepare_buffer(self->allocator, self->stream, buf)) {\n-\t\tgst_atomic_queue_push(self->queue, buf);\n+\t\tGLibLocker lock(GST_OBJECT(self));\n+\t\tself->queue->push_back(buf);\n \t\treturn GST_FLOW_ERROR;\n \t}\n \n@@ -64,9 +82,13 @@ static void\n gst_libcamera_pool_release_buffer(GstBufferPool *pool, GstBuffer *buffer)\n {\n \tGstLibcameraPool *self = GST_LIBCAMERA_POOL(pool);\n-\tbool do_notify = gst_atomic_queue_length(self->queue) == 0;\n+\tbool do_notify;\n \n-\tgst_atomic_queue_push(self->queue, buffer);\n+\t{\n+\t\tGLibLocker lock(GST_OBJECT(self));\n+\t\tdo_notify = self->queue->empty();\n+\t\tself->queue->push_back(buffer);\n+\t}\n \n \tif (do_notify)\n \t\tg_signal_emit(self, signals[SIGNAL_BUFFER_NOTIFY], 0);\n@@ -75,7 +97,7 @@ gst_libcamera_pool_release_buffer(GstBufferPool *pool, GstBuffer *buffer)\n static void\n gst_libcamera_pool_init(GstLibcameraPool *self)\n {\n-\tself->queue = gst_atomic_queue_new(4);\n+\tself->queue = new std::deque<GstBuffer *>();\n }\n \n static void\n@@ -84,10 +106,10 @@ gst_libcamera_pool_finalize(GObject *object)\n \tGstLibcameraPool *self = GST_LIBCAMERA_POOL(object);\n \tGstBuffer *buf;\n \n-\twhile ((buf = GST_BUFFER(gst_atomic_queue_pop(self->queue))))\n+\twhile ((buf = gst_libcamera_pool_pop_buffer(self)))\n \t\tgst_buffer_unref(buf);\n \n-\tgst_atomic_queue_unref(self->queue);\n+\tdelete self->queue;\n \tg_object_unref(self->allocator);\n \n \tG_OBJECT_CLASS(gst_libcamera_pool_parent_class)->finalize(object);\n@@ -122,7 +144,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n \tgsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n \tfor (gsize i = 0; i < pool_size; i++) {\n \t\tGstBuffer *buffer = gst_buffer_new();\n-\t\tgst_atomic_queue_push(pool->queue, buffer);\n+\t\tpool->queue->push_back(buffer);\n \t}\n \n \treturn pool;\n", "prefixes": [ "v1" ] }