Created attachment 348759 [details] [review]
mpdparser: Allow inherit Segment{Base,Template} from Period

We handled this for SegmentList in bug #768460. SegmentBase and SegmentTemplate also could be inherited from Period level

Created attachment 348760 [details] [review]
mpdparser: Relex sanity condition of Segment Base Information

The existence condition of duration attribute and SegmentTimeline
should be checked in Representation level. Higher level elements
might not have them.

This bug seems to duplication of bug #763454, but the previous one seems to no action for a long time, and we can simply inherit them like bug #768460.

Thiago, can you take a look at this? You reviewed/wrote patches for the above bugs, do these patches here now fix everything?

Review of attachment 348760 [details] [review]:

I don't like how we need to modify and add this extra parameter everywhere to achieve this. Maybe it would be simpler to just split parsing and validating into 2 steps. Then we only need to do the validation at the end when the final structures are formed and we can check for any missing information required for playback.

Created attachment 349714 [details] [review]
mpdparser: Do sanity check of Segment Base Information only at Repesentation level

Spec 5.3.9.2 is saying about the existence of duration and SegmentTimeline
only for Representation level. Other level such as Period or AdaptationSet
might not have the attributes.

(In reply to Thiago Sousa Santos from comment #5)
> Review of attachment 348760 [details] [review] [review]:
> 

I found much simpler way for checking it :) Please review new patch set.

Created attachment 349762 [details] [review]
tests: dash: add tests for period segmenttemplate inheritance

This relaxing of the check doesn't catch the case where the segmenttemplate is completely specified under the Period as the test attached depicts. It would be nice to also provide more unit tests so we can confirm this works and keeps working in the long run.

My impression is that it is getting too complicated to check this during parsing. I'd just eliminate the checking during parsing (unless it is something mandatory that can't be inherited) and instead of check for the sanity of the values afterwards as a post-processing step.

(In reply to Thiago Sousa Santos from comment #9)
> My impression is that it is getting too complicated to check this during
> parsing. I'd just eliminate the checking during parsing (unless it is
> something mandatory that can't be inherited) and instead of check for the
> sanity of the values afterwards as a post-processing step.

I agree that parser seems to become complicated more and more. 
If you are willing to work about this, that sounds really great :)

I'm extending the unit tests at the moment to make sure we cover more cases, will try to get that out by the end of this week.

I'm currently working on a dash-specific validate testsuite, if you have streams that expose this issue, please tell me so I can add it to the list.

commit 22c037df6c6511859e6dfa0c627a9e768151425e
Author: Thiago Santos <thiagossantos@gmail.com>
Date:   Sat Apr 15 18:17:29 2017 -0700

    tests: dash_mpd: add some inheritance tests
    
    Tests regarding inheritance of segment template attributes

commit 68ac72431c3e68346ee918580bf41d7f0f90929d
Author: Seungha Yang <sh.yang@lge.com>
Date:   Wed Apr 12 16:58:10 2017 +0900

    mpdparser: Do sanity check of Segment Base Information only at Repesentation level
    
    Spec 5.3.9.2 is saying about the existence of duration and SegmentTimeline
    only for Representation level. Other level such as Period or AdaptationSet
    might not have the attributes.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=780570

commit 6167dab39ee0f8c683f0ab840ceaa005780cfa5e
Author: Seungha Yang <sh.yang@lge.com>
Date:   Mon Mar 27 10:06:30 2017 +0900

    mpdparser: Allow inherit Segment{Base,Template} from Period
    
    Similar to SegmentList, Representation can inherit Segment{Base,Template}
    from Period
    
    https://bugzilla.gnome.org/show_bug.cgi?id=780570

Is there a public dashstream that uses this so we can it to the dash regression suite ?

(In reply to Edward Hervey from comment #14)
> Is there a public dashstream that uses this so we can it to the dash
> regression suite ?

My issue was tested on private stream and related bug #763454 seems to be also private one.

Anyway bug #778237 seems to be similar to this issue but not exactly same. 
If we can use the url which was opened by the reporter, I expect it's also useful :)

http://akamai-progressive.irt.de/irt_avstandard_2014/M4S/1280x720p50/V-1280x720p50_libx264_high_yuv420p_gop100_bit3584k_term5-2-5-14-1/seg2_frag2_hbbtv15_live_multi/V-1280x720p50_libx264_high_yuv420p_gop100_bit3584k_term5-2-5-14-1.mpd