Closed Bug 1151299 Opened 5 years ago Closed 5 years ago

If the first frame decoding failed with WAITING_FOR_DATA, it will never re-attempt to get the first frame.

Categories

(Core :: Audio/Video, defect)

x86
Windows Vista
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox38 --- fixed
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

Attachments

(2 files)

The problem appears with patch from bug 1149278 applied.

Following bug 1149278, we never do blocking read with MSE. If a decode error occurs for the first frame, it seems that it never attempts to decode the first frame again, leading to stalls.
Only attempt to decode the first frame when we know we have data available, and don't assume that the first decoder in our array is the one with the first sample.
Attachment #8588823 - Flags: review?(matt.woodrow)
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Once a demuxer is marked as EOS it will be forever (or until the MP4Reader is flushed). More data may be added following a EOS. This prevent a stall under some circumstances (partial segment received)
Attachment #8588824 - Flags: review?(matt.woodrow)
Attachment #8588823 - Flags: review?(matt.woodrow) → review+
Attachment #8588824 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/fdd90d58cb14
https://hg.mozilla.org/mozilla-central/rev/86efbe39200a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Jean-Yves: should we uplift his fix to Beta 38 for Adobe's EME testing?
Blocks: 1032660
Flags: needinfo?(jyavenard)
OS: Mac OS X → Windows Vista
We should uplift this for EME.
Flags: needinfo?(cpearce)
Comment on attachment 8588823 [details] [diff] [review]
Part1. Only attempt to decode first frame when available

Approval Request Comment
[Feature/regressing bug #]: EME+MSE, bug 1149278.
[User impact if declined]: We need bug 1149278 to ship EME, and to make MSE safer, but the patch in there exposes a new bug where we can stall playback. Without the patches here, we can stall playback.
[Describe test coverage new/current, TreeHerder]: Lots of MSE mochitests
[Risks and why]: Seems low; fixes a stall.
[String/UUID change made/needed]: None
Attachment #8588823 - Flags: approval-mozilla-beta?
Attachment #8588823 - Flags: approval-mozilla-aurora?
Comment on attachment 8588824 [details] [diff] [review]
Part2. Clear EOS flag when new data is received

Approval Request Comment
[Feature/regressing bug #]: EME+MSE, bug 1149278.
[User impact if declined]: We need bug 1149278 to ship EME, and to make MSE safer, but the patch in there exposes a new bug where we can stall playback. Without the patches here, we can stall playback.
[Describe test coverage new/current, TreeHerder]: Lots of MSE mochitests
[Risks and why]: Seems low; fixes a stall.
[String/UUID change made/needed]: None
Attachment #8588824 - Flags: approval-mozilla-beta?
Attachment #8588824 - Flags: approval-mozilla-aurora?
Flags: needinfo?(jyavenard)
Comment on attachment 8588823 [details] [diff] [review]
Part1. Only attempt to decode first frame when available

Should be in 38 beta 3.
Attachment #8588823 - Flags: approval-mozilla-beta?
Attachment #8588823 - Flags: approval-mozilla-beta+
Attachment #8588823 - Flags: approval-mozilla-aurora?
Attachment #8588823 - Flags: approval-mozilla-aurora+
Attachment #8588824 - Flags: approval-mozilla-beta?
Attachment #8588824 - Flags: approval-mozilla-beta+
Attachment #8588824 - Flags: approval-mozilla-aurora?
Attachment #8588824 - Flags: approval-mozilla-aurora+
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.