[{"id":30460,"web_url":"https://patchwork.libcamera.org/comment/30460/","msgid":"<172175035276.392292.3285203495047431608@ping.linuxembedded.co.uk>","date":"2024-07-23T15:59:12","subject":"Re: [PATCH v1] gstreamer: pool: Replace GstAtomicQueue with deque\n\tand mutex","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Nicolas Dufresne (2024-06-05 20:41:20)\n> From: Nicolas Dufresne \n> \n> The GstAtomicQueue only supports 2 threads, one pushing, and one popping. We\n> pop and push on error cases and we may have multiple threads downstream\n> returning buffer (using tee), which breaks this assumption.\n> \n> On top of which, the release function, that notify when the queue goes from\n\ns/notify/notifies/\n\n> empty to not-empty rely on a racy empty check. The downstream thread that\n\ns/rely on a/relies on an/\n\n> does this check if effectively concurrent with our thread calling acquire().\n\ns/if/is/\n\n\n> Fix this by replacing the GstAtomicQueue with a std::deque, and protect access\n> to that using the object lock.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=201\n> Signed-off-by: Nicolas Dufresne \n\nThis all looks reasonable to me - and following the bugzilla I see the\ncomment : https://bugs.libcamera.org/show_bug.cgi?id=201#c10 which says\nit improved things, but that the issue might still persist (which could\nbe a separate race/issue still?)\n\n\nNicolas, what are your thoughts - do you think this patch is ready for\nmerge already?\n\nFor the code itself:\n\n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/gstreamer/gstlibcamerapool.cpp | 40 +++++++++++++++++++++++-------\n> 1 file changed, 31 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> index 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 \n> #include \n> \n> #include \"gstlibcamera-utils.h\"\n> @@ -24,24 +25,41 @@ static guint signals[N_SIGNALS];\n> struct _GstLibcameraPool {\n> GstBufferPool parent;\n> \n> - GstAtomicQueue *queue;\n> + std::deque *queue;\n> GstLibcameraAllocator *allocator;\n> Stream *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> + GLibLocker lock(GST_OBJECT(self));\n> + GstBuffer *buf;\n> +\n> + if (self->queue->empty())\n> + return nullptr;\n> +\n> + buf = self->queue->front();\n> + self->queue->pop_front();\n> +\n> + return buf;\n> +}\n> +\n> static GstFlowReturn\n> gst_libcamera_pool_acquire_buffer(GstBufferPool *pool, GstBuffer **buffer,\n> [[maybe_unused]] GstBufferPoolAcquireParams *params)\n> {\n> GstLibcameraPool *self = GST_LIBCAMERA_POOL(pool);\n> - GstBuffer *buf = GST_BUFFER(gst_atomic_queue_pop(self->queue));\n> + GstBuffer *buf = gst_libcamera_pool_pop_buffer(self);\n> +\n> if (!buf)\n> return GST_FLOW_ERROR;\n> \n> if (!gst_libcamera_allocator_prepare_buffer(self->allocator, self->stream, buf)) {\n> - gst_atomic_queue_push(self->queue, buf);\n> + GLibLocker lock(GST_OBJECT(self));\n> + self->queue->push_back(buf);\n> return GST_FLOW_ERROR;\n> }\n> \n> @@ -64,9 +82,13 @@ static void\n> gst_libcamera_pool_release_buffer(GstBufferPool *pool, GstBuffer *buffer)\n> {\n> GstLibcameraPool *self = GST_LIBCAMERA_POOL(pool);\n> - bool do_notify = gst_atomic_queue_length(self->queue) == 0;\n> + bool do_notify;\n> \n> - gst_atomic_queue_push(self->queue, buffer);\n> + {\n> + GLibLocker lock(GST_OBJECT(self));\n> + do_notify = self->queue->empty();\n> + self->queue->push_back(buffer);\n> + }\n> \n> if (do_notify)\n> g_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> - self->queue = gst_atomic_queue_new(4);\n> + self->queue = new std::deque();\n> }\n> \n> static void\n> @@ -84,10 +106,10 @@ gst_libcamera_pool_finalize(GObject *object)\n> GstLibcameraPool *self = GST_LIBCAMERA_POOL(object);\n> GstBuffer *buf;\n> \n> - while ((buf = GST_BUFFER(gst_atomic_queue_pop(self->queue))))\n> + while ((buf = gst_libcamera_pool_pop_buffer(self)))\n> gst_buffer_unref(buf);\n> \n> - gst_atomic_queue_unref(self->queue);\n> + delete self->queue;\n> g_object_unref(self->allocator);\n> \n> G_OBJECT_CLASS(gst_libcamera_pool_parent_class)->finalize(object);\n> @@ -122,7 +144,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n> for (gsize i = 0; i < pool_size; i++) {\n> GstBuffer *buffer = gst_buffer_new();\n> - gst_atomic_queue_push(pool->queue, buffer);\n> + pool->queue->push_back(buffer);\n> }\n> \n> return pool;\n> -- \n> 2.45.1\n>","headers":{"Return-Path":"","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 509CBC323E\n\tfor ;\n\tTue, 23 Jul 2024 15:59:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7AAA7619A3;\n\tTue, 23 Jul 2024 17:59:16 +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 B774E619A1\n\tfor ;\n\tTue, 23 Jul 2024 17:59:15 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3F3417E2;\n\tTue, 23 Jul 2024 17:58:33 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZSJubzPz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1721750313;\n\tbh=fHMHCBvIgQsf0WKM9pdez3/23we3126cwUQQOOGXuro=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ZSJubzPz+DcMIcQB6t7GveMlHaRppv5oQXZYKJjfgXLh1IxhhjoRK8T1XyVynQTMs\n\thtHGhb3KdK6xrq9B04Pvcnw7RtNrcNDrUCvn2fi+ohxEdLFZPGNz/Yx2dim0wJKNt7\n\thoIjLi6pRO6zCNYmAvK5bNWCj1snC9gO8//Mpn8c=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240605194120.152960-1-nicolas@ndufresne.ca>","References":"<20240605194120.152960-1-nicolas@ndufresne.ca>","Subject":"Re: [PATCH v1] gstreamer: pool: Replace GstAtomicQueue with deque\n\tand mutex","From":"Kieran Bingham ","Cc":"Nicolas Dufresne ","To":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 23 Jul 2024 16:59:12 +0100","Message-ID":"<172175035276.392292.3285203495047431608@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30461,"web_url":"https://patchwork.libcamera.org/comment/30461/","msgid":"<5466919fbf0c968d18092a864aa4b7e5f8fcaf2d.camel@collabora.com>","date":"2024-07-23T16:08:33","subject":"Re: [PATCH v1] gstreamer: pool: Replace GstAtomicQueue with deque\n\tand mutex","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Hi,\n\nLe mardi 23 juillet 2024 à 16:59 +0100, Kieran Bingham a écrit :\n> Quoting Nicolas Dufresne (2024-06-05 20:41:20)\n> > From: Nicolas Dufresne \n> > \n> > The GstAtomicQueue only supports 2 threads, one pushing, and one popping. We\n> > pop and push on error cases and we may have multiple threads downstream\n> > returning buffer (using tee), which breaks this assumption.\n> > \n> > On top of which, the release function, that notify when the queue goes from\n> \n> s/notify/notifies/\n> \n> > empty to not-empty rely on a racy empty check. The downstream thread that\n> \n> s/rely on a/relies on an/\n> \n> > does this check if effectively concurrent with our thread calling acquire().\n> \n> s/if/is/\n> \n> \n> > Fix this by replacing the GstAtomicQueue with a std::deque, and protect access\n> > to that using the object lock.\n> > \n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=201\n> > Signed-off-by: Nicolas Dufresne \n> \n> This all looks reasonable to me - and following the bugzilla I see the\n> comment : https://bugs.libcamera.org/show_bug.cgi?id=201#c10 which says\n> it improved things, but that the issue might still persist (which could\n> be a separate race/issue still?)\n> \n> \n> Nicolas, what are your thoughts - do you think this patch is ready for\n> merge already?\n> \n> For the code itself:\n> \n> \n> Reviewed-by: Kieran Bingham \n\nWith the suggested typo, yes, I'd picked it up as it aligns the code with\nsimilar fixes that happen upstream. I still have to find time to look into the\nremaining, but shouldn't block this one, GstAtomicQueue the way we use it now\nmust go away anyway.\n\nNicolas\n\n> \n> > ---\n> > src/gstreamer/gstlibcamerapool.cpp | 40 +++++++++++++++++++++++-------\n> > 1 file changed, 31 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> > index 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 \n> > #include \n> > \n> > #include \"gstlibcamera-utils.h\"\n> > @@ -24,24 +25,41 @@ static guint signals[N_SIGNALS];\n> > struct _GstLibcameraPool {\n> > GstBufferPool parent;\n> > \n> > - GstAtomicQueue *queue;\n> > + std::deque *queue;\n> > GstLibcameraAllocator *allocator;\n> > Stream *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> > + GLibLocker lock(GST_OBJECT(self));\n> > + GstBuffer *buf;\n> > +\n> > + if (self->queue->empty())\n> > + return nullptr;\n> > +\n> > + buf = self->queue->front();\n> > + self->queue->pop_front();\n> > +\n> > + return buf;\n> > +}\n> > +\n> > static GstFlowReturn\n> > gst_libcamera_pool_acquire_buffer(GstBufferPool *pool, GstBuffer **buffer,\n> > [[maybe_unused]] GstBufferPoolAcquireParams *params)\n> > {\n> > GstLibcameraPool *self = GST_LIBCAMERA_POOL(pool);\n> > - GstBuffer *buf = GST_BUFFER(gst_atomic_queue_pop(self->queue));\n> > + GstBuffer *buf = gst_libcamera_pool_pop_buffer(self);\n> > +\n> > if (!buf)\n> > return GST_FLOW_ERROR;\n> > \n> > if (!gst_libcamera_allocator_prepare_buffer(self->allocator, self->stream, buf)) {\n> > - gst_atomic_queue_push(self->queue, buf);\n> > + GLibLocker lock(GST_OBJECT(self));\n> > + self->queue->push_back(buf);\n> > return GST_FLOW_ERROR;\n> > }\n> > \n> > @@ -64,9 +82,13 @@ static void\n> > gst_libcamera_pool_release_buffer(GstBufferPool *pool, GstBuffer *buffer)\n> > {\n> > GstLibcameraPool *self = GST_LIBCAMERA_POOL(pool);\n> > - bool do_notify = gst_atomic_queue_length(self->queue) == 0;\n> > + bool do_notify;\n> > \n> > - gst_atomic_queue_push(self->queue, buffer);\n> > + {\n> > + GLibLocker lock(GST_OBJECT(self));\n> > + do_notify = self->queue->empty();\n> > + self->queue->push_back(buffer);\n> > + }\n> > \n> > if (do_notify)\n> > g_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> > - self->queue = gst_atomic_queue_new(4);\n> > + self->queue = new std::deque();\n> > }\n> > \n> > static void\n> > @@ -84,10 +106,10 @@ gst_libcamera_pool_finalize(GObject *object)\n> > GstLibcameraPool *self = GST_LIBCAMERA_POOL(object);\n> > GstBuffer *buf;\n> > \n> > - while ((buf = GST_BUFFER(gst_atomic_queue_pop(self->queue))))\n> > + while ((buf = gst_libcamera_pool_pop_buffer(self)))\n> > gst_buffer_unref(buf);\n> > \n> > - gst_atomic_queue_unref(self->queue);\n> > + delete self->queue;\n> > g_object_unref(self->allocator);\n> > \n> > G_OBJECT_CLASS(gst_libcamera_pool_parent_class)->finalize(object);\n> > @@ -122,7 +144,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> > gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n> > for (gsize i = 0; i < pool_size; i++) {\n> > GstBuffer *buffer = gst_buffer_new();\n> > - gst_atomic_queue_push(pool->queue, buffer);\n> > + pool->queue->push_back(buffer);\n> > }\n> > \n> > return pool;\n> > -- \n> > 2.45.1\n> >","headers":{"Return-Path":"","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 0501DBDB1C\n\tfor ;\n\tTue, 23 Jul 2024 16:08:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A008619A1;\n\tTue, 23 Jul 2024 18:08:39 +0200 (CEST)","from madrid.collaboradmins.com (madrid.collaboradmins.com\n\t[46.235.227.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 25A2B619A1\n\tfor ;\n\tTue, 23 Jul 2024 18:08:37 +0200 (CEST)","from nicolas-tpx395.localdomain (cola.collaboradmins.com\n\t[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 1E9EA378206B; \n\tTue, 23 Jul 2024 16:08:36 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"gieeLJ2+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1721750916;\n\tbh=WlQr+TfR4jFK5JXbR15xCLY6JQmldc9tnTeXgfvzw8w=;\n\th=Subject:From:To:Date:In-Reply-To:References:From;\n\tb=gieeLJ2+JGuM6MjQ3DMb/PtKO/jboLf2arhebbzMMJlW0WOMnSlTtYvNi9hE2eg5V\n\t8n685QSLt23NQEobnNJclXEyUS9uX7ZCbGuHTFVZ3IwkLE0eKewk/4P3RiqY/G2K/I\n\ttvkjaRyZLeeD+mLs+T8dheeS+1KsRVP2PtcoyqryOuDzvTj/L9DUg39K3BtB3E8pQj\n\tHKyIS6m85l7KVICY6xBfCzmiN4Vm2WB2AuG6jQnI36tZsR6K9fyaezYzskqgu+/R6E\n\tcGz2urf4H18FFtc6WXEVXrJsaAp0ccqzrpmcEE9VMjwKt8zcbeM74xiGuB3/XQaG1F\n\tuRM8jQ0Y5e5EQ==","Message-ID":"<5466919fbf0c968d18092a864aa4b7e5f8fcaf2d.camel@collabora.com>","Subject":"Re: [PATCH v1] gstreamer: pool: Replace GstAtomicQueue with deque\n\tand mutex","From":"Nicolas Dufresne ","To":"Kieran Bingham , \n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 23 Jul 2024 12:08:33 -0400","In-Reply-To":"<172175035276.392292.3285203495047431608@ping.linuxembedded.co.uk>","References":"<20240605194120.152960-1-nicolas@ndufresne.ca>\n\t<172175035276.392292.3285203495047431608@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.52.2 (3.52.2-1.fc40) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30478,"web_url":"https://patchwork.libcamera.org/comment/30478/","msgid":"","date":"2024-07-25T08:55:30","subject":"Re: [PATCH v1] gstreamer: pool: Replace GstAtomicQueue with deque\n\tand mutex","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Nicolas\n\nOn Wed, Jun 05, 2024 at 03:41:20PM GMT, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne \n>\n> The GstAtomicQueue only supports 2 threads, one pushing, and one popping. We\n> pop and push on error cases and we may have multiple threads downstream\n> returning buffer (using tee), which breaks this assumption.\n>\n> On top of which, the release function, that notify when the queue goes from\n> empty to not-empty rely on a racy empty check. The downstream thread that\n> does this check if effectively concurrent with our thread calling acquire().\n>\n> Fix this by replacing the GstAtomicQueue with a std::deque, and protect access\n> to that using the object lock.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=201\n> Signed-off-by: Nicolas Dufresne \n\nLooks sane to me even if the gst code base is not familiar to me\nAcked-by: Jacopo Mondi \n\nThanks\n j\n> ---\n> src/gstreamer/gstlibcamerapool.cpp | 40 +++++++++++++++++++++++-------\n> 1 file changed, 31 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> index 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 \n> #include \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 *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();\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> --\n> 2.45.1\n>","headers":{"Return-Path":"","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 F2B60C323E\n\tfor ;\n\tThu, 25 Jul 2024 08:55:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3A8C63369;\n\tThu, 25 Jul 2024 10:55:34 +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 A0ECE6199A\n\tfor ;\n\tThu, 25 Jul 2024 10:55:33 +0200 (CEST)","from ideasonboard.com (mob-5-90-42-5.net.vodafone.it [5.90.42.5])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 008EC45B;\n\tThu, 25 Jul 2024 10:54:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HgjfZMkY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1721897690;\n\tbh=OdmgDWVCNYrkPgeok3+OWk5evssd5xcOvXvyxLYoHuQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HgjfZMkYDryy4QRPsLyztW6zQpGnmBANXLEfoqX8xN3AuFvCULD++rEjuH2In4ZP5\n\tdQouurxF+X0fkDfoNeqZv1JSErnLr0MKg4F+pOchfz9IiqsWpwJOWdfKT6GfiA0D6G\n\tgZXIEGp+xDDqtz/kBvMNzeG1HYOu8j9DJ5bWCOHo=","Date":"Thu, 25 Jul 2024 10:55:30 +0200","From":"Jacopo Mondi ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org, \n\tNicolas Dufresne ","Subject":"Re: [PATCH v1] gstreamer: pool: Replace GstAtomicQueue with deque\n\tand mutex","Message-ID":"","References":"<20240605194120.152960-1-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240605194120.152960-1-nicolas@ndufresne.ca>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]