From 3eb78e5280ec39e420172be7a4deb6431b6497b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= <francois@centricular.com>
Date: Wed, 14 Jun 2023 10:08:51 +0200
Subject: [PATCH] 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/4860>
---
 .../gst-plugins-bad/ext/srtp/gstsrtpdec.c     | 40 ++++++++++++++-----
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/subprojects/gst-plugins-bad/ext/srtp/gstsrtpdec.c b/subprojects/gst-plugins-bad/ext/srtp/gstsrtpdec.c
index a640690010..153712acb6 100644
--- a/subprojects/gst-plugins-bad/ext/srtp/gstsrtpdec.c
+++ b/subprojects/gst-plugins-bad/ext/srtp/gstsrtpdec.c
@@ -1307,7 +1307,7 @@ decorate_stream_id_private (GstElement * element, const gchar * stream_id)
   return new_stream_id;
 }
 
-static void
+static gboolean
 gst_srtp_dec_push_early_events (GstSrtpDec * filter, GstPad * pad,
     GstPad * otherpad, gboolean is_rtcp)
 {
@@ -1351,7 +1351,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);
   }
 
@@ -1361,8 +1362,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)
@@ -1370,6 +1379,10 @@ gst_srtp_dec_push_early_events (GstSrtpDec * filter, GstPad * pad,
   else
     filter->rtp_has_segment = TRUE;
 
+  return TRUE;
+
+err:
+  return FALSE;
 }
 
 /*
@@ -1547,15 +1560,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;