hlsdemux: Detect media sequence number inconsistencies and fail

Without failing, we would play back random parts of the stream which is
arguably a worse user experience, and failing is also recommended by the
spec here.

And also handle live streams without any media sequence numbers at all
properly, that is, make sure the sequence numbers are increasing instead
of starting again at 0 every time.

https://bugzilla.gnome.org/show_bug.cgi?id=775665

1 files changed, 170 insertions, 8 deletions

diff --git a/ext/hls/m3u8.c b/ext/hls/m3u8.c
index 788743f68..76cf50f30 100644
--- a/ext/hls/m3u8.c
+++ b/ext/hls/m3u8.c
@@ -306,6 +306,141 @@ gst_hls_variant_stream_compare_by_bitrate (gconstpointer a, gconstpointer b)
   return vs_a->bandwidth - vs_b->bandwidth;
 }
 
+static gboolean
+gst_m3u8_update_check_consistent_media_seqnums (GstM3U8 * self,
+    gboolean have_mediasequence, GList * previous_files)
+{
+  if (!previous_files)
+    return TRUE;
+
+  /* If we have MEDIA-SEQUENCE, ensure that it's consistent. If it is not,
+   * the client SHOULD halt playback (6.3.4), which is what we do then.
+   *
+   * If we don't have MEDIA-SEQUENCE, we check URIs in the previous and
+   * current playlist to calculate the/a correct MEDIA-SEQUENCE for the new
+   * playlist in relation to the old. That is, same URIs get the same number
+   * and later URIs get higher numbers */
+  if (have_mediasequence) {
+    GList *l, *m;
+    GstM3U8MediaFile *f1 = NULL, *f2 = NULL;
+
+    /* Find first case of higher/equal sequence number in new playlist or
+     * same URI. From there on we can linearly step ahead */
+    for (l = self->files; l; l = l->next) {
+      gboolean match = FALSE;
+
+      f1 = l->data;
+      for (m = previous_files; m; m = m->next) {
+        f2 = m->data;
+
+        if (f1->sequence >= f2->sequence || g_str_equal (f1->uri, f2->uri)) {
+          match = TRUE;
+          break;
+        }
+      }
+      if (match)
+        break;
+    }
+
+    if (!l) {
+      /* No match, no sequence in the new playlist was higher than
+       * any in the old, and no URI was found again. This is bad! */
+      GST_ERROR ("Media sequences inconsistent, ignoring");
+      return FALSE;
+    } else {
+      g_assert (f1 != NULL);
+      g_assert (f2 != NULL);
+
+      for (; l && m; l = l->next, m = m->next) {
+        f1 = l->data;
+        f2 = m->data;
+
+        if (f1->sequence == f2->sequence) {
+          if (!g_str_equal (f1->uri, f2->uri)) {
+            /* Same sequence, different URI. This is bad! */
+            GST_ERROR ("Media sequences inconsistent, ignoring");
+            return FALSE;
+          } else {
+            /* Good case, we advance and check the next one */
+          }
+        } else if (g_str_equal (f1->uri, f2->uri)) {
+          /* Same URIs but different sequences, this is bad! */
+          GST_ERROR ("Media sequences inconsistent, ignoring");
+          return FALSE;
+        } else {
+          /* Not same URI, not same sequence but by construction sequence
+           * must be higher in the new one. All good in that case, if it
+           * isn't then this means that sequence numbers are decreasing
+           * or files were inserted */
+          if (f1->sequence < f2->sequence) {
+            GST_ERROR ("Media sequences inconsistent, ignoring");
+            return FALSE;
+          }
+        }
+      }
+
+      /* All good if we're getting here */
+    }
+  } else {
+    GList *l, *m;
+    GstM3U8MediaFile *f1 = NULL, *f2 = NULL;
+    gint64 mediasequence;
+
+    for (l = self->files; l; l = l->next) {
+      gboolean match = FALSE;
+
+      f1 = l->data;
+      for (m = previous_files; m; m = m->next) {
+        f2 = m->data;
+
+        if (g_str_equal (f1->uri, f2->uri)) {
+          match = TRUE;
+          break;
+        }
+      }
+
+      if (match)
+        break;
+    }
+
+    if (!l) {
+      /* No match, this means f2 is the last item in the previous playlist
+       * and we have to start our new playlist at that sequence */
+      mediasequence = f2->sequence + 1;
+
+      for (l = self->files; l; l = l->next) {
+        f1 = l->data;
+        f1->sequence = mediasequence;
+        mediasequence++;
+      }
+    } else {
+      /* Match, check that all following ones are matching too and continue
+       * sequence numbers from there on */
+
+      mediasequence = f2->sequence;
+
+      for (; l; l = l->next) {
+        f1 = l->data;
+        f2 = m ? m->data : NULL;
+
+        f1->sequence = mediasequence;
+        mediasequence++;
+
+        if (f2) {
+          if (!g_str_equal (f1->uri, f2->uri)) {
+            GST_WARNING ("Inconsistent URIs after playlist update");
+          }
+        }
+
+        if (m)
+          m = m->next;
+      }
+    }
+  }
+
+  return TRUE;
+}
+
 /*
  * @data: a m3u8 playlist text data, taking ownership
  */
@@ -321,6 +456,8 @@ gst_m3u8_update (GstM3U8 * self, gchar * data)
   guint8 iv[16] = { 0, };
   gint64 size = -1, offset = -1;
   gint64 mediasequence;
+  GList *previous_files = NULL;
+  gboolean have_mediasequence = FALSE;
 
   g_return_val_if_fail (self != NULL, FALSE);
   g_return_val_if_fail (data != NULL, FALSE);
@@ -354,11 +491,8 @@ gst_m3u8_update (GstM3U8 * self, gchar * data)
   self->last_data = data;
 
   self->current_file = NULL;
-  if (self->files) {
-    g_list_foreach (self->files, (GFunc) gst_m3u8_media_file_unref, NULL);
-    g_list_free (self->files);
-    self->files = NULL;
-  }
+  previous_files = self->files;
+  self->files = NULL;
   self->duration = GST_CLOCK_TIME_NONE;
   mediasequence = 0;
 
@@ -461,8 +595,10 @@ gst_m3u8_update (GstM3U8 * self, gchar * data)
         if (int_from_string (data + 22, &data, &val))
           self->targetduration = val * GST_SECOND;
       } else if (g_str_has_prefix (data_ext_x, "MEDIA-SEQUENCE:")) {
-        if (int_from_string (data + 22, &data, &val))
+        if (int_from_string (data + 22, &data, &val)) {
           mediasequence = val;
+          have_mediasequence = TRUE;
+        }
       } else if (g_str_has_prefix (data_ext_x, "DISCONTINUITY")) {
         discontinuity = TRUE;
       } else if (g_str_has_prefix (data_ext_x, "PROGRAM-DATE-TIME:")) {
@@ -545,22 +681,48 @@ gst_m3u8_update (GstM3U8 * self, gchar * data)
   g_free (current_key);
   current_key = NULL;
 
+  self->files = g_list_reverse (self->files);
+
+  if (previous_files) {
+    gboolean consistent = gst_m3u8_update_check_consistent_media_seqnums (self,
+        have_mediasequence, previous_files);
+
+    g_list_foreach (previous_files, (GFunc) gst_m3u8_media_file_unref, NULL);
+    g_list_free (previous_files);
+    previous_files = NULL;
+
+    /* error was reported above already */
+    if (!consistent)
+      return FALSE;
+  }
+
   if (self->files == NULL) {
     GST_ERROR ("Invalid media playlist, it does not contain any media files");
     GST_M3U8_UNLOCK (self);
     return FALSE;
   }
 
-  self->files = g_list_reverse (self->files);
-
   /* calculate the start and end times of this media playlist. */
   {
     GList *walk;
     GstM3U8MediaFile *file;
     GstClockTime duration = 0;
 
+    mediasequence = -1;
+
     for (walk = self->files; walk; walk = walk->next) {
       file = walk->data;
+
+      if (mediasequence == -1) {
+        mediasequence = file->sequence;
+      } else if (mediasequence >= file->sequence) {
+        GST_ERROR ("Non-increasing media sequence");
+        GST_M3U8_UNLOCK (self);
+        return FALSE;
+      } else {
+        mediasequence = file->sequence;
+      }
+
       duration += file->duration;
       if (file->sequence > self->highest_sequence_number) {
         if (self->highest_sequence_number >= 0) {