[{"id":3877,"web_url":"https://patchwork.libcamera.org/comment/3877/","msgid":"<20200229151115.GX18738@pendragon.ideasonboard.com>","date":"2020-02-29T15:11:15","subject":"Re: [libcamera-devel] [PATCH v2 26/27] gst: libcamerasrc: Prevent\n\tsrc task deadlock on exhausted buffer pool","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas and Jakub,\n\nThank you for the patch.\n\nOn Thu, Feb 27, 2020 at 03:04:06PM -0500, Nicolas Dufresne wrote:\n> From: Jakub Adam <jakub.adam@collabora.com>\n> \n> Allow GstLibcameraPool to notify the source when a new buffer has become\n> available in a previously exhausted buffer pool. This can be used to\n> resume a src task that got paused because it couldn't acquire a buffer.\n> \n> Without this change the src task will never resume from pause once the\n> pool gets exhausted.\n> \n> To trigger the deadlock (it doesn't happen every time), run:\n> \n>   gst-launch-1.0 libcamerasrc ! queue ! glimagesink\n> \n> Signed-off-by: Jakub Adam <jakub.adam@collabora.com>\n> ---\n>  src/gstreamer/gstlibcamerapool.cpp | 16 ++++++++++++++++\n>  src/gstreamer/gstlibcamerapool.h   |  6 ++++++\n>  src/gstreamer/gstlibcamerasrc.cpp  | 14 ++++++++++++--\n>  3 files changed, 34 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> index f85c3aa..5aa0eeb 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -18,6 +18,8 @@ struct _GstLibcameraPool {\n>  \n>  \tGstAtomicQueue *queue;\n>  \tGstLibcameraAllocator *allocator;\n> +\tGstLibcameraBufferNotify buffer_notify;\n> +\tgpointer buffer_notify_data;\n>  \tStream *stream;\n>  };\n>  \n> @@ -54,7 +56,12 @@ 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> +\n>  \tgst_atomic_queue_push(self->queue, buffer);\n> +\n> +\tif (do_notify && self->buffer_notify)\n> +\t\tself->buffer_notify(self->buffer_notify_data);\n>  }\n>  \n>  static void\n> @@ -108,6 +115,15 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n>  \treturn pool;\n>  }\n>  \n> +void\n> +gst_libcamera_pool_set_buffer_notify(GstLibcameraPool *self,\n> +\t\t\t\t     GstLibcameraBufferNotify notify,\n> +\t\t\t\t     gpointer data)\n> +{\n> +\tself->buffer_notify = notify;\n> +\tself->buffer_notify_data = data;\n> +}\n> +\n>  Stream *\n>  gst_libcamera_pool_get_stream(GstLibcameraPool *self)\n>  {\n> diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> index 6fd2913..545be01 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -20,9 +20,15 @@\n>  #define GST_TYPE_LIBCAMERA_POOL gst_libcamera_pool_get_type()\n>  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)\n>  \n> +typedef void (*GstLibcameraBufferNotify)(gpointer data);\n> +\n>  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n>  \t\t\t\t\t libcamera::Stream *stream);\n>  \n> +void gst_libcamera_pool_set_buffer_notify(GstLibcameraPool *self,\n> +\t\t\t\t\t  GstLibcameraBufferNotify notify,\n> +\t\t\t\t\t  gpointer data);\n> +\n>  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);\n>  \n>  libcamera::Stream *gst_libcamera_buffer_get_stream(GstBuffer *buffer);\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index fe60584..ecbfe37 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -434,6 +434,10 @@ gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)\n>  \t\tconst StreamConfiguration &stream_cfg = state->config->at(i);\n>  \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n>  \t\t\t\t\t\t\t\tstream_cfg.stream());\n> +\t\tgst_libcamera_pool_set_buffer_notify(pool,\n> +\t\t\t\t\t\t     (GstLibcameraBufferNotify)gst_libcamera_resume_task,\n> +\t\t\t\t\t\t     task);\n> +\n>  \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n>  \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n>  \t}\n> @@ -468,8 +472,14 @@ gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)\n>  \n>  \tstate->cam->stop();\n>  \n> -\tfor (GstPad *srcpad : state->srcpads)\n> -\t\tgst_libcamera_pad_set_pool(srcpad, NULL);\n> +\tfor (GstPad *srcpad : state->srcpads) {\n> +\t\tauto *pool = gst_libcamera_pad_get_pool(srcpad);\n> +\n> +\t\tif (pool) {\n> +\t\t\tgst_libcamera_pool_set_buffer_notify(pool, NULL, NULL);\n> +\t\t\tgst_libcamera_pad_set_pool(srcpad, NULL);\n\ns/NULL/nullptr/\n\nUp to you, but you could also use a Signal. This would however require\nC++ objects, so some refactoring would be needed, and it may not be\nworth it, but adding ad-hoc callback registration is a step on the\nslippery slope to towards spaghetti code :-)\n\n> +\t\t}\n> +\t}\n>  \n>  \tg_clear_object(&self->allocator);\n>  \tg_clear_pointer(&self->flow_combiner,","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A1B262689\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 29 Feb 2020 16:11:39 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A400E33E;\n\tSat, 29 Feb 2020 16:11:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582989098;\n\tbh=INiJdIoCr9Fw8q7vhwnunER30/1I8nv4xtTIuBhXwjU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Zo8pLeNxGyMeqzJKBB8JGKX4Sk9bjIkEd7ZTp3HJL6lhLPq/e1HvyZ22RpOKpDuj3\n\tZdSjwiWN+JqczapnpQHX6xQaAXfdMDmNKV5l77N0grp+dxLiGZ30CCxvehAtlVJCAU\n\tXvx0/0LjTXSA460fumcjohlTlJtbcV0/O9qdYUgo=","Date":"Sat, 29 Feb 2020 17:11:15 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tJakub Adam <jakub.adam@collabora.com>","Message-ID":"<20200229151115.GX18738@pendragon.ideasonboard.com>","References":"<20200227200407.490616-1-nicolas.dufresne@collabora.com>\n\t<20200227200407.490616-27-nicolas.dufresne@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200227200407.490616-27-nicolas.dufresne@collabora.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 26/27] gst: libcamerasrc: Prevent\n\tsrc task deadlock on exhausted buffer pool","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>","X-List-Received-Date":"Sat, 29 Feb 2020 15:11:39 -0000"}},{"id":3885,"web_url":"https://patchwork.libcamera.org/comment/3885/","msgid":"<5382ffc8911b251ff446365a644fde3456a504d9.camel@collabora.com>","date":"2020-02-29T15:35:21","subject":"Re: [libcamera-devel] [PATCH v2 26/27] gst: libcamerasrc: Prevent\n\tsrc task deadlock on exhausted buffer pool","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le samedi 29 février 2020 à 17:11 +0200, Laurent Pinchart a écrit :\n> Hi Nicolas and Jakub,\n> \n> Thank you for the patch.\n> \n> On Thu, Feb 27, 2020 at 03:04:06PM -0500, Nicolas Dufresne wrote:\n> > From: Jakub Adam <jakub.adam@collabora.com>\n> > \n> > Allow GstLibcameraPool to notify the source when a new buffer has become\n> > available in a previously exhausted buffer pool. This can be used to\n> > resume a src task that got paused because it couldn't acquire a buffer.\n> > \n> > Without this change the src task will never resume from pause once the\n> > pool gets exhausted.\n> > \n> > To trigger the deadlock (it doesn't happen every time), run:\n> > \n> >   gst-launch-1.0 libcamerasrc ! queue ! glimagesink\n> > \n> > Signed-off-by: Jakub Adam <jakub.adam@collabora.com>\n> > ---\n> >  src/gstreamer/gstlibcamerapool.cpp | 16 ++++++++++++++++\n> >  src/gstreamer/gstlibcamerapool.h   |  6 ++++++\n> >  src/gstreamer/gstlibcamerasrc.cpp  | 14 ++++++++++++--\n> >  3 files changed, 34 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> > index f85c3aa..5aa0eeb 100644\n> > --- a/src/gstreamer/gstlibcamerapool.cpp\n> > +++ b/src/gstreamer/gstlibcamerapool.cpp\n> > @@ -18,6 +18,8 @@ struct _GstLibcameraPool {\n> >  \n> >  \tGstAtomicQueue *queue;\n> >  \tGstLibcameraAllocator *allocator;\n> > +\tGstLibcameraBufferNotify buffer_notify;\n> > +\tgpointer buffer_notify_data;\n> >  \tStream *stream;\n> >  };\n> >  \n> > @@ -54,7 +56,12 @@ 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> > +\n> >  \tgst_atomic_queue_push(self->queue, buffer);\n> > +\n> > +\tif (do_notify && self->buffer_notify)\n> > +\t\tself->buffer_notify(self->buffer_notify_data);\n> >  }\n> >  \n> >  static void\n> > @@ -108,6 +115,15 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> >  \treturn pool;\n> >  }\n> >  \n> > +void\n> > +gst_libcamera_pool_set_buffer_notify(GstLibcameraPool *self,\n> > +\t\t\t\t     GstLibcameraBufferNotify notify,\n> > +\t\t\t\t     gpointer data)\n> > +{\n> > +\tself->buffer_notify = notify;\n> > +\tself->buffer_notify_data = data;\n> > +}\n> > +\n> >  Stream *\n> >  gst_libcamera_pool_get_stream(GstLibcameraPool *self)\n> >  {\n> > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> > index 6fd2913..545be01 100644\n> > --- a/src/gstreamer/gstlibcamerapool.h\n> > +++ b/src/gstreamer/gstlibcamerapool.h\n> > @@ -20,9 +20,15 @@\n> >  #define GST_TYPE_LIBCAMERA_POOL gst_libcamera_pool_get_type()\n> >  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)\n> >  \n> > +typedef void (*GstLibcameraBufferNotify)(gpointer data);\n> > +\n> >  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n> >  \t\t\t\t\t libcamera::Stream *stream);\n> >  \n> > +void gst_libcamera_pool_set_buffer_notify(GstLibcameraPool *self,\n> > +\t\t\t\t\t  GstLibcameraBufferNotify notify,\n> > +\t\t\t\t\t  gpointer data);\n> > +\n> >  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);\n> >  \n> >  libcamera::Stream *gst_libcamera_buffer_get_stream(GstBuffer *buffer);\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index fe60584..ecbfe37 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -434,6 +434,10 @@ gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)\n> >  \t\tconst StreamConfiguration &stream_cfg = state->config->at(i);\n> >  \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> >  \t\t\t\t\t\t\t\tstream_cfg.stream());\n> > +\t\tgst_libcamera_pool_set_buffer_notify(pool,\n> > +\t\t\t\t\t\t     (GstLibcameraBufferNotify)gst_libcamera_resume_task,\n> > +\t\t\t\t\t\t     task);\n> > +\n> >  \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> >  \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> >  \t}\n> > @@ -468,8 +472,14 @@ gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)\n> >  \n> >  \tstate->cam->stop();\n> >  \n> > -\tfor (GstPad *srcpad : state->srcpads)\n> > -\t\tgst_libcamera_pad_set_pool(srcpad, NULL);\n> > +\tfor (GstPad *srcpad : state->srcpads) {\n> > +\t\tauto *pool = gst_libcamera_pad_get_pool(srcpad);\n> > +\n> > +\t\tif (pool) {\n> > +\t\t\tgst_libcamera_pool_set_buffer_notify(pool, NULL, NULL);\n> > +\t\t\tgst_libcamera_pad_set_pool(srcpad, NULL);\n> \n> s/NULL/nullptr/\n> \n> Up to you, but you could also use a Signal. This would however require\n> C++ objects, so some refactoring would be needed, and it may not be\n> worth it, but adding ad-hoc callback registration is a step on the\n> slippery slope to towards spaghetti code :-)\n\nAs this is a GObject, it could use a signal instead yes. I made that\ncomment internally to Jakub, but decided to send anyway. As I still\nhave many little typos and rebase errors, we'll have time to fix that\nby then.\n\n> \n> > +\t\t}\n> > +\t}\n> >  \n> >  \tg_clear_object(&self->allocator);\n> >  \tg_clear_pointer(&self->flow_combiner,","headers":{"Return-Path":"<nicolas.dufresne@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D8C5262689\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 29 Feb 2020 16:35:24 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id 31971263994"],"Message-ID":"<5382ffc8911b251ff446365a644fde3456a504d9.camel@collabora.com>","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Reply-To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tJakub Adam <jakub.adam@collabora.com>","Date":"Sat, 29 Feb 2020 10:35:21 -0500","In-Reply-To":"<20200229151115.GX18738@pendragon.ideasonboard.com>","References":"<20200227200407.490616-1-nicolas.dufresne@collabora.com>\n\t<20200227200407.490616-27-nicolas.dufresne@collabora.com>\n\t<20200229151115.GX18738@pendragon.ideasonboard.com>","Organization":"Collabora","Content-Type":"multipart/signed; micalg=\"pgp-sha1\";\n\tprotocol=\"application/pgp-signature\"; \n\tboundary=\"=-ithgVie51dGq20H2FPT/\"","User-Agent":"Evolution 3.34.4 (3.34.4-1.fc31) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH v2 26/27] gst: libcamerasrc: Prevent\n\tsrc task deadlock on exhausted buffer pool","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>","X-List-Received-Date":"Sat, 29 Feb 2020 15:35:25 -0000"}},{"id":3891,"web_url":"https://patchwork.libcamera.org/comment/3891/","msgid":"<20200302105528.GL11960@pendragon.ideasonboard.com>","date":"2020-03-02T10:55:28","subject":"Re: [libcamera-devel] [PATCH v2 26/27] gst: libcamerasrc: Prevent\n\tsrc task deadlock on exhausted buffer pool","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Sat, Feb 29, 2020 at 10:35:21AM -0500, Nicolas Dufresne wrote:\n> Le samedi 29 février 2020 à 17:11 +0200, Laurent Pinchart a écrit :\n> > On Thu, Feb 27, 2020 at 03:04:06PM -0500, Nicolas Dufresne wrote:\n> > > From: Jakub Adam <jakub.adam@collabora.com>\n> > > \n> > > Allow GstLibcameraPool to notify the source when a new buffer has become\n> > > available in a previously exhausted buffer pool. This can be used to\n> > > resume a src task that got paused because it couldn't acquire a buffer.\n> > > \n> > > Without this change the src task will never resume from pause once the\n> > > pool gets exhausted.\n> > > \n> > > To trigger the deadlock (it doesn't happen every time), run:\n> > > \n> > >   gst-launch-1.0 libcamerasrc ! queue ! glimagesink\n> > > \n> > > Signed-off-by: Jakub Adam <jakub.adam@collabora.com>\n> > > ---\n> > >  src/gstreamer/gstlibcamerapool.cpp | 16 ++++++++++++++++\n> > >  src/gstreamer/gstlibcamerapool.h   |  6 ++++++\n> > >  src/gstreamer/gstlibcamerasrc.cpp  | 14 ++++++++++++--\n> > >  3 files changed, 34 insertions(+), 2 deletions(-)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> > > index f85c3aa..5aa0eeb 100644\n> > > --- a/src/gstreamer/gstlibcamerapool.cpp\n> > > +++ b/src/gstreamer/gstlibcamerapool.cpp\n> > > @@ -18,6 +18,8 @@ struct _GstLibcameraPool {\n> > >  \n> > >  \tGstAtomicQueue *queue;\n> > >  \tGstLibcameraAllocator *allocator;\n> > > +\tGstLibcameraBufferNotify buffer_notify;\n> > > +\tgpointer buffer_notify_data;\n> > >  \tStream *stream;\n> > >  };\n> > >  \n> > > @@ -54,7 +56,12 @@ 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> > > +\n> > >  \tgst_atomic_queue_push(self->queue, buffer);\n> > > +\n> > > +\tif (do_notify && self->buffer_notify)\n> > > +\t\tself->buffer_notify(self->buffer_notify_data);\n> > >  }\n> > >  \n> > >  static void\n> > > @@ -108,6 +115,15 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> > >  \treturn pool;\n> > >  }\n> > >  \n> > > +void\n> > > +gst_libcamera_pool_set_buffer_notify(GstLibcameraPool *self,\n> > > +\t\t\t\t     GstLibcameraBufferNotify notify,\n> > > +\t\t\t\t     gpointer data)\n> > > +{\n> > > +\tself->buffer_notify = notify;\n> > > +\tself->buffer_notify_data = data;\n> > > +}\n> > > +\n> > >  Stream *\n> > >  gst_libcamera_pool_get_stream(GstLibcameraPool *self)\n> > >  {\n> > > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> > > index 6fd2913..545be01 100644\n> > > --- a/src/gstreamer/gstlibcamerapool.h\n> > > +++ b/src/gstreamer/gstlibcamerapool.h\n> > > @@ -20,9 +20,15 @@\n> > >  #define GST_TYPE_LIBCAMERA_POOL gst_libcamera_pool_get_type()\n> > >  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)\n> > >  \n> > > +typedef void (*GstLibcameraBufferNotify)(gpointer data);\n> > > +\n> > >  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n> > >  \t\t\t\t\t libcamera::Stream *stream);\n> > >  \n> > > +void gst_libcamera_pool_set_buffer_notify(GstLibcameraPool *self,\n> > > +\t\t\t\t\t  GstLibcameraBufferNotify notify,\n> > > +\t\t\t\t\t  gpointer data);\n> > > +\n> > >  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);\n> > >  \n> > >  libcamera::Stream *gst_libcamera_buffer_get_stream(GstBuffer *buffer);\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index fe60584..ecbfe37 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -434,6 +434,10 @@ gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)\n> > >  \t\tconst StreamConfiguration &stream_cfg = state->config->at(i);\n> > >  \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > >  \t\t\t\t\t\t\t\tstream_cfg.stream());\n> > > +\t\tgst_libcamera_pool_set_buffer_notify(pool,\n> > > +\t\t\t\t\t\t     (GstLibcameraBufferNotify)gst_libcamera_resume_task,\n> > > +\t\t\t\t\t\t     task);\n> > > +\n> > >  \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> > >  \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> > >  \t}\n> > > @@ -468,8 +472,14 @@ gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)\n> > >  \n> > >  \tstate->cam->stop();\n> > >  \n> > > -\tfor (GstPad *srcpad : state->srcpads)\n> > > -\t\tgst_libcamera_pad_set_pool(srcpad, NULL);\n> > > +\tfor (GstPad *srcpad : state->srcpads) {\n> > > +\t\tauto *pool = gst_libcamera_pad_get_pool(srcpad);\n> > > +\n> > > +\t\tif (pool) {\n> > > +\t\t\tgst_libcamera_pool_set_buffer_notify(pool, NULL, NULL);\n> > > +\t\t\tgst_libcamera_pad_set_pool(srcpad, NULL);\n> > \n> > s/NULL/nullptr/\n> > \n> > Up to you, but you could also use a Signal. This would however require\n> > C++ objects, so some refactoring would be needed, and it may not be\n> > worth it, but adding ad-hoc callback registration is a step on the\n> > slippery slope to towards spaghetti code :-)\n> \n> As this is a GObject, it could use a signal instead yes. I made that\n> comment internally to Jakub, but decided to send anyway. As I still\n> have many little typos and rebase errors, we'll have time to fix that\n> by then.\n\nI was referring to a libcamera Signal, but a glib signal works too :-)\n\n> > > +\t\t}\n> > > +\t}\n> > >  \n> > >  \tg_clear_object(&self->allocator);\n> > >  \tg_clear_pointer(&self->flow_combiner,","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 6AEA660427\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  2 Mar 2020 11:55:54 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8788854A;\n\tMon,  2 Mar 2020 11:55:53 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583146554;\n\tbh=qhyYRnI7BmxCOazj+EMCsSQhL1p74e5OYHAV1oQ4EjA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XVqR/lBjnVBNxtuUPauLWH9dpueoVLeJZqyzHhcs0cFpbz5vhcRidEdzpAIZpF+tH\n\tNx+oi5lNBIlDXuMjBKDNSckVHBPLE9ywohtEYeRErf7sbTBXniU6QigLi3TUTu6QIj\n\tdaRoF8Y+pWtudKEPqXQds1/OFjdTnJgw11chXFs4=","Date":"Mon, 2 Mar 2020 12:55:28 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tJakub Adam <jakub.adam@collabora.com>","Message-ID":"<20200302105528.GL11960@pendragon.ideasonboard.com>","References":"<20200227200407.490616-1-nicolas.dufresne@collabora.com>\n\t<20200227200407.490616-27-nicolas.dufresne@collabora.com>\n\t<20200229151115.GX18738@pendragon.ideasonboard.com>\n\t<5382ffc8911b251ff446365a644fde3456a504d9.camel@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<5382ffc8911b251ff446365a644fde3456a504d9.camel@collabora.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 26/27] gst: libcamerasrc: Prevent\n\tsrc task deadlock on exhausted buffer pool","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>","X-List-Received-Date":"Mon, 02 Mar 2020 10:55:54 -0000"}}]