playback: Fix a small race on decodebin/parsebin shutdown.
authorJan Schmidt <jan@centricular.com>
Mon, 2 Jan 2017 15:23:43 +0000 (02:23 +1100)
committerJan Schmidt <jan@centricular.com>
Mon, 2 Jan 2017 15:27:51 +0000 (02:27 +1100)
When shutting down decodebin2 and parsebin, they set their
output pads to flushing, and there is a very small window
where elements might send a sticky event such as a tag event
(which silently fails due to flushing) and then sends a buffer,
and the buffer will return GST_FLOW_ERROR because it can't
forward sticky events. The element will then send an error
message on the bus. This can also happen when elements send EOS
just as shutdown is happening. Since we're about to destroy all
the elements inside parsebin and decodebin anyway, just discard
error messages from them.

A nicer but more difficult fix for GStreamer 2.0 is to make
all event pushing / handling in core return a GstFlowReturn
like buffers do, so we can report a FLUSHING state cleanly.

gst/playback/gstdecodebin2.c
gst/playback/gstparsebin.c

index ace3a14..0a14597 100644 (file)
@@ -5390,12 +5390,22 @@ gst_decode_bin_handle_message (GstBin * bin, GstMessage * msg)
   gboolean drop = FALSE;
 
   if (GST_MESSAGE_TYPE (msg) == GST_MESSAGE_ERROR) {
-    GST_OBJECT_LOCK (dbin);
-    drop = (g_list_find (dbin->filtered, GST_MESSAGE_SRC (msg)) != NULL);
-    if (drop)
-      dbin->filtered_errors =
-          g_list_prepend (dbin->filtered_errors, gst_message_ref (msg));
-    GST_OBJECT_UNLOCK (dbin);
+    /* Don't pass errors when shutting down. Sometimes,
+     * elements can generate spurious errors because we set the
+     * output pads to flushing, and they can't detect that if they
+     * send an event at exactly the wrong moment */
+    DYN_LOCK (dbin);
+    drop = dbin->shutdown;
+    DYN_UNLOCK (dbin);
+
+    if (!drop) {
+      GST_OBJECT_LOCK (dbin);
+      drop = (g_list_find (dbin->filtered, GST_MESSAGE_SRC (msg)) != NULL);
+      if (drop)
+        dbin->filtered_errors =
+            g_list_prepend (dbin->filtered_errors, gst_message_ref (msg));
+      GST_OBJECT_UNLOCK (dbin);
+    }
   } else if (GST_MESSAGE_TYPE (msg) == GST_MESSAGE_BUFFERING) {
     gint perc, msg_perc;
     gint smaller_perc = 100;
index c0fdf89..26d8769 100644 (file)
@@ -4355,12 +4355,23 @@ gst_parse_bin_handle_message (GstBin * bin, GstMessage * msg)
 
   switch (GST_MESSAGE_TYPE (msg)) {
     case GST_MESSAGE_ERROR:{
-      GST_OBJECT_LOCK (parsebin);
-      drop = (g_list_find (parsebin->filtered, GST_MESSAGE_SRC (msg)) != NULL);
-      if (drop)
-        parsebin->filtered_errors =
-            g_list_prepend (parsebin->filtered_errors, gst_message_ref (msg));
-      GST_OBJECT_UNLOCK (parsebin);
+      /* Don't pass errors when shutting down. Sometimes,
+       * elements can generate spurious errors because we set the
+       * output pads to flushing, and they can't detect that if they
+       * send an event at exactly the wrong moment */
+      DYN_LOCK (parsebin);
+      drop = parsebin->shutdown;
+      DYN_UNLOCK (parsebin);
+
+      if (!drop) {
+        GST_OBJECT_LOCK (parsebin);
+        drop =
+            (g_list_find (parsebin->filtered, GST_MESSAGE_SRC (msg)) != NULL);
+        if (drop)
+          parsebin->filtered_errors =
+              g_list_prepend (parsebin->filtered_errors, gst_message_ref (msg));
+        GST_OBJECT_UNLOCK (parsebin);
+      }
       break;
     }
     default: