[{"id":23615,"web_url":"https://patchwork.libcamera.org/comment/23615/","msgid":"","date":"2022-06-27T18:10:04","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hello Laurent,\n\nI am not sure if I am well versed enough in gstreaner to review this,\nbut here are few of my comments, pardon me if they are wrong ;)\n\nOn Fri, Jun 24, 2022 at 1:22 AM Laurent Pinchart\n wrote:\n>\n> The gst_libcamera_resume_task() helper is an implementation of the\n> gst_task_resume() function that predates its addition to GStreamer. Use\n> gst_task_resume() when available, and rename gst_libcamera_resume_task()\n> to gst_task_resume() to support older GStreamer versions.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n> src/gstreamer/gstlibcamera-utils.h | 4 +++-\n> src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> 3 files changed, 15 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index 3f2422863c03..c97c0d438de2 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> stream_cfg.size.height = height;\n> }\n>\n> -void\n> -gst_libcamera_resume_task(GstTask *task)\n> +#if !GST_CHECK_VERSION(1, 17, 1)\n> +gboolean\n> +gst_task_resume(GstTask *task)\n> {\n> /* We only want to resume the task if it's paused. */\n> GLibLocker lock(GST_OBJECT(task));\n> - if (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n> - GST_TASK_STATE(task) = GST_TASK_STARTED;\n> - GST_TASK_SIGNAL(task);\n> - }\n> + if (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n> + return FALSE;\n> +\n> + GST_TASK_STATE(task) = GST_TASK_STARTED;\n> + GST_TASK_SIGNAL(task);\n> + return TRUE;\n> }\n> +#endif\n\nI had a small doubt, shouldn't this replicate the function defined\nhere ? https://gitlab.freedesktop.org/gstreamer/gstreamer/blob/1.18.4/gst/gsttask.c#L865\n\n>\n> G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n> static std::weak_ptr cm_singleton_ptr;\n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index d54f15885d0c..164189a28965 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n> GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> GstCaps *caps);\n> -void gst_libcamera_resume_task(GstTask *task);\n> +#if !GST_CHECK_VERSION(1, 17, 1)\n> +gboolean gst_task_resume(GstTask *task);\n> +#endif\n> std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n>\n> /**\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 46fd02d207be..9d6be075a474 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> gst_libcamera_pad_queue_buffer(srcpad, buffer);\n> }\n>\n> - gst_libcamera_resume_task(this->src_->task);\n> + gst_task_resume(src_->task);\n> }\n>\n> static bool\n> @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> stream_cfg.stream());\n> g_signal_connect_swapped(pool, \"buffer-notify\",\n> - G_CALLBACK(gst_libcamera_resume_task), task);\n> + G_CALLBACK(gst_task_resume), task);\n>\n> gst_libcamera_pad_set_pool(srcpad, pool);\n> gst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n\nRegards,\nVedant Paranjape","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 78AC4BE173\n\tfor ;\n\tMon, 27 Jun 2022 18:10:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B05C465637;\n\tMon, 27 Jun 2022 20:10:18 +0200 (CEST)","from mail-yw1-x1134.google.com (mail-yw1-x1134.google.com\n\t[IPv6:2607:f8b0:4864:20::1134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C90026059B\n\tfor ;\n\tMon, 27 Jun 2022 20:10:16 +0200 (CEST)","by mail-yw1-x1134.google.com with SMTP id\n\t00721157ae682-31772f8495fso93696137b3.4\n\tfor ;\n\tMon, 27 Jun 2022 11:10:16 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656353418;\n\tbh=281W/CcJ+wVZYvEiRXqKDl6wNt2ns4acUxr6MtqQi/0=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=nbRjKe0VSTeal0gBHgFhw/ttIppc0+QwEc2L686+ONvQDg3ozsIbH9qAPeIjLji/q\n\ta75+BLLUUnC2SSra0L12tD/nB8eqVIPkWC7dpuNTSDV0vtQwhN8A6uPLcAqmtu9k9X\n\tg1VKZfUinFgdYDjR3C93GR+9J4PjOBWa9WWFT1qqFuwt1JZnuK2Xq0GI1ddi8qEXjH\n\tmCWdedSQ4LqD5VsI+nE5PvnKKKjlnOJn5HU0Wl1dqUyF9HaB3urFrspG1nppP70B2p\n\t21i2hwyGe1QgPaTS7SaYNLAY2GY0Wgr819gPGZzhdH1uqKVCsKxRNTRWDyRMN+XOGJ\n\t+9V9byrDBtDoQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=O1+a5RDinM9PDfFJkI4oeCO/G/SHKDzBbdCH7SX9dis=;\n\tb=qhiBuRiT7a0FPbH6VOGBkp0bMXhr1IggquzRH88SUcYeZqqvjyKjMEafd1Xlvnbkk8\n\txXWvkcN1opVf8YaSZXd0VqC4tdkReEEWcyZK7p4kLgT2xzapWTBpLqgtrvRSX/hy/9ka\n\tpOzZlvekNmNj7IBP4FlC++iW8WFb+dNmKQ5PrTM+/89TJodAbK6ulenY4/pp3gtSB5JJ\n\tm2E1v/2bKH5KK+dnQLJg06M73BKNh1yNpAAzYVtR5w5Nze5Scwiq9VYU5l+l7t8+uvCI\n\tYma2qYjiTNjLZOGeCnmMCTw7YXBGm+tcAAhiX/9bghg7g2KsJA1GZQTY+ckoYYpjm8AJ\n\tcEAw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"qhiBuRiT\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=O1+a5RDinM9PDfFJkI4oeCO/G/SHKDzBbdCH7SX9dis=;\n\tb=BjHwmpVr8fo4m2csSft10qCDaCELObn/QI8CStR7FzWBaXSChc/OP4BDgUjvI6UBcz\n\tCc9gnDOJZ06Y/AKGaSAhnb/WVuqT//yOPAEUWUk6zt2yXr/Pwx79bN/ZlYFyaw5DrhJM\n\taYHi0UdecXsXs8oVgGg8QhbcnoeIpmuhN2OX4duaFeXF9bfVmCQUUscUQ+6ysOPRRHKM\n\tCaBzBpRmBBwMANlyCLoGnXo16ZBhuETm3THAgBQtD8slQSptNjC/2f5P5jCfP93vEaAh\n\tzVXcXaUWtk2NwH9QvR7Ri9qfPOObRU6zQmP1rKDFOY+csGTJ/gXESlsQeOp7dUmZNnN+\n\tSKuw==","X-Gm-Message-State":"AJIora8D97uGCziyqOGXyljm6p4I/djorQlnyhGpatJ5DL/RTo51aa+c\n\tBvHz7lKadn62uk8gwk/HshAk6Fh4CMDyU8NdU9c=","X-Google-Smtp-Source":"AGRyM1tY6wjwyA+AMD3rkF64Xiw6GygF9cU2GOabmsZ5HY3/Pgd0GVwTVvk8wKc0/+oLfCdwWQkOZUWpJzuLvK/ZPy4=","X-Received":"by 2002:a05:690c:810:b0:317:9432:7f28 with SMTP id\n\tbx16-20020a05690c081000b0031794327f28mr16914514ywb.305.1656353415498;\n\tMon, 27 Jun 2022 11:10:15 -0700 (PDT)","MIME-Version":"1.0","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>","Date":"Mon, 27 Jun 2022 20:10:04 +0200","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Vedant Paranjape via libcamera-devel\n\t","Reply-To":"Vedant Paranjape ","Cc":"LibCamera Devel ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23616,"web_url":"https://patchwork.libcamera.org/comment/23616/","msgid":"","date":"2022-06-27T19:00:36","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Vedant,\n\nOn Mon, Jun 27, 2022 at 08:10:04PM +0200, Vedant Paranjape wrote:\n> Hello Laurent,\n> \n> I am not sure if I am well versed enough in gstreaner to review this,\n> but here are few of my comments, pardon me if they are wrong ;)\n\nReviews are always appreciated :-)\n\n> On Fri, Jun 24, 2022 at 1:22 AM Laurent Pinchart wrote:\n> >\n> > The gst_libcamera_resume_task() helper is an implementation of the\n> > gst_task_resume() function that predates its addition to GStreamer. Use\n> > gst_task_resume() when available, and rename gst_libcamera_resume_task()\n> > to gst_task_resume() to support older GStreamer versions.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n> > src/gstreamer/gstlibcamera-utils.h | 4 +++-\n> > src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> > 3 files changed, 15 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > index 3f2422863c03..c97c0d438de2 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> > stream_cfg.size.height = height;\n> > }\n> >\n> > -void\n> > -gst_libcamera_resume_task(GstTask *task)\n> > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > +gboolean\n> > +gst_task_resume(GstTask *task)\n> > {\n> > /* We only want to resume the task if it's paused. */\n> > GLibLocker lock(GST_OBJECT(task));\n> > - if (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n> > - GST_TASK_STATE(task) = GST_TASK_STARTED;\n> > - GST_TASK_SIGNAL(task);\n> > - }\n> > + if (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n> > + return FALSE;\n> > +\n> > + GST_TASK_STATE(task) = GST_TASK_STARTED;\n> > + GST_TASK_SIGNAL(task);\n> > + return TRUE;\n> > }\n> > +#endif\n> \n> I had a small doubt, shouldn't this replicate the function defined\n> here ? https://gitlab.freedesktop.org/gstreamer/gstreamer/blob/1.18.4/gst/gsttask.c#L865\n\nIdeally, yes, but that calls gst_task_set_state_unlocked(), which is an\ninternal static function. I could copy its code here too, but as far as\nI understand, the above implementation should be functionally equivalent\nfor our use cases (I think that was Nicolas' intent at least).\n\n> > G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n> > static std::weak_ptr cm_singleton_ptr;\n> > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> > index d54f15885d0c..164189a28965 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.h\n> > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n> > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> > GstCaps *caps);\n> > -void gst_libcamera_resume_task(GstTask *task);\n> > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > +gboolean gst_task_resume(GstTask *task);\n> > +#endif\n> > std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n> >\n> > /**\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 46fd02d207be..9d6be075a474 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> > gst_libcamera_pad_queue_buffer(srcpad, buffer);\n> > }\n> >\n> > - gst_libcamera_resume_task(this->src_->task);\n> > + gst_task_resume(src_->task);\n> > }\n> >\n> > static bool\n> > @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > stream_cfg.stream());\n> > g_signal_connect_swapped(pool, \"buffer-notify\",\n> > - G_CALLBACK(gst_libcamera_resume_task), task);\n> > + G_CALLBACK(gst_task_resume), task);\n> >\n> > gst_libcamera_pad_set_pool(srcpad, pool);\n> > gst_flow_combiner_add_pad(self->flow_combiner, srcpad);","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 3DB08BD808\n\tfor ;\n\tMon, 27 Jun 2022 19:00:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9151F633A7;\n\tMon, 27 Jun 2022 21:00:57 +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 D456B6059B\n\tfor ;\n\tMon, 27 Jun 2022 21:00:55 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D0061C82;\n\tMon, 27 Jun 2022 21:00:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656356457;\n\tbh=pQzzVpGjMaE9buXxmUoXKOnZ8ID5CZhzAbtMNHB564I=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=VhwJx/7IyvQzYOrw488CVemc3jBft/meZgnuK/vLtuvo3yavAF8WLOglnokTMtXWi\n\tgLPeXR8LfHyKJEyJGv2ZoQUeYHomM3cDMr3vEwtArvhCZGiq4bLGF1ydA13aWlWytu\n\tetnn8/aczEUWnzH7m3YQRciPAwWqinUrc4OtX8trhYXpfwGCYV/wyk7RRKlih8pkIc\n\t7gJRTx8r6jiBkcdFTcPNsyVqINOUloWS8NjoewxQNJg+2fuB7VAuEDx2FE0wu2FLwP\n\tmI62bi5tJLu3GnX3t9p6gAxPM7idbPAIGNPLGeV76KtHhvXfp0+cjdUdtCi6q/TfOn\n\tp//YhQppyXGdQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656356455;\n\tbh=pQzzVpGjMaE9buXxmUoXKOnZ8ID5CZhzAbtMNHB564I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UhcGqY4jaUUgwgYRRANP26vlAMJpPNAqlrOkA0x+F4QMgKqLhbPG7uIcAWz/Mif4C\n\tx3Z/sG/haH6BVydvYGEAluN4SZhcC1bSqn+ZNOL81VswUuuy/Tbzmw58+Ua3qRdLOt\n\tiBQg3pWCrzQ46KozS6cOtIgusnd5tvaNutDuNaF8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UhcGqY4j\"; dkim-atps=neutral","Date":"Mon, 27 Jun 2022 22:00:36 +0300","To":"Vedant Paranjape ","Message-ID":"","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"LibCamera Devel ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23617,"web_url":"https://patchwork.libcamera.org/comment/23617/","msgid":"","date":"2022-06-27T20:33:13","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le vendredi 24 juin 2022 à 02:21 +0300, Laurent Pinchart a écrit :\n> > The gst_libcamera_resume_task() helper is an implementation of the\n> > gst_task_resume() function that predates its addition to GStreamer. Use\n> > gst_task_resume() when available, and rename gst_libcamera_resume_task()\n> > to gst_task_resume() to support older GStreamer versions.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n> > src/gstreamer/gstlibcamera-utils.h | 4 +++-\n> > src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> > 3 files changed, 15 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > index 3f2422863c03..c97c0d438de2 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> > \tstream_cfg.size.height = height;\n> > }\n> > \n> > -void\n> > -gst_libcamera_resume_task(GstTask *task)\n> > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > +gboolean\n> > +gst_task_resume(GstTask *task)\n> > {\n> > \t/* We only want to resume the task if it's paused. */\n> > \tGLibLocker lock(GST_OBJECT(task));\n> > -\tif (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n> > -\t\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> > -\t\tGST_TASK_SIGNAL(task);\n> > -\t}\n> > +\tif (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n> > +\t\treturn FALSE;\n> > +\n> > +\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> > +\tGST_TASK_SIGNAL(task);\n> > +\treturn TRUE;\n> > }\n> > +#endif\n\nHave you also considered bumping GStreamer requirement to 1.18.0 and up ? It\nalso been a while since we have bumped this.\n\nWith or without any changes here, code looks equivalent, with a return value\ninstead of void.\n\nReviewed-by: Nicolas Dufresne \n\np.s. I saw Vedant comment, though the code looks like proper reduction of the\nupstream one.\n\n> > \n> > G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n> > static std::weak_ptr cm_singleton_ptr;\n> > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> > index d54f15885d0c..164189a28965 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.h\n> > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n> > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> > \t\t\t\t\t GstCaps *caps);\n> > -void gst_libcamera_resume_task(GstTask *task);\n> > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > +gboolean gst_task_resume(GstTask *task);\n> > +#endif\n> > std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n> > \n> > /**\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 46fd02d207be..9d6be075a474 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> > \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n> > \t}\n> > \n> > -\tgst_libcamera_resume_task(this->src_->task);\n> > +\tgst_task_resume(src_->task);\n> > }\n> > \n> > static bool\n> > @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > \t\t\t\t\t\t\t\tstream_cfg.stream());\n> > \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> > -\t\t\t\t\t G_CALLBACK(gst_libcamera_resume_task), task);\n> > +\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n> > \n> > \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> > \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);","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 85BBDBD808\n\tfor ;\n\tMon, 27 Jun 2022 20:33:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD5E865635;\n\tMon, 27 Jun 2022 22:33:24 +0200 (CEST)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D98C96059B\n\tfor ;\n\tMon, 27 Jun 2022 22:33:22 +0200 (CEST)","from nicolas-tpx395.localdomain (192-222-136-102.qc.cable.ebox.net\n\t[192.222.136.102])\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 madras.collabora.co.uk (Postfix) with ESMTPSA id CCBEE6601667;\n\tMon, 27 Jun 2022 21:33:21 +0100 (BST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656362004;\n\tbh=Jy4PZm2vyv+mBuXb9FUlxJDCZwM4Ge+errGPmKOkV+4=;\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=t0IyYT9/5yRAvw8qFcODTgJjTDX6NgXjXGbXpuTxT/xKHgFZ500GTQHCqWOxZAl1b\n\thG13E4LEjdofz2dHZe6kgBPap1p+nnZPstF/5Bqkl1SpJbU2fCI2OLPGJjfwQin2sL\n\t0H7BgA3DdHaEBbLQr2cKuLe9E6PGB/0GbXmT8tAZAbeZx0Fn5Hom2wcfZFe3awqFI+\n\thgWc+CwMeADNbBY8MpQ0R39hCjhIAZ/SmsIybmYWgm1mCilCIXP8OFNCkaoiZoJH80\n\tvgudAQIN83grykFvy/LCpSMbKx2fgCVzIixlWpi7w1+I3A4i4x77nR/Fm1lnaJRj80\n\to8DXb4auglIRQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1656362002;\n\tbh=Jy4PZm2vyv+mBuXb9FUlxJDCZwM4Ge+errGPmKOkV+4=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=nytz1/zlQp8OWt7sRUtZRinqyYcpF54XZkXAaaCwAyh+P75PEBT+K5fUeDUEqi7N0\n\tDuBbp3lg4PtPVtwVLKJZqVmkq67C6DyDO9gz5NTp+mTXOosULLCFm+xPSYAMbfD1bv\n\tXKzwe0wPTNBT1mkRSdrZnzkaJloPsivnloMPFJr47UfAhaR4HCw3URfLbODrMatxBN\n\tXDI6E21EdQkyEbJ0IWWHxS6MOAYvE/KCY6mckhNJ4mZkUe9bKYlFKfqt0IViy0qflu\n\tUMxsnfPEV1PXgSaTVlSifCj4pXQUQMz7jgcUwWSk0WDGXOjeLPXWFVRdRw2z4Vc3w4\n\tyGCvclSJoSjHg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"nytz1/zl\"; dkim-atps=neutral","Message-ID":"","To":"Laurent Pinchart , \n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 27 Jun 2022 16:33:13 -0400","In-Reply-To":"<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.44.2 (3.44.2-1.fc36) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Nicolas Dufresne via libcamera-devel\n\t","Reply-To":"Nicolas Dufresne ","Cc":"Vedant Paranjape ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23621,"web_url":"https://patchwork.libcamera.org/comment/23621/","msgid":"","date":"2022-06-27T20:47:53","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the review.\n\nOn Mon, Jun 27, 2022 at 04:33:13PM -0400, Nicolas Dufresne wrote:\n> Le vendredi 24 juin 2022 à 02:21 +0300, Laurent Pinchart a écrit :\n> > > The gst_libcamera_resume_task() helper is an implementation of the\n> > > gst_task_resume() function that predates its addition to GStreamer. Use\n> > > gst_task_resume() when available, and rename gst_libcamera_resume_task()\n> > > to gst_task_resume() to support older GStreamer versions.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n> > > src/gstreamer/gstlibcamera-utils.h | 4 +++-\n> > > src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> > > 3 files changed, 15 insertions(+), 9 deletions(-)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > > index 3f2422863c03..c97c0d438de2 100644\n> > > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > > @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> > > \tstream_cfg.size.height = height;\n> > > }\n> > > \n> > > -void\n> > > -gst_libcamera_resume_task(GstTask *task)\n> > > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > > +gboolean\n> > > +gst_task_resume(GstTask *task)\n> > > {\n> > > \t/* We only want to resume the task if it's paused. */\n> > > \tGLibLocker lock(GST_OBJECT(task));\n> > > -\tif (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n> > > -\t\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> > > -\t\tGST_TASK_SIGNAL(task);\n> > > -\t}\n> > > +\tif (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n> > > +\t\treturn FALSE;\n> > > +\n> > > +\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> > > +\tGST_TASK_SIGNAL(task);\n> > > +\treturn TRUE;\n> > > }\n> > > +#endif\n> \n> Have you also considered bumping GStreamer requirement to 1.18.0 and up ? It\n> also been a while since we have bumped this.\n\nWe could. I haven't considered that, mostly out of laziness as I didn't\nspend time checking if distros usually shipped GStreamer 1.18.0 nowadays\n:-) We have a little bit of backward-compatibility code in the\nlibcamerasrc unit test too, which we could then drop, but as it's so\nlittle at this point it doesn't hurt much to keep it for a bit longer.\n\n> With or without any changes here, code looks equivalent, with a return value\n> instead of void.\n> \n> Reviewed-by: Nicolas Dufresne \n> \n> p.s. I saw Vedant comment, though the code looks like proper reduction of the\n> upstream one.\n> \n> > > \n> > > G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n> > > static std::weak_ptr cm_singleton_ptr;\n> > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> > > index d54f15885d0c..164189a28965 100644\n> > > --- a/src/gstreamer/gstlibcamera-utils.h\n> > > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > > @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n> > > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> > > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> > > \t\t\t\t\t GstCaps *caps);\n> > > -void gst_libcamera_resume_task(GstTask *task);\n> > > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > > +gboolean gst_task_resume(GstTask *task);\n> > > +#endif\n> > > std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n> > > \n> > > /**\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 46fd02d207be..9d6be075a474 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> > > \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n> > > \t}\n> > > \n> > > -\tgst_libcamera_resume_task(this->src_->task);\n> > > +\tgst_task_resume(src_->task);\n> > > }\n> > > \n> > > static bool\n> > > @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > > \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > > \t\t\t\t\t\t\t\tstream_cfg.stream());\n> > > \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> > > -\t\t\t\t\t G_CALLBACK(gst_libcamera_resume_task), task);\n> > > +\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n> > > \n> > > \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> > > \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\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 B3389BE173\n\tfor ;\n\tMon, 27 Jun 2022 20:48:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E618065635;\n\tMon, 27 Jun 2022 22:48:14 +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 0C8F86059B\n\tfor ;\n\tMon, 27 Jun 2022 22:48:13 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 517D21C82;\n\tMon, 27 Jun 2022 22:48:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656362895;\n\tbh=0uP+5Kig9OtHrBl0/Qjz8IiqNnC2z0KIO9fbuhlYYZc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=0LEwHb9i6/Rh3lGDDxXKZWwIHoEkT4lE1Y5JCOpkX4kZmjWrE8jK4zpgpOl/aexSQ\n\tBEu5fZk3bRERcOo9yRe03FLzEICRMiHTgN88Fu/NgZDruC3Z+ASbpf4Fw5Vtq/L4Rq\n\t/bjZZg+X53Q50M93q38KSy+MtieH6PIt/NiNpKT3/wC5i6igH89V3v0sy38kzVZP4l\n\tLKB9ef4cpWvuok5RRVIPex7rLdC57udU5FBkxX4wPTZdawwa9M8nBUii3+KdkvG/JK\n\t/2qHyTfXW9bZ5UH/qHe9sbOZDnpmLuJ39xjhkBMSr+ZbhpDSQv3g7bDce4mTd5tyzs\n\twDdiYP+1UPkVQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656362892;\n\tbh=0uP+5Kig9OtHrBl0/Qjz8IiqNnC2z0KIO9fbuhlYYZc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JD6cyY2G3uNYeyV2mjSyTb4D85QeiYawNjuHQSR72GH9BjhcoE3Hg8T7gxIPVICxw\n\tjPlLZhOzioS4PjkZIsfShAuxtDNLbNxaFIR0htnVBowcddjBAfwHDQ1jj3E+EVELY1\n\tD460cQ/bFDsU4eVOHJJ+Bs2SzotDZvGnNQxypowc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JD6cyY2G\"; dkim-atps=neutral","Date":"Mon, 27 Jun 2022 23:47:53 +0300","To":"Nicolas Dufresne ","Message-ID":"","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org,\n\tVedant Paranjape ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23657,"web_url":"https://patchwork.libcamera.org/comment/23657/","msgid":"<65c19055-5e81-0642-00ad-32620fb073d4@ideasonboard.com>","date":"2022-06-29T11:21:35","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi,\n\nOn 6/28/22 02:17, Laurent Pinchart via libcamera-devel wrote:\n> Hi Nicolas,\n>\n> Thank you for the review.\n>\n> On Mon, Jun 27, 2022 at 04:33:13PM -0400, Nicolas Dufresne wrote:\n>> Le vendredi 24 juin 2022 à 02:21 +0300, Laurent Pinchart a écrit :\n>>>> The gst_libcamera_resume_task() helper is an implementation of the\n>>>> gst_task_resume() function that predates its addition to GStreamer. Use\n>>>> gst_task_resume() when available, and rename gst_libcamera_resume_task()\n>>>> to gst_task_resume() to support older GStreamer versions.\n>>>>\n>>>> Signed-off-by: Laurent Pinchart \n>>>> ---\n>>>> src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n>>>> src/gstreamer/gstlibcamera-utils.h | 4 +++-\n>>>> src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n>>>> 3 files changed, 15 insertions(+), 9 deletions(-)\n>>>>\n>>>> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n>>>> index 3f2422863c03..c97c0d438de2 100644\n>>>> --- a/src/gstreamer/gstlibcamera-utils.cpp\n>>>> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n>>>> @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n>>>> \tstream_cfg.size.height = height;\n>>>> }\n>>>> \n>>>> -void\n>>>> -gst_libcamera_resume_task(GstTask *task)\n>>>> +#if !GST_CHECK_VERSION(1, 17, 1)\n>>>> +gboolean\n>>>> +gst_task_resume(GstTask *task)\n>>>> {\n>>>> \t/* We only want to resume the task if it's paused. */\n>>>> \tGLibLocker lock(GST_OBJECT(task));\n>>>> -\tif (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n>>>> -\t\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n>>>> -\t\tGST_TASK_SIGNAL(task);\n>>>> -\t}\n>>>> +\tif (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n>>>> +\t\treturn FALSE;\n>>>> +\n>>>> +\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n>>>> +\tGST_TASK_SIGNAL(task);\n>>>> +\treturn TRUE;\n>>>> }\n>>>> +#endif\n>> Have you also considered bumping GStreamer requirement to 1.18.0 and up ? It\n>> also been a while since we have bumped this.\n> We could. I haven't considered that, mostly out of laziness as I didn't\n> spend time checking if distros usually shipped GStreamer 1.18.0 nowadays\n\n\nI usually do a quick check the debian stable for such decisions on \nhttps://packages.debian.org\n\n> :-) We have a little bit of backward-compatibility code in the\n> libcamerasrc unit test too, which we could then drop, but as it's so\n> little at this point it doesn't hurt much to keep it for a bit longer.\n\n\nI'll take a look here later. I have few patches for gstreamer ongoing.\n\n>\n>> With or without any changes here, code looks equivalent, with a return value\n>> instead of void.\n>>\n>> Reviewed-by: Nicolas Dufresne \n\n\nFor this,\n\nReviewed-by: Umang Jain \n\n>>\n>> p.s. I saw Vedant comment, though the code looks like proper reduction of the\n>> upstream one.\n>>\n>>>> \n>>>> G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n>>>> static std::weak_ptr cm_singleton_ptr;\n>>>> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n>>>> index d54f15885d0c..164189a28965 100644\n>>>> --- a/src/gstreamer/gstlibcamera-utils.h\n>>>> +++ b/src/gstreamer/gstlibcamera-utils.h\n>>>> @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n>>>> GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n>>>> void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n>>>> \t\t\t\t\t GstCaps *caps);\n>>>> -void gst_libcamera_resume_task(GstTask *task);\n>>>> +#if !GST_CHECK_VERSION(1, 17, 1)\n>>>> +gboolean gst_task_resume(GstTask *task);\n>>>> +#endif\n>>>> std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n>>>> \n>>>> /**\n>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n>>>> index 46fd02d207be..9d6be075a474 100644\n>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>>>> @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>>>> \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n>>>> \t}\n>>>> \n>>>> -\tgst_libcamera_resume_task(this->src_->task);\n>>>> +\tgst_task_resume(src_->task);\n>>>> }\n>>>> \n>>>> static bool\n>>>> @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>>>> \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n>>>> \t\t\t\t\t\t\t\tstream_cfg.stream());\n>>>> \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n>>>> -\t\t\t\t\t G_CALLBACK(gst_libcamera_resume_task), task);\n>>>> +\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n>>>> \n>>>> \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n>>>> \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);","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 D3819BD808\n\tfor ;\n\tWed, 29 Jun 2022 11:21:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 47D2865635;\n\tWed, 29 Jun 2022 13:21:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4923D60412\n\tfor ;\n\tWed, 29 Jun 2022 13:21:42 +0200 (CEST)","from [IPV6:2401:4900:1f3f:ca21:e286:106b:5da4:9482] (unknown\n\t[IPv6:2401:4900:1f3f:ca21:e286:106b:5da4:9482])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B3B03D7;\n\tWed, 29 Jun 2022 13:21:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656501703;\n\tbh=AxFAIWr6X1g2qguJCJiCib1rlHx0rO2gfbX2yQtHnMY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=gDa7IASSZP5d1eEZou0Cs1fKD/jxGY+NAwCPQeGSC8N87hi6ncxnEp6WVb/HQkv7Y\n\tPhrJwZo88skd8NDZuKuLBU8I8QGrUvaVEQfc6nGD/eO9ok07j5f6VPTHsEPG7s1XW3\n\tMyS6RgECuecODxe3ZKT1c/c3Q2nvHxWRlNfJyLlYfgzH8Iz3JTkFxFZCpuQd9PqAUr\n\tZe+8zVKIWZ1e6jKIx/XGLw6P7+MXd/bHYFvmO0NK6+JP4Us98rt7etBgsffAzgetaG\n\tAb2/s6jyFx4Fxn3Wt/mN4GNTf/zohhoFXzGoWtc+V09kf3eYcoyaut54AfW/AbiKxL\n\t671Q975VxeP3g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656501701;\n\tbh=AxFAIWr6X1g2qguJCJiCib1rlHx0rO2gfbX2yQtHnMY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=fziHFg8IPs+hRlLjGl7QMyt2mTUuNzuWHXZQOcXKiYkjVnCdNvRMG8q3P0RSnFJj8\n\t3psTHRzbslkQgHbFGNVu4PSJQO4vO/bBMg4hgAU2k0SUZTvzrO0W7DvgCIeGFHXkQe\n\tulp5nb0M2K/Epl4GqmBP+uvTh8jPXsHVaIN8YpKM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"fziHFg8I\"; dkim-atps=neutral","Message-ID":"<65c19055-5e81-0642-00ad-32620fb073d4@ideasonboard.com>","Date":"Wed, 29 Jun 2022 16:51:35 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tNicolas Dufresne ","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>\n\t\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org,\n\tVedant Paranjape ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23665,"web_url":"https://patchwork.libcamera.org/comment/23665/","msgid":"","date":"2022-06-29T18:32:32","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jun 29, 2022 at 04:51:35PM +0530, Umang Jain wrote:\n> Hi,\n> \n> On 6/28/22 02:17, Laurent Pinchart via libcamera-devel wrote:\n> > Hi Nicolas,\n> >\n> > Thank you for the review.\n> >\n> > On Mon, Jun 27, 2022 at 04:33:13PM -0400, Nicolas Dufresne wrote:\n> >> Le vendredi 24 juin 2022 à 02:21 +0300, Laurent Pinchart a écrit :\n> >>>> The gst_libcamera_resume_task() helper is an implementation of the\n> >>>> gst_task_resume() function that predates its addition to GStreamer. Use\n> >>>> gst_task_resume() when available, and rename gst_libcamera_resume_task()\n> >>>> to gst_task_resume() to support older GStreamer versions.\n> >>>>\n> >>>> Signed-off-by: Laurent Pinchart \n> >>>> ---\n> >>>> src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n> >>>> src/gstreamer/gstlibcamera-utils.h | 4 +++-\n> >>>> src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> >>>> 3 files changed, 15 insertions(+), 9 deletions(-)\n> >>>>\n> >>>> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> >>>> index 3f2422863c03..c97c0d438de2 100644\n> >>>> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> >>>> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> >>>> @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> >>>> \tstream_cfg.size.height = height;\n> >>>> }\n> >>>> \n> >>>> -void\n> >>>> -gst_libcamera_resume_task(GstTask *task)\n> >>>> +#if !GST_CHECK_VERSION(1, 17, 1)\n> >>>> +gboolean\n> >>>> +gst_task_resume(GstTask *task)\n> >>>> {\n> >>>> \t/* We only want to resume the task if it's paused. */\n> >>>> \tGLibLocker lock(GST_OBJECT(task));\n> >>>> -\tif (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n> >>>> -\t\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> >>>> -\t\tGST_TASK_SIGNAL(task);\n> >>>> -\t}\n> >>>> +\tif (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n> >>>> +\t\treturn FALSE;\n> >>>> +\n> >>>> +\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> >>>> +\tGST_TASK_SIGNAL(task);\n> >>>> +\treturn TRUE;\n> >>>> }\n> >>>> +#endif\n> >> \n> >> Have you also considered bumping GStreamer requirement to 1.18.0 and up ? It\n> >> also been a while since we have bumped this.\n> > \n> > We could. I haven't considered that, mostly out of laziness as I didn't\n> > spend time checking if distros usually shipped GStreamer 1.18.0 nowadays\n> \n> I usually do a quick check the debian stable for such decisions on \n> https://packages.debian.org\n\nBoth Debian and Ubuntu now ship 1.20, but their old stable releases\nshipped 1.14. I'd prefer giving everybody a bit more time to upgrade,\ngiven that there's no urgency on our side to drop support for 1.14.\n\n> > :-) We have a little bit of backward-compatibility code in the\n> > libcamerasrc unit test too, which we could then drop, but as it's so\n> > little at this point it doesn't hurt much to keep it for a bit longer.\n> \n> I'll take a look here later. I have few patches for gstreamer ongoing.\n> \n> >> With or without any changes here, code looks equivalent, with a return value\n> >> instead of void.\n> >>\n> >> Reviewed-by: Nicolas Dufresne \n> \n> For this,\n> \n> Reviewed-by: Umang Jain \n> \n> >>\n> >> p.s. I saw Vedant comment, though the code looks like proper reduction of the\n> >> upstream one.\n> >>\n> >>>> \n> >>>> G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n> >>>> static std::weak_ptr cm_singleton_ptr;\n> >>>> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> >>>> index d54f15885d0c..164189a28965 100644\n> >>>> --- a/src/gstreamer/gstlibcamera-utils.h\n> >>>> +++ b/src/gstreamer/gstlibcamera-utils.h\n> >>>> @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n> >>>> GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> >>>> void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> >>>> \t\t\t\t\t GstCaps *caps);\n> >>>> -void gst_libcamera_resume_task(GstTask *task);\n> >>>> +#if !GST_CHECK_VERSION(1, 17, 1)\n> >>>> +gboolean gst_task_resume(GstTask *task);\n> >>>> +#endif\n> >>>> std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n> >>>> \n> >>>> /**\n> >>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> >>>> index 46fd02d207be..9d6be075a474 100644\n> >>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >>>> @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> >>>> \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n> >>>> \t}\n> >>>> \n> >>>> -\tgst_libcamera_resume_task(this->src_->task);\n> >>>> +\tgst_task_resume(src_->task);\n> >>>> }\n> >>>> \n> >>>> static bool\n> >>>> @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >>>> \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> >>>> \t\t\t\t\t\t\t\tstream_cfg.stream());\n> >>>> \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> >>>> -\t\t\t\t\t G_CALLBACK(gst_libcamera_resume_task), task);\n> >>>> +\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n> >>>> \n> >>>> \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> >>>> \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);","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 6127FBD808\n\tfor ;\n\tWed, 29 Jun 2022 18:32:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A32065635;\n\tWed, 29 Jun 2022 20:32:54 +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 0B5C160412\n\tfor ;\n\tWed, 29 Jun 2022 20:32:53 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3E7323D7;\n\tWed, 29 Jun 2022 20:32:52 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656527574;\n\tbh=oGz+ki8HZUqMMvy5QAVKXxyYSyM0wnaCF4VakL/PuR0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=B/A5qScf965ZXs0paXuPWfyuno4xzfYlbPSFbTHv6nlifah+21BfDhBvx1I6t22cK\n\tlTJCbah87UKsQ26UzNt7GVx6to3HjLYkvs1mY+hVyRFWAUzv0Gviw9Kh/WVtnwRSfg\n\tlRZofqNsRxTMSAWGceKK/vDn7jTyKlWi1lVCN2kptrKu6sMMI/RBUf6rFFBfCprJPv\n\tWZ157t6IAxt2B3zvIKXA0W5+UdSg6BfM96I9L7BzBIGzG6RDdSGnCVG7AUriZEn48Q\n\t5SqkoE4xmHR9kZCanlIUMCOo6bBs8/trtJBjRSFt1MlfoZW30SfbwCZuQaoByn4kGV\n\tBnXMMU8ghqzPQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656527572;\n\tbh=oGz+ki8HZUqMMvy5QAVKXxyYSyM0wnaCF4VakL/PuR0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=a2Ac13H2nqTNYfKXVRqq5og62mT0anLWp0dU19ZqLXM55mUqJ6IK6QR1Ni+IMd/fY\n\tnmBbkI03pBYQi9CWDtOZd+6XCyO+1SMxg1pmfbUoXJYnFjR61GbI/FDj1lqRpbLv2q\n\tCm4wJrM4LT2PdZI0i8n02hp7Y7b9Hux4WQk83N2k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"a2Ac13H2\"; dkim-atps=neutral","Date":"Wed, 29 Jun 2022 21:32:32 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t<65c19055-5e81-0642-00ad-32620fb073d4@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<65c19055-5e81-0642-00ad-32620fb073d4@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Vedant Paranjape ,\n\tNicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23682,"web_url":"https://patchwork.libcamera.org/comment/23682/","msgid":"","date":"2022-06-30T10:19:40","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hello Laurent,\n\nOn Mon, Jun 27, 2022 at 9:00 PM Laurent Pinchart\n wrote:\n>\n> Hi Vedant,\n>\n> On Mon, Jun 27, 2022 at 08:10:04PM +0200, Vedant Paranjape wrote:\n> > Hello Laurent,\n> >\n> > I am not sure if I am well versed enough in gstreaner to review this,\n> > but here are few of my comments, pardon me if they are wrong ;)\n>\n> Reviews are always appreciated :-)\n>\n> > On Fri, Jun 24, 2022 at 1:22 AM Laurent Pinchart wrote:\n> > >\n> > > The gst_libcamera_resume_task() helper is an implementation of the\n> > > gst_task_resume() function that predates its addition to GStreamer. Use\n> > > gst_task_resume() when available, and rename gst_libcamera_resume_task()\n> > > to gst_task_resume() to support older GStreamer versions.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n\nReviewed-by: Vedant Paranjape \n\n> > > ---\n> > > src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n> > > src/gstreamer/gstlibcamera-utils.h | 4 +++-\n> > > src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> > > 3 files changed, 15 insertions(+), 9 deletions(-)\n> > >\n> > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > > index 3f2422863c03..c97c0d438de2 100644\n> > > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > > @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> > > stream_cfg.size.height = height;\n> > > }\n> > >\n> > > -void\n> > > -gst_libcamera_resume_task(GstTask *task)\n> > > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > > +gboolean\n> > > +gst_task_resume(GstTask *task)\n> > > {\n> > > /* We only want to resume the task if it's paused. */\n> > > GLibLocker lock(GST_OBJECT(task));\n> > > - if (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n> > > - GST_TASK_STATE(task) = GST_TASK_STARTED;\n> > > - GST_TASK_SIGNAL(task);\n> > > - }\n> > > + if (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n> > > + return FALSE;\n> > > +\n> > > + GST_TASK_STATE(task) = GST_TASK_STARTED;\n> > > + GST_TASK_SIGNAL(task);\n> > > + return TRUE;\n> > > }\n> > > +#endif\n> >\n> > I had a small doubt, shouldn't this replicate the function defined\n> > here ? https://gitlab.freedesktop.org/gstreamer/gstreamer/blob/1.18.4/gst/gsttask.c#L865\n>\n> Ideally, yes, but that calls gst_task_set_state_unlocked(), which is an\n> internal static function. I could copy its code here too, but as far as\n> I understand, the above implementation should be functionally equivalent\n> for our use cases (I think that was Nicolas' intent at least).\n\nAh, this makes sense.\n\n>\n> > > G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n> > > static std::weak_ptr cm_singleton_ptr;\n> > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> > > index d54f15885d0c..164189a28965 100644\n> > > --- a/src/gstreamer/gstlibcamera-utils.h\n> > > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > > @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n> > > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> > > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> > > GstCaps *caps);\n> > > -void gst_libcamera_resume_task(GstTask *task);\n> > > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > > +gboolean gst_task_resume(GstTask *task);\n> > > +#endif\n> > > std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n> > >\n> > > /**\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 46fd02d207be..9d6be075a474 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> > > gst_libcamera_pad_queue_buffer(srcpad, buffer);\n> > > }\n> > >\n> > > - gst_libcamera_resume_task(this->src_->task);\n> > > + gst_task_resume(src_->task);\n> > > }\n> > >\n> > > static bool\n> > > @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > > GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > > stream_cfg.stream());\n> > > g_signal_connect_swapped(pool, \"buffer-notify\",\n> > > - G_CALLBACK(gst_libcamera_resume_task), task);\n> > > + G_CALLBACK(gst_task_resume), task);\n> > >\n> > > gst_libcamera_pad_set_pool(srcpad, pool);\n> > > gst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 39FFEBD808\n\tfor ;\n\tThu, 30 Jun 2022 10:19:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B75FE6564F;\n\tThu, 30 Jun 2022 12:19:53 +0200 (CEST)","from mail-yb1-xb33.google.com (mail-yb1-xb33.google.com\n\t[IPv6:2607:f8b0:4864:20::b33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F55E6559A\n\tfor ;\n\tThu, 30 Jun 2022 12:19:52 +0200 (CEST)","by mail-yb1-xb33.google.com with SMTP id i7so32854816ybe.11\n\tfor ;\n\tThu, 30 Jun 2022 03:19:52 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656584393;\n\tbh=1GquYq+BmqVtMDwyBd6xR3TfTHH+FCAcCgrpMmAZ8qg=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tKXn79NC8fGiQLrTtlI4B0dHNmQc6ZZYOM3nC76A9Fb0iYfg483lL07CVGQ5JnxvH\n\tiK8Qmg3qc20SzhdvuL8WI8W9UgV/YZgRbY9cII3JkGLvtldKQSuAg8MTzeYQJ2V29F\n\t6LSRJHReXrvUYl3y4uNvyGfEaKpWm3zuQyjYrJb44prYxAB/K9J/6DAVqfFi+c02WN\n\t5THgL6zlMavlapSmeqrTPN8XPW7XecpXkdsn5iQ0Kn/EYp/4D/eOfT50ziQmqCQQx9\n\thHmLU1+saJA9oQVLLYbhD6c1KLSVK+LOj2dTH+ZLTZMYNGorupDUWllGiv54SUo4K2\n\tBWb+FmaenxeYg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=xaceO4cRnu4bKLkplaVmlkT2JVuP1dJix8LKAT/svno=;\n\tb=kcnfq77S/xoLFfIFS+b216FIPtALKAU8iykCEneRDaqOMoNd4dCuEDDom4UWIi9FpH\n\tswg2MGm1iZ5JkBKDREjvgG7Bbh4yqfx31muxdQX1opQGN9KbFKFyTfcb7Sj6fkieG8zL\n\tvAf9LHvKepIByDoXfXJMFhOeqY4mMZy9C2n1bdSvOBTT+bO8JGBHZ3OAFN4PAbzze9ie\n\tokdjFoc0QKW9ovuOsgEBFQvB87vohY2//RRdwPvdbbH6QaCaVthNkYayuknBwhxiexd2\n\th73qZj+1OkWM0sHlpbevTTy7o+I+duw8wfDpq2eT7/NFois8mvjZx09dX8ZxDSDQ1dHU\n\t/Axw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"kcnfq77S\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=xaceO4cRnu4bKLkplaVmlkT2JVuP1dJix8LKAT/svno=;\n\tb=v6dYgzNlwfF4N3p9VUm4/aU/QRP6sws7Zih+Rf7OBkOFL53b3XQrjSbG1ayyIwVNqG\n\tVbDFhS7dK/2PNpbte1GayHPNZjeXYOqO2Elny8IqQw1u7GetZYYZXG3T2e9UBKJFiuu4\n\tsba1bgzJuL64tVXcpOUh9EA+kg6B7+X2Q2YsGLfUbZ0LQJ5Nppv3Lw3TSr7rx+hoZlNY\n\t0cfkevcTO01aGU1m/lXbE2xqpTg+uYrUsH/wPWr+EowpwAh9IbLZAcegqFMDwhdgbwME\n\tX9xQXOT7Lcvv5vUziAepNQlSTaljJ8qrgWes+p6NYSAE4odLo7FN2Ry7r8UhDrOH3e0U\n\tdpaw==","X-Gm-Message-State":"AJIora9zwniWpVh3+V+8UNwCbj4Iry9ZEvtVXRjebrmCBKgVB72Rqnt+\n\t652mDKVmSyGA8dLW7cvW077QYib6+S0E+I5nTyhdPqSd2ijQkw==","X-Google-Smtp-Source":"AGRyM1vsFvITyAixXHm3qwd7XI2m19S2V+g4mpiipddgWouGB/Pi7Wh3KSe6976iPwAAops73aNa6V6dQ7l1/8W//RQ=","X-Received":"by 2002:a05:6902:14e:b0:64f:d2eb:2df0 with SMTP id\n\tp14-20020a056902014e00b0064fd2eb2df0mr8516018ybh.557.1656584391388;\n\tThu, 30 Jun 2022 03:19:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>\n\t\n\t","In-Reply-To":"","Date":"Thu, 30 Jun 2022 12:19:40 +0200","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Vedant Paranjape via libcamera-devel\n\t","Reply-To":"Vedant Paranjape ","Cc":"LibCamera Devel ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23687,"web_url":"https://patchwork.libcamera.org/comment/23687/","msgid":"","date":"2022-06-30T19:25:32","subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le mercredi 29 juin 2022 à 16:51 +0530, Umang Jain a écrit :\n> Hi,\n> \n> On 6/28/22 02:17, Laurent Pinchart via libcamera-devel wrote:\n> > Hi Nicolas,\n> > \n> > Thank you for the review.\n> > \n> > On Mon, Jun 27, 2022 at 04:33:13PM -0400, Nicolas Dufresne wrote:\n> > > Le vendredi 24 juin 2022 à 02:21 +0300, Laurent Pinchart a écrit :\n> > > > > The gst_libcamera_resume_task() helper is an implementation of the\n> > > > > gst_task_resume() function that predates its addition to GStreamer. Use\n> > > > > gst_task_resume() when available, and rename gst_libcamera_resume_task()\n> > > > > to gst_task_resume() to support older GStreamer versions.\n> > > > > \n> > > > > Signed-off-by: Laurent Pinchart \n> > > > > ---\n> > > > > src/gstreamer/gstlibcamera-utils.cpp | 16 ++++++++++------\n> > > > > src/gstreamer/gstlibcamera-utils.h | 4 +++-\n> > > > > src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> > > > > 3 files changed, 15 insertions(+), 9 deletions(-)\n> > > > > \n> > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > > > > index 3f2422863c03..c97c0d438de2 100644\n> > > > > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > > > > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > > > > @@ -224,16 +224,20 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> > > > > \tstream_cfg.size.height = height;\n> > > > > }\n> > > > > \n> > > > > -void\n> > > > > -gst_libcamera_resume_task(GstTask *task)\n> > > > > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > > > > +gboolean\n> > > > > +gst_task_resume(GstTask *task)\n> > > > > {\n> > > > > \t/* We only want to resume the task if it's paused. */\n> > > > > \tGLibLocker lock(GST_OBJECT(task));\n> > > > > -\tif (GST_TASK_STATE(task) == GST_TASK_PAUSED) {\n> > > > > -\t\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> > > > > -\t\tGST_TASK_SIGNAL(task);\n> > > > > -\t}\n> > > > > +\tif (GST_TASK_STATE(task) != GST_TASK_PAUSED)\n> > > > > +\t\treturn FALSE;\n> > > > > +\n> > > > > +\tGST_TASK_STATE(task) = GST_TASK_STARTED;\n> > > > > +\tGST_TASK_SIGNAL(task);\n> > > > > +\treturn TRUE;\n> > > > > }\n> > > > > +#endif\n> > > Have you also considered bumping GStreamer requirement to 1.18.0 and up ? It\n> > > also been a while since we have bumped this.\n> > We could. I haven't considered that, mostly out of laziness as I didn't\n> > spend time checking if distros usually shipped GStreamer 1.18.0 nowadays\n> \n> \n> I usually do a quick check the debian stable for such decisions on \n> https://packages.debian.org\n\nhttps://packages.debian.org/search?keywords=libgstreamer1.0-0\n1.18.4\n\nAnd the gst_task_resume() is \"Since : 1.18\". So if that is the rule, then yes,\nwe are fine.\n\n> \n> > :-) We have a little bit of backward-compatibility code in the\n> > libcamerasrc unit test too, which we could then drop, but as it's so\n> > little at this point it doesn't hurt much to keep it for a bit longer.\n> \n> \n> I'll take a look here later. I have few patches for gstreamer ongoing.\n> \n> > \n> > > With or without any changes here, code looks equivalent, with a return value\n> > > instead of void.\n> > > \n> > > Reviewed-by: Nicolas Dufresne \n> \n> \n> For this,\n> \n> Reviewed-by: Umang Jain \n> \n> > > \n> > > p.s. I saw Vedant comment, though the code looks like proper reduction of the\n> > > upstream one.\n> > > \n> > > > > \n> > > > > G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n> > > > > static std::weak_ptr cm_singleton_ptr;\n> > > > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> > > > > index d54f15885d0c..164189a28965 100644\n> > > > > --- a/src/gstreamer/gstlibcamera-utils.h\n> > > > > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > > > > @@ -18,7 +18,9 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\n> > > > > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> > > > > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> > > > > \t\t\t\t\t GstCaps *caps);\n> > > > > -void gst_libcamera_resume_task(GstTask *task);\n> > > > > +#if !GST_CHECK_VERSION(1, 17, 1)\n> > > > > +gboolean gst_task_resume(GstTask *task);\n> > > > > +#endif\n> > > > > std::shared_ptr gst_libcamera_get_camera_manager(int &ret);\n> > > > > \n> > > > > /**\n> > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > > index 46fd02d207be..9d6be075a474 100644\n> > > > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > > @@ -192,7 +192,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> > > > > \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n> > > > > \t}\n> > > > > \n> > > > > -\tgst_libcamera_resume_task(this->src_->task);\n> > > > > +\tgst_task_resume(src_->task);\n> > > > > }\n> > > > > \n> > > > > static bool\n> > > > > @@ -453,7 +453,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > > > > \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > > > > \t\t\t\t\t\t\t\tstream_cfg.stream());\n> > > > > \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> > > > > -\t\t\t\t\t G_CALLBACK(gst_libcamera_resume_task), task);\n> > > > > +\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n> > > > > \n> > > > > \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> > > > > \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);","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 3820EBE173\n\tfor ;\n\tThu, 30 Jun 2022 19:25:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 345566564E;\n\tThu, 30 Jun 2022 21:25:43 +0200 (CEST)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89B796559A\n\tfor ;\n\tThu, 30 Jun 2022 21:25:41 +0200 (CEST)","from nicolas-tpx395.localdomain (192-222-136-102.qc.cable.ebox.net\n\t[192.222.136.102])\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 madras.collabora.co.uk (Postfix) with ESMTPSA id 7C0956601969;\n\tThu, 30 Jun 2022 20:25:40 +0100 (BST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656617143;\n\tbh=melHMR2V4Oa1HREtQCgCFGx6T7n98wBoVlacbLS1ekA=;\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=hY/sE4yLHNp5N4UtHJcueKGzaXFVAl0cG7mfCXZrsqurZIH3sn4YyNy+3p1ldh/sD\n\tXMfu9biJEB/DoanCJQnbgAe0tTZBj82y2AoIwpW87NNLttsFIFu00ydsYsLntu9prf\n\t3MKhI3dOMtPaHxDuTdytSGrbg/hHA+pgg2yZkWTaUFVTmHA2ppD8H9l4tb/zp8Ov01\n\tXZWSNvFmWdLQfqtT4Qi2R2l0tB7NOHNNOzf8RsjYrzqUjnRRjof86ZXLlmKSDoBfF/\n\t9hCI7d7tg0hOirYjNhBVgKuPYnAwqFDikQnZFFl2Wg4AkZKTHoDMF+28Yr652xViQm\n\t3MsRdjNmkeGEg==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1656617141;\n\tbh=melHMR2V4Oa1HREtQCgCFGx6T7n98wBoVlacbLS1ekA=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=iopltpFO/GraebGt/tJJ1whPcHbhN/p4PxnQYx3PeW4B4gPwNtbSYC08KgZqufAoV\n\tDgDjPlob6O2WoijL7nB2nylnikEre0mDmrf6SKLB0StinR4T0ge7QYLmMqQ28erOVO\n\ttEPyDJ6CQC2AUU42EFSCEZrCDNh0NCWAPUk7OtnVHZ+zIKtPFpvLLRrG2fXYS7TKzt\n\tq87uoV/wDx8ZPAtw9imKbJ7QzqC2K5ceoZrgY3PUPMSnJgzlVTam/0O1U+zHUkU0bV\n\tt7GQZDxvL7rTtDle4aE+CPFSFXCQbyAcZiUuM+qc5pjE12zWW/YmDAPxeiW+/k4DL2\n\tKoB4sxBp1m8OA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"iopltpFO\"; dkim-atps=neutral","Message-ID":"","To":"Umang Jain , Laurent Pinchart\n\t","Date":"Thu, 30 Jun 2022 15:25:32 -0400","In-Reply-To":"<65c19055-5e81-0642-00ad-32620fb073d4@ideasonboard.com>","References":"<20220623232210.18742-1-laurent.pinchart@ideasonboard.com>\n\t<20220623232210.18742-2-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t<65c19055-5e81-0642-00ad-32620fb073d4@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.44.2 (3.44.2-1.fc36) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH 01/13] gstreamer: Use\n\tgst_task_resume() when available","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","From":"Nicolas Dufresne via libcamera-devel\n\t","Reply-To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org,\n\tVedant Paranjape ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]