Closed Bug 1484720 Opened Last year Closed 5 months ago

Check Services.appinfo.is64Bit instead of UA string when disabling AV1-in-MP4 test on Win32

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

x86
Windows
enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox-esr60 --- wontfix
firefox67 --- wontfix
firefox67.0.1 --- wontfix
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: jya, Assigned: cpeterson)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Add playback test for bug 1417050 (av1 in mp4)
Rank: 25
Priority: -- → P3

AV1-in-MP4 support was disabled on Win32 in bug 1417050 and re-enabled in bug 1475564. This Win32 check should have been removed at that time to re-enable this AV1-in-MP4 test on Win32.

isSlowPlatform() is always false because we no longer support Android 2.3 or B2G.

Also remove unused functions isWindows32() and isAndroid(). They were added in bug 1485875, but the callers were later removed in Windows bug 1475564 and Android bug 1368843.

Depends on D33423

I stumbled upon this test code when looking for test code that looks for "x64" or "Win64" in Firefox's User-Agent string. In bug 1556223, I am proposing to remove "x64" from the Windows User-Agent string. In that case, this tests manifestNavigator().userAgent.includes("x64"))) check would stop working. But that's OK because this AV1 test case was supposed to have been re-enabled on Win32 by bug 1475564.

Blocks: 1556223, 1485875
Depends on: 1475564, 1368843
OS: Unspecified → Windows
Hardware: Unspecified → x86
Summary: Add mochitest for bug 1417050 → Add mochitest for AV1-in-MP4 bug 1417050
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e1fee5983139
Part 1: Enable AV1-in-MP4 test on Win32. r=jya
https://hg.mozilla.org/integration/autoland/rev/bc4c88c01a02
Part 2: Remove always-false isSlowPlatform() test function. r=jya
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dbe9ada72c1a
Backed out 2 changesets for assertion failures at RemoteDecoderManagerParent.cpp

Backed out 2 changesets (bug 1484720) for assertion failures at RemoteDecoderManagerParent.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/dbe9ada72c1a9666101fc3ace0d662307a880175

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&group_state=expanded&revision=bc4c88c01a027bdf4aebe17d1f072a19c66c33d5&selectedJob=249900077

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=249900077&repo=autoland&lineNumber=6175

Log snippet:

11:05:57 INFO - GECKO(5064) | [Parent 5268, Main Thread] WARNING: NS_ENSURE_TRUE(mSuspendCount > 0) failed: file z:/b
11:05:57 INFO - GECKO(5064) | uild/build/src/netwerk/protocol/http/nsHttpChannel.cpp, line 9441
11:05:57 INFO - GECKO(5064) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to c:\users\task_1559642874\appdata\local\temp\tmpc9yrlp.mozrunner\runtests_leaks_rdd_pid1560.log
11:05:57 INFO - GECKO(5064) | Assertion failure: (((HRESULT)(hr)) >= 0), at z:/build/build/src/dom/media/ipc/RemoteDecoderManagerParent.cpp:87
11:06:24 INFO - GECKO(5064) | #01: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1176]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #02: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/threads/nsThreadUtils.cpp:486]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #03: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:333]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #04: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:315]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #05: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:309]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #06: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:291]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #07: nsThread::ThreadFunc(void *) [xpcom/threads/nsThread.cpp:457]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #08: _PR_NativeRunThread [nsprpub/pr/src/threads/combined/pruthr.c:397]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #09: static unsigned int pr_root(void *) [nsprpub/pr/src/md/windows/w95thred.c:137]
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #10: ucrtbase.DLL + 0x3e16f
11:06:24 INFO -
11:06:24 INFO - GECKO(5064) | #11: kernel32.dll + 0x4ef3c

Flags: needinfo?(cpeterson)

11:05:57 INFO - GECKO(5064) | Assertion failure: (((HRESULT)(hr)) >= 0), at z:/build/build/src/dom/media/ipc/RemoteDecoderManagerParent.cpp:87

CoInitializeEx() is reliably failing with E_OUTOFMEMORY in RemoteDecoderManagerParent::StartupThreads() on 32-bit Windows 7:

https://searchfox.org/mozilla-central/rev/9eb2f739c165b4e294929f7b99fbb4d90f8a396b/dom/media/ipc/RemoteDecoderManagerParent.cpp#85-87

Flags: needinfo?(cpeterson)

(In reply to Chris Peterson [:cpeterson] from comment #7)

CoInitializeEx() is reliably failing with E_OUTOFMEMORY in RemoteDecoderManagerParent::StartupThreads() on 32-bit Windows 7:

Adding av1.mp4 to the list of media files to play causes CoInitializeEx() to return E_OUTOFMEMORY but only in debug builds. (I added a MOZ_RELEASE_ASSERT to confirm the opt build does not return any error.) What (COM?) resources would our AV1 decoder use up, but only in a debug build?

The av1.mp4 test file in an opt build on 32-bit Windows 7 fails but doesn't crash:

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=250098309&repo=try&lineNumber=2430-2474

07:48:45     INFO - TEST-PASS | dom/media/test/test_playback.html | [started av1.mp4-57 t=66.367] Length of array should match number of running tests 
07:48:45     INFO - TEST-PASS | dom/media/test/test_playback.html | av1.mp4: Name should match #3 
07:48:45     INFO - TEST-PASS | dom/media/test/test_playback.html | av1.mp4: Name should match #1 
07:48:45     INFO - TEST-PASS | dom/media/test/test_playback.html | av1.mp4 duration (0.999984) should be around 1 
07:48:45     INFO - TEST-PASS | dom/media/test/test_playback.html | av1.mp4 isEncrypted should be true if we have decryption keys 
07:48:45     INFO - Buffered messages finished
07:48:45     INFO - TEST-UNEXPECTED-FAIL | dom/media/test/test_playback.html | av1.mp4 should not fire 'error' event NS_ERROR_DOM_MEDIA_FATAL_ERR (0x806e0005) 

The av1.mp4 test file in an opt build on 32-bit Windows 7 fails but doesn't crash

And if I make RemoteDecoderManagerParent::StartupThreads() ignore E_OUTOFMEMORY in a debug build instead of asserting, the av1.mp4 test case seems to pass, even though the same test case fails in an opt build..?!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9a4b2f54438d62e43d459a04e5b4b02443282a2

Here is my current understanding of my Try test results. (I don't have a Windows machine on which to test.) Bug 1475564 re-enabled AV1 on Win32, but we still seem to have some problems on Win32.

OS Firefox Build av1.mp4 Test
32-bit Windows 7 Debug CoInitializeEx() returns E_OUTOFMEMORY and asserts, but the test seems to pass if this error is ignored?!
32-bit Windows 7 Opt CoInitializeEx() returns S_OK, but the test then fails with NS_ERROR_DOM_MEDIA_FATAL_ERR
64-bit Windows 10 Debug Test passes
64-bit Windows 10 Opt Test passes

I don't have the Windows dev environment or expertise to debug these AV1 failures on 32-bit Windows 7, but I'd like to land a patch to update the AV1 test case's Win32 check to use SpecialPowers.Services.appinfo.is64Bit instead of sniffing the UA string. I hope to remove the CPU architecture ("x64") from the UA string in bug 1556223. Tests won't be able to distinguish Win32 from Win64 using the UA string then.

Keywords: leave-open
Summary: Add mochitest for AV1-in-MP4 bug 1417050 → AV1-in-MP4 test fails on 32-bit Windows 7 with CoInitializeEx() E_OUTOFMEMORY
Attachment #9069243 - Attachment description: Bug 1484720 - Part 1: Enable AV1-in-MP4 test on Win32. r?jya → Bug 1484720 - Part 1: Check Services.appinfo.is64Bit instead of UA string when disabling AV1-in-MP4 test. r?jya
Attachment #9069244 - Attachment description: Bug 1484720 - Part 2: Remove always-false isSlowPlatform() test function. r?jya → Bug 1484720 - Part 2: Remove always-false isSlowPlatform() test function. r=jya
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc9bef9ad09c
Part 1: Check Services.appinfo.is64Bit instead of UA string when disabling AV1-in-MP4 test. r=jya
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2a4bff3a6a0
Part 2: Remove always-false isSlowPlatform() test function. r=jya
https://hg.mozilla.org/integration/mozilla-inbound/rev/50ea5ea76349
Part 3: Remove some media tests' obsolete checks for B2G. r=jya

I'll file a new bug for the actual test failure that prevents us from re-enabling the AV1-in-MP4 on Win32.

Assignee: nobody → cpeterson
Keywords: leave-open
Summary: AV1-in-MP4 test fails on 32-bit Windows 7 with CoInitializeEx() E_OUTOFMEMORY → Check Services.appinfo.is64Bit instead of UA string when disabling AV1-in-MP4 test on Win32
You need to log in before you can comment on or make changes to this bug.