Closed Bug 1188238 Opened 7 years ago Closed 7 years ago

Frames with a duration of 0 could be improperly inserted or cause later insertion to be incorrect


(Core :: Audio/Video: Playback, defect, P2)




Tracking Status
firefox42 --- fixed
firefox43 --- fixed


(Reporter: jya, Assigned: jya)


(Blocks 1 open bug)



(2 files)

Encountered a DASH video in bug 1186149 where the first three frames demuxed have a size of 0.

They got me thinking on how those could mess the current frame handling inside the track buffer.

If those frames are found right at the beginning and within a block of other frames, then they will be properly inserted.

However, when searching where to insert our next block of frame, if we have detected a discontinuity, we rely on finding the first frame intersecting our block of new frames.

If that frame happened to have a duration of 0, the time interval they cover is empty, and as such they will never intersect with anything.

What would occur here, is the next block of frame would be inserted *after* that particular frame (which also happen to be a key frame).
It would render all frames depending on that 0s frame undecodable.

Need to find a new algorithm or modify TimeIntervals so it can handle to interval of 0 duration.
Assignee: nobody → jyavenard
Priority: -- → P2
With H264, often the first frame of a media segment has no duration ; as such the time interval it represents is empty and will never intersect with anything.
Attachment #8659757 - Flags: review?(gsquelart)
Instead return an error which will terminate the video element.
Attachment #8659758 - Flags: review?(gsquelart)
Attachment #8659757 - Flags: review?(gsquelart) → review+
Attachment #8659758 - Flags: review?(gsquelart) → review+
Blocks: 1197083
Duplicate of this bug: 1199676
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8659757 [details] [diff] [review]
[MSE] P1. Don't use Interval::Intersect to find the first frame of an interval.

Approval is for the two patches

Approval Request Comment
[Feature/regressing bug #]:1171330
[User impact if declined]: Crashes ; stall or YouTube reverting to Flash
[Describe test coverage new/current, TreeHerder]: in central, manual test
[Risks and why]: Very low. We now return an error rather than crashing. We also now better calculate where to insert our frames
[String/UUID change made/needed]: None
Attachment #8659757 - Flags: approval-mozilla-aurora?
Not sure what's the policy is about the MSE blanker approval ; if it's something I could have simply pushed right away without explicit aurora+ .
Comment on attachment 8659757 [details] [diff] [review]
[MSE] P1. Don't use Interval::Intersect to find the first frame of an interval.

Indeed, you have to use the regular uplift process.
Attachment #8659757 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8659758 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.