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

RESOLVED FIXED in Firefox 29

Status

()

Core
Audio/Video
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: RyanVM, Assigned: cpearce)

Tracking

({intermittent-failure})

Trunk
mozilla30
x86
Windows XP
intermittent-failure
Points:
---

Firefox Tracking Flags

(firefox28 wontfix, firefox29 fixed, firefox30 fixed, firefox-esr24 wontfix)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=21727685&tree=Mozilla-Inbound
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 17

4 years ago
Created attachment 8384094 [details] [diff] [review]
Patch

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)
(Assignee)

Updated

4 years ago
Blocks: 973408
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+
(Assignee)

Comment 19

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/382a65f86218
(Assignee)

Comment 20

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
(Reporter)

Comment 22

4 years ago
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)
(Assignee)

Comment 23

4 years ago
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)
(Assignee)

Comment 24

4 years ago
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+
(Reporter)

Comment 26

4 years ago
<3

https://hg.mozilla.org/releases/mozilla-aurora/rev/5e935f77e615
status-firefox28: affected → wontfix
status-firefox29: affected → fixed
You need to log in before you can comment on or make changes to this bug.