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 748360 - rtspsrc: teardown usually never happens
rtspsrc: teardown usually never happens
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.8.2
Other Linux
: Normal normal
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
https://bugzilla.gnome.org/show_bug.c...
: 750387 756441 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-04-23 10:50 UTC by Kseniya Vasilchuk
Modified: 2017-07-14 20:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
send teardown request patch (2.13 KB, patch)
2016-09-06 15:18 UTC, Kseniya Vasilchuk
none Details | Review
refactoring (1.72 KB, patch)
2016-09-07 13:26 UTC, Kseniya Vasilchuk
committed Details | Review

Description Kseniya Vasilchuk 2015-04-23 10:50:36 UTC
https://bugzilla.gnome.org/show_bug.cgi?id=739265

I had the same situation with TEARDOWN like Aleix with bug in URL above so I tried using progress messages. When EOS happens, I change pipeline state to STATE_PAUSE and wait for progress messages (this works fine), after I get GST_PROGRESS_TYPE_COMPLETE I set pipeline to STATE_READY to close connection and wait for progress messages again, but this time I get no PROGRESS_MESSAGE_COMPLETE.

When a state changes from PAUSE to READY I expect that CMD_CLOSE will be sent and other commands will be canceled. But after gst_rtspsrc_thread executes CMD_PAUSE, it changes current command to CMD_LOOP, so it will never be interrupted and CMD_CLOSE will never be executed because CMD_CLOSE can only interrupt CMD_PAUSE but not CMD_LOOP.

Aleix's patch (even without GST_RTSP_STREAM_LOCK/UNLOCK) solves this problem for me so I think it should be submitted.
Comment 1 Sebastian Dröge (slomo) 2015-07-06 07:20:38 UTC
*** Bug 750387 has been marked as a duplicate of this bug. ***
Comment 2 Sebastian Dröge (slomo) 2015-11-05 10:17:38 UTC
*** Bug 756441 has been marked as a duplicate of this bug. ***
Comment 3 Kseniya Vasilchuk 2016-09-06 15:18:03 UTC
Created attachment 334916 [details] [review]
send teardown request patch

Still actual for 1.8.2.

I've attached Aleix Conchillo Flaqué patch from previous bug without unnecessary lock/unlock that fix the problem.
Comment 4 Sebastian Dröge (slomo) 2016-09-07 07:52:49 UTC
Review of attachment 334916 [details] [review]:

Please keep the original author in the commit in the future :)

Also some further explanation of why/how this solves the problem in the commit message would be useful

::: gst/rtsp/gstrtspsrc.c
@@ +7361,3 @@
+   * might end up calling request_key (with SRTP) while caps are still
+   * being configured. */
+  gst_rtspsrc_set_state (src, GST_STATE_PLAYING);

This is already done in the line above :)
Comment 5 Kseniya Vasilchuk 2016-09-07 13:26:12 UTC
Created attachment 334984 [details] [review]
refactoring

I've changed the author name and remove the unnecessary code. Please watch.
Comment 6 Matt Staples 2016-12-03 00:13:05 UTC
Is this fix a candidate for 1.11?

This solves the problem nicely for me as well. 

Without this fix, I've found that if the RTSP server doesn't support PAUSE, then the CMD_PAUSE completes much more quickly, causing CMD_CLOSE to never get executed.  For servers that do implement PAUSE, the CMD_CLOSE usually interrupts CMD_PAUSE, so it at least tries to call TEARDOWN.  It doesn't always successfully complete TEARDOWN though, depending entirely on timing. 

With this fix, my app can set the pipeline to READY, then wait (with some timeout) for the progress message telling it that close is completed, and then proceed from there. But if I one wants to set the pipeline straight to NULL, it will still transition to that state without blocking, which I understand was the original design goal.

Another note: none of that works with playbin, as that component seems to internally set rtspsrc to NULL on its own.  That is, when I set the pipeline to READY, playbin transitions rtspsrc all the way to NULL, leaving the application no way to guarantee that TEARDOWN happens.  (That's not a problem for me, as my app doesn't use playbin, but I found that problem while testing fixes to this problem via gst-play-1.0.)
Comment 7 Sebastian Dröge (slomo) 2016-12-05 09:34:38 UTC
commit bc9f06f37b856be0b3270e62e58feac635239556
Author: Aleix Conchillo Flaque <aleix@oblong.com>
Date:   Wed Sep 7 16:10:27 2016 +0300

    rtspsrc: always send teardown request
    
    Allow CMD_CLOSE to cancel all commands not only CMD_PAUSE
    and ignore CMD_WAIT while closing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=748360