Closed Bug 861136 Opened 13 years ago Closed 12 years ago

Intermittent Windows XP test_chaining.html,test_fragment_play.html,test_framebuffer.html,test_load_source.html | Test timed out.

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla30
Tracking Flags:
Tracking Status
firefox28 --- wontfix
firefox29 --- fixed
firefox30 --- fixed
firefox-esr24 --- wontfix

People

(Reporter: ryanvm, Assigned: cpearce)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Patch
1.34 KB, patch
roc
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=21702712&tree=Mozilla-Central Rev3 WINNT 5.1 mozilla-central pgo test mochitest-1 on 2013-04-11 08:29:53 PDT for push 7b8ed29c6bc0 slave: talos-r3-xp-096 15:49:59 INFO - 200402 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_chaining.html | Test timed out. 15:55:29 INFO - 201224 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_fragment_play.html | Test timed out. 16:00:59 INFO - 201232 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_framebuffer.html | Test timed out. 16:06:29 INFO - 201748 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_load_source.html | Test timed out. 16:06:29 INFO - 201749 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | 4 test timeouts, giving up. 16:06:29 INFO - 201750 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | Skipping 210 remaining tests.
Attached patch Patch — — Splinter Review
I've been debugging this, since it was high frequency with my patches in bug 973408 applied. This failure happens when we've decoded the entire media before script calls play(), and elements that are preload=metadata. When we load a preload=metadata media, we suspend the load once the media has decoded metadata. We call Decoder::Suspend(). If/when we/script calls HTMLMediaElement.play(), we check if the load was suspended after we reached the first frame, and if so we call MediaDecoder::Resume(true), which moves the state machine into BUFFERING state. However, when we've finished decoding the entire media, the state machine moves to COMPLETED state. If we reach COMPLETED state before play() is called, we'll overwrite the COMPLETED state with BUFFERING state, but we'll never be able to get out of buffering state since the decode won't advance because we're already at the end. If this happens just so, we don't run the state machine in COMPLETED state after finishing decoding the media, so we don't dispatch an "ended" event, which causes the test timeouts. This can happen in test_chaining because one of the chaining test files has total duration just less than 1 second, which is how much audio we currently buffer in advance of the current playback position. So we can decode the entire file before the loaded metadata event runs in JS (which is the thing that calls play() in test_chaining.html). So MediaDecoderStateMachine::StartBuffering() should only overwrite mState if we're in DECODING state I think. That makes the test pass at least. I bet this affects other tests which have short media and call play() manually too.
Assignee: nobody → cpearce
Status: NEW → ASSIGNED
Attachment #8384094 - Flags: review?(roc)
Comment on attachment 8384094 [details] [diff] [review] Patch Review of attachment 8384094 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/MediaDecoderStateMachine.cpp @@ +2447,5 @@ > > void MediaDecoderStateMachine::StartBuffering() > { > AssertCurrentThreadInMonitor(); > + trailing whitespace
Attachment #8384094 - Flags: review?(roc) → review+
I forgot to remove the whitespace like you asked, fixed that: https://hg.mozilla.org/integration/mozilla-inbound/rev/c1eafeaad416
https://hg.mozilla.org/mozilla-central/rev/382a65f86218 https://hg.mozilla.org/mozilla-central/rev/c1eafeaad416
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Can we please nominate this for Aurora/Beta uplift? :)
status-firefox28: --- → affected
status-firefox29: --- → affected
status-firefox30: --- → fixed
status-firefox-esr24: --- → wontfix
Flags: needinfo?(cpearce)
Comment on attachment 8384094 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug present in HTML5 video since feature first implemented. User impact if declined: random orange, angry sheriffs, and possible timeouts in scripts depending on detecting end of playback for short sounds. Testing completed (on m-c, etc.): Been on m-c for a couple of days. Risk to taking this patch (and alternatives if risky): low. String or IDL/UUID changes made by this patch: None.
Attachment #8384094 - Flags: approval-mozilla-aurora?
Flags: needinfo?(cpearce)
We can skip beta uplift, we're getting pretty close to merge day.
We want sheriffs to be happy. Approved );
Attachment #8384094 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: