Make sure that when a pipeline goes to PLAYING, that data has actually hit the sink.

Original commit message from CVS:
2005-08-18  Andy Wingo  <wingo@pobox.com>

Make sure that when a pipeline goes to PLAYING, that data has
actually hit the sink.

* check/states/sinks.c (test_sink): A sink that doesn't get any
data shouldn't return SUCCESS for going to either PLAYING or
PAUSED. Test also the return values on the way back down.

* gst/gstelement.c (gst_element_set_state): When changing the
state of an element currently changing state asynchronously, go to
lost-state after commiting the pending state. Makes future calls
to get_state continue to return ASYNC.

* gst/base/gstbasesink.c (gst_base_sink_change_state): Return
ASYNC when going to PLAYING if we still don't have preroll, as can
happen with live sources.

diff --git a/ChangeLog b/ChangeLog
index dc3a60b656aa5fff1ed92b3f09da05eff2286cd8..548e2ce49cce2a533a1448e9801ecbdb644771be 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2005-08-18  Andy Wingo  <wingo@pobox.com>
+
+       Make sure that when a pipeline goes to PLAYING, that data has
+       actually hit the sink.
+
+       * check/states/sinks.c (test_sink): A sink that doesn't get any
+       data shouldn't return SUCCESS for going to either PLAYING or
+       PAUSED. Test also the return values on the way back down.
+
+       * gst/gstelement.c (gst_element_set_state): When changing the
+       state of an element currently changing state asynchronously, go to
+       lost-state after commiting the pending state. Makes future calls
+       to get_state continue to return ASYNC.
+
+       * gst/base/gstbasesink.c (gst_base_sink_change_state): Return
+       ASYNC when going to PLAYING if we still don't have preroll, as can
+       happen with live sources.
+
 2005-08-18  Jan Schmidt  <thaytan@mad.scientist.com>
 
        * docs/pwg/advanced-types.xml:

diff --git a/check/states/sinks.c b/check/states/sinks.c
index afbb86ea6ac4bc7a0985211f56e1f0648df27e35..ae295be5d34f8049b23ba8927dbd424479586de3 100644 (file)
--- a/check/states/sinks.c
+++ b/check/states/sinks.c
@@ -28,6 +28,7 @@ GST_START_TEST (test_sink)
   GstElement *sink;
   GstElementStateReturn ret;
   GstElementState current, pending;
+  GTimeVal tv;
 
   sink = gst_element_factory_make ("fakesink", "sink");
 
@@ -35,12 +36,22 @@ GST_START_TEST (test_sink)
   fail_unless (ret == GST_STATE_ASYNC, "no async state return");
 
   ret = gst_element_set_state (sink, GST_STATE_PLAYING);
-  fail_unless (ret == GST_STATE_SUCCESS, "cannot force play");
+  fail_unless (ret == GST_STATE_ASYNC, "no forced async state change");
 
-  ret = gst_element_get_state (sink, &current, &pending, NULL);
-  fail_unless (ret == GST_STATE_SUCCESS, "not playing");
-  fail_unless (current == GST_STATE_PLAYING, "not playing");
-  fail_unless (pending == GST_STATE_VOID_PENDING, "not playing");
+  GST_TIME_TO_TIMEVAL ((GstClockTime) 0, tv);
+
+  ret = gst_element_get_state (sink, &current, &pending, &tv);
+  fail_unless (ret == GST_STATE_ASYNC, "not changing state async");
+  fail_unless (current == GST_STATE_PAUSED, "bad current state");
+  fail_unless (pending == GST_STATE_PLAYING, "bad pending state");
+
+  ret = gst_element_set_state (sink, GST_STATE_PAUSED);
+  fail_unless (ret == GST_STATE_ASYNC, "no async going back to paused");
+
+  ret = gst_element_set_state (sink, GST_STATE_READY);
+  fail_unless (ret == GST_STATE_SUCCESS, "failed to go to ready");
+
+  gst_object_unref (sink);
 }
 
 GST_END_TEST

diff --git a/gst/base/gstbasesink.c b/gst/base/gstbasesink.c
index 50f59e2c7be54fb8fb08a4cc9e87f20693b33500..23b6a22f53b6c3931d44e7a0ebb3644f806feb32 100644 (file)
--- a/gst/base/gstbasesink.c
+++ b/gst/base/gstbasesink.c
@@ -528,6 +528,9 @@ gst_base_sink_handle_object (GstBaseSink * basesink, GstPad * pad,
       basesink->preroll_queued,
       basesink->buffers_queued, basesink->events_queued);
 
+  if (basesink->playing_async)
+    goto playing_async;
+
   /* check if we are prerolling */
   if (!basesink->need_preroll)
     goto no_preroll;
@@ -626,6 +629,34 @@ no_preroll:
     ret = gst_base_sink_preroll_queue_empty (basesink, pad);
     GST_PREROLL_UNLOCK (pad);
 
+    return ret;
+  }
+playing_async:
+  {
+    GstFlowReturn ret;
+    gint t;
+
+    basesink->have_preroll = FALSE;
+    basesink->playing_async = FALSE;
+
+    /* handle buffer first */
+    ret = gst_base_sink_preroll_queue_empty (basesink, pad);
+
+    /* unroll locks, commit state, reacquire stream lock */
+    GST_PREROLL_UNLOCK (pad);
+    t = GST_STREAM_UNLOCK_FULL (pad);
+    GST_DEBUG ("released stream lock %d times", t);
+    if (t <= 0) {
+      GST_WARNING ("STREAM_LOCK should have been locked !!");
+      g_warning ("STREAM_LOCK should have been locked !!");
+    }
+    GST_STATE_LOCK (basesink);
+    GST_DEBUG ("commit state %p >", basesink);
+    gst_element_commit_state (GST_ELEMENT (basesink));
+    GST_STATE_UNLOCK (basesink);
+    if (t > 0)
+      GST_STREAM_LOCK_FULL (pad, t);
+
     return ret;
   }
 flushing:
@@ -1104,11 +1135,18 @@ gst_base_sink_change_state (GstElement * element)
        * thread to do this. */
       if (basesink->eos) {
         gst_base_sink_preroll_queue_empty (basesink, basesink->sinkpad);
+      } else if (!basesink->have_preroll) {
+        /* don't need preroll, but do queue a commit_state */
+        basesink->need_preroll = FALSE;
+        basesink->playing_async = TRUE;
+        ret = GST_STATE_ASYNC;
+        /* we know it's not waiting, no need to signal */
+      } else {
+        /* don't need the preroll anymore */
+        basesink->need_preroll = FALSE;
+        /* now let it play */
+        GST_PREROLL_SIGNAL (basesink->sinkpad);
       }
-      /* don't need the preroll anymore */
-      basesink->need_preroll = FALSE;
-      /* now let it play */
-      GST_PREROLL_SIGNAL (basesink->sinkpad);
       GST_PREROLL_UNLOCK (basesink->sinkpad);
       break;
     }
@@ -1133,6 +1171,8 @@ gst_base_sink_change_state (GstElement * element)
       }
       GST_UNLOCK (basesink);
 
+      basesink->playing_async = FALSE;
+
       /* unlock any subclasses */
       if (bclass->unlock)
         bclass->unlock (basesink);

diff --git a/gst/base/gstbasesink.h b/gst/base/gstbasesink.h
index 60bf9a0583d97e365e4ecdd5c51512e2e2a66ed1..e58b9cef5648a5573e0dce57097e9d3b56d0ec2e 100644 (file)
--- a/gst/base/gstbasesink.h
+++ b/gst/base/gstbasesink.h
@@ -81,6 +81,7 @@ struct _GstBaseSink {
   gboolean       eos;
   gboolean       need_preroll;
   gboolean       have_preroll;
+  gboolean       playing_async;
 
   /*< private >*/
   gpointer       _gst_reserved[GST_PADDING];

diff --git a/gst/gstelement.c b/gst/gstelement.c
index 6d1ccc1142cfcd0d38838335519e271c305b2f66..790ec555588cbae031ed284db61720702f3778cb 100644 (file)
--- a/gst/gstelement.c
+++ b/gst/gstelement.c
@@ -1778,6 +1778,7 @@ gst_element_set_state (GstElement * element, GstElementState state)
   GST_STATE_FINAL (element) = state;
   if (ret == GST_STATE_ASYNC) {
     gst_element_commit_state (element);
+    gst_element_lost_state (element);
   }
 
   /* start with the current state */

diff --git a/libs/gst/base/gstbasesink.c b/libs/gst/base/gstbasesink.c
index 50f59e2c7be54fb8fb08a4cc9e87f20693b33500..23b6a22f53b6c3931d44e7a0ebb3644f806feb32 100644 (file)
--- a/libs/gst/base/gstbasesink.c
+++ b/libs/gst/base/gstbasesink.c
@@ -528,6 +528,9 @@ gst_base_sink_handle_object (GstBaseSink * basesink, GstPad * pad,
       basesink->preroll_queued,
       basesink->buffers_queued, basesink->events_queued);
 
+  if (basesink->playing_async)
+    goto playing_async;
+
   /* check if we are prerolling */
   if (!basesink->need_preroll)
     goto no_preroll;
@@ -626,6 +629,34 @@ no_preroll:
     ret = gst_base_sink_preroll_queue_empty (basesink, pad);
     GST_PREROLL_UNLOCK (pad);
 
+    return ret;
+  }
+playing_async:
+  {
+    GstFlowReturn ret;
+    gint t;
+
+    basesink->have_preroll = FALSE;
+    basesink->playing_async = FALSE;
+
+    /* handle buffer first */
+    ret = gst_base_sink_preroll_queue_empty (basesink, pad);
+
+    /* unroll locks, commit state, reacquire stream lock */
+    GST_PREROLL_UNLOCK (pad);
+    t = GST_STREAM_UNLOCK_FULL (pad);
+    GST_DEBUG ("released stream lock %d times", t);
+    if (t <= 0) {
+      GST_WARNING ("STREAM_LOCK should have been locked !!");
+      g_warning ("STREAM_LOCK should have been locked !!");
+    }
+    GST_STATE_LOCK (basesink);
+    GST_DEBUG ("commit state %p >", basesink);
+    gst_element_commit_state (GST_ELEMENT (basesink));
+    GST_STATE_UNLOCK (basesink);
+    if (t > 0)
+      GST_STREAM_LOCK_FULL (pad, t);
+
     return ret;
   }
 flushing:
@@ -1104,11 +1135,18 @@ gst_base_sink_change_state (GstElement * element)
        * thread to do this. */
       if (basesink->eos) {
         gst_base_sink_preroll_queue_empty (basesink, basesink->sinkpad);
+      } else if (!basesink->have_preroll) {
+        /* don't need preroll, but do queue a commit_state */
+        basesink->need_preroll = FALSE;
+        basesink->playing_async = TRUE;
+        ret = GST_STATE_ASYNC;
+        /* we know it's not waiting, no need to signal */
+      } else {
+        /* don't need the preroll anymore */
+        basesink->need_preroll = FALSE;
+        /* now let it play */
+        GST_PREROLL_SIGNAL (basesink->sinkpad);
       }
-      /* don't need the preroll anymore */
-      basesink->need_preroll = FALSE;
-      /* now let it play */
-      GST_PREROLL_SIGNAL (basesink->sinkpad);
       GST_PREROLL_UNLOCK (basesink->sinkpad);
       break;
     }
@@ -1133,6 +1171,8 @@ gst_base_sink_change_state (GstElement * element)
       }
       GST_UNLOCK (basesink);
 
+      basesink->playing_async = FALSE;
+
       /* unlock any subclasses */
       if (bclass->unlock)
         bclass->unlock (basesink);

diff --git a/libs/gst/base/gstbasesink.h b/libs/gst/base/gstbasesink.h
index 60bf9a0583d97e365e4ecdd5c51512e2e2a66ed1..e58b9cef5648a5573e0dce57097e9d3b56d0ec2e 100644 (file)
--- a/libs/gst/base/gstbasesink.h
+++ b/libs/gst/base/gstbasesink.h
@@ -81,6 +81,7 @@ struct _GstBaseSink {
   gboolean       eos;
   gboolean       need_preroll;
   gboolean       have_preroll;
+  gboolean       playing_async;
 
   /*< private >*/
   gpointer       _gst_reserved[GST_PADDING];

diff --git a/tests/check/generic/sinks.c b/tests/check/generic/sinks.c
index afbb86ea6ac4bc7a0985211f56e1f0648df27e35..ae295be5d34f8049b23ba8927dbd424479586de3 100644 (file)
--- a/tests/check/generic/sinks.c
+++ b/tests/check/generic/sinks.c
@@ -28,6 +28,7 @@ GST_START_TEST (test_sink)
   GstElement *sink;
   GstElementStateReturn ret;
   GstElementState current, pending;
+  GTimeVal tv;
 
   sink = gst_element_factory_make ("fakesink", "sink");
 
@@ -35,12 +36,22 @@ GST_START_TEST (test_sink)
   fail_unless (ret == GST_STATE_ASYNC, "no async state return");
 
   ret = gst_element_set_state (sink, GST_STATE_PLAYING);
-  fail_unless (ret == GST_STATE_SUCCESS, "cannot force play");
+  fail_unless (ret == GST_STATE_ASYNC, "no forced async state change");
 
-  ret = gst_element_get_state (sink, &current, &pending, NULL);
-  fail_unless (ret == GST_STATE_SUCCESS, "not playing");
-  fail_unless (current == GST_STATE_PLAYING, "not playing");
-  fail_unless (pending == GST_STATE_VOID_PENDING, "not playing");
+  GST_TIME_TO_TIMEVAL ((GstClockTime) 0, tv);
+
+  ret = gst_element_get_state (sink, &current, &pending, &tv);
+  fail_unless (ret == GST_STATE_ASYNC, "not changing state async");
+  fail_unless (current == GST_STATE_PAUSED, "bad current state");
+  fail_unless (pending == GST_STATE_PLAYING, "bad pending state");
+
+  ret = gst_element_set_state (sink, GST_STATE_PAUSED);
+  fail_unless (ret == GST_STATE_ASYNC, "no async going back to paused");
+
+  ret = gst_element_set_state (sink, GST_STATE_READY);
+  fail_unless (ret == GST_STATE_SUCCESS, "failed to go to ready");
+
+  gst_object_unref (sink);
 }
 
 GST_END_TEST