After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755732 - rtmpsrc: Slow teardown of connection when pipeline is interrupted
rtmpsrc: Slow teardown of connection when pipeline is interrupted
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-09-28 13:07 UTC by John Slade
Modified: 2018-11-03 13:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to use GstPoll on sockets (4.35 KB, patch)
2015-09-28 13:14 UTC, John Slade
none Details | Review
Fix indentation in gstrtmpsrc.c with gst-indent (903 bytes, patch)
2015-09-28 13:14 UTC, John Slade
committed Details | Review
Patch to use GstPoll on sockets (4.39 KB, patch)
2015-10-12 11:52 UTC, John Slade
needs-work Details | Review

Description John Slade 2015-09-28 13:07:15 UTC
The RTMP plugin can be slow to stop when interrupted (with SIGINT) as it waits for the network socket to timeout before stopping.  librtmp has this timeout set by default at 30 seconds.

Here is an example output with timestamps:

Sep 25 16:38:13 Setting pipeline to PAUSED ...
Sep 25 16:38:13 Pipeline is PREROLLING ...
Sep 25 16:38:15 Redistribute latency...
Sep 25 16:38:15 Pipeline is PREROLLED ...
Sep 25 16:38:15 Setting pipeline to PLAYING ...
Sep 25 16:38:15 New clock: GstSystemClock
Sep 25 16:38:16 handling interrupt.             <-------------- interrupt with SIGINT
Sep 25 16:38:16 Interrupt: Stopping pipeline ...
Sep 25 16:38:16 Execution ended after 0:00:00.951296656
Sep 25 16:38:16 Setting pipeline to PAUSED ...
Sep 25 16:38:16 Setting pipeline to READY ...
Sep 25 16:38:46 Setting pipeline to NULL ...    <-------------- 30 seconds delay 
Sep 25 16:38:46 Freeing pipeline ...

The log shows going from READY->NULL took 30 seconds.
Comment 1 John Slade 2015-09-28 13:14:08 UTC
Created attachment 312302 [details] [review]
Patch to use GstPoll on sockets

I have created a patch (0002-rtmpsrc-Poll-changes-on-RTMP-socket.patch) for this which uses GstPoll so that we don't block the plugin inside librtmp.  Using GstPoll means we can block inside gst_rtmp_src_create() which gives us control to unblock quickly when unlocking the plugin.

I think this might also help with https://bugzilla.gnome.org/show_bug.cgi?id=729099 and unsafe multi-threading of librtmp.

Note there is an existing indentation problem on line 437 of gstrtmpsrc.c that   gst-indent fixes.  I've got this in a separate patch (0001-rtmpsrc-Fix-indentation-with-gst-indent.patch).
Comment 2 John Slade 2015-09-28 13:14:56 UTC
Created attachment 312303 [details] [review]
Fix indentation in gstrtmpsrc.c with gst-indent
Comment 3 Sebastian Dröge (slomo) 2015-10-02 12:17:23 UTC
Review of attachment 312302 [details] [review]:

::: ext/rtmp/gstrtmpsrc.c
@@ +343,3 @@
+     * to timeout on its recv call. */
+    poll_state = gst_poll_wait (src->poll, GST_CLOCK_TIME_NONE);
+    if ((poll_state == -1) && (errno == EBUSY)) {

The extra parenthesis are not needed here :)

@@ +661,2 @@
   if (src->rtmp) {
+    gst_poll_set_flushing (src->poll, FALSE);

Shouldn't this be flushing in stop(), and non-flushing in start()?
Comment 4 John Slade 2015-10-09 14:52:20 UTC
>@@ +661,2 @@
>   if (src->rtmp) {
>+    gst_poll_set_flushing (src->poll, FALSE);
>
> Shouldn't this be flushing in stop(), and non-flushing in start()?

My assumption was that create and stop are in the same thread so we can't simultaneously by blocked in create and in stop.  Where as unlock was a separate thread (and can flush the polling).  Is this correct view of how it works?

I put this non-flushing in stop() to just reset the poll to a good state as this is where we clear up src->rtmp and close any sockets.  I could also do the same in start() to ensure the the poll is non-flushing before we start to use it.
Comment 5 Sebastian Dröge (slomo) 2015-10-11 09:55:04 UTC
create() and stop() are not in the same thread
Comment 6 John Slade 2015-10-12 11:52:00 UTC
Created attachment 313114 [details] [review]
Patch to use GstPoll on sockets

Made updates to patch as suggested:
* Poll is flushed in stop and non-flushing in start.
* Removed extra parenthesis.
Comment 7 Sebastian Dröge (slomo) 2015-10-12 12:31:36 UTC
Comment on attachment 313114 [details] [review]
Patch to use GstPoll on sockets

Now only misses the set_flushing(FALSE) in unlock_stop().

The other question is, if RTMP_Read() does anything else than reading... like writing to the socket for keep-alive or similar things if no data is available for reading.
Comment 8 Sebastian Dröge (slomo) 2015-10-24 12:11:19 UTC
John, are you going to update the patch?
Comment 9 GStreamer system administrator 2018-11-03 13:40:56 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/306.