Closed
Bug 1481139
Opened 6 years ago
Closed 6 years ago
Firefox for Android failed to load gmp-openh264 v1.7.1
Categories
(Core :: WebRTC: Audio/Video, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
relnote-firefox | --- | 62+ |
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox61 | --- | wontfix |
firefox62 | + | verified |
firefox63 | + | fixed |
People
(Reporter: hankpeng, Assigned: jhlin)
References
Details
(Keywords: regression)
Attachments
(4 files)
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/x-phabricator-request
|
jld
:
review+
n.nethercote
:
review+
|
Details |
(deleted),
patch
|
jhlin
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
While verifying the release candidate of gmp-openh264 v1.8.0 with Nightly, we fount Firefox for Android cannot load the new plugin.
We then tested Firefox for Android Nightly (version firefox-63.0a1-2018-08-05, installed with Play Store) and Firefox for Android (version firefox-61.0) with the official gmp-openh264 v1.7.1, and it turns out neither can load the plugin.
Steps:
1. Launch Firefox by command
adb shell am start -a android.intent.action.VIEW -d https://mozilla.github.io/webrtc-landing/pc_test.html --es env0 NSPR_LOG_MODULES=timestamp,GMP:5 org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp
or
adb shell am start -a android.intent.action.VIEW -d https://mozilla.github.io/webrtc-landing/pc_test.html --es env0 NSPR_LOG_MODULES=timestamp,GMP:5 org.mozilla.firefox/org.mozilla.gecko.BrowserApp
2. In the page https://mozilla.github.io/webrtc-landing/pc_test.html, check "Use Fake Audio/Video for one stream" and "Require H.264 video" and uncheck "Enable Identity Provider", and click "Start!"
3. Allow to share the camera and microphone
Result: only preview video, no remote video is shown
Note this only happens on Firefox for Android. We verified that Firefox on Windows, macOS and Linux doesn't have such issue.
Comment 3•6 years ago
|
||
(In reply to Hank Peng from comment #0)
> We then tested Firefox for Android Nightly (version
> firefox-63.0a1-2018-08-05, installed with Play Store) and Firefox for
> Android (version firefox-61.0) with the official gmp-openh264 v1.7.1, and it
> turns out neither can load the plugin.
In that case it would be interesting to know which was the last version that still worked.
Comment 4•6 years ago
|
||
Moving this to the webrtc component where it should get more visibility.
Using mozregression can help with a regression date. https://mozilla.github.io/mozregression/install.html#mozregression
Component: Audio/Video → WebRTC
Product: Firefox for Android → Core
Hardware: Unspecified → All
Version: unspecified → Trunk
Summary: Failed to load gmp-openh264 v1.7.1 → Firefox for Android failed to load gmp-openh264 v1.7.1
Updated•6 years ago
|
Rank: 11
Component: WebRTC → WebRTC: Audio/Video
Priority: -- → P2
Assignee | ||
Comment 7•6 years ago
|
||
Saw the following in log:
08-05 22:45:18.709 3865-3892/? I/Gecko: 2018-08-06 05:45:18.709186 UTC - [(null) 3865: Main Thread]: D/GMP GMPChild[pid=3865] GMPChild ctor
2018-08-06 05:45:18.709583 UTC - [GMP 3865: Main Thread]: D/GMP GMPChild[pid=3865] Init pluginPath=/data/data/org.mozilla.fennec_aurora/files/mozilla/ubse88vm.default/gmp-gmpopenh264/1.7.1
08-05 22:45:18.717 3865-3902/? I/Gecko: [GMP 3865, Chrome_ChildThread] WARNING: pipe error: Socket operation on non-socket: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 726
[GMP 3865, Chrome_ChildThread] WARNING: pipe error: Socket operation on non-socket: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 726
[GMP 3865, Chrome_ChildThread] WARNING: pipe error: Socket operation on non-socket: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 726
08-05 22:45:48.170 2289-2409/? I/Gecko: 2018-08-06 05:45:48.170734 UTC - [(null) 2289: GMPThread]: D/GMP GMPParent[0x9e32c6b0|childPid=0] LoadProcess: Failed to launch new child process
Not sure whether those pipe errors are the cause or effect, though.
Comment 8•6 years ago
|
||
Also what Android versions have you been testing with? It could equally be that some change in more recent Android version broke things.
Comment 9•6 years ago
|
||
i submitted this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1473583 which have been closed for being a duplicate of this one.
If those bugs are related which is probably the case, it's happened to me with android 4.4.2 kernel version 3.10.0-4756355 with firefox 61, it's more likely a change in firefox code between the 61 version and the previous one rather than a android code change.
Reporter | ||
Comment 10•6 years ago
|
||
We tested various phones and tablets. The android versions include 5.0 and 6.0.1. These devices were used for gmp-openh264 plugin integration test before and worked then.
Comment 11•6 years ago
|
||
Release Note Request (optional, but appreciated)
[Why is this notable]: Unable to talk to Safari or other H264 only WebRTC clients
[Affects Firefox for Android]: Yes, affects release
[Suggested wording]:
[Links (documentation, blog post, etc)]: this bug
relnote-firefox:
--- → ?
Assignee | ||
Comment 12•6 years ago
|
||
Did some bisecting and found it's broken since bug 1438678.
The patch there introduces a new file descriptor for pref. ContentParent adds it to the fds_to_remap table of GeckoChildProcessHost[2], and it is then passed to the child process. GMPProcessParent does not add it, so the index used to retrieve the IPC channel fd is off by one on Android[2].
Bug 1471025 added another fd for pref map and the index is off by two now.
I've created a patch to make GMPProcessParent add dummy values for these two fds, and verify that it works locally. Will upload it for review and request backporting for 61 and 62 once it lands.
[1] https://hg.mozilla.org/mozilla-central/diff/32d6774930e5/dom/ipc/ContentParent.cpp#l1.122
[2] https://hg.mozilla.org/mozilla-central/diff/32d6774930e5/ipc/glue/GeckoChildProcessHost.cpp#l1.68
Assignee: nobody → jolin
Status: NEW → ASSIGNED
status-firefox61:
--- → affected
status-firefox62:
--- → affected
status-firefox63:
--- → affected
Assignee | ||
Comment 13•6 years ago
|
||
Bug 1481139 - p1: handle invalid file descriptors.
Bug 1481139 - p2: add dummy fds for GMP process.
Two file descriptors were added in bug 1438678 and 1471025 for content/child
process but not GMP process, and it breaks the IPC channel on Android.
Add dummy values to make it work for now before bug 1440207 clean up the mess.
Updated•6 years ago
|
Comment 14•6 years ago
|
||
Comment on attachment 9001731 [details]
Bug 1481139 - fix GMP process IPC channel error on Android.
Jed Davis [:jld] (⏰UTC-6) has approved the revision.
Attachment #9001731 -
Flags: review+
Comment 15•6 years ago
|
||
Comment on attachment 9001731 [details]
Bug 1481139 - fix GMP process IPC channel error on Android.
Nicholas Nethercote [:njn] has approved the revision.
Attachment #9001731 -
Flags: review+
Comment 16•6 years ago
|
||
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/060aa1c70723
fix GMP process IPC channel error on Android. r=jld,njn
Comment 17•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•6 years ago
|
Keywords: regression
Comment 18•6 years ago
|
||
Too late for 61 at this point, but it sounds like we definitely need a Beta uplift request.
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Flags: needinfo?(jolin)
Updated•6 years ago
|
tracking-firefox62:
--- → +
tracking-firefox63:
--- → +
Assignee | ||
Comment 19•6 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)
> Too late for 61 at this point, but it sounds like we definitely need a Beta
> uplift request.
I filed bug 1484749 for the uplifting but forgot to mention it here. :$
Blocks: 1484749
Flags: needinfo?(jolin)
Comment 20•6 years ago
|
||
Why did we need a separate bug for uplifting? That makes tracking more difficult.
Flags: needinfo?(jolin)
Assignee | ||
Comment 21•6 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #20)
> Why did we need a separate bug for uplifting? That makes tracking more
> difficult.
You're right. I did that because the status of this bug has been changed to resolved and fixed, but it's obvious a bad decision. Sorry about that.
Flags: needinfo?(jolin)
Assignee | ||
Comment 22•6 years ago
|
||
Move and carry the r+ (:jld and :njn) from bug 1484749.
Attachment #9002817 -
Flags: review+
Attachment #9002817 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 24•6 years ago
|
||
Comment on attachment 9002817 [details] [diff] [review]
Patch for beta(62) branch
Approval Request Comment
[Feature/Bug causing the regression]: bug 1438678
[User impact if declined]: WebRTC video won't work between Firefox and Safari
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: yes. Please follow the steps in bug 1481139 comment 0
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low
[Why is the change risky/not risky?]: the patch changes only GMP process init code and adds the sanity check
[String changes made/needed]: none
Comment 25•6 years ago
|
||
Comment on attachment 9002817 [details] [diff] [review]
Patch for beta(62) branch
Thanks for moving things around. Approved for Fennec 62.0b21 (due to ship *next* Tuesday).
Attachment #9002817 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•6 years ago
|
Flags: qe-verify+
Comment 26•6 years ago
|
||
bugherder uplift |
Comment 27•6 years ago
|
||
Oana is handling the Bug verification for the next beta and will take care of this.
Flags: needinfo?(oana.horvath)
Comment 28•6 years ago
|
||
Verified on Beta 62.0b21 that both videos work, based on STR from comment 0.
Device: Sony Xperia Z5 Premium (Android 6.0.1)
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(oana.horvath)
Added to Fennec 62 release notes.
You need to log in
before you can comment on or make changes to this bug.
Description
•