SampleInfo.duration, size, cts_offset incorrect type conversion
Reported by
bbcrdd...@gmail.com,
Sep 27 2016
|
|||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.116 Safari/537.36 Example URL: See below Steps to reproduce the problem: Unfortunately I can't share anything which helps to debug this other than what is available at https://github.com/Dash-Industry-Forum/dash.js/issues/1592 What is the expected behavior? Segments containing samples with sample_duration > MAX_INT32 should be appended correctly. What went wrong? track_run_iterator.cc has a static method PopulateSampleInfo which fills a SampleInfo with information from the TrackRunBox. SampleInfo.duration is defined as in int, whereas the source value trun.sample_durations[i] is uint32. For very large values of sample_durations[i], this causes SampleInfo.duration to be interpreted as negative which, when used with MSE, causes all further samples and therefore the segment to be rejected with the error "Parsed buffers not in DTS sequence" Did this work before? N/A Is it a problem with Flash or HTML5? HTML5 Does this work in other browsers? Yes Chrome version: 53.0.2785.116 Channel: stable OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Shockwave Flash 23.0 r0 Obviously it is very unlikely that anyone would be using very large sample_duration (this was found whilst debugging another issue (https://github.com/Dash-Industry-Forum/dash.js/issues/1592)). Firefox correctly interprets the sample_duration as enormous.
,
Sep 27 2016
,
Sep 27 2016
Thanks for reporting. Details for repro from: https://github.com/Dash-Industry-Forum/dash.js/issues/1592 1. dash.js 2. mpd: http://wowza-test.streamroot.io/liveOrigin/sintel-live.smil/manifest.mpd 3. let it play a few minutes to the credits. Observe decode error and chrome://media-internals logs like: 00:13:50 654 error Parsed buffers not in DTS sequence 00:13:50 654 error Append: stream parsing failed. Data size=33697 append_window_start=0 append_window_end=inf 00:13:50 654 pipeline_error pipeline: decode error 00:13:50 654 pipeline_state kStopping 00:13:50 664 pipeline_state kStopped
,
Sep 27 2016
SampleInfo.size (should be uint32, not int) and SampleInfo.cts_offset, and TrackFragmentRun.sample_composition_time_offsets are also not always the right type, either (cts can be unsigned32 or signed32 depending on box version field).
,
Sep 27 2016
,
Sep 27 2016
,
Sep 30 2016
trun sample_composition_time_offsets unfortunately need to be treated regardless of box version as int32_t, since encoders like ffmpeg commonly do this incorrectly per spec, and very large offsets don't commonly make sense.
,
Dec 15 2016
@#7, I've queried ffmpeg upstream about probably incorrect muxing by ffmpeg when cts_offset is negative at http://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/204434.html.
,
Sep 12 2017
=> sandersd@ since he's been fixing related bits of that code recently.
,
Sep 12 2017
When I went through this code recently, the main difficulty in fixing these quickly was that (-1) is used as a default value for many of them. The correct fix will probably be to switch to base::Optional. There is at least one path where values are coming from both uint64_t and int64_t fields (specifically: DTS and edit list offset). Since we don't have a cross-platform int128_t, something more clever may be required. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dalecur...@chromium.org
, Sep 27 2016