[{"id":28062,"web_url":"https://patchwork.libcamera.org/comment/28062/","msgid":"","date":"2023-11-07T18:40:10","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\nI am not too familiar with gstreamer, so I cannot comment on that part,\nbut I believe using a single `std::atomic` object would be better. Please see below.\n\n\n2023. november 6., hétfő 17:52 keltezéssel, Jaslo Ziska via libcamera-devel írta:\n\n> ---\n> Hi,\n> \n> I recently implemented basic EOS handling for the libcamerasrc gstreamer element.\n> \n> I basically looked at how GstBaseSrc does it and tried to do it similarly.\n> \n> I have no idea whether the locking is correct or if this is thread safe but so far it worked without any issues for my purposes.\n> \n> You can also now run the following command and receive a working video file:\n> \n> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! h264parse ! mp4mux ! filesink location=test.mp4\n> \n> Looking forward for feedback!\n> \n> Cheers,\n> \n> Jaslo\n> \n> src/gstreamer/gstlibcamerasrc.cpp | 47 +++++++++++++++++++++++++++++++\n> 1 file changed, 47 insertions(+)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 63d99571..a4681e1e 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> \tgchar *camera_name;\n> \tcontrols::AfModeEnum auto_focus_mode = controls::AfModeManual;\n> \n> +\tGstEvent *pending_eos;\n> +\tint has_pending_eos;\n\nstd::atomic instead of the above two?\n\n\n> +\n> \tGstLibcameraSrcState *state;\n> \tGstLibcameraAllocator *allocator;\n> \tGstFlowCombiner *flow_combiner;\n> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer user_data)\n> \n> \tbool doResume = false;\n> \n> +\tif (g_atomic_int_get(&self->has_pending_eos)) {\n> +\t\tg_atomic_int_set(&self->has_pending_eos, FALSE);\n\nThen you could just do\n\n if (auto *event = self->pending_eos.exchange(nullptr)) {\n ...\n }\n\n> +\n> +\t\tGST_OBJECT_LOCK(self);\n> +\t\tGstEvent *event = self->pending_eos;\n> +\t\tself->pending_eos = NULL;\n> +\t\tGST_OBJECT_UNLOCK(self);\n> +\n> +\t\tfor (GstPad *srcpad : state->srcpads_)\n> +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(event));\n> +\t\tgst_event_unref(event);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> \t/*\n> \t * Create and queue one request. If no buffers are available the\n> \t * function returns -ENOBUFS, which we ignore here as that's not a\n> @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement *element, GstStateChange transition)\n> \treturn ret;\n> }\n> \n> +static gboolean\n> +gst_libcamera_src_send_event(GstElement *element, GstEvent *event)\n> +{\n> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> +\tgboolean ret = FALSE;\n> +\n> +\tswitch (GST_EVENT_TYPE(event)) {\n> +\tcase GST_EVENT_EOS: {\n> +\t\tGST_OBJECT_LOCK(self);\n> +\t\tg_atomic_int_set(&self->has_pending_eos, TRUE);\n> +\t\tif (self->pending_eos)\n> +\t\t\tgst_event_unref(self->pending_eos);\n> +\t\tself->pending_eos = event;\n> +\t\tGST_OBJECT_UNLOCK(self);\n\nAnd you could do\n\n if (auto *old_event = self->pending_eos.exchange(event))\n gst_event_unref(old_event);\n\n\n> +\n> +\t\tret = TRUE;\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tgst_event_unref(event);\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> static void\n> gst_libcamera_src_finalize(GObject *object)\n> {\n> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc *self)\n> \tstate->srcpads_.push_back(gst_pad_new_from_template(templ, \"src\"));\n> \tgst_element_add_pad(GST_ELEMENT(self), state->srcpads_.back());\n> \n> +\tGST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> +\n> \t/* C-style friend. */\n> \tstate->src_ = self;\n> \tself->state = state;\n> @@ -844,6 +890,7 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> \telement_class->request_new_pad = gst_libcamera_src_request_new_pad;\n> \telement_class->release_pad = gst_libcamera_src_release_pad;\n> \telement_class->change_state = gst_libcamera_src_change_state;\n> +\telement_class->send_event = gst_libcamera_src_send_event;\n> \n> \tgst_element_class_set_metadata(element_class,\n> \t\t\t\t \"libcamera Source\", \"Source/Video\",\n> --\n> 2.42.1\n\n\nRegards,\nBarnabás Pőcze","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 EAF5CBD16B\n\tfor ;\n\tTue, 7 Nov 2023 18:40:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 414FF629AE;\n\tTue, 7 Nov 2023 19:40:19 +0100 (CET)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29ABA61DC1\n\tfor ;\n\tTue, 7 Nov 2023 19:40:17 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699382419;\n\tbh=jDXr/U+ixLEapnSeOpe7Op6ur0RdruggXF0HK0RT2vg=;\n\th=Date:To: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=LeSzUo6RTo92/7ypozuFvu7zVYEafp1yCyoA2A5o9pai51ZY7d2PWncMk8/CpAYmH\n\tSdMZMQnNFDbTdJ9MDx2LPUnBG6gTzyQ11+4E3BLpTT8omKVeY7Nrqra7x0dizmnD4N\n\tO4TUczsfK4SNk2v5cQdijSTFfgarWkqbfzBzL1qHZFjIx/uPINn9rExpPNhqTg7jV/\n\tg1t3eIV9OUU2mJay3K1bwU3GVNqOAwIJpkirBAUnHAu/fa4eLgc9PdhUw0L/3FIUbL\n\tj/aomGvAbnLdrQVPwjdfqondNe1CK1fwUbpXzg2L1ahPbDlmirACbIVlTbVrQI6EA2\n\t4YeDeNMK+9tYg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1699382416; x=1699641616;\n\tbh=0vZHSbJSGAFn/T9voJe/YvWky/mCuYiF7gHpvKI804E=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=IjxvifOycMv9pm9oAkqX+3W6A2y6bQ1vWR1xj0eB4a+nxAnzkI+kOSiNb+MyhEkfS\n\tVy41scqg07R5O9/3jNjUbT7m7NKTQtzhGkZQFR5/2+Pq4VT2b3iXBra343Ijq6LWZp\n\tpShEpkH0LxEXRd01HzouxtIIZ5BjaJ2n61lIgs87WGfy1SFEFq52RqbyarneuE/ZBU\n\tmn/mOQVQOwmFYPmKUZE3q5vi9GjQs97IKcVHsjNWYhPWS7f2SlVhe92BXP4YHo5ezf\n\tpD5+rTh+NhdCgPBtIbaHjN0469eilwfppo4/c0Eaf4DbhbFmD7V8+/VWQl2208iT2/\n\tUZI0oD8tdjwKg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"IjxvifOy\"; dkim-atps=neutral","Date":"Tue, 07 Nov 2023 18:40:10 +0000","To":"Jaslo Ziska ","Message-ID":"","In-Reply-To":"<20231106165545.47116-1-jaslo@ziska.de>","References":"<20231106165545.47116-1-jaslo@ziska.de>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28063,"web_url":"https://patchwork.libcamera.org/comment/28063/","msgid":"<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>","date":"2023-11-08T07:36:38","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jalso\n\nOn 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n> ---\n> Hi,\n>\n> I recently implemented basic EOS handling for the libcamerasrc gstreamer element.\n\nAh thanks, I remember we do track a bug report here:\nhttps://bugs.libcamera.org/show_bug.cgi?id=91\n\n>\n> I basically looked at how GstBaseSrc does it and tried to do it similarly.\n>\n> I have no idea whether the locking is correct or if this is thread safe but so far it worked without any issues for my purposes.\n>\n> You can also now run the following command and receive a working video file:\n>\n> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! h264parse ! mp4mux ! filesink location=test.mp4\n\nGood to see that the diff is tested :-D\n>\n> Looking forward for feedback!\n\nIt looks logical and on the right track to me atleast. But there might \nbe other implications regarding the threading and locking mechanisms.\n> Cheers,\n>\n> Jaslo\n>\n> src/gstreamer/gstlibcamerasrc.cpp | 47 +++++++++++++++++++++++++++++++\n> 1 file changed, 47 insertions(+)\n>\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 63d99571..a4681e1e 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> \tgchar *camera_name;\n> \tcontrols::AfModeEnum auto_focus_mode = controls::AfModeManual;\n>\n> +\tGstEvent *pending_eos;\n> +\tint has_pending_eos;\n> +\n> \tGstLibcameraSrcState *state;\n> \tGstLibcameraAllocator *allocator;\n> \tGstFlowCombiner *flow_combiner;\n> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer user_data)\n>\n> \tbool doResume = false;\n>\n> +\tif (g_atomic_int_get(&self->has_pending_eos)) {\n> +\t\tg_atomic_int_set(&self->has_pending_eos, FALSE);\n> +\n> +\t\tGST_OBJECT_LOCK(self);\n> +\t\tGstEvent *event = self->pending_eos;\n> +\t\tself->pending_eos = NULL;\n> +\t\tGST_OBJECT_UNLOCK(self);\n> +\n> +\t\tfor (GstPad *srcpad : state->srcpads_)\n> +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(event));\n> +\t\tgst_event_unref(event);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> \t/*\n> \t * Create and queue one request. If no buffers are available the\n> \t * function returns -ENOBUFS, which we ignore here as that's not a\n> @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement *element, GstStateChange transition)\n> \treturn ret;\n> }\n>\n> +static gboolean\n> +gst_libcamera_src_send_event(GstElement *element, GstEvent *event)\n> +{\n> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> +\tgboolean ret = FALSE;\n> +\n> +\tswitch (GST_EVENT_TYPE(event)) {\n> +\tcase GST_EVENT_EOS: {\n> +\t\tGST_OBJECT_LOCK(self);\n> +\t\tg_atomic_int_set(&self->has_pending_eos, TRUE);\n> +\t\tif (self->pending_eos)\n> +\t\t\tgst_event_unref(self->pending_eos);\n> +\t\tself->pending_eos = event;\n> +\t\tGST_OBJECT_UNLOCK(self);\n> +\n> +\t\tret = TRUE;\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tgst_event_unref(event);\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> static void\n> gst_libcamera_src_finalize(GObject *object)\n> {\n> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc *self)\n> \tstate->srcpads_.push_back(gst_pad_new_from_template(templ, \"src\"));\n> \tgst_element_add_pad(GST_ELEMENT(self), state->srcpads_.back());\n>\n> +\tGST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> +\n\nI always thought libcamerasrc was inheriting from GstBaseSrc, but in \nfact it's not.\nSo, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n\nApart from a missing commit mesasge, the patch looks good to me. I'll \nput this on my radar soon for testing it rigorously.\n> \t/* C-style friend. */\n> \tstate->src_ = self;\n> \tself->state = state;\n> @@ -844,6 +890,7 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> \telement_class->request_new_pad = gst_libcamera_src_request_new_pad;\n> \telement_class->release_pad = gst_libcamera_src_release_pad;\n> \telement_class->change_state = gst_libcamera_src_change_state;\n> +\telement_class->send_event = gst_libcamera_src_send_event;\n>\n> \tgst_element_class_set_metadata(element_class,\n> \t\t\t\t \"libcamera Source\", \"Source/Video\",\n> --\n> 2.42.1","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 A2D8ABDE6B\n\tfor ;\n\tWed, 8 Nov 2023 07:36:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECA48629B3;\n\tWed, 8 Nov 2023 08:36:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 360FA6299E\n\tfor ;\n\tWed, 8 Nov 2023 08:36:45 +0100 (CET)","from [192.168.1.110] (unknown [103.251.226.64])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1D28F836;\n\tWed, 8 Nov 2023 08:36:22 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699429007;\n\tbh=A9khz9jY3xSlrMslxIF63PMLXj5FrnOyKg59yLHuiLs=;\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:\n\tFrom;\n\tb=ZlR5Cy3jmAuG+RCbjB0n4YLPoSxKb02oEa/MTU0qs5qpAvoKQbOJD6gNcqDsqNWcv\n\tgIYeNEPgSOsXsTNAL72bMtKCy+QirL+7+RmXej+llaLNj8Qm3EehHnFkl9wlOI9LD/\n\tgqih/FZP2bvN57CagHEqpuDBIF9w/9MtRr3qGdxs4f4wCHHm/7fIALTb6bLiSWC6aF\n\tzAWoABax5pxELQHi4mmySW8Narc1nhaJN/170DHbsetozXeo7VVySdf//qO0oOI+3n\n\tt8BU948rPl58Xwj/JUmlnIvflH+JTOGKCz4GG3jFlacamYLiHW/TVZN6g652jdGKK3\n\tv7GzJ5x6MGFsQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1699428983;\n\tbh=A9khz9jY3xSlrMslxIF63PMLXj5FrnOyKg59yLHuiLs=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=MZT1kGvY4x2Go+d33qmFPO91ADzlz0qI2t+kd1l81r+35PEjmbpT+5opzVmb6/TEY\n\tpFzNem5bkX3UxxqEsAels19/mMbQhI6O3/XKP9DjkN+0R1ShXZmCgbSPQ+aZdbEDCQ\n\tBmawO67+Qc7tA4peqOzmNVdy5OoWWFMvCxwydnOQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MZT1kGvY\"; dkim-atps=neutral","Message-ID":"<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>","Date":"Wed, 8 Nov 2023 13:06:38 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.10.0","To":"Jaslo Ziska , libcamera-devel@lists.libcamera.org","References":"<20231106165545.47116-1-jaslo@ziska.de>","Content-Language":"en-US","In-Reply-To":"<20231106165545.47116-1-jaslo@ziska.de>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28066,"web_url":"https://patchwork.libcamera.org/comment/28066/","msgid":"<87cywj9jq6.fsf@ziska.de>","date":"2023-11-09T12:34:15","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi Barnabás,\n\nthanks for the suggestion! I tested it and it does work and makes \nthe code a lot more readable in my opinion.\n\nI just do not know how gstreamer- / glib-like the code is supposed \nto stay as other elements for example use GMutex or GRecMutex \ninstead of the C++ native ones. Does anyone else have an opinion \non this?\n\nRegards,\n\nJaslo\n\nBarnabás Pőcze writes:\n\n> Hi\n>\n>\n> I am not too familiar with gstreamer, so I cannot comment on \n> that part,\n> but I believe using a single `std::atomic` object would be \n> better. Please see below.\n>\n>\n> 2023. november 6., hétfő 17:52 keltezéssel, Jaslo Ziska via \n> libcamera-devel írta:\n>\n>> ---\n>> Hi,\n>>\n>> I recently implemented basic EOS handling for the libcamerasrc \n>> gstreamer element.\n>>\n>> I basically looked at how GstBaseSrc does it and tried to do it \n>> similarly.\n>>\n>> I have no idea whether the locking is correct or if this is \n>> thread safe but so far it worked without any issues for my \n>> purposes.\n>>\n>> You can also now run the following command and receive a \n>> working video file:\n>>\n>> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! \n>> h264parse ! mp4mux ! filesink location=test.mp4\n>>\n>> Looking forward for feedback!\n>>\n>> Cheers,\n>>\n>> Jaslo\n>>\n>> src/gstreamer/gstlibcamerasrc.cpp | 47 \n>> +++++++++++++++++++++++++++++++\n>> 1 file changed, 47 insertions(+)\n>>\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n>> b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 63d99571..a4681e1e 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n>> \tgchar *camera_name;\n>> \tcontrols::AfModeEnum auto_focus_mode = \n>> controls::AfModeManual;\n>>\n>> +\tGstEvent *pending_eos;\n>> +\tint has_pending_eos;\n>\n> std::atomic instead of the above two?\n>\n>\n>> +\n>> \tGstLibcameraSrcState *state;\n>> \tGstLibcameraAllocator *allocator;\n>> \tGstFlowCombiner *flow_combiner;\n>> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer \n>> user_data)\n>>\n>> \tbool doResume = false;\n>>\n>> +\tif (g_atomic_int_get(&self->has_pending_eos)) {\n>> +\t\tg_atomic_int_set(&self->has_pending_eos, FALSE);\n>\n> Then you could just do\n>\n> if (auto *event = self->pending_eos.exchange(nullptr)) {\n> ...\n> }\n>\n>> +\n>> +\t\tGST_OBJECT_LOCK(self);\n>> +\t\tGstEvent *event = self->pending_eos;\n>> +\t\tself->pending_eos = NULL;\n>> +\t\tGST_OBJECT_UNLOCK(self);\n>> +\n>> +\t\tfor (GstPad *srcpad : state->srcpads_)\n>> +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(event));\n>> +\t\tgst_event_unref(event);\n>> +\n>> +\t\treturn;\n>> +\t}\n>> +\n>> \t/*\n>> \t * Create and queue one request. If no buffers are \n>> available the\n>> \t * function returns -ENOBUFS, which we ignore here as \n>> that's not a\n>> @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement \n>> *element, GstStateChange transition)\n>> \treturn ret;\n>> }\n>>\n>> +static gboolean\n>> +gst_libcamera_src_send_event(GstElement *element, GstEvent \n>> *event)\n>> +{\n>> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n>> +\tgboolean ret = FALSE;\n>> +\n>> +\tswitch (GST_EVENT_TYPE(event)) {\n>> +\tcase GST_EVENT_EOS: {\n>> +\t\tGST_OBJECT_LOCK(self);\n>> +\t\tg_atomic_int_set(&self->has_pending_eos, TRUE);\n>> +\t\tif (self->pending_eos)\n>> +\t\t\tgst_event_unref(self->pending_eos);\n>> +\t\tself->pending_eos = event;\n>> +\t\tGST_OBJECT_UNLOCK(self);\n>\n> And you could do\n>\n> if (auto *old_event = self->pending_eos.exchange(event))\n> gst_event_unref(old_event);\n>\n>\n>> +\n>> +\t\tret = TRUE;\n>> +\t\tbreak;\n>> +\t}\n>> +\tdefault:\n>> +\t\tgst_event_unref(event);\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\treturn ret;\n>> +}\n>> +\n>> static void\n>> gst_libcamera_src_finalize(GObject *object)\n>> {\n>> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc \n>> *self)\n>> \tstate->srcpads_.push_back(gst_pad_new_from_template(templ, \n>> \"src\"));\n>> \tgst_element_add_pad(GST_ELEMENT(self), \n>> state->srcpads_.back());\n>>\n>> +\tGST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n>> +\n>> \t/* C-style friend. */\n>> \tstate->src_ = self;\n>> \tself->state = state;\n>> @@ -844,6 +890,7 @@ \n>> gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>> \telement_class->request_new_pad = \n>> gst_libcamera_src_request_new_pad;\n>> \telement_class->release_pad = \n>> gst_libcamera_src_release_pad;\n>> \telement_class->change_state = \n>> gst_libcamera_src_change_state;\n>> +\telement_class->send_event = gst_libcamera_src_send_event;\n>>\n>> \tgst_element_class_set_metadata(element_class,\n>> \t\t\t\t \"libcamera Source\", \"Source/Video\",\n>> --\n>> 2.42.1\n>\n>\n> Regards,\n> Barnabás Pőcze","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 32014C3284\n\tfor ;\n\tThu, 9 Nov 2023 12:48:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D6FB629AF;\n\tThu, 9 Nov 2023 13:48:04 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.218])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 33E6B629AD\n\tfor ;\n\tThu, 9 Nov 2023 13:48:02 +0100 (CET)","from archlinux by smtp.strato.de (RZmta 49.9.1 AUTH)\n\twith ESMTPSA id 6bb150zA9Clx3iZ\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tThu, 9 Nov 2023 13:47:59 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699534084;\n\tbh=iRel+KeAqyQzU73gPUvEiLYXZQt4oyBu4R4OdtEfxn8=;\n\th=References:To:Date: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=A8sgdFDqA9OrkbAC6r40hll1c79ji5Wo4Zmf11biAwPB9fP3Sq48pUYJjxieBNjfB\n\tXhPSifG92E3hLBte8P9KR3NZa06JVAsp46ln2r15Bqwk3rt9UemomUO/nX4+UPFmJN\n\t7DJ5Hdw1dNNSTwuAuCh2lFdhzbDtfyPHMcAb5qNXHAaYSwnipyCHnd68kBdMkDTWzS\n\tayc3u2EEr2Q9cCpeZPBSvhQqu9SqyLxP7QBIbUDqz5tr/mYYXEOpXfNlVRL89D3t3E\n\tp76147HSsu4K14nO1PZt0GpfqyyUmFEn/0FxtiKMttZGHK16ECJFCpP2jr8iLVfZyX\n\tz9lMp2ZOjfz/Q==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1699534079;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=SyRcelzZ0uxmwdvyAw3RLD2DdFu96mYF4b1Nq7+ZeKs=;\n\tb=gqKtRNi1JggSPC1LYSu2ZF/WDnPYlTtpvyly+pxGdmcAsMkUfHU/iByzor9adDB5dz\n\tkaZjO9RNrGwqpHV/qCvqFMSuxsasNUfKdnSsnntCOFC81/4844va+/VcTdLMSAt3EqiR\n\trZbiK281pUylaa9gz+8DRHDAkoAqMvpTeDviboA4pGmQHJW7cntplRLIUbKTPDa8sy9L\n\tQ6BZzpbEE8AdG8YQOVGzM9IGfHPUXAXTyly1gS8nH1CD8lwxw5hLrrifIWiNfrW05yq3\n\tG2O1ceZSPqu0/jLAaiHGS5YTcnwS1whKGdoKsDma1WO75tgK1jhrSohmy2ExGyMCg5GR\n\t+9YA==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1699534079;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=SyRcelzZ0uxmwdvyAw3RLD2DdFu96mYF4b1Nq7+ZeKs=;\n\tb=yq7m0IbKSZ7RKljTU/0yTgJ98NkD97UZn7CyLSEwJy7mvqCnwccbLppb92GQhWBfyG\n\tAc1MPWzR/Or0RYnt5sDg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"gqKtRNi1\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"yq7m0IbK\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1699534079; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=r51tHAhyRr9OC/3FyEvVUAUhQHBda5raF91lQ/DUVeQfmADF55e3D663ZVIv/qQVNX\n\tYTwL/1DsKr+CMBntWXK97gAR9hUHSq4pUOX7nq0pR4t1G7l0JdlJvGewkmcoqcxXHfhx\n\tu5Q+nF9tH0/8PaNQ7GFPNm77YrXmFaKcdpFB6wAIvTCKWE0E6XNU84BPESR7OGJlGMPM\n\t1PmR+x46uVOUok4R8At3c7RVFafy/JrUufSZ/q+7cdnBjBnLtlzb1X/ieZ7Lt0juk0ij\n\tRD3CIT896wBl1Pup8b/RY1adOneWicstTCw3QnY4uadRnq/AO7Ap9C2VdLTUUEp9V3Du\n\tEChw==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1699534079;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=SyRcelzZ0uxmwdvyAw3RLD2DdFu96mYF4b1Nq7+ZeKs=;\n\tb=sm1JonhRDyUk9MsBCcqFqfrFoHw3AEN89Y/N6r4+BiRPrCs8v7MY2aYc9Gu79BY/8l\n\tdM6/w20mfCTaFYcBpbGHAae0kT28pqBq755FYhOTj3lwdfgimIWGTEu4vuuD6i7w6Qd0\n\t8z28ka/MSfmLdH5s5aBuArzd4Tyi6zOZ9nRQ/GOWPnHAl7EX6cwTmmophuQNABPLa67z\n\tKod+eeMbzh3+zQe8CGFWLxo4xx+qFQBMb9ztSl59KCk3+wZkrn3VDzYUUvtKceVROxx6\n\tPaRx+pJ7HGhtjN2l8jgZn2jwN9ZwbM9Q0WWu2hyAvGwg7aK8fUkwtsT6CmXMQG5PNALW\n\tp0ng==","ARC-Authentication-Results":"i=1; strato.com;\n arc=none;\n dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusQvJpnsql/jCNNVfDSgNWRnQi1f6u5kqcO+yfkVJzm55/pOeaUCFmlYZOuqHs+kg==\"","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t","User-agent":"mu4e 1.10.7; emacs 29.1","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Date":"Thu, 09 Nov 2023 13:34:15 +0100","In-reply-to":"","Message-ID":"<87cywj9jq6.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"Jaslo Ziska via libcamera-devel ","Reply-To":"Jaslo Ziska ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28067,"web_url":"https://patchwork.libcamera.org/comment/28067/","msgid":"<878r779iy3.fsf@ziska.de>","date":"2023-11-09T12:51:15","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi,\n\nUmang Jain writes:\n> Hi Jalso\n>\n> On 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n>> ---\n>> Hi,\n>>\n>> I recently implemented basic EOS handling for the libcamerasrc \n>> gstreamer element.\n>\n> Ah thanks, I remember we do track a bug report here:\n> https://bugs.libcamera.org/show_bug.cgi?id=91\n\nAh yes, I remember that I stumbled upon this when I was first \ninvestigating this, I will update the bug report.\n\n>>\n>> I basically looked at how GstBaseSrc does it and tried to do it \n>> similarly.\n>>\n>> I have no idea whether the locking is correct or if this is \n>> thread safe but so far it worked without any issues for my \n>> purposes.\n>>\n>> You can also now run the following command and receive a \n>> working video file:\n>>\n>> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! \n>> h264parse ! mp4mux ! filesink location=test.mp4\n>\n> Good to see that the diff is tested :-D\n\nSpeaking of testing, I just ran the test suite and, except for the \nmulti_stream_test (which was skipped) the other tests succeeded.\n\n>>\n>> Looking forward for feedback!\n>\n> It looks logical and on the right track to me atleast. But there \n> might be other implications regarding the threading and\n> locking mechanisms.\n\nThanks! That is what I worry about too, maybe someone can review \nthis.\n\n>> Cheers,\n>>\n>> Jaslo\n>>\n>> src/gstreamer/gstlibcamerasrc.cpp | 47 \n>> +++++++++++++++++++++++++++++++\n>> 1 file changed, 47 insertions(+)\n>>\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n>> b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 63d99571..a4681e1e 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n>> \tgchar *camera_name;\n>> \tcontrols::AfModeEnum auto_focus_mode = \n>> controls::AfModeManual;\n>>\n>> +\tGstEvent *pending_eos;\n>> +\tint has_pending_eos;\n>> +\n>> \tGstLibcameraSrcState *state;\n>> \tGstLibcameraAllocator *allocator;\n>> \tGstFlowCombiner *flow_combiner;\n>> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer \n>> user_data)\n>>\n>> \tbool doResume = false;\n>>\n>> +\tif (g_atomic_int_get(&self->has_pending_eos)) {\n>> +\t\tg_atomic_int_set(&self->has_pending_eos, FALSE);\n>> +\n>> +\t\tGST_OBJECT_LOCK(self);\n>> +\t\tGstEvent *event = self->pending_eos;\n>> +\t\tself->pending_eos = NULL;\n>> +\t\tGST_OBJECT_UNLOCK(self);\n>> +\n>> +\t\tfor (GstPad *srcpad : state->srcpads_)\n>> +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(event));\n>> +\t\tgst_event_unref(event);\n>> +\n>> +\t\treturn;\n>> +\t}\n>> +\n>> \t/*\n>> \t * Create and queue one request. If no buffers are \n>> available the\n>> \t * function returns -ENOBUFS, which we ignore here as \n>> that's not a\n>> @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement \n>> *element, GstStateChange transition)\n>> \treturn ret;\n>> }\n>>\n>> +static gboolean\n>> +gst_libcamera_src_send_event(GstElement *element, GstEvent \n>> *event)\n>> +{\n>> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n>> +\tgboolean ret = FALSE;\n>> +\n>> +\tswitch (GST_EVENT_TYPE(event)) {\n>> +\tcase GST_EVENT_EOS: {\n>> +\t\tGST_OBJECT_LOCK(self);\n>> +\t\tg_atomic_int_set(&self->has_pending_eos, TRUE);\n>> +\t\tif (self->pending_eos)\n>> +\t\t\tgst_event_unref(self->pending_eos);\n>> +\t\tself->pending_eos = event;\n>> +\t\tGST_OBJECT_UNLOCK(self);\n>> +\n>> +\t\tret = TRUE;\n>> +\t\tbreak;\n>> +\t}\n>> +\tdefault:\n>> +\t\tgst_event_unref(event);\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\treturn ret;\n>> +}\n>> +\n>> static void\n>> gst_libcamera_src_finalize(GObject *object)\n>> {\n>> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc \n>> *self)\n>> \tstate->srcpads_.push_back(gst_pad_new_from_template(templ, \n>> \"src\"));\n>> \tgst_element_add_pad(GST_ELEMENT(self), \n>> state->srcpads_.back());\n>>\n>> +\tGST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n>> +\n>\n> I always thought libcamerasrc was inheriting from GstBaseSrc, \n> but in fact it's not.\n> So, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n\nThis flag is actually required to receive the eos signal when it \nis send to the pipeline bin and not the element directly. Took me \na while to figure out.\n\n> Apart from a missing commit mesasge, the patch looks good to me. \n> I'll put this on my radar soon for testing it\n> rigorously.\n\nOh yeah, a commit message would indeed be nice, completely forgot \nthat, thanks :)\n\n>> \t/* C-style friend. */\n>> \tstate->src_ = self;\n>> \tself->state = state;\n>> @@ -844,6 +890,7 @@ \n>> gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>> \telement_class->request_new_pad = \n>> gst_libcamera_src_request_new_pad;\n>> \telement_class->release_pad = \n>> gst_libcamera_src_release_pad;\n>> \telement_class->change_state = \n>> gst_libcamera_src_change_state;\n>> +\telement_class->send_event = gst_libcamera_src_send_event;\n>>\n>> \tgst_element_class_set_metadata(element_class,\n>> \t\t\t\t \"libcamera Source\", \"Source/Video\",\n>> --\n>> 2.42.1\n\nRegards,\n\nJaslo","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 9CFF5BDE6B\n\tfor ;\n\tThu, 9 Nov 2023 13:04:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14C0A629AF;\n\tThu, 9 Nov 2023 14:04:52 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.216])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4EA7629AD\n\tfor ;\n\tThu, 9 Nov 2023 14:04:49 +0100 (CET)","from archlinux by smtp.strato.de (RZmta 49.9.1 AUTH)\n\twith ESMTPSA id 6bb150zA9D4m3pf\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tThu, 9 Nov 2023 14:04:48 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699535092;\n\tbh=K76z7MFKhP4QLx9ZS9J3GT/g6i3LPoYzEhTOnTmkhxo=;\n\th=References:To:Date: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=H70A8SKUiIHUgmrgS1dc14rTLs+1K4YHabK7qp71d1ZoxGtkZCVZ2/LFM0TRbZpdM\n\ts/rA7+NRYzWb7KjOFNl4YZIh0JVCUibFr1cbBypyhr8uBqysCkklcDuLr3HYrTfV4W\n\tETiXVI0lST7SeAV7zOJv0Ij0eoyb3twO9cHP6mk9nqXkBuHuvrTt256oDoM1aPqWaf\n\t6mjuzV+tqNJq12OkC5fA2kpAhRqjPAz1VWk6puHK8uy1ISKEyOcWomsQFxtkRqVGBK\n\t0eFcpTZUiwRGR1zc9vr0p3ZLRMvpDN6kn2vxLhmW6zYWE0WP3V1sF8jF/Oy43G7huh\n\tjxr2nESMk1lsg==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1699535089;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=J0Klut07u2NJgsY4fJQetRZm4s7zvaPnl+G6b+vPbv0=;\n\tb=M8dKHcz6HlDxzB/tEyqoioM+Ja21LZy+4T+NEJUdr/TvqNeK3M5sTK5LnGFoh5IyZg\n\tFn7LDgBMwR6/+5QIL1pcgUVVOKLYYaItJbly2i5Ty+yIbDhS9YPWJKSxsiFXYUr6M466\n\tWYdCXcGykhRvzm/tDQIjt4I7H0SUrNVCAXk4fe5AlilAzcqMrCpI7qfvLUAIE1eqqDJe\n\tyrrod44uwamsbRxqxHOTDAOlH5y9LkWsW1gwqVCMG71GYUYmSQoQMjrdM9RzgO9ZwwME\n\tez+7XmT7vZuE/fnqT3xfULHFa6fdLMvwC4LVG1GHulH2iD/JaDQcoIXlRuBXyGdJMrhF\n\tcb4w==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1699535089;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=J0Klut07u2NJgsY4fJQetRZm4s7zvaPnl+G6b+vPbv0=;\n\tb=nyz5wbuiEq+cKHRQWH3/TRs7rNgD/UP1o68R8zZj77zo2L2mfoAtDS1G2r1AGro5ZJ\n\texnU+V54+2UGORt9vKBw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"M8dKHcz6\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"nyz5wbui\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1699535089; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=Xt+u7WWxLJkstgnpVTu7T3eVY64htt2NjOjcuKPS11H8oHs/Va8PghM2qIf8PaFIlV\n\tKcsX/yo8DZZ8zTXpviBYxEuCzvj4Ve6Mjo1hIUTLYRaFIxKiVeWdl/ep4I59+MJWnYQY\n\t+5F1WSxVHlJeWkW21wSQIZ5vBHs2SrzIUUZPiYfluf2bo9l+D4rxecCyRKMlfwykJlig\n\toc3LmwmHb7yMJD29hYz+ZiKX7reiiFQFUjJZC4J37jw33n5pBXFRXiwxuHSeIzX3QzCl\n\tWvypLpxMhwQ3to6CdoRu3tJnk/OywoykrQLthunqBajtLaGtom6kgVjmPIzBTxZvxBvp\n\tO21A==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1699535089;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=J0Klut07u2NJgsY4fJQetRZm4s7zvaPnl+G6b+vPbv0=;\n\tb=CdnGq6dhu4GTaKR9R+tqWLSQg6tlTbXYitSzm/ugIgYFxW0MSTZ9nwT9rwXrQaQUDO\n\t/UAR2Iz1ubW43ol8lH3Dbmxyhug+3sP8ZSCulHjW/cnnkOeInUpMi+tldF+tbrRzG3cB\n\tJW7xenq2bfT3WgI7Kgbf17XZC0M9fQvcVVet9Bwb0vn62PyoC9+wUlQpmdzE+yoNhvxJ\n\tMXdAenTaEojC61x8hHVcC140Sxpg7+V2mClgtQur9TBx3zBXMpp2Rm8jzsDug/jP0+Ht\n\tI64K+r66O5JM00CoQuxlWPedVTmzRzwir2B6CuqrkLKYi8xxcDIH2zLbVY5l3bvSyrey\n\t2/LQ==","ARC-Authentication-Results":"i=1; strato.com;\n arc=none;\n dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusQvJpnsql/jCNNVfDSgNWRnQi1f6u5kqcO+yfkVJzm55/pOeaUCFmlYZOuqHs+kg==\"","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>","User-agent":"mu4e 1.10.7; emacs 29.1","To":"Umang Jain ","Date":"Thu, 09 Nov 2023 13:51:15 +0100","In-reply-to":"<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>","Message-ID":"<878r779iy3.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"Jaslo Ziska via libcamera-devel ","Reply-To":"Jaslo Ziska ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28068,"web_url":"https://patchwork.libcamera.org/comment/28068/","msgid":"<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>","date":"2023-11-09T13:40:58","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jaslo Ziska via libcamera-devel (2023-11-09 12:51:15)\n> Hi,\n> \n> Umang Jain writes:\n> > Hi Jalso\n> >\n> > On 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n> >> ---\n> >> Hi,\n> >>\n> >> I recently implemented basic EOS handling for the libcamerasrc \n> >> gstreamer element.\n> >\n> > Ah thanks, I remember we do track a bug report here:\n> > https://bugs.libcamera.org/show_bug.cgi?id=91\n> \n> Ah yes, I remember that I stumbled upon this when I was first \n> investigating this, I will update the bug report.\n> \n> >>\n> >> I basically looked at how GstBaseSrc does it and tried to do it \n> >> similarly.\n> >>\n> >> I have no idea whether the locking is correct or if this is \n> >> thread safe but so far it worked without any issues for my \n> >> purposes.\n> >>\n> >> You can also now run the following command and receive a \n> >> working video file:\n> >>\n> >> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! \n> >> h264parse ! mp4mux ! filesink location=test.mp4\n> >\n> > Good to see that the diff is tested :-D\n> \n> Speaking of testing, I just ran the test suite and, except for the \n> multi_stream_test (which was skipped) the other tests succeeded.\n> \n> >>\n> >> Looking forward for feedback!\n> >\n> > It looks logical and on the right track to me atleast. But there \n> > might be other implications regarding the threading and\n> > locking mechanisms.\n> \n> Thanks! That is what I worry about too, maybe someone can review \n> this.\n> \n> >> Cheers,\n> >>\n> >> Jaslo\n> >>\n> >> src/gstreamer/gstlibcamerasrc.cpp | 47 \n> >> +++++++++++++++++++++++++++++++\n> >> 1 file changed, 47 insertions(+)\n> >>\n> >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n> >> b/src/gstreamer/gstlibcamerasrc.cpp\n> >> index 63d99571..a4681e1e 100644\n> >> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> >> gchar *camera_name;\n> >> controls::AfModeEnum auto_focus_mode = \n> >> controls::AfModeManual;\n> >>\n> >> + GstEvent *pending_eos;\n> >> + int has_pending_eos;\n> >> +\n> >> GstLibcameraSrcState *state;\n> >> GstLibcameraAllocator *allocator;\n> >> GstFlowCombiner *flow_combiner;\n> >> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer \n> >> user_data)\n> >>\n> >> bool doResume = false;\n> >>\n> >> + if (g_atomic_int_get(&self->has_pending_eos)) {\n> >> + g_atomic_int_set(&self->has_pending_eos, FALSE);\n> >> +\n> >> + GST_OBJECT_LOCK(self);\n> >> + GstEvent *event = self->pending_eos;\n> >> + self->pending_eos = NULL;\n> >> + GST_OBJECT_UNLOCK(self);\n> >> +\n> >> + for (GstPad *srcpad : state->srcpads_)\n> >> + gst_pad_push_event(srcpad, gst_event_ref(event));\n> >> + gst_event_unref(event);\n> >> +\n> >> + return;\n> >> + }\n> >> +\n> >> /*\n> >> * Create and queue one request. If no buffers are \n> >> available the\n> >> * function returns -ENOBUFS, which we ignore here as \n> >> that's not a\n> >> @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement \n> >> *element, GstStateChange transition)\n> >> return ret;\n> >> }\n> >>\n> >> +static gboolean\n> >> +gst_libcamera_src_send_event(GstElement *element, GstEvent \n> >> *event)\n> >> +{\n> >> + GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> >> + gboolean ret = FALSE;\n> >> +\n> >> + switch (GST_EVENT_TYPE(event)) {\n> >> + case GST_EVENT_EOS: {\n> >> + GST_OBJECT_LOCK(self);\n> >> + g_atomic_int_set(&self->has_pending_eos, TRUE);\n> >> + if (self->pending_eos)\n> >> + gst_event_unref(self->pending_eos);\n> >> + self->pending_eos = event;\n> >> + GST_OBJECT_UNLOCK(self);\n> >> +\n> >> + ret = TRUE;\n> >> + break;\n> >> + }\n> >> + default:\n> >> + gst_event_unref(event);\n> >> + break;\n> >> + }\n> >> +\n> >> + return ret;\n> >> +}\n> >> +\n> >> static void\n> >> gst_libcamera_src_finalize(GObject *object)\n> >> {\n> >> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc \n> >> *self)\n> >> state->srcpads_.push_back(gst_pad_new_from_template(templ, \n> >> \"src\"));\n> >> gst_element_add_pad(GST_ELEMENT(self), \n> >> state->srcpads_.back());\n> >>\n> >> + GST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> >> +\n> >\n> > I always thought libcamerasrc was inheriting from GstBaseSrc, \n> > but in fact it's not.\n> > So, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n> \n> This flag is actually required to receive the eos signal when it \n> is send to the pipeline bin and not the element directly. Took me \n> a while to figure out.\n> \n> > Apart from a missing commit mesasge, the patch looks good to me. \n> > I'll put this on my radar soon for testing it\n> > rigorously.\n> \n> Oh yeah, a commit message would indeed be nice, completely forgot \n> that, thanks :)\n\nCan you also add/clarify /how/ the EOS event is being sent in the commit\nmessage please?\n\nIs this issue occuring when you run the pipeline and press ctrl-c for\ninstance? or is there some other detail?\n\n--\nKieran\n\n\n> \n> >> /* C-style friend. */\n> >> state->src_ = self;\n> >> self->state = state;\n> >> @@ -844,6 +890,7 @@ \n> >> gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >> element_class->request_new_pad = \n> >> gst_libcamera_src_request_new_pad;\n> >> element_class->release_pad = \n> >> gst_libcamera_src_release_pad;\n> >> element_class->change_state = \n> >> gst_libcamera_src_change_state;\n> >> + element_class->send_event = gst_libcamera_src_send_event;\n> >>\n> >> gst_element_class_set_metadata(element_class,\n> >> \"libcamera Source\", \"Source/Video\",\n> >> --\n> >> 2.42.1\n> \n> Regards,\n> \n> Jaslo","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 E790EC3284\n\tfor ;\n\tThu, 9 Nov 2023 13:41:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 260C2629BC;\n\tThu, 9 Nov 2023 14:41:02 +0100 (CET)","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 1D665629AD\n\tfor ;\n\tThu, 9 Nov 2023 14:41:01 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B76CB6EF;\n\tThu, 9 Nov 2023 14:40:38 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699537262;\n\tbh=3FzHOeNTpVh+0om/3QkrtWoKuYPb769o204nPJGHPIg=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tSG/lkmSBFtypyv42PqIEbRMxzC2AeUIQq1FWiT3LqjzHbzBqlFpkQPGEjYSxLk/B\n\tJC5pGekwfJMSncSE6IPlYDQQgR2obP5cP5myl6vAGr4CJjV2JedXf9WRby1v6qqR4L\n\tGVUVjijoK5xQ31Y7fSa00U/DWpEa4GxBzVPJtSnaW+0xDbLvEx64PGc2eJGu/6a+IM\n\tCA+KZwBSdLhXzpEAJBtBleGoKPWaS/58Ww3GEvwxdjIWvXV5w1Km5tX0zEgBGC+W1N\n\tgg3JyoCwtX01RlTAZly3v3q35kNqTxLZxtaTT5UCkUv0mt8t1LNBvnL0bHgHr6gCcO\n\teM/LV8yu+BPyA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1699537238;\n\tbh=3FzHOeNTpVh+0om/3QkrtWoKuYPb769o204nPJGHPIg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=U3R73t41MinADoPL7H2N+KEPvXqg1kIfAuFNVlOvoHmhGlJtp9zQJ0N43cUOeOR9H\n\t8MHE0AmwhM9Al4B/XUrn9w3pd1opIG9hI4M9WKXlfH648Sya/or6/JnOMkxnN2+BP2\n\tdPLj5t6pudBqSwElZ4hpF3cDo4x/LFxEKlfgjp5g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"U3R73t41\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<878r779iy3.fsf@ziska.de>","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>\n\t<878r779iy3.fsf@ziska.de>","To":"Jaslo Ziska ,\n\tJaslo Ziska via libcamera-devel ,\n\tUmang Jain ","Date":"Thu, 09 Nov 2023 13:40:58 +0000","Message-ID":"<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28069,"web_url":"https://patchwork.libcamera.org/comment/28069/","msgid":"<874jhv9f5k.fsf@ziska.de>","date":"2023-11-09T14:20:22","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi Kieran,\n\nKieran Bingham writes:\n> Quoting Jaslo Ziska via libcamera-devel (2023-11-09 12:51:15)\n>> Hi,\n>>\n>> Umang Jain writes:\n>> > Hi Jalso\n>> >\n>> > On 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n>> >> ---\n>> >> Hi,\n>> >>\n>> >> I recently implemented basic EOS handling for the \n>> >> libcamerasrc\n>> >> gstreamer element.\n>> >\n>> > Ah thanks, I remember we do track a bug report here:\n>> > https://bugs.libcamera.org/show_bug.cgi?id=91\n>>\n>> Ah yes, I remember that I stumbled upon this when I was first\n>> investigating this, I will update the bug report.\n>>\n>> >>\n>> >> I basically looked at how GstBaseSrc does it and tried to do \n>> >> it\n>> >> similarly.\n>> >>\n>> >> I have no idea whether the locking is correct or if this is\n>> >> thread safe but so far it worked without any issues for my\n>> >> purposes.\n>> >>\n>> >> You can also now run the following command and receive a\n>> >> working video file:\n>> >>\n>> >> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc !\n>> >> h264parse ! mp4mux ! filesink location=test.mp4\n>> >\n>> > Good to see that the diff is tested :-D\n>>\n>> Speaking of testing, I just ran the test suite and, except for \n>> the\n>> multi_stream_test (which was skipped) the other tests \n>> succeeded.\n>>\n>> >>\n>> >> Looking forward for feedback!\n>> >\n>> > It looks logical and on the right track to me atleast. But \n>> > there\n>> > might be other implications regarding the threading and\n>> > locking mechanisms.\n>>\n>> Thanks! That is what I worry about too, maybe someone can \n>> review\n>> this.\n>>\n>> >> Cheers,\n>> >>\n>> >> Jaslo\n>> >>\n>> >> src/gstreamer/gstlibcamerasrc.cpp | 47\n>> >> +++++++++++++++++++++++++++++++\n>> >> 1 file changed, 47 insertions(+)\n>> >>\n>> >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>> >> b/src/gstreamer/gstlibcamerasrc.cpp\n>> >> index 63d99571..a4681e1e 100644\n>> >> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> >> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> >> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n>> >> gchar *camera_name;\n>> >> controls::AfModeEnum auto_focus_mode =\n>> >> controls::AfModeManual;\n>> >>\n>> >> + GstEvent *pending_eos;\n>> >> + int has_pending_eos;\n>> >> +\n>> >> GstLibcameraSrcState *state;\n>> >> GstLibcameraAllocator *allocator;\n>> >> GstFlowCombiner *flow_combiner;\n>> >> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer\n>> >> user_data)\n>> >>\n>> >> bool doResume = false;\n>> >>\n>> >> + if (g_atomic_int_get(&self->has_pending_eos)) {\n>> >> + g_atomic_int_set(&self->has_pending_eos, \n>> >> FALSE);\n>> >> +\n>> >> + GST_OBJECT_LOCK(self);\n>> >> + GstEvent *event = self->pending_eos;\n>> >> + self->pending_eos = NULL;\n>> >> + GST_OBJECT_UNLOCK(self);\n>> >> +\n>> >> + for (GstPad *srcpad : state->srcpads_)\n>> >> + gst_pad_push_event(srcpad, \n>> >> gst_event_ref(event));\n>> >> + gst_event_unref(event);\n>> >> +\n>> >> + return;\n>> >> + }\n>> >> +\n>> >> /*\n>> >> * Create and queue one request. If no buffers are\n>> >> available the\n>> >> * function returns -ENOBUFS, which we ignore here as\n>> >> that's not a\n>> >> @@ -747,6 +765,32 @@ \n>> >> gst_libcamera_src_change_state(GstElement\n>> >> *element, GstStateChange transition)\n>> >> return ret;\n>> >> }\n>> >>\n>> >> +static gboolean\n>> >> +gst_libcamera_src_send_event(GstElement *element, GstEvent\n>> >> *event)\n>> >> +{\n>> >> + GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n>> >> + gboolean ret = FALSE;\n>> >> +\n>> >> + switch (GST_EVENT_TYPE(event)) {\n>> >> + case GST_EVENT_EOS: {\n>> >> + GST_OBJECT_LOCK(self);\n>> >> + g_atomic_int_set(&self->has_pending_eos, TRUE);\n>> >> + if (self->pending_eos)\n>> >> + gst_event_unref(self->pending_eos);\n>> >> + self->pending_eos = event;\n>> >> + GST_OBJECT_UNLOCK(self);\n>> >> +\n>> >> + ret = TRUE;\n>> >> + break;\n>> >> + }\n>> >> + default:\n>> >> + gst_event_unref(event);\n>> >> + break;\n>> >> + }\n>> >> +\n>> >> + return ret;\n>> >> +}\n>> >> +\n>> >> static void\n>> >> gst_libcamera_src_finalize(GObject *object)\n>> >> {\n>> >> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc\n>> >> *self)\n>> >> state->srcpads_.push_back(gst_pad_new_from_template(templ,\n>> >> \"src\"));\n>> >> gst_element_add_pad(GST_ELEMENT(self),\n>> >> state->srcpads_.back());\n>> >>\n>> >> + GST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n>> >> +\n>> >\n>> > I always thought libcamerasrc was inheriting from GstBaseSrc,\n>> > but in fact it's not.\n>> > So, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n>>\n>> This flag is actually required to receive the eos signal when \n>> it\n>> is send to the pipeline bin and not the element directly. Took \n>> me\n>> a while to figure out.\n>>\n>> > Apart from a missing commit mesasge, the patch looks good to \n>> > me.\n>> > I'll put this on my radar soon for testing it\n>> > rigorously.\n>>\n>> Oh yeah, a commit message would indeed be nice, completely \n>> forgot\n>> that, thanks :)\n>\n> Can you also add/clarify /how/ the EOS event is being sent in \n> the commit\n> message please?\n\nSure, do you mean I should describe how the libcamerasrc element \nreceives the EOS event from the pipeline or how the element is \nsending the event to its pads (or both)?\n\nAlso should I create a new patch with only the new commit message \nright now or wait for some actual changes?\n\n>\n> Is this issue occuring when you run the pipeline and press \n> ctrl-c for\n> instance? or is there some other detail?\n\nYes, exactly, this is what is being fixed, as gst-launch-1.0 sends \nthe EOS event to the pipeline bin and the bin apparently looks for \nthe GST_ELEMENT_FLAG_SOURCE flag on an element which than receives \nthe EOS event.\n\nRegards,\n\nJaslo","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 D2D67BDE6B\n\tfor ;\n\tThu, 9 Nov 2023 14:26:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 22AE4629BC;\n\tThu, 9 Nov 2023 15:26:47 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.216])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CEA24629AD\n\tfor ;\n\tThu, 9 Nov 2023 15:26:45 +0100 (CET)","from archlinux by smtp.strato.de (RZmta 49.9.1 AUTH)\n\twith ESMTPSA id 6bb150zA9EQi4Tu\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tThu, 9 Nov 2023 15:26:44 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699540007;\n\tbh=qd3S3KVPi3Isw7rDHvRvLUW25gWiWj5x15ux5g/sBQ8=;\n\th=References:To:Date: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=hi9L+UkUHC60C2kdFhFRpA3PKIKMBpQ6MlwdVr4yvaBhkbRv3YQKdGovHa/QR3MbD\n\tLtWHGJFN1rW3cW5GXvvk+yLa9T++6cdeJBJn6Oc9BbUXWBo63vkAe/9DJKpQ8iRuKo\n\tdeB9cf11VQrajySizrlqOTDx9prkwJZP+d8kMQzuwOLfV4xUnP0XmGQiA5TEVV/ZSi\n\t3WyR0L7dhC7BNyplAbeQdBKKODoQ/+yCYso33bEO4aEB6VWoHYzNav7rdH2rEv6kdX\n\t8clbbngxPQcPcepvDNC0kh21jMH5cKWEkTWuwZbJkQDFZgLuUhIFEzyhdYVfZSrD1c\n\trXdbLeXpRD+zg==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1699540004;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=0Gc4F7sfeiG3KX5GWKel7JWTB3EWh+ZY2hNvUncwiMw=;\n\tb=WIJJoI4GLplyyu0hkynYpimTbDgi+dX1JGcEywtMOecX2k378IMaI9YKWNIJFv+WGN\n\t0Puqqna9HbSyCG1QgUxhyMocam6iRdfdkujHHLxuacyFuJXRq83co7cXTf4HGf5vu6Yb\n\tV1CNFtpfCJ3A4XdWxN2KThdW9AoIUAA5AGHnO8HD6dwlnYT2fyHDMBKGXPvcczNJG5V+\n\t+jAHgq90SsT3Ky8pA5AUyLmx678/QHiRlU2k2mht2hlOtEVOOoBm0qA70hL6LKDtg8gp\n\tF9gBXYA3fSRhCHLI7Q64PHJw5lpEkKvhTvCrInTe0pejvugs0+pccD6ohNVaMjONnvfY\n\tEZ2g==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1699540004;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=0Gc4F7sfeiG3KX5GWKel7JWTB3EWh+ZY2hNvUncwiMw=;\n\tb=hvz9k+3KJM+9RGOicwUWgC8SuOHiIBMJ9cXqQCZT6/PMxpSn2JNJfMR2HXiL2CmRHq\n\tWFKJAhDe5Sf8YMjzNKDA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"WIJJoI4G\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"hvz9k+3K\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1699540004; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=dH2thcTDjcK/QcbF9773hoQWf33Pe9DkpQNFqnK96ZEUc6Xo0QHDZzw+KykSDLlJGd\n\tSRV3nlypWhCtrCY8Ewb9wSaqxGXKt1x6Jf4mNt0LGjPGgs+nZ1U1DX/BYVYku4qYfijW\n\thcnbO9bxneENbRNkTi7ate2RZLI5ffdhZWfN3Drfl21Lk1UntAe33pZT8Z65z4KGPxjm\n\tfXZNluxzgNhOEhLRx84Et/gqOtmwTTRVhjGhOuSQ3OpyixZJzoULRfefo+ztWY3RpiTP\n\t/Q9DfX4OPBLGSF7n6y6UNMtsAn/7jHksg3Hwbb7QfOPxHVcZXX5zpDwGOwOG4P/0TwOo\n\tDveg==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1699540004;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=0Gc4F7sfeiG3KX5GWKel7JWTB3EWh+ZY2hNvUncwiMw=;\n\tb=GhMevdwlv6C9NELOzxPEjRiaU7SAz4CsNtAqifg6CZRHnzygxTmYIMuAjRp9ZyY7Q3\n\tMoi95s14amHd/L3FZ/rat+EWX3grdk/zgXxchtwXxaPb4boQiGTHsBoB2PgNLBQuf/5t\n\tVopAl7fkHdjWx242wsP2pLBzMNEE2PFK2pGSL8PREmTEMSE4RLDOkAETy8yKSMLARCDk\n\t/a+kiWwIG44gjTPrRnsPEzFk7PNxf2cG9HHtWehWt11It5tSjIn26CDbAVSjvOFQHCrQ\n\tjGvmqO6FeAljxt/uYII1HN7RMHxTlo6OBMfcyCJlkWA4STfmZp948+byZYPZpI4C3lyD\n\t5Xpw==","ARC-Authentication-Results":"i=1; strato.com;\n arc=none;\n dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusQvJpnsql/jCNNVfDSgNWRnQi1f6u5kqcO+yfkVJzm55/pOeaUCFmlYZOuqHs+kg==\"","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>\n\t<878r779iy3.fsf@ziska.de>\n\t<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>","User-agent":"mu4e 1.10.7; emacs 29.1","To":"Kieran Bingham ","Date":"Thu, 09 Nov 2023 15:20:22 +0100","In-reply-to":"<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>","Message-ID":"<874jhv9f5k.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"Jaslo Ziska via libcamera-devel ","Reply-To":"Jaslo Ziska ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28070,"web_url":"https://patchwork.libcamera.org/comment/28070/","msgid":"<169954233534.1233626.15655541530209584232@ping.linuxembedded.co.uk>","date":"2023-11-09T15:05:35","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"+Nicolas\n\nQuoting Jaslo Ziska (2023-11-09 14:20:22)\n> Hi Kieran,\n> \n> Kieran Bingham writes:\n> > Quoting Jaslo Ziska via libcamera-devel (2023-11-09 12:51:15)\n> >> Hi,\n> >>\n> >> Umang Jain writes:\n> >> > Hi Jalso\n> >> >\n> >> > On 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n> >> >> ---\n> >> >> Hi,\n> >> >>\n> >> >> I recently implemented basic EOS handling for the \n> >> >> libcamerasrc\n> >> >> gstreamer element.\n> >> >\n> >> > Ah thanks, I remember we do track a bug report here:\n> >> > https://bugs.libcamera.org/show_bug.cgi?id=91\n> >>\n> >> Ah yes, I remember that I stumbled upon this when I was first\n> >> investigating this, I will update the bug report.\n> >>\n> >> >>\n> >> >> I basically looked at how GstBaseSrc does it and tried to do \n> >> >> it\n> >> >> similarly.\n> >> >>\n> >> >> I have no idea whether the locking is correct or if this is\n> >> >> thread safe but so far it worked without any issues for my\n> >> >> purposes.\n> >> >>\n> >> >> You can also now run the following command and receive a\n> >> >> working video file:\n> >> >>\n> >> >> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc !\n> >> >> h264parse ! mp4mux ! filesink location=test.mp4\n> >> >\n> >> > Good to see that the diff is tested :-D\n> >>\n> >> Speaking of testing, I just ran the test suite and, except for \n> >> the\n> >> multi_stream_test (which was skipped) the other tests \n> >> succeeded.\n> >>\n> >> >>\n> >> >> Looking forward for feedback!\n> >> >\n> >> > It looks logical and on the right track to me atleast. But \n> >> > there\n> >> > might be other implications regarding the threading and\n> >> > locking mechanisms.\n> >>\n> >> Thanks! That is what I worry about too, maybe someone can \n> >> review\n> >> this.\n> >>\n> >> >> Cheers,\n> >> >>\n> >> >> Jaslo\n> >> >>\n> >> >> src/gstreamer/gstlibcamerasrc.cpp | 47\n> >> >> +++++++++++++++++++++++++++++++\n> >> >> 1 file changed, 47 insertions(+)\n> >> >>\n> >> >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> >> >> b/src/gstreamer/gstlibcamerasrc.cpp\n> >> >> index 63d99571..a4681e1e 100644\n> >> >> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >> >> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >> >> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> >> >> gchar *camera_name;\n> >> >> controls::AfModeEnum auto_focus_mode =\n> >> >> controls::AfModeManual;\n> >> >>\n> >> >> + GstEvent *pending_eos;\n> >> >> + int has_pending_eos;\n> >> >> +\n> >> >> GstLibcameraSrcState *state;\n> >> >> GstLibcameraAllocator *allocator;\n> >> >> GstFlowCombiner *flow_combiner;\n> >> >> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer\n> >> >> user_data)\n> >> >>\n> >> >> bool doResume = false;\n> >> >>\n> >> >> + if (g_atomic_int_get(&self->has_pending_eos)) {\n> >> >> + g_atomic_int_set(&self->has_pending_eos, \n> >> >> FALSE);\n> >> >> +\n> >> >> + GST_OBJECT_LOCK(self);\n> >> >> + GstEvent *event = self->pending_eos;\n> >> >> + self->pending_eos = NULL;\n> >> >> + GST_OBJECT_UNLOCK(self);\n> >> >> +\n> >> >> + for (GstPad *srcpad : state->srcpads_)\n> >> >> + gst_pad_push_event(srcpad, \n> >> >> gst_event_ref(event));\n> >> >> + gst_event_unref(event);\n> >> >> +\n> >> >> + return;\n> >> >> + }\n> >> >> +\n> >> >> /*\n> >> >> * Create and queue one request. If no buffers are\n> >> >> available the\n> >> >> * function returns -ENOBUFS, which we ignore here as\n> >> >> that's not a\n> >> >> @@ -747,6 +765,32 @@ \n> >> >> gst_libcamera_src_change_state(GstElement\n> >> >> *element, GstStateChange transition)\n> >> >> return ret;\n> >> >> }\n> >> >>\n> >> >> +static gboolean\n> >> >> +gst_libcamera_src_send_event(GstElement *element, GstEvent\n> >> >> *event)\n> >> >> +{\n> >> >> + GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> >> >> + gboolean ret = FALSE;\n> >> >> +\n> >> >> + switch (GST_EVENT_TYPE(event)) {\n> >> >> + case GST_EVENT_EOS: {\n> >> >> + GST_OBJECT_LOCK(self);\n> >> >> + g_atomic_int_set(&self->has_pending_eos, TRUE);\n> >> >> + if (self->pending_eos)\n> >> >> + gst_event_unref(self->pending_eos);\n> >> >> + self->pending_eos = event;\n> >> >> + GST_OBJECT_UNLOCK(self);\n> >> >> +\n> >> >> + ret = TRUE;\n> >> >> + break;\n> >> >> + }\n> >> >> + default:\n> >> >> + gst_event_unref(event);\n> >> >> + break;\n> >> >> + }\n> >> >> +\n> >> >> + return ret;\n> >> >> +}\n> >> >> +\n> >> >> static void\n> >> >> gst_libcamera_src_finalize(GObject *object)\n> >> >> {\n> >> >> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc\n> >> >> *self)\n> >> >> state->srcpads_.push_back(gst_pad_new_from_template(templ,\n> >> >> \"src\"));\n> >> >> gst_element_add_pad(GST_ELEMENT(self),\n> >> >> state->srcpads_.back());\n> >> >>\n> >> >> + GST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> >> >> +\n> >> >\n> >> > I always thought libcamerasrc was inheriting from GstBaseSrc,\n> >> > but in fact it's not.\n> >> > So, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n> >>\n> >> This flag is actually required to receive the eos signal when \n> >> it\n> >> is send to the pipeline bin and not the element directly. Took \n> >> me\n> >> a while to figure out.\n> >>\n> >> > Apart from a missing commit mesasge, the patch looks good to \n> >> > me.\n> >> > I'll put this on my radar soon for testing it\n> >> > rigorously.\n> >>\n> >> Oh yeah, a commit message would indeed be nice, completely \n> >> forgot\n> >> that, thanks :)\n> >\n> > Can you also add/clarify /how/ the EOS event is being sent in \n> > the commit\n> > message please?\n> \n> Sure, do you mean I should describe how the libcamerasrc element \n> receives the EOS event from the pipeline or how the element is \n> sending the event to its pads (or both)?\n> \n> Also should I create a new patch with only the new commit message \n> right now or wait for some actual changes?\n\nI would hold off a few more days.\n\nNicolas - is this something you could cast an eye over please?\n\n--\nKieran\n\n> \n> >\n> > Is this issue occuring when you run the pipeline and press \n> > ctrl-c for\n> > instance? or is there some other detail?\n> \n> Yes, exactly, this is what is being fixed, as gst-launch-1.0 sends \n> the EOS event to the pipeline bin and the bin apparently looks for \n> the GST_ELEMENT_FLAG_SOURCE flag on an element which than receives \n> the EOS event.\n> \n> Regards,\n> \n> Jaslo","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 084C7C3284\n\tfor ;\n\tThu, 9 Nov 2023 15:05:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 664A4629BC;\n\tThu, 9 Nov 2023 16:05:39 +0100 (CET)","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 2D679629AD\n\tfor ;\n\tThu, 9 Nov 2023 16:05:38 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A59616EF;\n\tThu, 9 Nov 2023 16:05:15 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699542339;\n\tbh=22o+R6BgCmqySb4BHzRiL165qMBcTcEUci63RXrK0mM=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ddu44AqzU65Gk/mdmo0v/ADk3n/Od1gGVnk8a3HoMPpZy0ECvh2lU1E7jWqTe9XwV\n\tQI2CyClovn1KgfrgjzIWTK9eMoD7cSS8hhRsbmnz4v/itO3lkOmH5mdp1L2gw8JK3K\n\tvZuArQMCV2y3V7wy/QG8yQz4PUc/gK3BqAtabt/sTBVGakxtNBl698m2G5ztKjHZPR\n\tXz++UxcSjqRJinTZ6g9FyAiihupMSRlhal7ggd/6k7uVj2Y3RmxR1qipoU8ZaWUOvg\n\txteaNpeBeGsU+KFR5egY6NQ3JQenATF5nmDTse70FlV/v+S7kTy88JlKkaymHUEmnA\n\tsrIfeZchKGHmg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1699542315;\n\tbh=22o+R6BgCmqySb4BHzRiL165qMBcTcEUci63RXrK0mM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=sK/F6cT30GYavFFfZgAmCA/FS2Bh7TxvgzX737uOolbPA5nZQWCP4zPXElaFUaHd+\n\tj3JjMoRuwzkmPxClDeCmlQuY6EYDQNhrInU7xDUvPL4vZ8ZsCRldab+QxW6RxHqKv2\n\tg1m0wh5ss3pAQdTJW/3o3j1cjWVoIh/bLiqSkW5o="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sK/F6cT3\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<874jhv9f5k.fsf@ziska.de>","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>\n\t<878r779iy3.fsf@ziska.de>\n\t<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>\n\t<874jhv9f5k.fsf@ziska.de>","To":"Jaslo Ziska , nicolas.dufresne@collabora.com,","Date":"Thu, 09 Nov 2023 15:05:35 +0000","Message-ID":"<169954233534.1233626.15655541530209584232@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28071,"web_url":"https://patchwork.libcamera.org/comment/28071/","msgid":"<4a7bb45555d19bcea4854ce6bfb0b938885f2f3e.camel@collabora.com>","date":"2023-11-09T15:07:38","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Thanks,\n\nwill review/test soon.\n\nLe jeudi 09 novembre 2023 à 15:05 +0000, Kieran Bingham a écrit :\n> +Nicolas\n> \n> Quoting Jaslo Ziska (2023-11-09 14:20:22)\n> > Hi Kieran,\n> > \n> > Kieran Bingham writes:\n> > > Quoting Jaslo Ziska via libcamera-devel (2023-11-09 12:51:15)\n> > > > Hi,\n> > > > \n> > > > Umang Jain writes:\n> > > > > Hi Jalso\n> > > > > \n> > > > > On 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n> > > > > > ---\n> > > > > > Hi,\n> > > > > > \n> > > > > > I recently implemented basic EOS handling for the \n> > > > > > libcamerasrc\n> > > > > > gstreamer element.\n> > > > > \n> > > > > Ah thanks, I remember we do track a bug report here:\n> > > > > https://bugs.libcamera.org/show_bug.cgi?id=91\n> > > > \n> > > > Ah yes, I remember that I stumbled upon this when I was first\n> > > > investigating this, I will update the bug report.\n> > > > \n> > > > > > \n> > > > > > I basically looked at how GstBaseSrc does it and tried to do \n> > > > > > it\n> > > > > > similarly.\n> > > > > > \n> > > > > > I have no idea whether the locking is correct or if this is\n> > > > > > thread safe but so far it worked without any issues for my\n> > > > > > purposes.\n> > > > > > \n> > > > > > You can also now run the following command and receive a\n> > > > > > working video file:\n> > > > > > \n> > > > > > gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc !\n> > > > > > h264parse ! mp4mux ! filesink location=test.mp4\n> > > > > \n> > > > > Good to see that the diff is tested :-D\n> > > > \n> > > > Speaking of testing, I just ran the test suite and, except for \n> > > > the\n> > > > multi_stream_test (which was skipped) the other tests \n> > > > succeeded.\n> > > > \n> > > > > > \n> > > > > > Looking forward for feedback!\n> > > > > \n> > > > > It looks logical and on the right track to me atleast. But \n> > > > > there\n> > > > > might be other implications regarding the threading and\n> > > > > locking mechanisms.\n> > > > \n> > > > Thanks! That is what I worry about too, maybe someone can \n> > > > review\n> > > > this.\n> > > > \n> > > > > > Cheers,\n> > > > > > \n> > > > > > Jaslo\n> > > > > > \n> > > > > > src/gstreamer/gstlibcamerasrc.cpp | 47\n> > > > > > +++++++++++++++++++++++++++++++\n> > > > > > 1 file changed, 47 insertions(+)\n> > > > > > \n> > > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> > > > > > b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > > > index 63d99571..a4681e1e 100644\n> > > > > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > > > @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> > > > > > gchar *camera_name;\n> > > > > > controls::AfModeEnum auto_focus_mode =\n> > > > > > controls::AfModeManual;\n> > > > > > \n> > > > > > + GstEvent *pending_eos;\n> > > > > > + int has_pending_eos;\n> > > > > > +\n> > > > > > GstLibcameraSrcState *state;\n> > > > > > GstLibcameraAllocator *allocator;\n> > > > > > GstFlowCombiner *flow_combiner;\n> > > > > > @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer\n> > > > > > user_data)\n> > > > > > \n> > > > > > bool doResume = false;\n> > > > > > \n> > > > > > + if (g_atomic_int_get(&self->has_pending_eos)) {\n> > > > > > + g_atomic_int_set(&self->has_pending_eos, \n> > > > > > FALSE);\n> > > > > > +\n> > > > > > + GST_OBJECT_LOCK(self);\n> > > > > > + GstEvent *event = self->pending_eos;\n> > > > > > + self->pending_eos = NULL;\n> > > > > > + GST_OBJECT_UNLOCK(self);\n> > > > > > +\n> > > > > > + for (GstPad *srcpad : state->srcpads_)\n> > > > > > + gst_pad_push_event(srcpad, \n> > > > > > gst_event_ref(event));\n> > > > > > + gst_event_unref(event);\n> > > > > > +\n> > > > > > + return;\n> > > > > > + }\n> > > > > > +\n> > > > > > /*\n> > > > > > * Create and queue one request. If no buffers are\n> > > > > > available the\n> > > > > > * function returns -ENOBUFS, which we ignore here as\n> > > > > > that's not a\n> > > > > > @@ -747,6 +765,32 @@ \n> > > > > > gst_libcamera_src_change_state(GstElement\n> > > > > > *element, GstStateChange transition)\n> > > > > > return ret;\n> > > > > > }\n> > > > > > \n> > > > > > +static gboolean\n> > > > > > +gst_libcamera_src_send_event(GstElement *element, GstEvent\n> > > > > > *event)\n> > > > > > +{\n> > > > > > + GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> > > > > > + gboolean ret = FALSE;\n> > > > > > +\n> > > > > > + switch (GST_EVENT_TYPE(event)) {\n> > > > > > + case GST_EVENT_EOS: {\n> > > > > > + GST_OBJECT_LOCK(self);\n> > > > > > + g_atomic_int_set(&self->has_pending_eos, TRUE);\n> > > > > > + if (self->pending_eos)\n> > > > > > + gst_event_unref(self->pending_eos);\n> > > > > > + self->pending_eos = event;\n> > > > > > + GST_OBJECT_UNLOCK(self);\n> > > > > > +\n> > > > > > + ret = TRUE;\n> > > > > > + break;\n> > > > > > + }\n> > > > > > + default:\n> > > > > > + gst_event_unref(event);\n> > > > > > + break;\n> > > > > > + }\n> > > > > > +\n> > > > > > + return ret;\n> > > > > > +}\n> > > > > > +\n> > > > > > static void\n> > > > > > gst_libcamera_src_finalize(GObject *object)\n> > > > > > {\n> > > > > > @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc\n> > > > > > *self)\n> > > > > > state->srcpads_.push_back(gst_pad_new_from_template(templ,\n> > > > > > \"src\"));\n> > > > > > gst_element_add_pad(GST_ELEMENT(self),\n> > > > > > state->srcpads_.back());\n> > > > > > \n> > > > > > + GST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> > > > > > +\n> > > > > \n> > > > > I always thought libcamerasrc was inheriting from GstBaseSrc,\n> > > > > but in fact it's not.\n> > > > > So, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n> > > > \n> > > > This flag is actually required to receive the eos signal when \n> > > > it\n> > > > is send to the pipeline bin and not the element directly. Took \n> > > > me\n> > > > a while to figure out.\n> > > > \n> > > > > Apart from a missing commit mesasge, the patch looks good to \n> > > > > me.\n> > > > > I'll put this on my radar soon for testing it\n> > > > > rigorously.\n> > > > \n> > > > Oh yeah, a commit message would indeed be nice, completely \n> > > > forgot\n> > > > that, thanks :)\n> > > \n> > > Can you also add/clarify /how/ the EOS event is being sent in \n> > > the commit\n> > > message please?\n> > \n> > Sure, do you mean I should describe how the libcamerasrc element \n> > receives the EOS event from the pipeline or how the element is \n> > sending the event to its pads (or both)?\n> > \n> > Also should I create a new patch with only the new commit message \n> > right now or wait for some actual changes?\n> \n> I would hold off a few more days.\n> \n> Nicolas - is this something you could cast an eye over please?\n> \n> --\n> Kieran\n> \n> > \n> > > \n> > > Is this issue occuring when you run the pipeline and press \n> > > ctrl-c for\n> > > instance? or is there some other detail?\n> > \n> > Yes, exactly, this is what is being fixed, as gst-launch-1.0 sends \n> > the EOS event to the pipeline bin and the bin apparently looks for \n> > the GST_ELEMENT_FLAG_SOURCE flag on an element which than receives \n> > the EOS event.\n> > \n> > Regards,\n> > \n> > Jaslo","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 B0087C3284\n\tfor ;\n\tThu, 9 Nov 2023 15:07:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 325C6629BC;\n\tThu, 9 Nov 2023 16:07:50 +0100 (CET)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[46.235.227.172])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C101629AD\n\tfor ;\n\tThu, 9 Nov 2023 16:07:49 +0100 (CET)","from [100.84.166.245] (cola.collaboradmins.com [195.201.22.229])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby madras.collabora.co.uk (Postfix) with ESMTPSA id 46F4B660741E;\n\tThu, 9 Nov 2023 15:07:48 +0000 (GMT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699542470;\n\tbh=vknAuTTWOkX84CcoV3PQgbgtB3TBlh+d1Nci9o6sNfQ=;\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=SrgghkbHE/ik93CL4XKWzj3bUvNwDkXmA9pTM9mC80r+zdfW5gN/SNDJRqG81fgr3\n\tllif2J08Og7jeQgT5AlRrCDFkpG7E3Nw1Pz3OmnZ6RSly2fn1aRM0S65JjHyVUijvK\n\tR0PLmJLaVwqKnJzWQ21eXO9Ck/hcku2UKehNgzQg4xtM8cYchbnQckgiMHdoqHQCie\n\tLJFM+AWziafTYiC4FNgkOO7PaBQYtMiaqFcKo0zAoAkml3QfdTIuYW8qVTt3Rn3A7s\n\tZQymbD5Iw/x+etlM8JhqnirR8UIPPcYzoXt7wX3T5zYYAXjkw0rxeIlj8s0HgtFXLe\n\tIdGo1sSlXH3ag==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1699542469;\n\tbh=vknAuTTWOkX84CcoV3PQgbgtB3TBlh+d1Nci9o6sNfQ=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=E64hEIobWgaIWAwRxpwwPSK2XxXhS8M6gIWzxKomu+ZxmQjeblq3iX+aIzc7A1yxt\n\tgLy+KcOlimrf8r+peMBn0FF0UE+kwuNtQxBerPBONMjnmhXz6A3/xWc0fqlntFrZBJ\n\t/AzquVTGpxYT4viCo6GvmZhvPLyveuz+0MKEjutuxCMGI07MoOLXQi5SDOoiYTrwLr\n\t1iCtaA0fY7D9Ge/vYkB/NeHKS5HZY6JQu4FQzDrXlbQNn8NGbclaMOLXkAY/4knXLU\n\tfJ0+MnDU3unjrgcjFzU9Gf3K9sc4B6vUusBiMFyFQC0USXfemfj0FwJLnDf7PIoS6s\n\taVw3xAWmJ0cjQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"E64hEIob\"; dkim-atps=neutral","Message-ID":"<4a7bb45555d19bcea4854ce6bfb0b938885f2f3e.camel@collabora.com>","To":"Kieran Bingham , Jaslo Ziska\n\t","Date":"Thu, 09 Nov 2023 10:07:38 -0500","In-Reply-To":"<169954233534.1233626.15655541530209584232@ping.linuxembedded.co.uk>","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>\n\t<878r779iy3.fsf@ziska.de>\n\t<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>\n\t<874jhv9f5k.fsf@ziska.de>\n\t<169954233534.1233626.15655541530209584232@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.4 (3.48.4-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28072,"web_url":"https://patchwork.libcamera.org/comment/28072/","msgid":"<5dd704d3aa1441dbc13421b8b20f1aab7b803475.camel@ndufresne.ca>","date":"2023-11-09T15:14:44","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Le jeudi 09 novembre 2023 à 13:34 +0100, Jaslo Ziska via libcamera-\ndevel a écrit :\n> Hi Barnabás,\n> \n> thanks for the suggestion! I tested it and it does work and makes \n> the code a lot more readable in my opinion.\n> \n> I just do not know how gstreamer- / glib-like the code is supposed \n> to stay as other elements for example use GMutex or GRecMutex \n> instead of the C++ native ones. Does anyone else have an opinion \n> on this?\n\nI've fine with using the C++ way.\n\nNicolas\n\n> \n> Regards,\n> \n> Jaslo\n> \n> Barnabás Pőcze writes:\n> \n> > Hi\n> > \n> > \n> > I am not too familiar with gstreamer, so I cannot comment on \n> > that part,\n> > but I believe using a single `std::atomic` object would be \n> > better. Please see below.\n> > \n> > \n> > 2023. november 6., hétfő 17:52 keltezéssel, Jaslo Ziska via \n> > libcamera-devel írta:\n> > \n> > > ---\n> > > Hi,\n> > > \n> > > I recently implemented basic EOS handling for the libcamerasrc \n> > > gstreamer element.\n> > > \n> > > I basically looked at how GstBaseSrc does it and tried to do it \n> > > similarly.\n> > > \n> > > I have no idea whether the locking is correct or if this is \n> > > thread safe but so far it worked without any issues for my \n> > > purposes.\n> > > \n> > > You can also now run the following command and receive a \n> > > working video file:\n> > > \n> > > gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! \n> > > h264parse ! mp4mux ! filesink location=test.mp4\n> > > \n> > > Looking forward for feedback!\n> > > \n> > > Cheers,\n> > > \n> > > Jaslo\n> > > \n> > > src/gstreamer/gstlibcamerasrc.cpp | 47 \n> > > +++++++++++++++++++++++++++++++\n> > > 1 file changed, 47 insertions(+)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n> > > b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 63d99571..a4681e1e 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> > > \tgchar *camera_name;\n> > > \tcontrols::AfModeEnum auto_focus_mode = \n> > > controls::AfModeManual;\n> > > \n> > > +\tGstEvent *pending_eos;\n> > > +\tint has_pending_eos;\n> > \n> > std::atomic instead of the above two?\n> > \n> > \n> > > +\n> > > \tGstLibcameraSrcState *state;\n> > > \tGstLibcameraAllocator *allocator;\n> > > \tGstFlowCombiner *flow_combiner;\n> > > @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer \n> > > user_data)\n> > > \n> > > \tbool doResume = false;\n> > > \n> > > +\tif (g_atomic_int_get(&self->has_pending_eos)) {\n> > > +\t\tg_atomic_int_set(&self->has_pending_eos, FALSE);\n> > \n> > Then you could just do\n> > \n> > if (auto *event = self->pending_eos.exchange(nullptr)) {\n> > ...\n> > }\n> > \n> > > +\n> > > +\t\tGST_OBJECT_LOCK(self);\n> > > +\t\tGstEvent *event = self->pending_eos;\n> > > +\t\tself->pending_eos = NULL;\n> > > +\t\tGST_OBJECT_UNLOCK(self);\n> > > +\n> > > +\t\tfor (GstPad *srcpad : state->srcpads_)\n> > > +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(event));\n> > > +\t\tgst_event_unref(event);\n> > > +\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > \t/*\n> > > \t * Create and queue one request. If no buffers are \n> > > available the\n> > > \t * function returns -ENOBUFS, which we ignore here as \n> > > that's not a\n> > > @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement \n> > > *element, GstStateChange transition)\n> > > \treturn ret;\n> > > }\n> > > \n> > > +static gboolean\n> > > +gst_libcamera_src_send_event(GstElement *element, GstEvent \n> > > *event)\n> > > +{\n> > > +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> > > +\tgboolean ret = FALSE;\n> > > +\n> > > +\tswitch (GST_EVENT_TYPE(event)) {\n> > > +\tcase GST_EVENT_EOS: {\n> > > +\t\tGST_OBJECT_LOCK(self);\n> > > +\t\tg_atomic_int_set(&self->has_pending_eos, TRUE);\n> > > +\t\tif (self->pending_eos)\n> > > +\t\t\tgst_event_unref(self->pending_eos);\n> > > +\t\tself->pending_eos = event;\n> > > +\t\tGST_OBJECT_UNLOCK(self);\n> > \n> > And you could do\n> > \n> > if (auto *old_event = self->pending_eos.exchange(event))\n> > gst_event_unref(old_event);\n> > \n> > \n> > > +\n> > > +\t\tret = TRUE;\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\tdefault:\n> > > +\t\tgst_event_unref(event);\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\n> > > +\treturn ret;\n> > > +}\n> > > +\n> > > static void\n> > > gst_libcamera_src_finalize(GObject *object)\n> > > {\n> > > @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc \n> > > *self)\n> > > \tstate->srcpads_.push_back(gst_pad_new_from_template(templ, \n> > > \"src\"));\n> > > \tgst_element_add_pad(GST_ELEMENT(self), \n> > > state->srcpads_.back());\n> > > \n> > > +\tGST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> > > +\n> > > \t/* C-style friend. */\n> > > \tstate->src_ = self;\n> > > \tself->state = state;\n> > > @@ -844,6 +890,7 @@ \n> > > gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> > > \telement_class->request_new_pad = \n> > > gst_libcamera_src_request_new_pad;\n> > > \telement_class->release_pad = \n> > > gst_libcamera_src_release_pad;\n> > > \telement_class->change_state = \n> > > gst_libcamera_src_change_state;\n> > > +\telement_class->send_event = gst_libcamera_src_send_event;\n> > > \n> > > \tgst_element_class_set_metadata(element_class,\n> > > \t\t\t\t \"libcamera Source\", \"Source/Video\",\n> > > --\n> > > 2.42.1\n> > \n> > \n> > Regards,\n> > Barnabás Pőcze","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 DC334BDE6B\n\tfor ;\n\tThu, 9 Nov 2023 15:14:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C421629BD;\n\tThu, 9 Nov 2023 16:14:48 +0100 (CET)","from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com\n\t[IPv6:2607:f8b0:4864:20::f2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A0D0E629AF\n\tfor ;\n\tThu, 9 Nov 2023 16:14:46 +0100 (CET)","by mail-qv1-xf2a.google.com with SMTP id\n\t6a1803df08f44-66fa16092c0so6491526d6.0\n\tfor ;\n\tThu, 09 Nov 2023 07:14:46 -0800 (PST)","from ?IPv6:2606:6d00:17:6dc0::c73? ([2606:6d00:17:6dc0::c73])\n\tby smtp.gmail.com with ESMTPSA id\n\tm12-20020ad44a0c000000b0065d051fc445sm2166647qvz.55.2023.11.09.07.14.45\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Nov 2023 07:14:45 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699542888;\n\tbh=aMDwOWX9h7Mh35xm8Hqhl/aPQ3FjmLIZF+MAfYZ+B7E=;\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=0XadXxDQTnIcMCk3M5qtZXmGqCyxD0S1TJm9GO5NxoysWab+gzx12PaFhpFkXCxtf\n\tzaI46ZsmwamrENN3nt6am4sPphDs6oEaParAn4zGEsF659/9JunBSL5rwfliErPAjc\n\tzQ5Fjozg1NU9iQK7ie1pOQxUFu7cO4l8Su8+FPOjG6eLMainYD0Tx2wb9QNrrEERuf\n\teshI6msB/tEX7eGRkvdaD4vcKqdmc6R0DtQxppFl3ReUeWVttLlz1qDLzgSQA8bmZl\n\tNINuB5M6rtY9NNRtH8F2nEmctoyDIjQClhDeN4F3X2M1w4IBKVUBKFxfFd9NBY+BwN\n\t97WfhsBmZziZw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1699542885;\n\tx=1700147685; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=mIauFsjCf3aFiOymXil0owjjjwqIL/Bji3GdBaCGLGg=;\n\tb=CYl8eERWEW2W/cnno897zyVtj8LYbq0tqtXDGpSy7E+Wsd9uNNmSzbXzso+HXiL5pC\n\tESsMRTU5VCpG7sXzhYJIQCPUKvkYdT3pA0JlwCdWlCP1848vOWOdcMe5CuCzZUAwcOjR\n\tWwofi87fA3zAkj2HIgsujsQ/AYYjRAjZDOA4uA7oQ9nYkom5LJW+aSGbN1bskJ2Mkrvc\n\t8o/xnUsx6v6h5ulDi8a5xU9fl8Z+ESKhljfKZDT4b1I0L/HyeoijTVDYDEk9gz/bFaI3\n\tOnKaGrBbb7sPItZZFiNSLBOOwqRCwkrGQkUbK7GRvMQa8Z37qoF46MzWJhVwEwjcGret\n\t9mvA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com header.b=\"CYl8eERW\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1699542885; x=1700147685;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=mIauFsjCf3aFiOymXil0owjjjwqIL/Bji3GdBaCGLGg=;\n\tb=g05gr+Kq5KZKUil66xtK7JXW1Yqm04urIBEvCuicvV9Qg5VwE6M+xcu25aSBmbjPgl\n\tmvo7ghAYzDnEEFV223R74++VbAJXvKJcXy9EHmDc3LBQvXothtCx93R2122NMRNv6Spq\n\tWd0rJkJNYZNDJjxZqOCv9b63IdkFK+juvKL8DPrMmZjkjY+iWFZXV2t62oz3L4llQ8cV\n\tc6fp3rVgaolvmXOFI+JQtf+KolZDLUIlYHChKpUAY3pDQd8NcCFcyS10h5JeWKXK5vL6\n\t8BMXhP+SfWh1EbemQRz3+vvnqGT2eW7baoo6heV65yAWCA8jSTO/5DZqdmcj4x0Q3KPA\n\tna3Q==","X-Gm-Message-State":"AOJu0Yxl0Tj7O+OWInpn6A+aKjdm30aUuCApK7zhpgedfbXmLRgjaFnL\n\tXytHZXuyI0LKkoo0gw/d9x3kn1b7pjkkGvZG7Atf0Q==","X-Google-Smtp-Source":"AGHT+IHO+oEaVm4f3EOZO4319hbz8DAHogxexnvVTvaaxE08+C69LQ9LVQF1ymq3a2qSAim7qJWvkw==","X-Received":"by 2002:a05:6214:d88:b0:670:18d6:6672 with SMTP id\n\te8-20020a0562140d8800b0067018d66672mr5236039qve.62.1699542885553; \n\tThu, 09 Nov 2023 07:14:45 -0800 (PST)","Message-ID":"<5dd704d3aa1441dbc13421b8b20f1aab7b803475.camel@ndufresne.ca>","To":"Jaslo Ziska , =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t","Date":"Thu, 09 Nov 2023 10:14:44 -0500","In-Reply-To":"<87cywj9jq6.fsf@ziska.de>","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t\n\t<87cywj9jq6.fsf@ziska.de>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.4 (3.48.4-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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@ndufresne.ca","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28073,"web_url":"https://patchwork.libcamera.org/comment/28073/","msgid":"","date":"2023-11-09T15:23:12","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Le jeudi 09 novembre 2023 à 13:40 +0000, Kieran Bingham via libcamera-\ndevel a écrit :\n> Quoting Jaslo Ziska via libcamera-devel (2023-11-09 12:51:15)\n> > Hi,\n> > \n> > Umang Jain writes:\n> > > Hi Jalso\n> > > \n> > > On 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n> > > > ---\n> > > > Hi,\n> > > > \n> > > > I recently implemented basic EOS handling for the libcamerasrc \n> > > > gstreamer element.\n> > > \n> > > Ah thanks, I remember we do track a bug report here:\n> > > https://bugs.libcamera.org/show_bug.cgi?id=91\n> > \n> > Ah yes, I remember that I stumbled upon this when I was first \n> > investigating this, I will update the bug report.\n> > \n> > > > \n> > > > I basically looked at how GstBaseSrc does it and tried to do it \n> > > > similarly.\n> > > > \n> > > > I have no idea whether the locking is correct or if this is \n> > > > thread safe but so far it worked without any issues for my \n> > > > purposes.\n> > > > \n> > > > You can also now run the following command and receive a \n> > > > working video file:\n> > > > \n> > > > gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! \n> > > > h264parse ! mp4mux ! filesink location=test.mp4\n> > > \n> > > Good to see that the diff is tested :-D\n> > \n> > Speaking of testing, I just ran the test suite and, except for the \n> > multi_stream_test (which was skipped) the other tests succeeded.\n> > \n> > > > \n> > > > Looking forward for feedback!\n> > > \n> > > It looks logical and on the right track to me atleast. But there \n> > > might be other implications regarding the threading and\n> > > locking mechanisms.\n> > \n> > Thanks! That is what I worry about too, maybe someone can review \n> > this.\n> > \n> > > > Cheers,\n> > > > \n> > > > Jaslo\n> > > > \n> > > > src/gstreamer/gstlibcamerasrc.cpp | 47 \n> > > > +++++++++++++++++++++++++++++++\n> > > > 1 file changed, 47 insertions(+)\n> > > > \n> > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n> > > > b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > index 63d99571..a4681e1e 100644\n> > > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> > > > gchar *camera_name;\n> > > > controls::AfModeEnum auto_focus_mode = \n> > > > controls::AfModeManual;\n> > > > \n> > > > + GstEvent *pending_eos;\n> > > > + int has_pending_eos;\n> > > > +\n> > > > GstLibcameraSrcState *state;\n> > > > GstLibcameraAllocator *allocator;\n> > > > GstFlowCombiner *flow_combiner;\n> > > > @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer \n> > > > user_data)\n> > > > \n> > > > bool doResume = false;\n> > > > \n> > > > + if (g_atomic_int_get(&self->has_pending_eos)) {\n> > > > + g_atomic_int_set(&self->has_pending_eos, FALSE);\n> > > > +\n> > > > + GST_OBJECT_LOCK(self);\n> > > > + GstEvent *event = self->pending_eos;\n> > > > + self->pending_eos = NULL;\n> > > > + GST_OBJECT_UNLOCK(self);\n> > > > +\n> > > > + for (GstPad *srcpad : state->srcpads_)\n> > > > + gst_pad_push_event(srcpad, gst_event_ref(event));\n> > > > + gst_event_unref(event);\n> > > > +\n> > > > + return;\n> > > > + }\n> > > > +\n> > > > /*\n> > > > * Create and queue one request. If no buffers are \n> > > > available the\n> > > > * function returns -ENOBUFS, which we ignore here as \n> > > > that's not a\n> > > > @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement \n> > > > *element, GstStateChange transition)\n> > > > return ret;\n> > > > }\n> > > > \n> > > > +static gboolean\n> > > > +gst_libcamera_src_send_event(GstElement *element, GstEvent \n> > > > *event)\n> > > > +{\n> > > > + GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> > > > + gboolean ret = FALSE;\n> > > > +\n> > > > + switch (GST_EVENT_TYPE(event)) {\n> > > > + case GST_EVENT_EOS: {\n> > > > + GST_OBJECT_LOCK(self);\n> > > > + g_atomic_int_set(&self->has_pending_eos, TRUE);\n> > > > + if (self->pending_eos)\n> > > > + gst_event_unref(self->pending_eos);\n> > > > + self->pending_eos = event;\n> > > > + GST_OBJECT_UNLOCK(self);\n> > > > +\n> > > > + ret = TRUE;\n> > > > + break;\n> > > > + }\n> > > > + default:\n> > > > + gst_event_unref(event);\n> > > > + break;\n> > > > + }\n> > > > +\n> > > > + return ret;\n> > > > +}\n> > > > +\n> > > > static void\n> > > > gst_libcamera_src_finalize(GObject *object)\n> > > > {\n> > > > @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc \n> > > > *self)\n> > > > state->srcpads_.push_back(gst_pad_new_from_template(templ, \n> > > > \"src\"));\n> > > > gst_element_add_pad(GST_ELEMENT(self), \n> > > > state->srcpads_.back());\n> > > > \n> > > > + GST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> > > > +\n> > > \n> > > I always thought libcamerasrc was inheriting from GstBaseSrc, \n> > > but in fact it's not.\n> > > So, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n> > \n> > This flag is actually required to receive the eos signal when it \n> > is send to the pipeline bin and not the element directly. Took me \n> > a while to figure out.\n> > \n> > > Apart from a missing commit mesasge, the patch looks good to me. \n> > > I'll put this on my radar soon for testing it\n> > > rigorously.\n> > \n> > Oh yeah, a commit message would indeed be nice, completely forgot \n> > that, thanks :)\n> \n> Can you also add/clarify /how/ the EOS event is being sent in the commit\n> message please?\n> \n> Is this issue occuring when you run the pipeline and press ctrl-c for\n> instance? or is there some other detail?\n\nThis patch covers cases where the application decides to stop the\npipeline by sending it an EOS. To CTRL+C combined with -e option of\ngst-launch-1.0 is a good example.\n\nThis patch does not cover (and this can be done later/seperatly):\n\n libcamerasrc ! identity eos-after=10 ! fakesink\n\nWhich is the case when downstream decides to stop. This case is a\nlittle more complex for multi-pad elements and is actually nice to\ntackle separately (its also more niche).\n\nNicolas\n\n> \n> --\n> Kieran\n> \n> \n> > \n> > > > /* C-style friend. */\n> > > > state->src_ = self;\n> > > > self->state = state;\n> > > > @@ -844,6 +890,7 @@ \n> > > > gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> > > > element_class->request_new_pad = \n> > > > gst_libcamera_src_request_new_pad;\n> > > > element_class->release_pad = \n> > > > gst_libcamera_src_release_pad;\n> > > > element_class->change_state = \n> > > > gst_libcamera_src_change_state;\n> > > > + element_class->send_event = gst_libcamera_src_send_event;\n> > > > \n> > > > gst_element_class_set_metadata(element_class,\n> > > > \"libcamera Source\", \"Source/Video\",\n> > > > --\n> > > > 2.42.1\n> > \n> > Regards,\n> > \n> > Jaslo","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 3C592C3284\n\tfor ;\n\tThu, 9 Nov 2023 15:23:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7AD60629BC;\n\tThu, 9 Nov 2023 16:23:15 +0100 (CET)","from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com\n\t[IPv6:2607:f8b0:4864:20::72c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73C9A629AD\n\tfor ;\n\tThu, 9 Nov 2023 16:23:14 +0100 (CET)","by mail-qk1-x72c.google.com with SMTP id\n\taf79cd13be357-777754138bdso64584285a.1\n\tfor ;\n\tThu, 09 Nov 2023 07:23:14 -0800 (PST)","from ?IPv6:2606:6d00:17:6dc0::c73? ([2606:6d00:17:6dc0::c73])\n\tby smtp.gmail.com with ESMTPSA id\n\tdy7-20020a05620a60c700b00770f2a690a8sm2080616qkb.53.2023.11.09.07.23.12\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Nov 2023 07:23:12 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699543395;\n\tbh=UOaNj1mI/wFrZejPsR7MUvmJuT0Eg+HubF2N1eAvY8g=;\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:\n\tFrom;\n\tb=hVDIJkMMNiJs5yG940tlNlxQU/h/RaNy8mKpSZhlyslu6iQ0yFS8Cx4Y5r7HCJ/zJ\n\tRAOJVPp4ofz0vj9KHqbhTNsm7WIlkhW0S66XkIzn65HoYOBbNz+4BeqIIJVtqQpo4h\n\t5sKnSqDQmGde5JaANyqDRfZYPfjVjNySRyzP91tvxTdXinhvO5s7clO2959ZLEVIVF\n\ts2YOYH6sznrmiXtoYwonDqTURNq/Gg2Zw36zS/h65VbjRHL/73l9gMN3Yi+89WjJyc\n\t1/EUrr7o6Tf8NB9yx/3QpI7v3riBWIhzs8RNtzSeucY0vvu6SZcUdKlirWMCx2rJDq\n\t/oNkeXW+6W7/g==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1699543393;\n\tx=1700148193; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=4nwQDEdOWZbxQXQ0uJmNKIBhUtrs2W08v6KemIxQz2s=;\n\tb=tZ//9BlSEEeHqY3tTSEkQIqii0UKnhQJA1bVNSJWjLnPlBwjbuygckgR0lBPBTiqWu\n\tIinHjYVCprFAo2rtIBR9h21JHwf71S4VKWBlcDpFV05C3W/SP+wgADf9V6ybMJOJ4MRF\n\t3slWXT3c5DzocMPw/b+xSCqaQIx89k0MBDuXWzKnftj3/aXBGHJcm7nREEHZcdawr/lf\n\t2tTGC9CsjkE1mcpPDwgQrOzQMhCerx26C5Jqxm3GKMbs+mGkTx23QQvVpDhSjBKOkVrm\n\tithmVHRN/dTzLvkwwzth8UDwfnf0BmNXLA3iDnSofu/rbn/hpRpm4Z9Z/qEdDsncTt9F\n\tf7Nw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com header.b=\"tZ//9BlS\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1699543393; x=1700148193;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=4nwQDEdOWZbxQXQ0uJmNKIBhUtrs2W08v6KemIxQz2s=;\n\tb=Wm8eQWeimCH6yQNuZ97i+37/tfM6STFApl+J2jjyctCJL1CjFIizuLAN8fViP8PJxA\n\tAjvxp7TBENHnQjPEPjO69LtwQfEPg0VyScAT8Bz9hjf1NMGH5fndCqnAgoVQx2ghnGri\n\tt/ONqpRcPVNXNEi3D2O+yMwJxjucfpJlv9xEFOb+1vPSVEEEl9QAAFUqWSJazC05N2QZ\n\twVN7L/SG0MDf0bwvD30LutZ3e0heH+J8kud0TQGtpRGtf4cF8QGZ+Y5ms1b1PRWEdq0n\n\tuwkiplrah0J5sB1GXFzDE0OQ7jUM1UZhs7mszlgGLTH5kuOXPClwbdjM2BUum3DX8uO5\n\tIMSA==","X-Gm-Message-State":"AOJu0YzvPmKb4dDHkUicmdT5neqolMG/4qZOYnj1zdE/3opUZ6DHRxNn\n\tBfJS8G0GULiuEloFCheA1+kaCA==","X-Google-Smtp-Source":"AGHT+IHoeCcobX6CVBNmrk7V4RNVGiPEMNWZ4VU8zRZF435LrlWqI85E3GwlisR1qkM/c77eMap9ZQ==","X-Received":"by 2002:a05:620a:8017:b0:774:1c2f:ac48 with SMTP id\n\tee23-20020a05620a801700b007741c2fac48mr6058352qkb.45.1699543393255; \n\tThu, 09 Nov 2023 07:23:13 -0800 (PST)","Message-ID":"","To":"Kieran Bingham , Jaslo Ziska\n\t, Jaslo Ziska via libcamera-devel\n\t, Umang Jain\n\t","Date":"Thu, 09 Nov 2023 10:23:12 -0500","In-Reply-To":"<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>\n\t<878r779iy3.fsf@ziska.de>\n\t<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.4 (3.48.4-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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@ndufresne.ca","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28074,"web_url":"https://patchwork.libcamera.org/comment/28074/","msgid":"<4b80af0d52d1f7970aa5b06e21d8bc0203045542.camel@ndufresne.ca>","date":"2023-11-09T16:21:15","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Jaslo,\n\nLe lundi 06 novembre 2023 à 17:52 +0100, Jaslo Ziska via libcamera-\ndevel a écrit :\n> ---\n> Hi,\n> \n> I recently implemented basic EOS handling for the libcamerasrc gstreamer element.\n> \n> I basically looked at how GstBaseSrc does it and tried to do it similarly.\n> \n> I have no idea whether the locking is correct or if this is thread safe but so far it worked without any issues for my purposes.\n> \n> You can also now run the following command and receive a working video file:\n> \n> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! h264parse ! mp4mux ! filesink location=test.mp4\n> \n> Looking forward for feedback!\n> \n> Cheers,\n> \n> Jaslo\n\nWith a proper commit message, and the C++ atomic changes suggested I\nthink this change is good and can be merged. Please include this in\nyour v2:\n\nAcked-by: Nicolas Dufresne \n\n> \n> src/gstreamer/gstlibcamerasrc.cpp | 47 +++++++++++++++++++++++++++++++\n> 1 file changed, 47 insertions(+)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 63d99571..a4681e1e 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n> \tgchar *camera_name;\n> \tcontrols::AfModeEnum auto_focus_mode = controls::AfModeManual;\n> \n> +\tGstEvent *pending_eos;\n> +\tint has_pending_eos;\n> +\n> \tGstLibcameraSrcState *state;\n> \tGstLibcameraAllocator *allocator;\n> \tGstFlowCombiner *flow_combiner;\n> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer user_data)\n> \n> \tbool doResume = false;\n> \n> +\tif (g_atomic_int_get(&self->has_pending_eos)) {\n> +\t\tg_atomic_int_set(&self->has_pending_eos, FALSE);\n> +\n> +\t\tGST_OBJECT_LOCK(self);\n> +\t\tGstEvent *event = self->pending_eos;\n> +\t\tself->pending_eos = NULL;\n> +\t\tGST_OBJECT_UNLOCK(self);\n> +\n> +\t\tfor (GstPad *srcpad : state->srcpads_)\n> +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(event));\n> +\t\tgst_event_unref(event);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> \t/*\n> \t * Create and queue one request. If no buffers are available the\n> \t * function returns -ENOBUFS, which we ignore here as that's not a\n> @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement *element, GstStateChange transition)\n> \treturn ret;\n> }\n> \n> +static gboolean\n> +gst_libcamera_src_send_event(GstElement *element, GstEvent *event)\n> +{\n> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> +\tgboolean ret = FALSE;\n> +\n> +\tswitch (GST_EVENT_TYPE(event)) {\n> +\tcase GST_EVENT_EOS: {\n> +\t\tGST_OBJECT_LOCK(self);\n> +\t\tg_atomic_int_set(&self->has_pending_eos, TRUE);\n> +\t\tif (self->pending_eos)\n> +\t\t\tgst_event_unref(self->pending_eos);\n> +\t\tself->pending_eos = event;\n> +\t\tGST_OBJECT_UNLOCK(self);\n> +\n> +\t\tret = TRUE;\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tgst_event_unref(event);\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> static void\n> gst_libcamera_src_finalize(GObject *object)\n> {\n> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc *self)\n> \tstate->srcpads_.push_back(gst_pad_new_from_template(templ, \"src\"));\n> \tgst_element_add_pad(GST_ELEMENT(self), state->srcpads_.back());\n> \n> +\tGST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n> +\n> \t/* C-style friend. */\n> \tstate->src_ = self;\n> \tself->state = state;\n> @@ -844,6 +890,7 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> \telement_class->request_new_pad = gst_libcamera_src_request_new_pad;\n> \telement_class->release_pad = gst_libcamera_src_release_pad;\n> \telement_class->change_state = gst_libcamera_src_change_state;\n> +\telement_class->send_event = gst_libcamera_src_send_event;\n> \n> \tgst_element_class_set_metadata(element_class,\n> \t\t\t\t \"libcamera Source\", \"Source/Video\",\n> --\n> 2.42.1","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 CFA72BDE6B\n\tfor ;\n\tThu, 9 Nov 2023 16:21:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 33289629BC;\n\tThu, 9 Nov 2023 17:21:19 +0100 (CET)","from mail-ua1-x92f.google.com (mail-ua1-x92f.google.com\n\t[IPv6:2607:f8b0:4864:20::92f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A5CE7629AD\n\tfor ;\n\tThu, 9 Nov 2023 17:21:17 +0100 (CET)","by mail-ua1-x92f.google.com with SMTP id\n\ta1e0cc1a2514c-7b9ba0c07f9so376079241.3\n\tfor ;\n\tThu, 09 Nov 2023 08:21:17 -0800 (PST)","from ?IPv6:2606:6d00:17:6dc0::c73? ([2606:6d00:17:6dc0::c73])\n\tby smtp.gmail.com with ESMTPSA id\n\tn3-20020a0cbe83000000b0066d0621bb67sm2195484qvi.114.2023.11.09.08.21.15\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Nov 2023 08:21:16 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699546879;\n\tbh=yf+vRjzCx5dHdIEcxuH75XfhSiDQUxD36AutmfvUARM=;\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:\n\tFrom;\n\tb=n1rX4nED8rqkZJDDTXzW31/lhKJU32bw/rQjtsUi/9VH7pMeiAFdO0nu0CVzcxBPp\n\tPwwtHvcElma/1I6I3OHwHOiKWo7GVklo7tX2smW3qIQCMbUuInDTIDoneOW3zk11Rc\n\tZNOsRTrwg4gZFKsonLz5pP0WP6ql/u1u12/ZpzYZ26OZ16K6FezMrFUGnArj/TDDvl\n\t1KuLsPGcpkXdFyfc8a61Mm9GLP9kdUqfOW2Ir+Pt9WgEclZlVGgt5nwk3fXj9ZsMbL\n\t3syPKJHHim8g4REwjWWRFBvzxpgS4DrKo2OHlfR4MMnrgeMhhyVSn7pGcx8ELmA14J\n\tr+M6PitpVZwQg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1699546876;\n\tx=1700151676; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=jr3BUD75eox5MLDEZ+X9hkecPuf/jkoHfF7poLTul4A=;\n\tb=BfQceCc/TU71g8BtaAKNa2DMEdam+jjf7+kRgbdvRqUeIuKuXQXTXeuxViTmMxVv+Y\n\tBD+Rmngt1KP0Wtuil4sU9DnISptDTlVBfithijysqwjBweREcfDAdq5Xt1g6Fat0dLCu\n\toMPCfmqsfrC1sRM79NlT6uyVzAA+JQhuK2TknsLjODxWGzAWvy6rhbn1tEeaOf0ee6Jw\n\tirgR8I+CmSidmo568cL3vgC8LHBi3QcJ8x45vpuUtD32S4wFFAxhBKiEezT5fG5Ry6J1\n\tj2z8vgowF4HEM/2DYUgJNUCccZKBr16/2+eCdf6hxGDH+eMHbgGkSWr0trWypedsi1o3\n\t8bsw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com header.b=\"BfQceCc/\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1699546876; x=1700151676;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=jr3BUD75eox5MLDEZ+X9hkecPuf/jkoHfF7poLTul4A=;\n\tb=XCuxW/egEIfalINGRQL2WE7SotR4R7/IIG8ex/IvduKYMCafh6uoXi/ClL+tOPovjk\n\toXSIChqZbK11sLKg4knDBxWQ92ZWvOO5z01FDBdc9pJJcNebTbQa76LpYgJk0cXQfCoz\n\t0YzuI+UoJQyueadomQvX5VBmYah/QHFH9sjsTNDLZSrbS3IvWYSamXK4Fs0TMxhqk7GN\n\tPidwr90l95FgH7pWX9fYHv4kWJYz5BbWENnHCVab34JoXhxQIIz1G2+1728LDnAu07GC\n\tHHPJeUY9iaUQFMCxZpC/CZVaRuDbsXrOgX7rURQ+xSr8ZkbqrDAPsbdowNiT6fW1DC5w\n\t5oRw==","X-Gm-Message-State":"AOJu0YyOQ35Yv23B++mPEXkrea25Qn6ezooKckTMk79QyX+Bkl/7rd8D\n\t9abf5SfGYEZ3bK9QMl/89Psw4ZzTtW5fD4ulMjgUqQ==","X-Google-Smtp-Source":"AGHT+IEodI12q78U3gGvFEU3lIhkd41InySb7DbxHClNzJUUV0Sxc65cljMl0uM9Kd8oNgJGzbrIqw==","X-Received":"by 2002:a67:e14b:0:b0:45d:c4d1:5bd8 with SMTP id\n\to11-20020a67e14b000000b0045dc4d15bd8mr4486165vsl.0.1699546876383; \n\tThu, 09 Nov 2023 08:21:16 -0800 (PST)","Message-ID":"<4b80af0d52d1f7970aa5b06e21d8bc0203045542.camel@ndufresne.ca>","To":"Jaslo Ziska , libcamera-devel@lists.libcamera.org","Date":"Thu, 09 Nov 2023 11:21:15 -0500","In-Reply-To":"<20231106165545.47116-1-jaslo@ziska.de>","References":"<20231106165545.47116-1-jaslo@ziska.de>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.4 (3.48.4-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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@ndufresne.ca","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28075,"web_url":"https://patchwork.libcamera.org/comment/28075/","msgid":"<87v8a927n9.fsf@ziska.de>","date":"2023-11-10T11:00:09","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi Nicolas,\n\nnicolas@ndufresne.ca writes:\n> Le jeudi 09 novembre 2023 à 13:40 +0000, Kieran Bingham via \n> libcamera-\n> devel a écrit :\n>> Quoting Jaslo Ziska via libcamera-devel (2023-11-09 12:51:15)\n>> > Hi,\n>> >\n>> > Umang Jain writes:\n>> > > Hi Jalso\n>> > >\n>> > > On 11/6/23 10:22 PM, Jaslo Ziska via libcamera-devel wrote:\n>> > > > ---\n>> > > > Hi,\n>> > > >\n>> > > > I recently implemented basic EOS handling for the \n>> > > > libcamerasrc\n>> > > > gstreamer element.\n>> > >\n>> > > Ah thanks, I remember we do track a bug report here:\n>> > > https://bugs.libcamera.org/show_bug.cgi?id=91\n>> >\n>> > Ah yes, I remember that I stumbled upon this when I was first\n>> > investigating this, I will update the bug report.\n>> >\n>> > > >\n>> > > > I basically looked at how GstBaseSrc does it and tried to \n>> > > > do it\n>> > > > similarly.\n>> > > >\n>> > > > I have no idea whether the locking is correct or if this \n>> > > > is\n>> > > > thread safe but so far it worked without any issues for \n>> > > > my\n>> > > > purposes.\n>> > > >\n>> > > > You can also now run the following command and receive a\n>> > > > working video file:\n>> > > >\n>> > > > gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc !\n>> > > > h264parse ! mp4mux ! filesink location=test.mp4\n>> > >\n>> > > Good to see that the diff is tested :-D\n>> >\n>> > Speaking of testing, I just ran the test suite and, except \n>> > for the\n>> > multi_stream_test (which was skipped) the other tests \n>> > succeeded.\n>> >\n>> > > >\n>> > > > Looking forward for feedback!\n>> > >\n>> > > It looks logical and on the right track to me atleast. But \n>> > > there\n>> > > might be other implications regarding the threading and\n>> > > locking mechanisms.\n>> >\n>> > Thanks! That is what I worry about too, maybe someone can \n>> > review\n>> > this.\n>> >\n>> > > > Cheers,\n>> > > >\n>> > > > Jaslo\n>> > > >\n>> > > > src/gstreamer/gstlibcamerasrc.cpp | 47\n>> > > > +++++++++++++++++++++++++++++++\n>> > > > 1 file changed, 47 insertions(+)\n>> > > >\n>> > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>> > > > b/src/gstreamer/gstlibcamerasrc.cpp\n>> > > > index 63d99571..a4681e1e 100644\n>> > > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> > > > @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {\n>> > > > gchar *camera_name;\n>> > > > controls::AfModeEnum auto_focus_mode =\n>> > > > controls::AfModeManual;\n>> > > >\n>> > > > + GstEvent *pending_eos;\n>> > > > + int has_pending_eos;\n>> > > > +\n>> > > > GstLibcameraSrcState *state;\n>> > > > GstLibcameraAllocator *allocator;\n>> > > > GstFlowCombiner *flow_combiner;\n>> > > > @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer\n>> > > > user_data)\n>> > > >\n>> > > > bool doResume = false;\n>> > > >\n>> > > > + if (g_atomic_int_get(&self->has_pending_eos)) {\n>> > > > + g_atomic_int_set(&self->has_pending_eos, \n>> > > > FALSE);\n>> > > > +\n>> > > > + GST_OBJECT_LOCK(self);\n>> > > > + GstEvent *event = self->pending_eos;\n>> > > > + self->pending_eos = NULL;\n>> > > > + GST_OBJECT_UNLOCK(self);\n>> > > > +\n>> > > > + for (GstPad *srcpad : state->srcpads_)\n>> > > > + gst_pad_push_event(srcpad, \n>> > > > gst_event_ref(event));\n>> > > > + gst_event_unref(event);\n>> > > > +\n>> > > > + return;\n>> > > > + }\n>> > > > +\n>> > > > /*\n>> > > > * Create and queue one request. If no buffers are\n>> > > > available the\n>> > > > * function returns -ENOBUFS, which we ignore here \n>> > > > as\n>> > > > that's not a\n>> > > > @@ -747,6 +765,32 @@ \n>> > > > gst_libcamera_src_change_state(GstElement\n>> > > > *element, GstStateChange transition)\n>> > > > return ret;\n>> > > > }\n>> > > >\n>> > > > +static gboolean\n>> > > > +gst_libcamera_src_send_event(GstElement *element, \n>> > > > GstEvent\n>> > > > *event)\n>> > > > +{\n>> > > > + GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n>> > > > + gboolean ret = FALSE;\n>> > > > +\n>> > > > + switch (GST_EVENT_TYPE(event)) {\n>> > > > + case GST_EVENT_EOS: {\n>> > > > + GST_OBJECT_LOCK(self);\n>> > > > + g_atomic_int_set(&self->has_pending_eos, \n>> > > > TRUE);\n>> > > > + if (self->pending_eos)\n>> > > > + gst_event_unref(self->pending_eos);\n>> > > > + self->pending_eos = event;\n>> > > > + GST_OBJECT_UNLOCK(self);\n>> > > > +\n>> > > > + ret = TRUE;\n>> > > > + break;\n>> > > > + }\n>> > > > + default:\n>> > > > + gst_event_unref(event);\n>> > > > + break;\n>> > > > + }\n>> > > > +\n>> > > > + return ret;\n>> > > > +}\n>> > > > +\n>> > > > static void\n>> > > > gst_libcamera_src_finalize(GObject *object)\n>> > > > {\n>> > > > @@ -779,6 +823,8 @@ \n>> > > > gst_libcamera_src_init(GstLibcameraSrc\n>> > > > *self)\n>> > > > state->srcpads_.push_back(gst_pad_new_from_template(templ,\n>> > > > \"src\"));\n>> > > > gst_element_add_pad(GST_ELEMENT(self),\n>> > > > state->srcpads_.back());\n>> > > >\n>> > > > + GST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);\n>> > > > +\n>> > >\n>> > > I always thought libcamerasrc was inheriting from \n>> > > GstBaseSrc,\n>> > > but in fact it's not.\n>> > > So, setting the GST_ELEMENT_FLAG_SOURCE makes sense here.\n>> >\n>> > This flag is actually required to receive the eos signal when \n>> > it\n>> > is send to the pipeline bin and not the element directly. \n>> > Took me\n>> > a while to figure out.\n>> >\n>> > > Apart from a missing commit mesasge, the patch looks good \n>> > > to me.\n>> > > I'll put this on my radar soon for testing it\n>> > > rigorously.\n>> >\n>> > Oh yeah, a commit message would indeed be nice, completely \n>> > forgot\n>> > that, thanks :)\n>>\n>> Can you also add/clarify /how/ the EOS event is being sent in \n>> the commit\n>> message please?\n>>\n>> Is this issue occuring when you run the pipeline and press \n>> ctrl-c for\n>> instance? or is there some other detail?\n>\n> This patch covers cases where the application decides to stop \n> the\n> pipeline by sending it an EOS. To CTRL+C combined with -e option \n> of\n> gst-launch-1.0 is a good example.\n>\n> This patch does not cover (and this can be done \n> later/seperatly):\n>\n> libcamerasrc ! identity eos-after=10 ! fakesink\n>\n> Which is the case when downstream decides to stop. This case is \n> a\n> little more complex for multi-pad elements and is actually nice \n> to\n> tackle separately (its also more niche).\n\nIsn't this already implemented when checking for GST_FLOW_EOS in \nGstLibcameraSrcState::processRequest()? Or is this something else?\n\nRegards,\n\nJaslo\n\n>\n> Nicolas\n>\n>>\n>> --\n>> Kieran\n>>\n>>\n>> >\n>> > > > /* C-style friend. */\n>> > > > state->src_ = self;\n>> > > > self->state = state;\n>> > > > @@ -844,6 +890,7 @@\n>> > > > gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>> > > > element_class->request_new_pad =\n>> > > > gst_libcamera_src_request_new_pad;\n>> > > > element_class->release_pad =\n>> > > > gst_libcamera_src_release_pad;\n>> > > > element_class->change_state =\n>> > > > gst_libcamera_src_change_state;\n>> > > > + element_class->send_event = \n>> > > > gst_libcamera_src_send_event;\n>> > > >\n>> > > > gst_element_class_set_metadata(element_class,\n>> > > > \"libcamera Source\", \n>> > > > \"Source/Video\",\n>> > > > --\n>> > > > 2.42.1\n>> >\n>> > Regards,\n>> >\n>> > Jaslo","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 1B4A5C3284\n\tfor ;\n\tFri, 10 Nov 2023 11:03:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D6D5629AB;\n\tFri, 10 Nov 2023 12:03:08 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.162])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A3999629AB\n\tfor ;\n\tFri, 10 Nov 2023 12:03:06 +0100 (CET)","from archlinux by smtp.strato.de (RZmta 49.9.1 AUTH)\n\twith ESMTPSA id 6bb150zAAB358cz\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tFri, 10 Nov 2023 12:03:05 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699614188;\n\tbh=1gv7Lwjb938UzEVxyIKfJa9Z59sj8LHMhxJJmVcS30w=;\n\th=References:To:Date: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=swPtblzR/qasM3WkwUKPTY5wH5JMAXXM1FNFw4hK/NNdUFmqjLoqr7ykZuw2ZN3kb\n\thi0GtkuSWgZUqvoMAgdf8xf4TLS4sszipqduHQXClpeVkyBfmR90tFzGnJGlzn8alC\n\tikoy9rKxoJy6MigKVCZ3R47W/JhCQlFPYdTX3gVap2TZXRXZCn0c8g7rsQef8vseda\n\tchEO6tOyDRkma6R16GVR0yg9wFfAshhJI2hv5WW7PAXiaWXxwAsyXxqO6bEVNd6mfT\n\t9Q/TcUmV6pPdnxzJmtQyR8a4NOOrg68pxePrg/iNJdVL57mFL+5vbfhCprA6lVLKQ8\n\t9aK6xkBggMiIQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1699614185;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=cn9/Q4b6JQ4K8GiIhZxV/FqbKAlqZX3GbVAt1qk+UIY=;\n\tb=quS/GKC44PbBjBDbQzGAVl5KF7pejkBKvLGt9qFOvRC0wfc6aBDSLeWnOSAmTOroZ6\n\teGINi8vxOD9f3fVbBHiKd1O9fnVobtp2r1ney8LHBe4aAEKD+KktKBLL9EuBIFrDBjob\n\tUt6U/BIJQr4D4QRuwjHlrQMpUkqSDAk9vKqwDmAOMxe5op4DrhOHCDzrO7E0El0stCqG\n\tSteG1iZK3x/HSpBwQA5fA+n0yL+gTOPgAyI8TKmjLIPdQO4jOi7QzKJa/nqpWlOM9ERk\n\t0ofW8cVzvIuSHEW0Dau2eY00IJ8VJCPwJiVH+Dxx6/I5tqQMW/UOdyb+IygtCcbvahAf\n\tBb1Q==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1699614185;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=cn9/Q4b6JQ4K8GiIhZxV/FqbKAlqZX3GbVAt1qk+UIY=;\n\tb=KxSUoJN/SkO+UwACpmzHPs5h1Cw289gxC2xyy1rfNLT6LaHpbhwAjrA1H+KAaRmqZ2\n\t1Sk4NywuyI7funcyNlCQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"quS/GKC4\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"KxSUoJN/\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1699614185; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=j+7K39zvFERTnzdpD/AEcJXOWJNA/mlyoxckYlneyUDsYaOg9I4ml+4ftwyOfPieGX\n\tvuqjfoZWU1FhlHhvhhLjXWC3P4Mhb+PBiIgYnontF8rb23a2oS/n9s5UcMcMIYf1l3cR\n\tSJiLcflkmJvqBI+bGnPgQzGUGLtRHcxpfEjwo7UjvO0SfVe6zY6MAw7xV8091k0zCLw3\n\tFnIK2YM0ZxrYGIaKb1k8/m2Ez8rpfCmvT5b3VxyJ0U/QlXEnR3gS6N9z1YM/9vZaIxYG\n\tt5U/m+xSRt2n9olS0s1kh/hT8mHgHQ5jE96n14drTTzWTwfOpIj+MJazok/Udbvx6k+x\n\tbGnQ==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1699614185;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=cn9/Q4b6JQ4K8GiIhZxV/FqbKAlqZX3GbVAt1qk+UIY=;\n\tb=YgkxOMIux8Yq2AWA9Ym39D26ySNzisWyC1T0TiP45MmM0etjEQwxR0MEpFMQ24nBoc\n\tZkS8vmHkqMh0nhfWkFWr70228yt4bhj/n1k8kvFFwp5HNh7JjC+gAuT+enUIf3J1F4hI\n\t5j9pkqO0K7S0zEXoCeNY4L91hLYlTtPpi1GXi7jwBM/LtvHMYU/xy9ajB9mUlbpaNFgC\n\tg68SXXXKH6l0ZYy8wHgduUaUqy75d61BqC/WjGgL1cmxibk7eF+d5QNAbSb6pZpQhq/R\n\t+n+tSokXR98tCL09ECJ0Dskb+e438Iq8Ts1IIYfSWyNK+5V8fiBvO8KFlrOy+8DANlEE\n\tK7ng==","ARC-Authentication-Results":"i=1; strato.com;\n arc=none;\n dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusQvJpnsql/jCNNVfDSgNWRnQi1f6u5kKcI94x51NJzihdQoifxxArMIehopBXtTqsh\"","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>\n\t<878r779iy3.fsf@ziska.de>\n\t<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>\n\t","User-agent":"mu4e 1.10.7; emacs 29.1","To":"nicolas@ndufresne.ca","Date":"Fri, 10 Nov 2023 12:00:09 +0100","In-reply-to":"","Message-ID":"<87v8a927n9.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"Jaslo Ziska via libcamera-devel ","Reply-To":"Jaslo Ziska ","Cc":"Jaslo Ziska via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28084,"web_url":"https://patchwork.libcamera.org/comment/28084/","msgid":"<58d5900846888febf94f885c780ad40396e446e5.camel@ndufresne.ca>","date":"2023-11-12T23:56:09","subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hi Jaslo,\n\nLe vendredi 10 novembre 2023 à 12:00 +0100, Jaslo Ziska a écrit :\n> > \n> > This patch does not cover (and this can be done \n> > later/seperatly):\n> > \n> >    libcamerasrc ! identity eos-after=10 ! fakesink\n> > \n> > Which is the case when downstream decides to stop. This case is \n> > a\n> > little more complex for multi-pad elements and is actually nice \n> > to\n> > tackle separately (its also more niche).\n> \n> Isn't this already implemented when checking for GST_FLOW_EOS in \n> GstLibcameraSrcState::processRequest()? Or is this something else?\n\nYou are correct, my bad. I didn't realize we had some handling. I'm not sure its\ndone properly though, since it seems pretty unfortunate that one FLOW_EOS on one\nstreams causes all streams to abort. I'd need to check demuxers, tee, rtspsrc\nand other implementation to understand what is expected.\n\n> \n> Regards,\n> \n> Jaslo\n\nNicolas","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 E1703C3213\n\tfor ;\n\tSun, 12 Nov 2023 23:56:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4CE9A629BA;\n\tMon, 13 Nov 2023 00:56:13 +0100 (CET)","from mail-qt1-x830.google.com (mail-qt1-x830.google.com\n\t[IPv6:2607:f8b0:4864:20::830])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D2C0761DB8\n\tfor ;\n\tMon, 13 Nov 2023 00:56:11 +0100 (CET)","by mail-qt1-x830.google.com with SMTP id\n\td75a77b69052e-41cc535cd5cso23306121cf.2\n\tfor ;\n\tSun, 12 Nov 2023 15:56:11 -0800 (PST)","from nicolas-tpx395.localdomain ([2606:6d00:17:6dc0::7a9])\n\tby smtp.gmail.com with ESMTPSA id\n\tk26-20020ac8605a000000b004182037f655sm1508730qtm.14.2023.11.12.15.56.09\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 12 Nov 2023 15:56:10 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699833373;\n\tbh=imGwSZbXtPi5QwDjIo46jz72S9opK4nOMEjL4wEN4hk=;\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=hGI6vPTj/AtBYnlUj8bq9S7QuH/9IJGo1HYjOzQ9fbP/N+9FjqpU7XEeOq9wfhYwG\n\teswdM1Ek4bsWsWloiGSZgg8d9kpRxIV1Ov0vGcjWYgnlI7Lf8goYgp6+xCo+eE2D8C\n\tyTHL4IAOaOvF44BprIMN2A1DvkXN+eTPp5HAOG8eczblCloubQxP/+eoM+4stynpki\n\t9sNnNj/OAFrEnn3o6Ugy6+is7t8HqJ7IF3nSEyx/vTQ/JVBrEkM1a70LscABDRdxQH\n\tTHj9V3Elu5OexgvU2puRKR2JCOWfDmbXktCC7nYYNUX3tiIXGiX+Hez/DfZEwS+e5r\n\trTmUvl3ZM4jqQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1699833370;\n\tx=1700438170; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=imGwSZbXtPi5QwDjIo46jz72S9opK4nOMEjL4wEN4hk=;\n\tb=pAs5B/Dmj15YRm+AYr9KNC8xgPANtuD2zgZy9vkVDIs6G6Je8rysGDJgDGJEpDBOno\n\t0VbbD8Br0A/Zn4MBNXasyKDltlpvn19TUBZc6lsA6mn1OGJvRw/hFT2VWWxYFkUrdMFc\n\tP25cUCYNJfl2zoP0TkbR3U/i871spGrnH+tl6p7yN6xYLMyXaLJlaIVWP6jowjrjqn6A\n\thMBn24rW5O7vAK6BJWuMnyzX3FrtSrVstuzx3ckolAZGn8AM0HeeYG3Nl59Y8TWlw2lq\n\tNnc5zBSWoHMTzRepPfoJ1WNosGJr/6u+6bdFGsc9v21NLWm1jd37rzkKqro2QZk9wY7b\n\tphZQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com header.b=\"pAs5B/Dm\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1699833370; x=1700438170;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=imGwSZbXtPi5QwDjIo46jz72S9opK4nOMEjL4wEN4hk=;\n\tb=NIq9Rywl02B9Q8NUpAdDimM3HVJ85QSSfykzW3ZZbdwyz0TIZBOofFWLY4WLYESNNY\n\tvwT1afvL63X2yMygL0S/S3eJ45T/QA5RYE+hjr77mLsU9dR0B7aRo5A1TL+xNwNMsh9T\n\trsuKc6DFqbahumYsuNgaq9BrnEDjV1mXbhkeqFQ7SvE358dVy4y71bq5dpRAfGU4kTAd\n\tSuOTSm01EvQVSpv6AnvmSmahzvhrinqGgHqDekVmELaRkmifSd/tV6tDdHDPF012lFbR\n\tmZU5ApNJ8ocNfeeoQrL8eyH7Fv8/i8pFaM65MGQVyHA6y4Q9W092r3LYseogxgTQFUDD\n\tF49A==","X-Gm-Message-State":"AOJu0YxuddWGSj3Cs52uMO/vR5af4jKdX0IszO2LMmIZDDxFMOSlcFAu\n\tK5wY0tS4qChpDJO19TYZ50Y/KUiY+zW/Sn/a+dGu5A==","X-Google-Smtp-Source":"AGHT+IGCYeG0QsqJK83LSrwxTTW3XLSHQR/jUWQHi2MhIfcoMWvh8RveyU672gX0DlyQpYxfITRSyg==","X-Received":"by 2002:a05:622a:1307:b0:41e:29ac:1300 with SMTP id\n\tv7-20020a05622a130700b0041e29ac1300mr7587326qtk.27.1699833370639; \n\tSun, 12 Nov 2023 15:56:10 -0800 (PST)","Message-ID":"<58d5900846888febf94f885c780ad40396e446e5.camel@ndufresne.ca>","To":"Jaslo Ziska ","Date":"Sun, 12 Nov 2023 18:56:09 -0500","In-Reply-To":"<87v8a927n9.fsf@ziska.de>","References":"<20231106165545.47116-1-jaslo@ziska.de>\n\t<841eb9cc-b2e1-1abd-8012-683cb941bc8e@ideasonboard.com>\n\t<878r779iy3.fsf@ziska.de>\n\t<169953725806.1233626.16677842308748774097@ping.linuxembedded.co.uk>\n\t\n\t<87v8a927n9.fsf@ziska.de>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.4 (3.48.4-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] gstreamer: Implement EOS handling","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":"Jaslo Ziska via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]