Closed Bug 1098990 Opened 5 years ago Closed 5 years ago

Trun segments don't necessarily contain contiguous samples

Categories

(Core :: Audio/Video, defect)

29 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This happens with the 'Microsoft #2' video on the dashif player.

The trun box for this file has the sample-composition-time-offsets-present flag present, so each sample has essentially arbitrary composition time.

We end up with a collection of samples for the 0-2 second range, and a bunch around 429 seconds.

Index dump: http://pastebin.mozilla.org/7297431

Our MoofParser code assumes that all the samples must be contiguous, so it edits them to make this true at [1]. Sample 47 in the dump is ~427 seconds long because of this editing.

MP4Demuxer also assumes this, so we use the time range for the Moof rather than adding up the individual samples in the index [2]. We need to fix both of these for buffered range calculations to be correct.

I can't see anything in the spec that would require the samples to be contiguous, anyone know why we assume this?


[1] http://mxr.mozilla.org/mozilla-central/source/media/libstagefright/binding/MoofParser.cpp#224
[2] http://mxr.mozilla.org/mozilla-central/source/media/libstagefright/binding/Index.cpp#156
Note that there only appears to be one track for this video.
For boxes that inherit from 'FullBox', the flags field is only 24bits and the top 8 bits are the version.

In version 1 of a 'trun' box, the cts offset it to be parsed as signed rather than unsigned.
Assignee: nobody → matt.woodrow
Attachment #8523573 - Flags: review?(ajones)
Comment on attachment 8523573 [details] [diff] [review]
cts is signed in version 1

Review of attachment 8523573 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libstagefright/binding/MoofParser.cpp
@@ +199,5 @@
> +    if (flags & 0x800) {
> +      if (version == 0) {
> +        ctsOffset = reader->ReadU32();
> +      } else {
> +        ctsOffset = (int32_t)reader->ReadU32();

Please add reader->Read32() to ByteReader rather than casting here.
Attachment #8523573 - Flags: review?(ajones) → review+
https://hg.mozilla.org/mozilla-central/rev/5492e9dc06f1
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.