Closed
Bug 1253959
Opened 8 years ago
Closed 8 years ago
[e10s] Various media tests timeout on WinXP debug
Categories
(Core :: Audio/Video, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: RyanVM, Assigned: jwwang)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
65.06 KB,
text/plain
|
Details | |
58 bytes,
text/x-review-board-request
|
cpearce
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta-
|
Details |
This is testing with e10s forced on (we're trying to get all platforms running e10s tests in CI). Only WinXP debug builds appear to be affected (opt builds ran the tests fine). I retriggered 5 times and hit the same failures each time (and the timeouts were always at the same point of each test, so I don't think this is just a "test run slow" issue). I do see the following in the logs in each of the timing out tests, not sure if it's relevant or not, though. Any thoughts, Anthony? 08:25:41 INFO - [Child 2532] WARNING: Decoder=bd86030 ReadMetadata failed, rv=80004005 HasValidMedia=0: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/MediaDecoderReader.cpp, line 219 08:25:41 INFO - [Child 2532] WARNING: Decoder=bd86030 Decode metadata failed, shutting down decoder: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp, line 2005 08:25:41 INFO - [Child 2532] WARNING: Decoder=bd86030 Decode error, changed state to ERROR: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp, line 1909 https://treeherder.mozilla.org/logviewer.html#?job_id=17670384&repo=try
Flags: needinfo?(ajones)
Blake - can we get some help from JW on this one?
Flags: needinfo?(bwu)
Assignee | ||
Comment 2•8 years ago
|
||
I am on PTO today will need to go out soon. I see lots of these messages in the logs: WARNING: Failed to load WMF DLLs: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/platforms/wmf/WMFUtils.cpp, line 169 It looks like some DLLs are not installed on that machine.
Flags: needinfo?(cpearce)
Flags: needinfo?(bwu)
Flags: needinfo?(ajones)
Comment 3•8 years ago
|
||
(In reply to JW Wang [:jwwang] from comment #2) > I am on PTO today will need to go out soon. > > I see lots of these messages in the logs: > WARNING: Failed to load WMF DLLs: file > c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/ > platforms/wmf/WMFUtils.cpp, line 169 > > It looks like some DLLs are not installed on that machine. Yes. WMF is not available on Windows XP. I imagine we're hitting that code path inside HTMLMediaElement.canPlayType(), when we query whether MP4/H.264/AAC is supported. I expected that we'd cache the result though so I wouldn't expect us to keep seeing error logging. I do however expect DirectShow to be installed on those machines, so the errors for DirectShowReader failing to load are unexpected.
Flags: needinfo?(cpearce)
Assignee | ||
Comment 4•8 years ago
|
||
I wonder why it happened on debug builds only. Could it be some kind of sandbox issue that fails to load the DLLs?
Comment 5•8 years ago
|
||
(In reply to JW Wang [:jwwang] from comment #4) > I wonder why it happened on debug builds only. Could it be some kind of > sandbox issue that fails to load the DLLs? In the DirectShow case, the error is in a call stack inside an #ifdef DEBUG chunk: http://mxr.mozilla.org/mozilla-central/source/dom/media/directshow/DirectShowReader.cpp#112 I wrote that code years ago, and IIRC its there so that you can attach the GraphEdit program to Firefox and see the filter graph that has been constructed. So we could probably just change that code to not error out on failure, or comment out or even remove that code entirely.
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/38403/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/38403/
Attachment #8727287 -
Flags: review?(cpearce)
Assignee | ||
Updated•8 years ago
|
Attachment #8727287 -
Flags: review?(cpearce)
Reporter | ||
Comment 8•8 years ago
|
||
Comment on attachment 8727287 [details] MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce. Looks like the patch works! https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b9aa7a1d715&group_state=expanded Now I've got a leak, but at least the tests all run! This patch also fixed a permafail in the web-platform-tests previous runs were showing.
Attachment #8727287 -
Flags: feedback+
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8727287 [details] MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38403/diff/1-2/
Attachment #8727287 -
Flags: feedback+ → review?(cpearce)
Comment 10•8 years ago
|
||
Comment on attachment 8727287 [details] MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce. https://reviewboard.mozilla.org/r/38403/#review35251 Would also accept just #ifdef'ing on something other than DEBUG, something that's not defined. That way the code isn't explicitly removed, just never built, as someday it may prove useful again.
Attachment #8727287 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 11•8 years ago
|
||
Thanks!
Assignee | ||
Comment 12•8 years ago
|
||
How about |#ifdef DIRECTSHOW_REGISTER_GRAPH|?
Assignee | ||
Comment 13•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/file/be593a64d7c6a826260514fe758ef32a6ee580f7/dom/media/directshow/DirectShowUtils.cpp#l82 Somehow runningObjectTable->Register() returns hr=80070005 (E_ACCESSDENIED) only in e10s mode. We don't have such an error in non-e10s mode. Hi Chris, Do you have any idea about that?
Flags: needinfo?(cpearce)
Assignee | ||
Comment 14•8 years ago
|
||
I will check the patch so we can move forward and open follow up bugs if necessary.
Assignee: nobody → jwwang
Comment 16•8 years ago
|
||
(In reply to JW Wang [:jwwang] from comment #12) > How about |#ifdef DIRECTSHOW_REGISTER_GRAPH|? Sounds good. Thanks! (In reply to JW Wang [:jwwang] from comment #13) > https://hg.mozilla.org/mozilla-central/file/ > be593a64d7c6a826260514fe758ef32a6ee580f7/dom/media/directshow/ > DirectShowUtils.cpp#l82 > > Somehow runningObjectTable->Register() returns hr=80070005 (E_ACCESSDENIED) > only in e10s mode. We don't have such an error in non-e10s mode. > > Hi Chris, > Do you have any idea about that? I don't know... Maybe there's some setup that we're not doing in e10s mode? I think we can just move on, we can fix it if and when we need to debug DirectShow again.
Flags: needinfo?(cpearce)
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/084c4f8baf48
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8727287 [details] MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce. Approval Request Comment [Feature/regressing bug #]:unknown [User impact if declined]:some audio/video won't play in e10s mode. [Describe test coverage new/current, TreeHerder]:TreeHerder [Risks and why]: low, the change is simple and just about disabling a feature that a user will never use. [String/UUID change made/needed]:none
Attachment #8727287 -
Flags: approval-mozilla-beta?
Attachment #8727287 -
Flags: approval-mozilla-aurora?
Comment 19•8 years ago
|
||
Marking affected for 46 since we're testing e10s in 46 beta. Can we verify this fix on 48?
status-firefox46:
--- → affected
Flags: qe-verify+
Comment 20•8 years ago
|
||
Is this something that affects users playing audio/video, or just tests? Do we have a way to test/verify?
Flags: needinfo?(cpearce)
Reporter | ||
Comment 21•8 years ago
|
||
This is debug-only code. It doesn't affect release users at all. Given that we aren't running WinXP e10s tests on 46, I think we should just uplift this a=NPOTB to Aurora and call it good.
Flags: needinfo?(cpearce)
Comment 22•8 years ago
|
||
Comment on attachment 8727287 [details] MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce. Debug-only, OK to uplift to aurora
Attachment #8727287 -
Flags: approval-mozilla-beta?
Attachment #8727287 -
Flags: approval-mozilla-beta-
Attachment #8727287 -
Flags: approval-mozilla-aurora?
Attachment #8727287 -
Flags: approval-mozilla-aurora+
Comment 23•8 years ago
|
||
Thanks Ryan, I missed that it was xp only, and you're right, NPOTB should be ok to land anyway without approval.
Reporter | ||
Updated•8 years ago
|
Reporter | ||
Comment 24•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/5cf762adb7a5
You need to log in
before you can comment on or make changes to this bug.
Description
•