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)

All
Android
defect

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)

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
Blocks: 1481142
Note this only happens on Firefox for Android. We verified that Firefox on Windows, macOS and Linux doesn't have such issue.
(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.
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
Rank: 11
Component: WebRTC → WebRTC: Audio/Video
Priority: -- → P2
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.
Also what Android versions have you been testing with? It could equally be that some change in more recent Android version broke things.
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.
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.
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: --- → ?
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
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.
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 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+
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/060aa1c70723
fix GMP process IPC channel error on Android. r=jld,njn
https://hg.mozilla.org/mozilla-central/rev/060aa1c70723
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Too late for 61 at this point, but it sounds like we definitely need a Beta uplift request.
(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)
Why did we need a separate bug for uplifting? That makes tracking more difficult.
Flags: needinfo?(jolin)
(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)
Move and carry the r+ (:jld and :njn) from bug 1484749.
Attachment #9002817 - Flags: review+
Attachment #9002817 - Flags: approval-mozilla-beta?
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
No longer blocks: 1484749
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+
Flags: qe-verify+
Oana is handling the Bug verification for the next beta and will take care of this.
Flags: needinfo?(oana.horvath)
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)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: