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 751832 - dashdemux: segment list inherits segment URLs element from parent node
dashdemux: segment list inherits segment URLs element from parent node
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-07-02 11:48 UTC by Florin Apostol
Modified: 2015-10-30 15:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
unit test reproducing the problem (8.44 KB, patch)
2015-07-02 11:49 UTC, Florin Apostol
none Details | Review
proposed patch (5.19 KB, patch)
2015-07-10 15:17 UTC, Florin Apostol
committed Details | Review
unit test reproducing the problem (6.35 KB, patch)
2015-10-30 15:04 UTC, Florin Apostol
committed Details | Review

Description Florin Apostol 2015-07-02 11:48:00 UTC
The standard says:
"SegmentBase, SegmentTemplate and SegmentList shall inherit attributes and elements from the same element on a higher level. If the same attribute or element is present on both levels, the one on the lower level shall take precedence over the one on the higher level."

However, the implementation will inherit the segment URL elements from the parent node, instead of overwriting them. For example, if the Adaptation has a segment list with 2 segment URL elements and the Representation also has a segment list with 2 segment URL elements, the stream will end up with 4 segments and this can lead to a total duration of more than period's duration.

My opinion is that the gst_mpdparser_parse_segment_list_node function should replace the inherited segment urls if the parsed node also contains segment url elements.
Comment 1 Florin Apostol 2015-07-02 11:49:56 UTC
Created attachment 306606 [details] [review]
unit test reproducing the problem

attached an unit test reproducing the problem
Comment 2 Thiago Sousa Santos 2015-07-05 17:00:05 UTC
I agree, it should replace the parent segment list. Can you provide a patch with a fix to be merged with these tests?
Comment 3 Florin Apostol 2015-07-10 15:17:44 UTC
Created attachment 307231 [details] [review]
proposed patch
Comment 4 Vincent Penquerc'h 2015-09-11 14:44:14 UTC
The patch to override looks ok.

I'm a bit confused as to the test, though: the SegmentURL appears twice in the tested XML bit, but they're not in a hierarchy, they're in parallel branches stemming from an AdaptationSet node.
Comment 5 A Ashley 2015-10-05 09:15:06 UTC
(In reply to Vincent Penquerc'h from comment #4)
> I'm a bit confused as to the test, though: the SegmentURL appears twice in
> the tested XML bit, but they're not in a hierarchy, they're in parallel
> branches stemming from an AdaptationSet node.

One set is at the AdaptationSet level and another is inside the Representation element. The Representation element is a child of the AdaptationSet element.
Comment 6 Vincent Penquerc'h 2015-10-30 13:06:19 UTC
Yes, but the SegmentList also is, so that seemed odd. But since that's confirmed to be OK by the standard semantics, I'll merge it.
Comment 7 Vincent Penquerc'h 2015-10-30 14:41:33 UTC
Those onto curent master get me a unit tests failure:

elements/dash_mpd.c:4582:F:complexMPD:dash_mpdparser_multiple_inherited_segmentURL:0: 'fragment.uri' (/TestMedia2) is not equal to '"/TestMedia0"' (/TestMedia0)
Comment 8 Florin Apostol 2015-10-30 15:04:09 UTC
Created attachment 314493 [details] [review]
unit test reproducing the problem
Comment 9 Vincent Penquerc'h 2015-10-30 15:07:18 UTC
Thanks, pushed:

commit 590df23cb1e0f9267835d3fd35238783412a59a5
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Fri Oct 30 15:02:35 2015 +0000

    dashdemux: unit testing reproducing inherited segment duration overflow
    
    unit test reproducing https://bugzilla.gnome.org/show_bug.cgi?id=751832

commit 013655d886db60c0d47b1e5efccd597e48fb84f0
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Fri Oct 30 14:31:21 2015 +0000

    dashdemux: inherited segment URLs are ignored if they are defined again in a lower SegmentList
    
    According to the standard:
    "SegmentBase, SegmentTemplate and SegmentList shall inherit
    attributes and elements from the same element on a higher level.
    If the same attribute or element is present on both levels,
    the one on the lower level shall take precedence over the one
    on the higher level."
    
    gst_mpdparser_parse_segment_list_node will now discard any inherited
    segment URLs if the parsed element defines some too.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=751832