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 739265 - rtspsrc: teardown usually never happens
rtspsrc: teardown usually never happens
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-10-27 22:17 UTC by Aleix Conchillo Flaqué
Modified: 2015-04-08 13:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
always send TEARDOWN (2.10 KB, patch)
2014-10-28 23:21 UTC, Aleix Conchillo Flaqué
none Details | Review
always send TEARDOWN fixup (2.04 KB, patch)
2014-10-29 16:44 UTC, Aleix Conchillo Flaqué
none Details | Review

Description Aleix Conchillo Flaqué 2014-10-27 22:17:34 UTC
I was testing a problem with gst-rtsp-server when TEARDOWN is sent by the client. Then, I saw that TEARDOWN was almost never being sent. I tracked it down to rtspsrc.

Basically,

    case GST_STATE_CHANGE_PAUSED_TO_READY:
      gst_rtspsrc_loop_send_cmd (rtspsrc, CMD_CLOSE, CMD_PAUSE);
      ret = GST_STATE_CHANGE_SUCCESS;
      break;
    case GST_STATE_CHANGE_READY_TO_NULL:
      gst_rtspsrc_stop (rtspsrc);
      ret = GST_STATE_CHANGE_SUCCESS;
      break;

gst_rtspsrc_stop is being called to soon and CMD_CLOSE is never executed, thus TEARDOWN is never sent.

I don't have a patch for this one. I think some sort of locking should happen and make gst_rtspsrc_stop to wait for it. But this is completely different than what gst_rtspsrc_stop is doing now.

To reproduce, just launch an gst-launch with rtspsrc and CTRL-C. You can check with wireshark or simply looking to the logs.
Comment 1 Aleix Conchillo Flaqué 2014-10-28 23:21:15 UTC
Created attachment 289556 [details] [review]
always send TEARDOWN

This fixes it for me. These are the changes:

(1) In GST_STATE_CHANGE_PAUSED_TO_READY we allow CMD_CLOSE to cancel all commands 
    not only CMD_PAUSE. It might be possible that CMD_PAUSE is already done and 
    CMD_LOOP is being executed, so we want to stop CMD_LOOP as well. So, now we 
    do:

    gst_rtspsrc_loop_send_cmd (rtspsrc, CMD_CLOSE, CMD_ALL);

(2) Because in (1) we might be interrupting CMD_LOOP and gst_rtspsrc_loop sends 
    a CMD_WAIT that would interrupt our CMD_CLOSE, we want to ignore that and 
    keep working on CMD_CLOSE.

(3) Finally, also in GST_STATE_CHANGE_PAUSED_TO_READY we wait on the stream 
    lock so we make sure teardown request was sent (or at least tried to).
Comment 2 Aleix Conchillo Flaqué 2014-10-29 16:44:59 UTC
Created attachment 289600 [details] [review]
always send TEARDOWN fixup

I removed a line by mistake in the previous patch. This one does exaclty what I said.
Comment 3 Wim Taymans 2014-10-29 16:49:52 UTC
This is by design, downward state changes should never block or wait. If  you want to wait for shutdown you should use the PROGRESS messages.
Comment 4 Aleix Conchillo Flaqué 2014-10-29 16:54:39 UTC
(In reply to comment #3)
> This is by design, downward state changes should never block or wait. 

Oh... I didn't know that.

> If  you want to wait for shutdown you should use the PROGRESS messages.

OK, this is new to me.

Thanks!
Comment 5 Kseniya Vasilchuk 2015-04-08 13:32:39 UTC
(In reply to Wim Taymans from comment #3)
> This is by design, downward state changes should never block or wait. If 
> you want to wait for shutdown you should use the PROGRESS messages.

I had the same situation with TEARDOWN like Aleix 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.