rtpvp*depay: possibly forward might-have-been-fec PacketLost events
authorMikhail Fludkov <misha@pexip.com>
Tue, 13 Oct 2020 23:28:50 +0000 (01:28 +0200)
committerMathieu Duponchelle <mathieu@centricular.com>
Thu, 29 Oct 2020 18:56:07 +0000 (19:56 +0100)
This is ad adaptation of a Pexip patch for dealing with spurious
GstRTPPacketLost events caused by lost ulpfec packets: as FEC packets
under that scheme are spliced in the same sequence domain as the media
packets, it is not generally possible to determine whether a lost packet
was a FEC packet or a media packet.

When upstreaming pexip's ulpfec patches, we decided to drop all lost
events at the base depayloader level, and where the original patch
from pexip was making use of picture ids and marker bits to determine
whether a packet should be forwarded, this patch makes use of those
to determine whether they should be dropped instead (by removing their
might-have-been-fec field).

Spurious lost events coming out of the depayloader can cause the
decoder to stop decoding until the next keyframe and / or request a new
keyframe, and while this is not desirable it makes sense to forward
that information when we have other means to determine whether a lost
packet was indeed a FEC packet, as is the case with VP8 / VP9 payloads
when they carry a picture id.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/769>

gst/rtp/gstrtpvp8depay.c
gst/rtp/gstrtpvp8depay.h
gst/rtp/gstrtpvp9depay.c
gst/rtp/gstrtpvp9depay.h
tests/check/elements/rtpvp8.c
tests/check/elements/rtpvp9.c

index e71baf8..396aa75 100644 (file)
@@ -43,6 +43,8 @@ static GstStateChangeReturn gst_rtp_vp8_depay_change_state (GstElement *
     element, GstStateChange transition);
 static gboolean gst_rtp_vp8_depay_handle_event (GstRTPBaseDepayload * depay,
     GstEvent * event);
+static gboolean gst_rtp_vp8_depay_packet_lost (GstRTPBaseDepayload * depay,
+    GstEvent * event);
 
 G_DEFINE_TYPE (GstRtpVP8Depay, gst_rtp_vp8_depay, GST_TYPE_RTP_BASE_DEPAYLOAD);
 
@@ -69,6 +71,9 @@ enum
   PROP_WAIT_FOR_KEYFRAME
 };
 
+#define PICTURE_ID_NONE (UINT_MAX)
+#define IS_PICTURE_ID_15BITS(pid) (((guint)(pid) & 0x8000) != 0)
+
 static void
 gst_rtp_vp8_depay_init (GstRtpVP8Depay * self)
 {
@@ -109,6 +114,7 @@ gst_rtp_vp8_depay_class_init (GstRtpVP8DepayClass * gst_rtp_vp8_depay_class)
 
   depay_class->process_rtp_packet = gst_rtp_vp8_depay_process;
   depay_class->handle_event = gst_rtp_vp8_depay_handle_event;
+  depay_class->packet_lost = gst_rtp_vp8_depay_packet_lost;
 
   GST_DEBUG_CATEGORY_INIT (gst_rtp_vp8_depay_debug, "rtpvp8depay", 0,
       "VP8 Video RTP Depayloader");
@@ -161,14 +167,75 @@ gst_rtp_vp8_depay_get_property (GObject * object, guint prop_id,
   }
 }
 
+static gint
+picture_id_compare (guint16 id0, guint16 id1)
+{
+  guint shift = 16 - (IS_PICTURE_ID_15BITS (id1) ? 15 : 7);
+  id0 = id0 << shift;
+  id1 = id1 << shift;
+  return ((gint16) (id1 - id0)) >> shift;
+}
+
+static void
+send_last_lost_event (GstRtpVP8Depay * self)
+{
+  if (self->last_lost_event) {
+    GST_ERROR_OBJECT (self,
+        "Sending the last stopped lost event: %" GST_PTR_FORMAT,
+        self->last_lost_event);
+    GST_RTP_BASE_DEPAYLOAD_CLASS (gst_rtp_vp8_depay_parent_class)
+        ->packet_lost (GST_RTP_BASE_DEPAYLOAD_CAST (self),
+        self->last_lost_event);
+    gst_event_replace (&self->last_lost_event, NULL);
+  }
+}
+
+static void
+send_last_lost_event_if_needed (GstRtpVP8Depay * self, guint new_picture_id)
+{
+  if (self->last_picture_id == PICTURE_ID_NONE)
+    return;
+
+  if (self->last_lost_event) {
+    gboolean send_lost_event = FALSE;
+    if (new_picture_id == PICTURE_ID_NONE) {
+      GST_DEBUG_OBJECT (self, "Dropping the last stopped lost event "
+          "(picture id does not exist): %" GST_PTR_FORMAT,
+          self->last_lost_event);
+    } else if (IS_PICTURE_ID_15BITS (self->last_picture_id) &&
+        !IS_PICTURE_ID_15BITS (new_picture_id)) {
+      GST_DEBUG_OBJECT (self, "Dropping the last stopped lost event "
+          "(picture id has less bits than before): %" GST_PTR_FORMAT,
+          self->last_lost_event);
+    } else if (picture_id_compare (self->last_picture_id, new_picture_id) != 1) {
+      GstStructure *s = gst_event_writable_structure (self->last_lost_event);
+
+      GST_DEBUG_OBJECT (self, "Sending the last stopped lost event "
+          "(gap in picture id %u %u): %" GST_PTR_FORMAT,
+          self->last_picture_id, new_picture_id, self->last_lost_event);
+      send_lost_event = TRUE;
+      /* Prevent rtpbasedepayload from dropping the event now
+       * that we have made sure the lost packet was not FEC */
+      gst_structure_remove_field (s, "might-have-been-fec");
+    }
+    if (send_lost_event)
+      GST_RTP_BASE_DEPAYLOAD_CLASS (gst_rtp_vp8_depay_parent_class)
+          ->packet_lost (GST_RTP_BASE_DEPAYLOAD_CAST (self),
+          self->last_lost_event);
+
+    gst_event_replace (&self->last_lost_event, NULL);
+  }
+}
+
 static GstBuffer *
 gst_rtp_vp8_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
 {
   GstRtpVP8Depay *self = GST_RTP_VP8_DEPAY (depay);
   GstBuffer *payload;
   guint8 *data;
-  guint hdrsize;
-  guint size;
+  guint hdrsize = 1;
+  guint picture_id = PICTURE_ID_NONE;
+  guint size = gst_rtp_buffer_get_payload_len (rtp);
 
   if (G_UNLIKELY (GST_BUFFER_IS_DISCONT (rtp->buffer))) {
     GST_LOG_OBJECT (self, "Discontinuity, flushing adapter");
@@ -179,24 +246,11 @@ gst_rtp_vp8_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
       self->waiting_for_keyframe = TRUE;
   }
 
-  size = gst_rtp_buffer_get_payload_len (rtp);
-
   /* At least one header and one vp8 byte */
   if (G_UNLIKELY (size < 2))
     goto too_small;
 
   data = gst_rtp_buffer_get_payload (rtp);
-
-  if (G_UNLIKELY (!self->started)) {
-    /* Check if this is the start of a VP8 frame, otherwise bail */
-    /* S=1 and PartID= 0 */
-    if ((data[0] & 0x17) != 0x10)
-      goto done;
-
-    self->started = TRUE;
-  }
-
-  hdrsize = 1;
   /* Check X optional header */
   if ((data[0] & 0x80) != 0) {
     hdrsize++;
@@ -206,8 +260,13 @@ gst_rtp_vp8_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
         goto too_small;
       hdrsize++;
       /* Check for 16 bits PictureID */
-      if ((data[2] & 0x80) != 0)
+      picture_id = data[2];
+      if ((data[2] & 0x80) != 0) {
+        if (G_UNLIKELY (size < 4))
+          goto too_small;
         hdrsize++;
+        picture_id = (picture_id << 8) | data[3];
+      }
     }
     /* Check L optional header */
     if ((data[1] & 0x40) != 0)
@@ -216,19 +275,46 @@ gst_rtp_vp8_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
     if ((data[1] & 0x20) != 0 || (data[1] & 0x10) != 0)
       hdrsize++;
   }
-  GST_DEBUG_OBJECT (depay, "hdrsize %u, size %u", hdrsize, size);
-
+  GST_DEBUG_OBJECT (depay, "hdrsize %u, size %u, picture id 0x%x", hdrsize,
+      size, picture_id);
   if (G_UNLIKELY (hdrsize >= size))
     goto too_small;
 
+  if (G_UNLIKELY (!self->started)) {
+    /* Check if this is the start of a VP8 frame, otherwise bail */
+    /* S=1 and PartID= 0 */
+    if ((data[0] & 0x17) != 0x10) {
+      GST_DEBUG_OBJECT (depay,
+          "The frame is missing the first packets, ignoring the packet");
+      if (self->stop_lost_events) {
+        send_last_lost_event (self);
+        self->stop_lost_events = FALSE;
+      }
+      goto done;
+    }
+
+    GST_DEBUG_OBJECT (depay, "Found the start of the frame");
+
+    if (self->stop_lost_events) {
+      send_last_lost_event_if_needed (self, picture_id);
+      self->stop_lost_events = FALSE;
+    }
+
+    self->started = TRUE;
+  }
+
   payload = gst_rtp_buffer_get_payload_subbuffer (rtp, hdrsize, -1);
   gst_adapter_push (self->adapter, payload);
+  self->last_picture_id = picture_id;
 
   /* Marker indicates that it was the last rtp packet for this frame */
   if (gst_rtp_buffer_get_marker (rtp)) {
     GstBuffer *out;
     guint8 header[10];
 
+    GST_DEBUG_OBJECT (depay,
+        "Found the end of the frame (%" G_GSIZE_FORMAT " bytes)",
+        gst_adapter_available (self->adapter));
     if (gst_adapter_available (self->adapter) < 10)
       goto too_small;
     gst_adapter_copy (self->adapter, &header, 0, 10);
@@ -284,6 +370,8 @@ gst_rtp_vp8_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
       self->waiting_for_keyframe = FALSE;
     }
 
+    if (picture_id != PICTURE_ID_NONE)
+      self->stop_lost_events = TRUE;
     return out;
   }
 
@@ -309,6 +397,10 @@ gst_rtp_vp8_depay_change_state (GstElement * element, GstStateChange transition)
       self->last_height = -1;
       self->last_width = -1;
       self->waiting_for_keyframe = TRUE;
+      self->caps_sent = FALSE;
+      self->last_picture_id = PICTURE_ID_NONE;
+      gst_event_replace (&self->last_lost_event, NULL);
+      self->stop_lost_events = FALSE;
       break;
     default:
       break;
@@ -329,6 +421,9 @@ gst_rtp_vp8_depay_handle_event (GstRTPBaseDepayload * depay, GstEvent * event)
       self->last_profile = -1;
       self->last_height = -1;
       self->last_width = -1;
+      self->last_picture_id = PICTURE_ID_NONE;
+      gst_event_replace (&self->last_lost_event, NULL);
+      self->stop_lost_events = FALSE;
       break;
     default:
       break;
@@ -339,6 +434,36 @@ gst_rtp_vp8_depay_handle_event (GstRTPBaseDepayload * depay, GstEvent * event)
       (gst_rtp_vp8_depay_parent_class)->handle_event (depay, event);
 }
 
+static gboolean
+gst_rtp_vp8_depay_packet_lost (GstRTPBaseDepayload * depay, GstEvent * event)
+{
+  GstRtpVP8Depay *self = GST_RTP_VP8_DEPAY (depay);
+  const GstStructure *s;
+  gboolean might_have_been_fec;
+
+  s = gst_event_get_structure (event);
+
+  if (self->stop_lost_events) {
+    if (gst_structure_get_boolean (s, "might-have-been-fec",
+            &might_have_been_fec)
+        && might_have_been_fec) {
+      GST_DEBUG_OBJECT (depay, "Stopping lost event %" GST_PTR_FORMAT, event);
+      gst_event_replace (&self->last_lost_event, event);
+      return TRUE;
+    }
+  } else if (self->last_picture_id != PICTURE_ID_NONE) {
+    GstStructure *s = gst_event_writable_structure (self->last_lost_event);
+
+    /* We are currently processing a picture, let's make sure the
+     * base depayloader doesn't drop this lost event */
+    gst_structure_remove_field (s, "might-have-been-fec");
+  }
+
+  return
+      GST_RTP_BASE_DEPAYLOAD_CLASS
+      (gst_rtp_vp8_depay_parent_class)->packet_lost (depay, event);
+}
+
 gboolean
 gst_rtp_vp8_depay_plugin_init (GstPlugin * plugin)
 {
index cde8e5e..c53ccaa 100644 (file)
@@ -54,10 +54,20 @@ struct _GstRtpVP8Depay
   GstAdapter *adapter;
   gboolean started;
 
+  gboolean caps_sent;
+  /* In between pictures, we might store GstRTPPacketLost events instead
+   * of forwarding them immediately, we check upon reception of a new
+   * picture id whether a gap was introduced, in which case we do forward
+   * the event. This is to avoid forwarding spurious lost events for FEC
+   * packets.
+   */
+  gboolean stop_lost_events;
+  GstEvent *last_lost_event;
   gboolean waiting_for_keyframe;
   gint last_profile;
   gint last_width;
   gint last_height;
+  guint last_picture_id;
 
   gboolean wait_for_keyframe;
 };
index c61affc..b95eb5d 100644 (file)
@@ -40,6 +40,8 @@ static GstStateChangeReturn gst_rtp_vp9_depay_change_state (GstElement *
     element, GstStateChange transition);
 static gboolean gst_rtp_vp9_depay_handle_event (GstRTPBaseDepayload * depay,
     GstEvent * event);
+static gboolean gst_rtp_vp9_depay_packet_lost (GstRTPBaseDepayload * depay,
+    GstEvent * event);
 
 G_DEFINE_TYPE (GstRtpVP9Depay, gst_rtp_vp9_depay, GST_TYPE_RTP_BASE_DEPAYLOAD);
 
@@ -58,6 +60,9 @@ GST_STATIC_PAD_TEMPLATE ("sink",
         "media = (string) \"video\","
         "encoding-name = (string) { \"VP9\", \"VP9-DRAFT-IETF-01\" }"));
 
+#define PICTURE_ID_NONE (UINT_MAX)
+#define IS_PICTURE_ID_15BITS(pid) (((guint)(pid) & 0x8000) != 0)
+
 static void
 gst_rtp_vp9_depay_init (GstRtpVP9Depay * self)
 {
@@ -89,6 +94,7 @@ gst_rtp_vp9_depay_class_init (GstRtpVP9DepayClass * gst_rtp_vp9_depay_class)
 
   depay_class->process_rtp_packet = gst_rtp_vp9_depay_process;
   depay_class->handle_event = gst_rtp_vp9_depay_handle_event;
+  depay_class->packet_lost = gst_rtp_vp9_depay_packet_lost;
 
   GST_DEBUG_CATEGORY_INIT (gst_rtp_vp9_depay_debug, "rtpvp9depay", 0,
       "VP9 Video RTP Depayloader");
@@ -109,6 +115,67 @@ gst_rtp_vp9_depay_dispose (GObject * object)
     G_OBJECT_CLASS (gst_rtp_vp9_depay_parent_class)->dispose (object);
 }
 
+static gint
+picture_id_compare (guint16 id0, guint16 id1)
+{
+  guint shift = 16 - (IS_PICTURE_ID_15BITS (id1) ? 15 : 7);
+  id0 = id0 << shift;
+  id1 = id1 << shift;
+  return ((gint16) (id1 - id0)) >> shift;
+}
+
+static void
+send_last_lost_event (GstRtpVP9Depay * self)
+{
+  if (self->last_lost_event) {
+    GST_DEBUG_OBJECT (self,
+        "Sending the last stopped lost event: %" GST_PTR_FORMAT,
+        self->last_lost_event);
+    GST_RTP_BASE_DEPAYLOAD_CLASS (gst_rtp_vp9_depay_parent_class)
+        ->packet_lost (GST_RTP_BASE_DEPAYLOAD_CAST (self),
+        self->last_lost_event);
+    gst_event_replace (&self->last_lost_event, NULL);
+  }
+}
+
+static void
+send_last_lost_event_if_needed (GstRtpVP9Depay * self, guint new_picture_id)
+{
+  if (self->last_picture_id == PICTURE_ID_NONE ||
+      self->last_picture_id == new_picture_id)
+    return;
+
+  if (self->last_lost_event) {
+    gboolean send_lost_event = FALSE;
+    if (new_picture_id == PICTURE_ID_NONE) {
+      GST_DEBUG_OBJECT (self, "Dropping the last stopped lost event "
+          "(picture id does not exist): %" GST_PTR_FORMAT,
+          self->last_lost_event);
+    } else if (IS_PICTURE_ID_15BITS (self->last_picture_id) &&
+        !IS_PICTURE_ID_15BITS (new_picture_id)) {
+      GST_DEBUG_OBJECT (self, "Dropping the last stopped lost event "
+          "(picture id has less bits than before): %" GST_PTR_FORMAT,
+          self->last_lost_event);
+    } else if (picture_id_compare (self->last_picture_id, new_picture_id) != 1) {
+      GstStructure *s = gst_event_writable_structure (self->last_lost_event);
+
+      GST_DEBUG_OBJECT (self, "Sending the last stopped lost event "
+          "(gap in picture id %u %u): %" GST_PTR_FORMAT,
+          self->last_picture_id, new_picture_id, self->last_lost_event);
+      send_lost_event = TRUE;
+      /* Prevent rtpbasedepayload from dropping the event now
+       * that we have made sure the lost packet was not FEC */
+      gst_structure_remove_field (s, "might-have-been-fec");
+    }
+    if (send_lost_event)
+      GST_RTP_BASE_DEPAYLOAD_CLASS (gst_rtp_vp9_depay_parent_class)
+          ->packet_lost (GST_RTP_BASE_DEPAYLOAD_CAST (self),
+          self->last_lost_event);
+
+    gst_event_replace (&self->last_lost_event, NULL);
+  }
+}
+
 static GstBuffer *
 gst_rtp_vp9_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
 {
@@ -118,6 +185,7 @@ gst_rtp_vp9_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
   guint hdrsize = 1;
   guint size;
   gint spatial_layer = 0;
+  guint picture_id = PICTURE_ID_NONE;
   gboolean i_bit, p_bit, l_bit, f_bit, b_bit, e_bit, v_bit;
 
   if (G_UNLIKELY (GST_BUFFER_IS_DISCONT (rtp->buffer))) {
@@ -141,14 +209,6 @@ gst_rtp_vp9_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
   e_bit = (data[0] & 0x04) != 0;
   v_bit = (data[0] & 0x02) != 0;
 
-  if (G_UNLIKELY (!self->started)) {
-    /* Check if this is the start of a VP9 layer frame, otherwise bail */
-    if (!b_bit)
-      goto done;
-
-    self->started = TRUE;
-  }
-
   GST_TRACE_OBJECT (self, "IPLFBEV : %d%d%d%d%d%d%d", i_bit, p_bit, l_bit,
       f_bit, b_bit, e_bit, v_bit);
 
@@ -157,11 +217,13 @@ gst_rtp_vp9_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
     hdrsize++;
     if (G_UNLIKELY (size < hdrsize + 1))
       goto too_small;
+    picture_id = data[1];
     /* Check M for 15 bits PictureID */
     if ((data[1] & 0x80) != 0) {
       hdrsize++;
       if (G_UNLIKELY (size < hdrsize + 1))
         goto too_small;
+      picture_id = (picture_id << 8) | data[2];
     }
   }
 
@@ -245,25 +307,51 @@ gst_rtp_vp9_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
     hdrsize += sssize;
   }
 
-  GST_DEBUG_OBJECT (depay, "hdrsize %u, size %u", hdrsize, size);
+  GST_DEBUG_OBJECT (depay, "hdrsize %u, size %u, picture id 0x%x",
+      hdrsize, size, picture_id);
 
   if (G_UNLIKELY (hdrsize >= size))
     goto too_small;
 
+  if (G_UNLIKELY (!self->started)) {
+    /* Check if this is the start of a VP9 layer frame, otherwise bail */
+    if (!b_bit) {
+      GST_DEBUG_OBJECT (depay,
+          "The layer is missing the first packets, ignoring the packet");
+      if (self->stop_lost_events) {
+        send_last_lost_event (self);
+        self->stop_lost_events = FALSE;
+      }
+      goto done;
+    }
+
+    GST_DEBUG_OBJECT (depay, "Found the start of the layer");
+    if (self->stop_lost_events) {
+      send_last_lost_event_if_needed (self, picture_id);
+      self->stop_lost_events = FALSE;
+    }
+    self->started = TRUE;
+  }
+
   payload = gst_rtp_buffer_get_payload_subbuffer (rtp, hdrsize, -1);
-  {
+  if (GST_LEVEL_MEMDUMP <= gst_debug_category_get_threshold (GST_CAT_DEFAULT)) {
     GstMapInfo map;
     gst_buffer_map (payload, &map, GST_MAP_READ);
     GST_MEMDUMP_OBJECT (self, "vp9 payload", map.data, 16);
     gst_buffer_unmap (payload, &map);
   }
   gst_adapter_push (self->adapter, payload);
+  self->last_picture_id = picture_id;
 
   /* Marker indicates that it was the last rtp packet for this frame */
   if (gst_rtp_buffer_get_marker (rtp)) {
     GstBuffer *out;
     gboolean key_frame_first_layer = !p_bit && spatial_layer == 0;
 
+    GST_DEBUG_OBJECT (depay,
+        "Found the end of the frame (%" G_GSIZE_FORMAT " bytes)",
+        gst_adapter_available (self->adapter));
+
     if (gst_adapter_available (self->adapter) < 10)
       goto too_small;
 
@@ -317,6 +405,8 @@ gst_rtp_vp9_depay_process (GstRTPBaseDepayload * depay, GstRTPBuffer * rtp)
       }
     }
 
+    if (picture_id != PICTURE_ID_NONE)
+      self->stop_lost_events = TRUE;
     return out;
   }
 
@@ -341,6 +431,9 @@ gst_rtp_vp9_depay_change_state (GstElement * element, GstStateChange transition)
       self->last_width = -1;
       self->last_height = -1;
       self->caps_sent = FALSE;
+      self->last_picture_id = PICTURE_ID_NONE;
+      gst_event_replace (&self->last_lost_event, NULL);
+      self->stop_lost_events = FALSE;
       break;
     default:
       break;
@@ -360,6 +453,9 @@ gst_rtp_vp9_depay_handle_event (GstRTPBaseDepayload * depay, GstEvent * event)
     case GST_EVENT_FLUSH_STOP:
       self->last_width = -1;
       self->last_height = -1;
+      self->last_picture_id = PICTURE_ID_NONE;
+      gst_event_replace (&self->last_lost_event, NULL);
+      self->stop_lost_events = FALSE;
       break;
     default:
       break;
@@ -370,6 +466,36 @@ gst_rtp_vp9_depay_handle_event (GstRTPBaseDepayload * depay, GstEvent * event)
       (gst_rtp_vp9_depay_parent_class)->handle_event (depay, event);
 }
 
+static gboolean
+gst_rtp_vp9_depay_packet_lost (GstRTPBaseDepayload * depay, GstEvent * event)
+{
+  GstRtpVP9Depay *self = GST_RTP_VP9_DEPAY (depay);
+  const GstStructure *s;
+  gboolean might_have_been_fec;
+
+  s = gst_event_get_structure (event);
+
+  if (self->stop_lost_events) {
+    if (gst_structure_get_boolean (s, "might-have-been-fec",
+            &might_have_been_fec)
+        && might_have_been_fec) {
+      GST_DEBUG_OBJECT (depay, "Stopping lost event %" GST_PTR_FORMAT, event);
+      gst_event_replace (&self->last_lost_event, event);
+      return TRUE;
+    }
+  } else if (self->last_picture_id != PICTURE_ID_NONE) {
+    GstStructure *s = gst_event_writable_structure (self->last_lost_event);
+
+    /* We are currently processing a picture, let's make sure the
+     * base depayloader doesn't drop this lost event */
+    gst_structure_remove_field (s, "might-have-been-fec");
+  }
+
+  return
+      GST_RTP_BASE_DEPAYLOAD_CLASS
+      (gst_rtp_vp9_depay_parent_class)->packet_lost (depay, event);
+}
+
 gboolean
 gst_rtp_vp9_depay_plugin_init (GstPlugin * plugin)
 {
index 6f783ab..2483068 100644 (file)
@@ -59,7 +59,16 @@ struct _GstRtpVP9Depay
   gint ss_height;
   gint last_width;
   gint last_height;
+  guint last_picture_id;
+  GstEvent *last_lost_event;
   gboolean caps_sent;
+  /* In between pictures, we might store GstRTPPacketLost events instead
+   * of forwarding them immediately, we check upon reception of a new
+   * picture id whether a gap was introduced, in which case we do forward
+   * the event. This is to avoid forwarding spurious lost events for FEC
+   * packets.
+   */
+  gboolean stop_lost_events;
 };
 
 GType gst_rtp_vp9_depay_get_type (void);
index 666bed5..4411289 100644 (file)
       g_memdup (vp8_bitstream_payload, sizeof (vp8_bitstream_payload)), \
       sizeof (vp8_bitstream_payload))
 
+static guint8 intra_picid6336_seqnum0[] = {
+  0x80, 0xe0, 0x00, 0x00, 0x9a, 0xbb, 0xe3, 0xb3, 0x8b, 0xe9, 0x1d, 0x61,
+  0x90, 0x80, 0x98, 0xc0, 0xf0, 0x07, 0x00, 0x9d, 0x01, 0x2a, 0xb0, 0x00,
+  0x90, 0x00, 0x06, 0x47, 0x08, 0x85, 0x85, 0x88, 0x99, 0x84, 0x88, 0x21,
+};
+
+static guint8 intra_picid24_seqnum0[] = {
+  0x80, 0xe0, 0x00, 0x00, 0x9a, 0xbb, 0xe3, 0xb3, 0x8b, 0xe9, 0x1d, 0x61,
+  0x90, 0x80, 0x18, 0xf0, 0x07, 0x00, 0x9d, 0x01, 0x2a, 0xb0, 0x00,
+  0x90, 0x00, 0x06, 0x47, 0x08, 0x85, 0x85, 0x88, 0x99, 0x84, 0x88, 0x21,
+};
+
+static guint8 intra_nopicid_seqnum0[] = {
+  0x80, 0xe0, 0x00, 0x00, 0x9a, 0xbb, 0xe3, 0xb3, 0x8b, 0xe9, 0x1d, 0x61,
+  0x90, 0x00, 0xf0, 0x07, 0x00, 0x9d, 0x01, 0x2a, 0xb0, 0x00,
+  0x90, 0x00, 0x06, 0x47, 0x08, 0x85, 0x85, 0x88, 0x99, 0x84, 0x88, 0x21,
+};
+
+static GstBuffer *
+create_rtp_vp8_buffer (guint seqnum, guint picid, gint picid_bits,
+    GstClockTime buf_pts)
+{
+  GstBuffer *ret;
+  guint8 *packet;
+  gsize size;
+
+  g_assert (picid_bits == 0 || picid_bits == 7 || picid_bits == 15);
+
+  if (picid_bits == 0) {
+    size = sizeof (intra_nopicid_seqnum0);
+    packet = g_memdup (intra_nopicid_seqnum0, size);
+  } else if (picid_bits == 7) {
+    size = sizeof (intra_picid24_seqnum0);
+    packet = g_memdup (intra_picid24_seqnum0, size);
+    packet[14] = picid & 0x7f;
+  } else {
+    size = sizeof (intra_picid6336_seqnum0);
+    packet = g_memdup (intra_picid6336_seqnum0, size);
+    packet[14] = ((picid >> 8) & 0xff) | 0x80;
+    packet[15] = (picid >> 0) & 0xff;
+  }
+
+  packet[2] = (seqnum >> 8) & 0xff;
+  packet[3] = (seqnum >> 0) & 0xff;
+
+  ret = gst_buffer_new_wrapped (packet, size);
+  GST_BUFFER_PTS (ret) = buf_pts;
+  return ret;
+}
+
 static void
 add_vp8_meta (GstBuffer * buffer, gboolean use_temporal_scaling,
     gboolean layer_sync, guint layer_id, guint tl0picidx)
@@ -445,6 +495,107 @@ GST_START_TEST (test_pay_tl0picidx_split_buffer)
 
 GST_END_TEST;
 
+typedef struct _DepayGapEventTestData
+{
+  gint seq_num;
+  gint picid;
+  gint picid_bits;
+} DepayGapEventTestData;
+
+static void
+test_depay_gap_event_base (const DepayGapEventTestData * data,
+    gboolean send_lost_event, gboolean expect_gap_event)
+{
+  GstEvent *event;
+  GstClockTime pts = 0;
+  GstHarness *h = gst_harness_new ("rtpvp8depay");
+  gst_harness_set_src_caps_str (h, RTP_VP8_CAPS_STR);
+
+  gst_harness_push (h, create_rtp_vp8_buffer (data[0].seq_num, data[0].picid,
+          data[0].picid_bits, pts));
+  pts += 33 * GST_MSECOND;
+
+  /* Preparation before pushing gap event. Getting rid of all events which
+   * came by this point - segment, caps, etc */
+  for (gint i = 0; i < 3; i++)
+    gst_event_unref (gst_harness_pull_event (h));
+  fail_unless_equals_int (gst_harness_events_in_queue (h), 0);
+
+  if (send_lost_event) {
+    gst_harness_push_event (h,
+        gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM,
+            gst_structure_new ("GstRTPPacketLost", "timestamp", G_TYPE_UINT64,
+                pts, "duration", G_TYPE_UINT64, 33 * GST_MSECOND,
+                "might-have-been-fec", G_TYPE_BOOLEAN, TRUE, NULL)));
+    pts += 33 * GST_MSECOND;
+  }
+
+  gst_harness_push (h, create_rtp_vp8_buffer (data[1].seq_num, data[1].picid,
+          data[1].picid_bits, pts));
+  fail_unless_equals_int (2, gst_harness_buffers_received (h));
+
+  if (expect_gap_event) {
+    /* Making sure the GAP event was pushed downstream */
+    event = gst_harness_pull_event (h);
+    fail_unless_equals_string ("gap",
+        gst_event_type_get_name (GST_EVENT_TYPE (event)));
+    gst_event_unref (event);
+  }
+  fail_unless_equals_int (gst_harness_events_in_queue (h), 0);
+
+  gst_harness_teardown (h);
+}
+
+/* Packet loss + no loss in picture ids */
+static const DepayGapEventTestData stop_gap_events_test_data[][2] = {
+  /* 7bit picture ids */
+  {{100, 24, 7}, {102, 25, 7}},
+
+  /* 15bit picture ids */
+  {{100, 250, 15}, {102, 251, 15}},
+
+  /* 7bit picture ids wrap */
+  {{100, 127, 7}, {102, 0, 7}},
+
+  /* 15bit picture ids wrap */
+  {{100, 32767, 15}, {102, 0, 15}},
+
+  /* 7bit to 15bit picture id */
+  {{100, 127, 7}, {102, 128, 15}},
+};
+
+GST_START_TEST (test_depay_stop_gap_events)
+{
+  test_depay_gap_event_base (&stop_gap_events_test_data[__i__][0], TRUE, FALSE);
+}
+
+GST_END_TEST;
+
+/* Packet loss + lost picture ids */
+static const DepayGapEventTestData resend_gap_event_test_data[][2] = {
+  /* 7bit picture ids */
+  {{100, 24, 7}, {102, 26, 7}},
+
+  /* 15bit picture ids */
+  {{100, 250, 15}, {102, 252, 15}},
+
+  /* 7bit picture ids wrap */
+  {{100, 127, 7}, {102, 1, 7}},
+
+  /* 15bit picture ids wrap */
+  {{100, 32767, 15}, {102, 1, 15}},
+
+  /* 7bit to 15bit picture id */
+  {{100, 126, 7}, {102, 129, 15}},
+};
+
+GST_START_TEST (test_depay_resend_gap_event)
+{
+  test_depay_gap_event_base (&resend_gap_event_test_data[__i__][0], TRUE, TRUE);
+}
+
+GST_END_TEST;
+
 static Suite *
 rtpvp8_suite (void)
 {
@@ -462,6 +613,10 @@ rtpvp8_suite (void)
       G_N_ELEMENTS (with_meta_test_data));
   tcase_add_test (tc_chain, test_pay_continuous_picture_id_and_tl0picidx);
   tcase_add_test (tc_chain, test_pay_tl0picidx_split_buffer);
+  tcase_add_loop_test (tc_chain, test_depay_stop_gap_events, 0,
+      G_N_ELEMENTS (stop_gap_events_test_data));
+  tcase_add_loop_test (tc_chain, test_depay_resend_gap_event, 0,
+      G_N_ELEMENTS (resend_gap_event_test_data));
 
   return s;
 }
index 123069e..f50ce3b 100644 (file)
@@ -25,7 +25,6 @@
 #define RTP_VP9_CAPS_STR \
   "application/x-rtp,media=video,encoding-name=VP9,clock-rate=90000,payload=96"
 
-
 GST_START_TEST (test_depay_flexible_mode)
 {
   /* b-bit, e-bit, f-bit and marker bit set */
@@ -96,17 +95,266 @@ GST_START_TEST (test_depay_non_flexible_mode)
 
 GST_END_TEST;
 
+static guint8 intra_picid6336_seqnum0[] = {
+  0x80, 0xf4, 0x00, 0x00, 0x49, 0x88, 0xd9, 0xf8, 0xa0, 0x6c, 0x65, 0x6c,
+  0x8c, 0x98, 0xc0, 0x87, 0x01, 0x02, 0x49, 0x3f, 0x1c, 0x12, 0x0e, 0x0c,
+  0xd0, 0x1b, 0xb9, 0x80, 0x80, 0xb0, 0x18, 0x0f, 0xa6, 0x4d, 0x01, 0xa5
+};
+
+static guint8 intra_picid24_seqnum0[] = {
+  0x80, 0xf4, 0x00, 0x00, 0x49, 0x88, 0xd9, 0xf8, 0xa0, 0x6c, 0x65, 0x6c,
+  0x8c, 0x18, 0x87, 0x01, 0x02, 0x49, 0x3f, 0x1c, 0x12, 0x0e, 0x0c,
+  0xd0, 0x1b, 0xb9, 0x80, 0x80, 0xb0, 0x18, 0x0f, 0xa6, 0x4d, 0x01, 0xa5
+};
+
+static guint8 intra_nopicid_seqnum0[] = {
+  0x80, 0xf4, 0x00, 0x00, 0x49, 0x88, 0xd9, 0xf8, 0xa0, 0x6c, 0x65, 0x6c,
+  0x0c, 0x87, 0x01, 0x02, 0x49, 0x3f, 0x1c, 0x12, 0x0e, 0x0c,
+  0xd0, 0x1b, 0xb9, 0x80, 0x80, 0xb0, 0x18, 0x0f, 0xa6, 0x4d, 0x01, 0xa5
+};
+
+enum
+{
+  BT_PLAIN_PICID_NONE,
+  BT_PLAIN_PICID_7,
+  BT_PLAIN_PICID_15,
+  /* Commented out for now, until added VP9 equvivalents.
+     BT_TS_PICID_NONE,
+     BT_TS_PICID_7,
+     BT_TS_PICID_15,
+     BT_TS_PICID_7_NO_TLOPICIDX,
+     BT_TS_PICID_7_NO_TID_Y_KEYIDX
+   */
+};
+
+static GstBuffer *
+create_rtp_vp9_buffer_full (guint seqnum, guint picid, guint buffer_type,
+    GstClockTime buf_pts, gboolean B_bit_start_of_frame, gboolean marker_bit)
+{
+  static struct BufferTemplate
+  {
+    guint8 *template;
+    gsize size;
+    gint picid_bits;
+  } templates[] = {
+    {
+    intra_nopicid_seqnum0, sizeof (intra_nopicid_seqnum0), 0}
+    , {
+    intra_picid24_seqnum0, sizeof (intra_picid24_seqnum0), 7}
+    , {
+    intra_picid6336_seqnum0, sizeof (intra_picid6336_seqnum0), 15}
+    ,
+        /*
+           { intra_nopicid_seqnum0_tl1_sync_tl0picidx12,
+           sizeof (intra_nopicid_seqnum0_tl1_sync_tl0picidx12),
+           0
+           },
+           { intra_picid24_seqnum0_tl1_sync_tl0picidx12,
+           sizeof (intra_picid24_seqnum0_tl1_sync_tl0picidx12),
+           7
+           },
+           { intra_picid6336_seqnum0_tl1_sync_tl0picidx12,
+           sizeof (intra_picid6336_seqnum0_tl1_sync_tl0picidx12),
+           15
+           },
+           { intra_picid24_seqnum0_tl1_sync_no_tl0picidx,
+           sizeof (intra_picid24_seqnum0_tl1_sync_no_tl0picidx),
+           7
+           },
+           { intra_picid24_seqnum0_notyk_tl0picidx12,
+           sizeof (intra_picid24_seqnum0_notyk_tl0picidx12),
+           7
+           }
+         */
+  };
+  struct BufferTemplate *template = &templates[buffer_type];
+  guint8 *packet = g_memdup (template->template, template->size);
+  GstBuffer *ret;
+
+  packet[2] = (seqnum >> 8) & 0xff;
+  packet[3] = (seqnum >> 0) & 0xff;
+
+  /* We're forcing the E-bit (EndOfFrame) together with the RTP marker bit here, which is a bit of a hack.
+   * If we're to enable spatial scalability tests, we need to take that into account when setting the E bit.
+   */
+  if (marker_bit) {
+    packet[1] |= 0x80;
+    packet[12] |= 0x4;
+  } else {
+    packet[1] &= ~0x80;
+    packet[12] &= ~0x4;
+  }
+
+  if (B_bit_start_of_frame)
+    packet[12] |= 0x8;
+  else
+    packet[12] &= ~0x8;
+
+  if (template->picid_bits == 7) {
+    /* Prerequisites for this to be correct:
+       ((packet[12] & 0x80) == 0x80); I bit set
+     */
+    g_assert ((packet[12] & 0x80) == 0x80);
+    packet[13] = picid & 0x7f;
+
+  } else if (template->picid_bits == 15) {
+    /* Prerequisites for this to be correct:
+       ((packet[12] & 0x80) == 0x80); I bit set
+     */
+    g_assert ((packet[12] & 0x80) == 0x80);
+    packet[13] = ((picid >> 8) & 0xff) | 0x80;
+    packet[14] = (picid >> 0) & 0xff;
+  }
+
+  ret = gst_buffer_new_wrapped (packet, template->size);
+  GST_BUFFER_PTS (ret) = buf_pts;
+  return ret;
+}
+
+static GstBuffer *
+create_rtp_vp9_buffer (guint seqnum, guint picid, guint buffer_type,
+    GstClockTime buf_pts)
+{
+  return create_rtp_vp9_buffer_full (seqnum, picid, buffer_type, buf_pts, TRUE,
+      TRUE);
+}
+
+typedef struct _DepayGapEventTestData
+{
+  gint seq_num;
+  gint picid;
+  guint buffer_type;
+} DepayGapEventTestData;
+
+typedef struct
+{
+  gint seq_num;
+  gint picid;
+  guint buffer_type;
+  gboolean s_bit;
+  gboolean marker_bit;
+} DepayGapEventTestDataFull;
+
+static void
+test_depay_gap_event_base (const DepayGapEventTestData * data,
+    gboolean send_lost_event, gboolean expect_gap_event, int iter)
+{
+  GstEvent *event;
+  GstClockTime pts = 0;
+  GstHarness *h = gst_harness_new ("rtpvp9depay");
+  if (send_lost_event == FALSE && expect_gap_event) {
+    /* Expect picture ID gaps to be concealed, so tell the element to do so. */
+    g_object_set (h->element, "hide-picture-id-gap", TRUE, NULL);
+  }
+  gst_harness_set_src_caps_str (h, RTP_VP9_CAPS_STR);
+
+  gst_harness_push (h, create_rtp_vp9_buffer (data[0].seq_num, data[0].picid,
+          data[0].buffer_type, pts));
+  pts += 33 * GST_MSECOND;
+
+  /* Preparation before pushing gap event. Getting rid of all events which
+   * came by this point - segment, caps, etc */
+  for (gint i = 0; i < 3; i++)
+    gst_event_unref (gst_harness_pull_event (h));
+  fail_unless_equals_int (gst_harness_events_in_queue (h), 0);
+
+  if (send_lost_event) {
+    gst_harness_push_event (h,
+        gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM,
+            gst_structure_new ("GstRTPPacketLost", "timestamp", G_TYPE_UINT64,
+                pts, "duration", G_TYPE_UINT64, 33 * GST_MSECOND,
+                "might-have-been-fec", G_TYPE_BOOLEAN, TRUE, NULL)));
+    pts += 33 * GST_MSECOND;
+  }
+
+  gst_harness_push (h, create_rtp_vp9_buffer (data[1].seq_num, data[1].picid,
+          data[1].buffer_type, pts));
+  fail_unless_equals_int (2, gst_harness_buffers_received (h));
+
+  if (expect_gap_event) {
+    gboolean noloss = FALSE;
+
+    /* Making sure the GAP event was pushed downstream */
+    event = gst_harness_pull_event (h);
+    fail_unless_equals_string ("gap",
+        gst_event_type_get_name (GST_EVENT_TYPE (event)));
+    gst_structure_get_boolean (gst_event_get_structure (event),
+        "no-packet-loss", &noloss);
+
+    /* If we didn't send GstRTPPacketLost event, the gap
+     * event should indicate that with 'no-packet-loss' parameter */
+    fail_unless_equals_int (noloss, !send_lost_event);
+    gst_event_unref (event);
+  }
+
+  fail_unless_equals_int (gst_harness_events_in_queue (h), 0);
+
+  gst_harness_teardown (h);
+}
+
+static const DepayGapEventTestData stop_gap_events_test_data[][2] = {
+  /* 7bit picture ids */
+  {{100, 24, BT_PLAIN_PICID_7}, {102, 25, BT_PLAIN_PICID_7}},
+
+  /* 15bit picture ids */
+  {{100, 250, BT_PLAIN_PICID_15}, {102, 251, BT_PLAIN_PICID_15}},
+
+  /* 7bit picture ids wrap */
+  {{100, 127, BT_PLAIN_PICID_7}, {102, 0, BT_PLAIN_PICID_7}},
+
+  /* 15bit picture ids wrap */
+  {{100, 32767, BT_PLAIN_PICID_15}, {102, 0, BT_PLAIN_PICID_15}},
+
+  /* 7bit to 15bit picture id */
+  {{100, 127, BT_PLAIN_PICID_7}, {102, 128, BT_PLAIN_PICID_15}},
+};
+
+GST_START_TEST (test_depay_stop_gap_events)
+{
+  test_depay_gap_event_base (&stop_gap_events_test_data[__i__][0], TRUE, FALSE,
+      __i__);
+}
+
+GST_END_TEST;
+
+/* Packet loss + lost picture ids */
+static const DepayGapEventTestData resend_gap_event_test_data[][2] = {
+  /* 7bit picture ids */
+  {{100, 24, BT_PLAIN_PICID_7}, {102, 26, BT_PLAIN_PICID_7}},
+
+  /* 15bit picture ids */
+  {{100, 250, BT_PLAIN_PICID_15}, {102, 252, BT_PLAIN_PICID_15}},
+
+  /* 7bit picture ids wrap */
+  {{100, 127, BT_PLAIN_PICID_7}, {102, 1, BT_PLAIN_PICID_7}},
 
+  /* 15bit picture ids wrap */
+  {{100, 32767, BT_PLAIN_PICID_15}, {102, 1, BT_PLAIN_PICID_15}},
+
+  /* 7bit to 15bit picture id */
+  {{100, 126, BT_PLAIN_PICID_7}, {102, 129, BT_PLAIN_PICID_15}},
+};
+
+GST_START_TEST (test_depay_resend_gap_event)
+{
+  test_depay_gap_event_base (&resend_gap_event_test_data[__i__][0], TRUE, TRUE,
+      __i__);
+}
+
+GST_END_TEST;
 
 static Suite *
 rtpvp9_suite (void)
 {
   Suite *s = suite_create ("rtpvp9");
   TCase *tc_chain;
-
   suite_add_tcase (s, (tc_chain = tcase_create ("vp9depay")));
   tcase_add_test (tc_chain, test_depay_flexible_mode);
   tcase_add_test (tc_chain, test_depay_non_flexible_mode);
+  tcase_add_loop_test (tc_chain, test_depay_stop_gap_events, 0,
+      G_N_ELEMENTS (stop_gap_events_test_data));
+  tcase_add_loop_test (tc_chain, test_depay_resend_gap_event, 0,
+      G_N_ELEMENTS (resend_gap_event_test_data));
 
   return s;
 }