[Bug 702705] New: rtspsrc does not pause properly (race condition)

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=702705
  GStreamer | gst-plugins-good | git

           Summary: rtspsrc does not pause properly (race condition)
    Classification: Platform
           Product: GStreamer
           Version: git
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: blocker
          Priority: Normal
         Component: gst-plugins-good
        AssignedTo: gstreamer-bugs at lists.freedesktop.org
        ReportedBy: youness.alaoui at collabora.co.uk
         QAContact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---


There is a race condition in rtspsrc when you change state from PLAYING to
PAUSED, as it will not pause its internal elements correctly.
When going from PLAYING to PAUSED, the rtspsrc must pause the rtpbin and send
the PAUSE request to the server, but this does not happen, causing various
issues. In my use case, the server will pause the playback and notify the
clients that they need to pause as well through a SET_PARAMETER. Without
properly doing the PAUSE request, the internal rtpbin to rtspsrc does not
change state, and does not stop its RTCP thread which makes it signal the
'on-timeout' error after 30 seconds, since the server stops transmitting, which
causes the rtspsrc to error.

The issue is that the way the PAUSE request is sent is by calling
gst_rtspsrc_loop_send_cmd (rtspsrc, CMD_PAUSE, CMD_LOOP) to tell the rtspsrc
task to send the CMD_PAUSE and handle the pause command, but the rtspsrc is
processing the "CMD_LOOP" command (to receive requests from the server), so for
that, a CMD_WAIT is first sent in order to interrupt the loop and pause the
rtspsrc task, then the CMD_PAUSE is sent. The race condition happens this way :
rtspsrc task : exeucting CMD_LOOP
change_state : send CMD_WAIT
rtspsrc task : CMD_LOOP interrupted (connection is set to flushing)
change_state : send CMD_PAUSE
rtspsrc task : 'pausing task, reason flushing' (connection is now non-flushing)
rtspsrc task : send CMD_WAIT (to pause the task)
send_cmd : cancelling CMD_PAUSE in favor of the newest command CMD_WAIT

and so the CMD_PAUSE is overwritten (pending_cmd) and is never executed.
Another issue with this is that if I do the set_state in the handle-request
signal handler, then this has no way of working because the task that needs to
be paused (CMD_LOOP that needs to be canceled) is the one calling my signal
handler.
I was able to work around this issue temporarily by making the change_state
function wait until the rtspsrc task is paused before sending the CMD_PAUSE
command to it, but this will cause a deadlock if the state gets changed from
within a signal handler that is called from within the task itself, which
forced me to create a g_idle_add to do my state change.
Do you have any suggestions on how to fix this properly ?

Here is my hackish workaround (along with state change in an idler in my
application) :
@@ -6969,6 +6975,13 @@ gst_rtspsrc_change_state (GstElement * element,
GstStateChange transition)
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
       /* unblock the tcp tasks and make the loop waiting */
       gst_rtspsrc_loop_send_cmd (rtspsrc, CMD_WAIT, CMD_LOOP);
+
+      GST_OBJECT_LOCK (rtspsrc);
+      while (rtspsrc->busy_cmd != CMD_WAIT) {
+        GST_OBJECT_UNLOCK (rtspsrc);
+        GST_OBJECT_LOCK (rtspsrc);
+      }
+      GST_OBJECT_UNLOCK (rtspsrc);
       break;
     case GST_STATE_CHANGE_PAUSED_TO_READY:
       break;

And here is another patch that was required to restore the CMD_LOOP
functionality of the source after the CMD_PAUSE has been executed :
@@ -6833,7 +6839,7 @@ gst_rtspsrc_thread (GstRTSPSrc * src)

   GST_OBJECT_LOCK (src);
   cmd = src->pending_cmd;
-  if (cmd == CMD_RECONNECT || cmd == CMD_PLAY || cmd == CMD_LOOP)
+  if (cmd == CMD_RECONNECT || cmd == CMD_PLAY || cmd == CMD_PAUSE || cmd ==
CMD_LOOP)
     src->pending_cmd = CMD_LOOP;
   else
     src->pending_cmd = CMD_WAIT;

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.