srtpdec: fix Got data flow before segment event

A race condition can occur in `srtpdec` during the READY -> NULL transition:
an RTCP buffer can make its way to `gst_srtp_dec_chain` while the element is
partially stopped, resulting in the following critical warning:

> Got data flow before segment event

The problematic sequence is the following:

1. An RTCP buffer is being handled by the chain function for the
   `rtcp_sinkpad`. Since, this is the first buffer, we try pushing the sticky
   events to `rtcp_srcpad`.
2. At the same moment, the element is being transitioned from PAUSED to READY.
3. While checking and pushing the sticky events for `rtcp_srcpad`, we reach the
   Segment event. For this, we try to get it from the "otherpad", in this case
   `rtp_srcpad`. In the problematic case, `rtp_srcpad` has already been
   deactivated so its sticky events have been cleared. We won't be pushing any
   Segment event to `rtcp_srcpad`.
4. We return to the chain function for `rtcp_sinkpad` and try pushing the
   buffer to `rtcp_srcpad` for which deactivation hasn't started yet, hence the
   "Got data flow before segment event".

This commit:

- Adds a boolean return value to `gst_srtp_dec_push_early_events`: in case the
  Segment event can't be retrieved, `gst_srtp_dec_chain` can return  an error
  instead of calling `gst_pad_push`.
- Replaces the obsolete `gst_pad_set_caps` with `gst_pad_push_event`. The
  additional preconditions checked by previous function are guaranteed here
  since we push a fixed Caps which was built in the same function.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4844>
This commit is contained in:
François Laignel 2023-06-14 10:08:51 +02:00 committed by GStreamer Marge Bot
parent 96450f4c59
commit 32fbad8d39

View file

@ -1260,7 +1260,7 @@ gst_srtp_dec_iterate_internal_links_rtcp (GstPad * pad, GstObject * parent)
return gst_srtp_dec_iterate_internal_links (pad, parent, TRUE);
}
static void
static gboolean
gst_srtp_dec_push_early_events (GstSrtpDec * filter, GstPad * pad,
GstPad * otherpad, gboolean is_rtcp)
{
@ -1304,7 +1304,8 @@ gst_srtp_dec_push_early_events (GstSrtpDec * filter, GstPad * pad,
else
caps = gst_caps_new_empty_simple ("application/x-rtp");
gst_pad_set_caps (pad, caps);
ev = gst_event_new_caps (caps);
gst_pad_push_event (pad, ev);
gst_caps_unref (caps);
}
@ -1314,8 +1315,16 @@ gst_srtp_dec_push_early_events (GstSrtpDec * filter, GstPad * pad,
} else {
ev = gst_pad_get_sticky_event (otherpad, GST_EVENT_SEGMENT, 0);
if (ev)
if (ev) {
gst_pad_push_event (pad, ev);
} else if (GST_PAD_IS_FLUSHING (otherpad)) {
/* We didn't get a Segment event from otherpad
* and otherpad is flushing => we are most likely shutting down */
goto err;
} else {
GST_WARNING_OBJECT (filter, "No Segment event to push");
goto err;
}
}
if (is_rtcp)
@ -1323,6 +1332,10 @@ gst_srtp_dec_push_early_events (GstSrtpDec * filter, GstPad * pad,
else
filter->rtp_has_segment = TRUE;
return TRUE;
err:
return FALSE;
}
/*
@ -1500,15 +1513,24 @@ push_out:
/* Push buffer to source pad */
if (is_rtcp) {
otherpad = filter->rtcp_srcpad;
if (!filter->rtcp_has_segment)
gst_srtp_dec_push_early_events (filter, filter->rtcp_srcpad,
filter->rtp_srcpad, TRUE);
if (!filter->rtcp_has_segment) {
if (!gst_srtp_dec_push_early_events (filter, filter->rtcp_srcpad,
filter->rtp_srcpad, TRUE)) {
ret = GST_FLOW_FLUSHING;
goto drop_buffer;
}
}
} else {
otherpad = filter->rtp_srcpad;
if (!filter->rtp_has_segment)
gst_srtp_dec_push_early_events (filter, filter->rtp_srcpad,
filter->rtcp_srcpad, FALSE);
if (!filter->rtp_has_segment) {
if (!gst_srtp_dec_push_early_events (filter, filter->rtp_srcpad,
filter->rtcp_srcpad, FALSE)) {
ret = GST_FLOW_FLUSHING;
goto drop_buffer;
}
}
}
ret = gst_pad_push (otherpad, buf);
return ret;