Intermittent dom/media/test/test_seek-4.html | Test timed out.

RESOLVED FIXED in Firefox 53

Status

()

Core
Audio/Video: Playback
P1
normal
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: Treeherder Bug Filer, Assigned: jwwang)

Tracking

({intermittent-failure})

unspecified
mozilla54
intermittent-failure
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 unaffected, firefox53 fixed, firefox54 fixed)

Details

(Whiteboard: [stockwell fixed])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

We're getting timeouts in pretty much all the seek tests recently...
Component: Audio/Video → Audio/Video: Playback
Flags: needinfo?(jyavenard)
Flags: needinfo?(gsquelart)
(Assignee)

Comment 2

a year ago
See bug 1321198 comment 17.

Comment 3

a year ago
12 failures in 722 pushes (0.017 failures/push) were associated with this bug in the last 7 days.  

Repository breakdown:
* mozilla-inbound: 8
* autoland: 4

Platform breakdown:
* windows8-64: 12

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1331070&startday=2017-01-09&endday=2017-01-15&tree=all
Flags: needinfo?(jyavenard)
Looks the same as bug 1321198 (a timeout in test_seek-4.html) but with a different message.
I hesitate to make this bug a duplicate, so we don't lose the associated OrangeFactor data; so I'm linking them through "See Also".

JW is looking into it, see bug 1321198 comment 17.
Flags: needinfo?(gsquelart)
See Also: → bug 1321198

Comment 5

a year ago
15 failures in 115 pushes (0.13 failures/push) were associated with this bug yesterday.  

Repository breakdown:
* mozilla-inbound: 8
* autoland: 6
* graphics: 1

Platform breakdown:
* windows8-64: 15

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1331070&startday=2017-01-19&endday=2017-01-19&tree=all

Comment 6

a year ago
38 failures in 690 pushes (0.055 failures/push) were associated with this bug in the last 7 days. 

This is the #44 most frequent failure this week. 

Repository breakdown:
* mozilla-inbound: 18
* autoland: 17
* graphics: 2
* mozilla-central: 1

Platform breakdown:
* windows8-64: 38

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1331070&startday=2017-01-16&endday=2017-01-22&tree=all

Comment 7

a year ago
38 failures in 749 pushes (0.051 failures/push) were associated with this bug in the last 7 days.  

Repository breakdown:
* autoland: 20
* mozilla-inbound: 14
* graphics: 3
* mozilla-central: 1

Platform breakdown:
* windows8-64: 38

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1331070&startday=2017-01-23&endday=2017-01-29&tree=all
any further updates here?  I see bug 1321198 having no updates in the last ~10 days.  Is there any information needed to help clarify this bug?
See Also: → bug 1331281, bug 1330880

Comment 9

a year ago
15 failures in 135 pushes (0.111 failures/push) were associated with this bug yesterday.  

Repository breakdown:
* mozilla-inbound: 7
* autoland: 6
* mozilla-central: 1
* graphics: 1

Platform breakdown:
* windows8-64: 15

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1331070&startday=2017-02-01&endday=2017-02-01&tree=all
57 failures in 733 pushes (0.078 failures/push) were associated with this bug in the last 7 days. 

This is the #21 most frequent failure this week. 

** This failure happened more than 50 times this week! Resolving this bug is a high priority. **

Repository breakdown:
* autoland: 31
* mozilla-inbound: 21
* mozilla-central: 4
* graphics: 1

Platform breakdown:
* windows8-64: 57

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1331070&startday=2017-01-30&endday=2017-02-05&tree=all
no action in bug 1321198, I have pinged for help there, but if we don't get any traction in the next week or two I will recommend disabling affected tests.
(Assignee)

Updated

a year ago
Duplicate of this bug: 1321198
(Assignee)

Updated

a year ago
Duplicate of this bug: 1331281
(Assignee)

Updated

a year ago
Duplicate of this bug: 1330880
(Assignee)

Updated

a year ago
Duplicate of this bug: 1337966
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8835231 - Flags: review?(kaku)

Comment 17

a year ago
mozreview-review
Comment on attachment 8835231 [details]
Bug 1331070 - delay seek request until decoding first frames for non-MSE media.

https://reviewboard.mozilla.org/r/110920/#review112226

No problem to the code modification.

But, the treeherder log in bug 1321198 comment 17 is missing, so I am not able to figure out the root cause. Is this patch an attempt? Or it is the right fix?

::: dom/media/MediaDecoderStateMachine.cpp:604
(Diff revision 1)
> +    if (mMaster->mIsMSE) {
> +      return StateObject::HandleSeek(aTarget);
> +    }
> +    // Delay seek request until decoding first frames for non-MSE media.
> +    SLOG("Not Enough Data to seek at this stage, queuing seek");
> +    mPendingSeek.RejectIfExists(__func__);
> +    mPendingSeek.mTarget.emplace(aTarget);
> +    return mPendingSeek.mPromise.Ensure(__func__);

This is not really an issue, just some ideas.

Here we have two different logics for handling seek. Totally, we have three kinds over all states:
(1) reject the seek: used by DecodeMetaData, ShutDownState.
(2) postpone the seek: used by WaitForCDM and non-MSE DecodingFirstFrame.
(3) execute the seek: used by others.

How about having a new class, say SeekHandler, with three kinds of inheritances and we compose the right SeekHandler into SeekObjects. By this way, we can avoid the if-statement here.
Attachment #8835231 - Flags: review?(kaku) → review+
(Assignee)

Comment 18

a year ago
(In reply to Tzuhao Kuo [:kaku] from comment #17)
> But, the treeherder log in bug 1321198 comment 17 is missing, so I am not
> able to figure out the root cause. Is this patch an attempt? Or it is the
> right fix?

The issue is if we try to seek on a non-MSE stream while decoding first frames, sometimes we get decode errors on win8 x64 opt. The patch is a workaround to delay processing the seek request until decoding first frames is done. Note somehow this issue seems to happen on win8 x64 opt. I think this is probably a problem of the windows decoder.
(Assignee)

Comment 19

a year ago
(In reply to Tzuhao Kuo [:kaku] from comment #17)
> Here we have two different logics for handling seek. Totally, we have three
> kinds over all states:
> (1) reject the seek: used by DecodeMetaData, ShutDownState.
> (2) postpone the seek: used by WaitForCDM and non-MSE DecodingFirstFrame.
> (3) execute the seek: used by others.
> 
> How about having a new class, say SeekHandler, with three kinds of
> inheritances and we compose the right SeekHandler into SeekObjects. By this
> way, we can avoid the if-statement here.

Can you open a new bug and have a WIP to illustrate the idea?
(Assignee)

Comment 20

a year ago
Thanks for the review!
Assignee: nobody → jwwang
Priority: -- → P1

Comment 21

a year ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e059386f261
delay seek request until decoding first frames for non-MSE media. r=kaku
(Assignee)

Updated

a year ago
Duplicate of this bug: 1338045

Comment 23

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1e059386f261
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Based on when this got filed, it certainly appears that 53 *should* be affected, but I'm not seeing any OrangeFactor data from mozilla-aurora to support it either. Can you please nominate this for Aurora approval assuming the underlying code defect is in fact there?
status-firefox52: --- → unaffected
status-firefox53: --- → ?
Flags: needinfo?(jwwang)
(Assignee)

Comment 25

a year ago
Created attachment 8835838 [details] [diff] [review]
1331070_fix_aurora.patch

Approval Request Comment
[Feature/Bug causing the regression]:1331070
[User impact if declined]:mochitest timeout
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:no
[Why is the change risky/not risky?]:The change is not complicated and has been tested on Try.
[String changes made/needed]:none
Flags: needinfo?(jwwang)
Attachment #8835838 - Flags: review+
Attachment #8835838 - Flags: approval-mozilla-aurora?
Comment on attachment 8835838 [details] [diff] [review]
1331070_fix_aurora.patch

Should help prevent test timeouts, let's uplift to aurora.
Attachment #8835838 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 27

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/a6042b35e7ce
status-firefox53: ? → fixed
Flags: in-testsuite+

Comment 28

11 months ago
37 failures in 836 pushes (0.044 failures/push) were associated with this bug in the last 7 days.  

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **

Repository breakdown:
* mozilla-inbound: 16
* autoland: 16
* graphics: 3
* mozilla-central: 2

Platform breakdown:
* windows8-64: 37

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1331070&startday=2017-02-06&endday=2017-02-12&tree=all
(Assignee)

Updated

11 months ago
Duplicate of this bug: 1332019
(Assignee)

Updated

11 months ago
Duplicate of this bug: 1331401
(Assignee)

Updated

11 months ago
Duplicate of this bug: 1331201
(Assignee)

Updated

11 months ago
Duplicate of this bug: 1329638
Whiteboard: [stockwell fixed]
You need to log in before you can comment on or make changes to this bug.