dashdemux: corrected computation of period's duration

According to ISO/IEC 23009-1:2014(E), chapter 5.3.2.1
"The Period extends until the PeriodStart of the next Period, or until
the end of the Media Presentation in the case of the last Period."

This means that a configured value for optional attribute period duration
should be ignored if the next period contains a start attribute or it is
the last period and the MPD contains a mediaPresentationDuration attribute.

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

2 files changed, 116 insertions, 3 deletions

diff --git a/ext/dash/gstmpdparser.c b/ext/dash/gstmpdparser.c
index 482326828..003e1d5db 100644
--- a/ext/dash/gstmpdparser.c
+++ b/ext/dash/gstmpdparser.c
@@ -3391,6 +3391,12 @@ gst_mpd_client_setup_media_presentation (GstMpdClient * client)
     period_node = (GstPeriodNode *) list->data;
     if (period_node->start != -1) {
       /* we have a regular period */
+      /* start cannot be smaller than previous start */
+      if (list != g_list_first (client->mpd_node->Periods)
+          && start >= period_node->start * GST_MSECOND) {
+        /* Invalid MPD file: duration would be negative or zero */
+        goto syntax_error;
+      }
       start = period_node->start * GST_MSECOND;
     } else if (duration != GST_CLOCK_TIME_NONE) {
       /* start time inferred from previous period, this is still a regular period */
@@ -3405,13 +3411,26 @@ gst_mpd_client_setup_media_presentation (GstMpdClient * client)
       goto early;
     }
 
-    if (period_node->duration != -1) {
-      duration = period_node->duration * GST_MSECOND;
-    } else if ((next = g_list_next (list)) != NULL) {
+    /* compute duration.
+       If there is a start time for the next period, or this is the last period
+       and mediaPresentationDuration was set, those values will take precedence
+       over a configured period duration in computing this period's duration
+
+       ISO/IEC 23009-1:2014(E), chapter 5.3.2.1
+       "The Period extends until the PeriodStart of the next Period, or until
+       the end of the Media Presentation in the case of the last Period."
+     */
+    if ((next = g_list_next (list)) != NULL) {
       /* try to infer this period duration from the start time of the next period */
       GstPeriodNode *next_period_node = next->data;
       if (next_period_node->start != -1) {
+        if (start >= next_period_node->start * GST_MSECOND) {
+          /* Invalid MPD file: duration would be negative or zero */
+          goto syntax_error;
+        }
         duration = next_period_node->start * GST_MSECOND - start;
+      } else if (period_node->duration != -1) {
+        duration = period_node->duration * GST_MSECOND;
       } else if (client->mpd_node->type == GST_MPD_FILE_TYPE_DYNAMIC) {
         /* might be a live file, ignore unspecified duration */
       } else {
@@ -3420,8 +3439,14 @@ gst_mpd_client_setup_media_presentation (GstMpdClient * client)
       }
     } else if (client->mpd_node->mediaPresentationDuration != -1) {
       /* last Period of the Media Presentation */
+      if (client->mpd_node->mediaPresentationDuration * GST_MSECOND <= start) {
+        /* Invalid MPD file: duration would be negative or zero */
+        goto syntax_error;
+      }
       duration =
           client->mpd_node->mediaPresentationDuration * GST_MSECOND - start;
+    } else if (period_node->duration != -1) {
+      duration = period_node->duration * GST_MSECOND;
     } else if (client->mpd_node->type == GST_MPD_FILE_TYPE_DYNAMIC) {
       /* might be a live file, ignore unspecified duration */
     } else {
diff --git a/tests/check/elements/dash_mpd.c b/tests/check/elements/dash_mpd.c
index 2abe5b0b1..0b4a87ccd 100644
--- a/tests/check/elements/dash_mpd.c
+++ b/tests/check/elements/dash_mpd.c
@@ -500,6 +500,90 @@ GST_START_TEST (dash_mpdparser_no_default_namespace)
 GST_END_TEST;
 
 /*
+ * Test handling wrong period duration during attempts to
+ * infer a period duration from the start time of the next period
+ */
+GST_START_TEST (dash_mpdparser_wrong_period_duration_inferred_from_next_period)
+{
+  const gchar *periodName;
+
+  const gchar *xml =
+      "<?xml version=\"1.0\"?>"
+      "<MPD xmlns=\"urn:mpeg:dash:schema:mpd:2011\""
+      " profiles=\"urn:mpeg:dash:profile:isoff-main:2011\""
+      " availabilityStartTime=\"2015-03-24T0:0:0\""
+      " mediaPresentationDuration=\"P0Y0M0DT3H3M30S\">"
+      "<Period id=\"Period0\" duration=\"P0Y0M0DT1H1M0S\"></Period>"
+      "<Period id=\"Period1\"></Period>"
+      "<Period id=\"Period2\" start=\"P0Y0M0DT0H0M10S\"></Period></MPD>";
+
+  gboolean ret;
+  GstMpdClient *mpdclient = gst_mpd_client_new ();
+
+  ret = gst_mpd_parse (mpdclient, xml, (gint) strlen (xml));
+
+  assert_equals_int (ret, TRUE);
+
+  /* period_idx should be 0 and we should have no active periods */
+  assert_equals_uint64 (mpdclient->period_idx, 0);
+  fail_unless (mpdclient->periods == NULL);
+
+  /* process the xml data */
+  ret = gst_mpd_client_setup_media_presentation (mpdclient);
+  assert_equals_int (ret, TRUE);
+
+  /* Period0 should be present */
+  fail_unless (mpdclient->periods != NULL);
+  periodName = gst_mpd_client_get_period_id (mpdclient);
+  assert_equals_string (periodName, "Period0");
+
+  /* Period1 should not be present due to wrong duration */
+  ret = gst_mpd_client_set_period_index (mpdclient, 1);
+  assert_equals_int (ret, FALSE);
+
+  gst_mpd_client_free (mpdclient);
+}
+
+GST_END_TEST;
+
+/*
+ * Test handling wrong period duration during attempts to
+ * infer a period duration from the mediaPresentationDuration
+ */
+GST_START_TEST
+    (dash_mpdparser_wrong_period_duration_inferred_from_next_mediaPresentationDuration)
+{
+  const gchar *xml =
+      "<?xml version=\"1.0\"?>"
+      "<MPD xmlns=\"urn:mpeg:dash:schema:mpd:2011\""
+      " profiles=\"urn:mpeg:dash:profile:isoff-main:2011\""
+      " availabilityStartTime=\"2015-03-24T0:0:0\""
+      " mediaPresentationDuration=\"P0Y0M0DT3H3M30S\">"
+      "<Period id=\"Period0\" start=\"P0Y0M0DT4H0M0S\"></Period></MPD>";
+
+  gboolean ret;
+  GstMpdClient *mpdclient = gst_mpd_client_new ();
+
+  ret = gst_mpd_parse (mpdclient, xml, (gint) strlen (xml));
+
+  assert_equals_int (ret, TRUE);
+
+  /* period_idx should be 0 and we should have no active periods */
+  assert_equals_uint64 (mpdclient->period_idx, 0);
+  fail_unless (mpdclient->periods == NULL);
+
+  /* process the xml data
+   * should fail due to wrong duration in Period0 (start > mediaPresentationDuration)
+   */
+  ret = gst_mpd_client_setup_media_presentation (mpdclient);
+  assert_equals_int (ret, FALSE);
+
+  gst_mpd_client_free (mpdclient);
+}
+
+GST_END_TEST;
+
+/*
  * create a test suite containing all dash testcases
  */
 static Suite *
@@ -534,6 +618,10 @@ dash_suite (void)
   tcase_add_test (tc_negativeTests, dash_mpdparser_missing_mpd);
   tcase_add_test (tc_negativeTests, dash_mpdparser_no_end_tag);
   tcase_add_test (tc_negativeTests, dash_mpdparser_no_default_namespace);
+  tcase_add_test (tc_negativeTests,
+      dash_mpdparser_wrong_period_duration_inferred_from_next_period);
+  tcase_add_test (tc_negativeTests,
+      dash_mpdparser_wrong_period_duration_inferred_from_next_mediaPresentationDuration);
 
   suite_add_tcase (s, tc_simpleMPD);
   suite_add_tcase (s, tc_complexMPD);