After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 751650 - dashdemux: extra validations needed when parsing the representation element
dashdemux: extra validations needed when parsing the representation element
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-06-29 13:39 UTC by Florin Apostol
Modified: 2015-10-28 15:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
check segment lists have either duration or timeline (22.36 KB, patch)
2015-09-10 16:53 UTC, Vincent Penquerc'h
none Details | Review
check segment lists have either duration or timeline (22.32 KB, patch)
2015-09-11 10:07 UTC, Vincent Penquerc'h
committed Details | Review

Description Florin Apostol 2015-06-29 13:39:20 UTC
Chapter 5.3.9.2 Segment base information mentions:

"
If the Representation contains more than one Media Segment, then either the attribute @duration or the element SegmentTimeline shall be present. The attribute @duration and the element SegmentTimeline shall not be present at the same time.
"

I created a xml with no duration and no segmentTimeline:
  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 start=\"P0Y0M0DT0H0M10S\">"
      "  <AdaptationSet mimeType=\"video/mp4\">"
      "    <Representation>"
      "      <SegmentList>"
      "        <SegmentURL"
      "           media=\"TestMedia\" mediaRange=\"100-200\""
      "           index=\"TestIndex\" indexRange=\"300-400\""
      "        ></SegmentURL>"
      "      </SegmentList>"
      "    </Representation>"
      "  </AdaptationSet>"
      "</Period>"
      "</MPD>";

It parses fine, and I can setup streaming, but the gst_mpd_client_get_next_fragment will return a fragment with duration 0:
elements/dash_mpd.c:3281:F:complexMPD:dash_mpdparser_segment_list:0: 'fragment.duration' (0) is not equal to 'expectedDuration * ((1000000 * (__extension__ (1000LL))) / (__extension__ (1000LL)))' (11000000000000)

This is obvious due to missing duration or timeline in SegmentList's MultipleSegmentBaseType.

I propose to add checks at the end of gst_mpdparser_parse_representation_node to validate that either duration or timeline was specified. This means changing the return type of gst_mpdparser_parse_representation_node, gst_mpdparser_parse_adaptation_set_node, gst_mpdparser_parse_period_node, gst_mpdparser_parse_root_node. If any will fail, the gst_mpd_parse function will return false.

Will these extra validations be accepted?
Comment 1 Vincent Penquerc'h 2015-09-10 16:53:18 UTC
Created attachment 311096 [details] [review]
check segment lists have either duration or timeline
Comment 2 Florin Apostol 2015-09-11 09:57:18 UTC
Review of attachment 311096 [details] [review]:

::: ext/dash/gstmpdparser.c
@@ +1449,3 @@
+
+  if (!has_duration && !has_timeline) {
+    GST_ERROR ("has: duration %d, timeline %d", has_duration, has_timeline);

this will always print "has: duration 0, timeline 0"
I suggest to replace it with "segment has neither duration neither timeline"

@@ +3150,1 @@
+    return ret;

remove this.
Comment 3 Vincent Penquerc'h 2015-09-11 10:07:49 UTC
Created attachment 311127 [details] [review]
check segment lists have either duration or timeline

Done.
Comment 4 Florin Apostol 2015-09-11 10:13:21 UTC
Review of attachment 311127 [details] [review]:

looks ok
Comment 5 Vincent Penquerc'h 2015-10-28 15:53:22 UTC
commit 60133b1472acebdd99628a7af92adfd781376895
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Wed Oct 28 15:34:29 2015 +0000

    mpdparser: check segment lists have either duration or timeline
    
    And add error checking along the way.
    
    Add duration where appropriate so unit tests still pass.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=751650