rtpbasedepayload: Drop gap events before first buffer

Before a gap event is pushed downstream a segment event must be pushed
since the gap event can cause packet concealment downstream and hence
data flow. Since concealment before receiving any data packets usually
doesn't make any sense, the gap event is not sent downstream.

Alternatively one could generate a default caps and segment event, but
no need to complicate things until it's proven necessary.

https://bugzilla.gnome.org/show_bug.cgi?id=773104
https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/301

diff --git a/gst-libs/gst/rtp/gstrtpbasedepayload.c b/gst-libs/gst/rtp/gstrtpbasedepayload.c
index c9c5204..5476193 100644 (file)
--- a/gst-libs/gst/rtp/gstrtpbasedepayload.c
+++ b/gst-libs/gst/rtp/gstrtpbasedepayload.c
@@ -913,6 +913,15 @@ gst_rtp_base_depayload_packet_lost (GstRTPBaseDepayload * filter,
     return FALSE;
   }
 
+  sevent = gst_pad_get_sticky_event (filter->srcpad, GST_EVENT_SEGMENT, 0);
+  if (G_UNLIKELY (!sevent)) {
+    /* Typically happens if lost event arrives before first buffer */
+    GST_DEBUG_OBJECT (filter,
+        "Ignore packet loss because segment event missing");
+    return FALSE;
+  }
+  gst_event_unref (sevent);
+
   if (!gst_structure_get_boolean (s, "might-have-been-fec",
           &might_have_been_fec) || !might_have_been_fec) {
     /* send GAP event */

diff --git a/tests/check/libs/rtpbasedepayload.c b/tests/check/libs/rtpbasedepayload.c
index 5e6f8a3..b133e4e 100644 (file)
--- a/tests/check/libs/rtpbasedepayload.c
+++ b/tests/check/libs/rtpbasedepayload.c
@@ -934,6 +934,60 @@ GST_START_TEST (rtp_base_depayload_packet_lost_test)
 }
 
 GST_END_TEST
+/* If a lost event is received before the first buffer, the rtp base
+ * depayloader will not send a gap event downstream. Alternatively it should
+ * make sure that stream-start, caps and segment events are sent in correct
+ * order before the gap event so that packet loss concealment can take place
+ * downstream, but this is more complicated and without any real benefit since
+ * concealment before any data is received is not very useful. */
+GST_START_TEST (rtp_base_depayload_packet_lost_before_first_buffer_test)
+{
+  GstHarness *h;
+  GstEvent *event;
+  GstRtpDummyDepay *depay;
+  const GstEventType etype[] = {
+    GST_EVENT_STREAM_START, GST_EVENT_CAPS, GST_EVENT_SEGMENT
+  };
+  gint i;
+
+  depay = rtp_dummy_depay_new ();
+  h = gst_harness_new_with_element (GST_ELEMENT_CAST (depay), "sink", "src");
+  gst_harness_set_src_caps_str (h, "application/x-rtp");
+
+  /* Verify that depayloader has received setup events */
+  for (i = 0; i < 3; i++) {
+    event = gst_pad_get_sticky_event (h->srcpad, etype[i], 0);
+    fail_unless (event != NULL);
+    gst_event_unref (event);
+  }
+
+  /* Send loss event to depayloader */
+  gst_harness_push_event (h, gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM,
+          gst_structure_new ("GstRTPPacketLost",
+              "seqnum", G_TYPE_UINT, (guint) 0,
+              "timestamp", G_TYPE_UINT64, (guint64) 0,
+              "duration", G_TYPE_UINT64, (guint64) 10 * GST_MSECOND, NULL)));
+
+  /* When a buffer is pushed, an updated (and more accurate) segment event
+   * should aslo be sent. */
+  gst_harness_push (h, gst_rtp_buffer_new_allocate (0, 0, 0));
+
+  /* Verify that setup events are sent before gap event */
+  for (i = 0; i < 3; i++) {
+    fail_unless (event = gst_harness_pull_event (h));
+    fail_unless_equals_int (GST_EVENT_TYPE (event), etype[i]);
+    gst_event_unref (event);
+  }
+  fail_unless_equals_int (gst_harness_events_in_queue (h), 0);
+
+  gst_buffer_unref (gst_harness_pull (h));
+  fail_unless_equals_int (gst_harness_buffers_in_queue (h), 0);
+
+  g_object_unref (depay);
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
 /* rtp base depayloader should set DISCONT flag on buffer in case of a large
  * sequence number gap, and it's not set already by upstream. This tests a
  * certain code path where the buffer needs to be made writable to set the
@@ -1341,6 +1395,8 @@ rtp_basepayloading_suite (void)
   tcase_add_test (tc_chain, rtp_base_depayload_without_negotiation_test);
 
   tcase_add_test (tc_chain, rtp_base_depayload_packet_lost_test);
+  tcase_add_test (tc_chain,
+      rtp_base_depayload_packet_lost_before_first_buffer_test);
   tcase_add_test (tc_chain, rtp_base_depayload_seq_discont_test);
 
   tcase_add_test (tc_chain, rtp_base_depayload_repeated_caps_test);