From 4ffa6350e8d42323fd476bd1b01f58898fbbaf49 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 6 Feb 2020 14:29:13 +0200 Subject: [PATCH] webrtc: In all blocking pad probes except for sink pads also handle serialized events Otherwise it can happen that e.g. the stream-start event is tried to be sent as part of pushing the first buffer. Downstream might not be in PAUSED/PLAYING yet, so the event is rejected with GST_FLOW_FLUSHING and because it's an event would not cause the blocking pad probe to trigger first. This would then return GST_FLOW_FLUSHING for the buffer and shut down all of upstream. To solve this we return GST_PAD_PROBE_DROP for all events. In case of sticky events they would be resent again later once we unblocked after blocking on the buffer and everything works fine. Don't handle events specifically in sink pad blocking pad probes as here downstream is not linked yet and we are actually waiting for the following CAPS event before unblocking can happen. Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/1172 --- ext/webrtc/gstwebrtcbin.c | 30 ++++++++++++++++++++++++------ ext/webrtc/transportreceivebin.c | 16 +++++++++++++--- ext/webrtc/transportsendbin.c | 16 +++++++++++++--- 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/ext/webrtc/gstwebrtcbin.c b/ext/webrtc/gstwebrtcbin.c index 9a82672..9c2cb40 100644 --- a/ext/webrtc/gstwebrtcbin.c +++ b/ext/webrtc/gstwebrtcbin.c @@ -1857,8 +1857,19 @@ _on_sctp_notify_dtls_state (GstWebRTCDTLSTransport * transport, } static GstPadProbeReturn -pad_block (GstPad * pad, GstPadProbeInfo * info, gpointer unused) -{ +sctp_pad_block (GstPad * pad, GstPadProbeInfo * info, gpointer unused) +{ + /* Drop all events: we don't care about them and don't want to block on + * them. Sticky events would be forwarded again later once we unblock + * and we don't want to forward them here already because that might + * cause a spurious GST_FLOW_FLUSHING */ + if (GST_IS_EVENT (info->data)) + return GST_PAD_PROBE_DROP; + + /* But block on any actual data-flow so we don't accidentally send that + * to a pad that is not ready yet, causing GST_FLOW_FLUSHING and everything + * to silently stop. + */ GST_LOG_OBJECT (pad, "blocking pad with data %" GST_PTR_FORMAT, info->data); return GST_PAD_PROBE_OK; @@ -1912,9 +1923,8 @@ _get_or_create_data_channel_transports (GstWebRTCBin * webrtc, guint session_id) "data_src"); sctp_transport->sctpdec_block_id = gst_pad_add_probe (receive_srcpad, - GST_PAD_PROBE_TYPE_BLOCK | - GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST, - (GstPadProbeCallback) pad_block, NULL, NULL); + GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_DATA_DOWNSTREAM, + (GstPadProbeCallback) sctp_pad_block, NULL, NULL); gst_object_unref (receive_srcpad); } @@ -5457,6 +5467,14 @@ gst_webrtc_bin_change_state (GstElement * element, GstStateChange transition) return ret; } +static GstPadProbeReturn +sink_pad_block (GstPad * pad, GstPadProbeInfo * info, gpointer unused) +{ + GST_LOG_OBJECT (pad, "blocking pad with data %" GST_PTR_FORMAT, info->data); + + return GST_PAD_PROBE_OK; +} + static GstPad * gst_webrtc_bin_request_new_pad (GstElement * element, GstPadTemplate * templ, const gchar * name, const GstCaps * caps) @@ -5494,7 +5512,7 @@ gst_webrtc_bin_request_new_pad (GstElement * element, GstPadTemplate * templ, pad->block_id = gst_pad_add_probe (GST_PAD (pad), GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST, - (GstPadProbeCallback) pad_block, NULL, NULL); + (GstPadProbeCallback) sink_pad_block, NULL, NULL); webrtc->priv->pending_sink_transceivers = g_list_append (webrtc->priv->pending_sink_transceivers, gst_object_ref (pad)); diff --git a/ext/webrtc/transportreceivebin.c b/ext/webrtc/transportreceivebin.c index 9654eb6..c8bbdee 100644 --- a/ext/webrtc/transportreceivebin.c +++ b/ext/webrtc/transportreceivebin.c @@ -101,7 +101,17 @@ _receive_state_to_string (ReceiveState state) static GstPadProbeReturn pad_block (GstPad * pad, GstPadProbeInfo * info, TransportReceiveBin * receive) { - + /* Drop all events: we don't care about them and don't want to block on + * them. Sticky events would be forwarded again later once we unblock + * and we don't want to forward them here already because that might + * cause a spurious GST_FLOW_FLUSHING */ + if (GST_IS_EVENT (info->data)) + return GST_PAD_PROBE_DROP; + + /* But block on any actual data-flow so we don't accidentally send that + * to a pad that is not ready yet, causing GST_FLOW_FLUSHING and everything + * to silently stop. + */ GST_LOG_OBJECT (pad, "blocking pad with data %" GST_PTR_FORMAT, info->data); return GST_PAD_PROBE_OK; @@ -222,7 +232,7 @@ transport_receive_bin_change_state (GstElement * element, receive->rtp_block->block_id = gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_BLOCK | - GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST, + GST_PAD_PROBE_TYPE_DATA_DOWNSTREAM, (GstPadProbeCallback) pad_block, receive, NULL); gst_object_unref (pad); @@ -238,7 +248,7 @@ transport_receive_bin_change_state (GstElement * element, receive->rtcp_block->block_id = gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_BLOCK | - GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST, + GST_PAD_PROBE_TYPE_DATA_DOWNSTREAM, (GstPadProbeCallback) pad_block, receive, NULL); gst_object_unref (pad); diff --git a/ext/webrtc/transportsendbin.c b/ext/webrtc/transportsendbin.c index 7939b8e..727506a 100644 --- a/ext/webrtc/transportsendbin.c +++ b/ext/webrtc/transportsendbin.c @@ -146,6 +146,17 @@ transport_send_bin_get_property (GObject * object, guint prop_id, static GstPadProbeReturn pad_block (GstPad * pad, GstPadProbeInfo * info, gpointer unused) { + /* Drop all events: we don't care about them and don't want to block on + * them. Sticky events would be forwarded again later once we unblock + * and we don't want to forward them here already because that might + * cause a spurious GST_FLOW_FLUSHING */ + if (GST_IS_EVENT (info->data)) + return GST_PAD_PROBE_DROP; + + /* But block on any actual data-flow so we don't accidentally send that + * to a pad that is not ready yet, causing GST_FLOW_FLUSHING and everything + * to silently stop. + */ GST_LOG_OBJECT (pad, "blocking pad with data %" GST_PTR_FORMAT, info->data); return GST_PAD_PROBE_OK; @@ -165,9 +176,8 @@ block_peer_pad (GstElement * elem, const gchar * pad_name) peer = gst_pad_get_peer (pad); block = _create_pad_block (elem, peer, 0, NULL, NULL); block->block_id = gst_pad_add_probe (peer, - GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER | - GST_PAD_PROBE_TYPE_BUFFER_LIST, (GstPadProbeCallback) pad_block, NULL, - NULL); + GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_DATA_DOWNSTREAM, + (GstPadProbeCallback) pad_block, NULL, NULL); gst_object_unref (pad); gst_object_unref (peer); return block; -- 2.7.4