From c2a91d2cfd5cacf4267a0020294889171c46dd12 Mon Sep 17 00:00:00 2001 From: Jan Schmidt Date: Tue, 3 Jan 2017 02:23:43 +1100 Subject: [PATCH] playback: Fix a small race on decodebin/parsebin shutdown. 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 | 22 ++++++++++++++++------ gst/playback/gstparsebin.c | 23 +++++++++++++++++------ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/gst/playback/gstdecodebin2.c b/gst/playback/gstdecodebin2.c index ace3a1473f..0a14597867 100644 --- a/gst/playback/gstdecodebin2.c +++ b/gst/playback/gstdecodebin2.c @@ -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; diff --git a/gst/playback/gstparsebin.c b/gst/playback/gstparsebin.c index c0fdf899de..26d8769cf1 100644 --- a/gst/playback/gstparsebin.c +++ b/gst/playback/gstparsebin.c @@ -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: