Closed Bug 1290075 Opened 3 years ago Closed 3 years ago

Can't open audio inputs via GetUserMedia() in Android (or desktop without full_duplex)

Categories

(Core :: WebRTC: Audio/Video, defect, P1)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox48 --- unaffected
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: gcp, Assigned: jesup)

References

Details

Attachments

(1 file)

Nexus 4, Android 5.0.1

1) Go to https://mozilla.github.io/webrtc-landing/gum_test.html
2) Click audio & video
3) Tap home button
4) Observe that "Fennec - Camera and Microphone are on" message stays
5) Start Fennec again or return via task manager
6) Reload the gum page
7) Observe "Notfounderror: The object can not be found here"
To make sure it's explicit: (4) is also an error, as at the very least the camera is supposed to be released as soon as the app is backgrounded.
Priority: -- → P1
FWIW bug 874572 contains the UX discussion that decided the camera should be released when Firefox is backgrounded.
So we *are* releasing the camera when we get backgrounded.  (Proof: open Camera while backgrounded, which didn't work before bug 874572 landed)
There is a UI issue in that it shows the camera as in-use in notifications even when backgrounded (step 4 above)

The error in step 7 (this bug) is actually a problem with audio - you can only use gum audio once; a second use (without fennec being killed and restarted) will fail.  Desktop will fail also if you set full_duplex to false in about:config

The problem is that the VoiceEngine is now being shut down when no one is using it (Bug 1273206), and if you try to use the voiceengine after that, it fails unless Init() is called.  full_duplex doesn't have this issue since it gets input from cubeb.

Simple solution for uplift: always Init() the voice engine in EnumerateAudioDevices.  I may land a more complex/better patch, but non-full-duplex is going away soon anyways, so it may not be worth the effort
Assignee: nobody → rjesup
Rank: 10
Component: Audio/Video → WebRTC: Audio/Video
Product: Firefox for Android → Core
Summary: Camera isn't properly closed or shut down when leaving app/pages → Can't open audio inputs via GetUserMedia() in Android (or desktop without full_duplex)
Note: we may still need a bug on whatever is going on with video and backgrounding (UI probably, since I think opening the Camera proves we're stopping capture, but there are better verifications we can do)
in voe_base_impl.cc:
int VoEBaseImpl::Init(AudioDeviceModule* external_adm,
                      AudioProcessing* audioproc)
{
...
    if (_shared->statistics().Initialized())
    {
        return 0;
    }
...
    return _shared->statistics().SetInitialized();
}
Initialized() is basically just a flag; Init() sets it, and Terminate() unsets it.
Comment on attachment 8778017 [details] [diff] [review]
Always Init() the VoiceEngine when enumerating audio inputs

Review of attachment 8778017 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm.
Attachment #8778017 - Flags: review?(jib) → review+
Pushed by rjesup@wgate.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1ff3a7826a3
Always Init() the VoiceEngine when enumerating audio inputs r=jib
Comment on attachment 8778017 [details] [diff] [review]
Always Init() the VoiceEngine when enumerating audio inputs

Approval Request Comment
[Feature/regressing bug #]: bug 1273206

[User impact if declined]:
Android will be unable to use getUserMedia for audio more than once.  We plan to disable full_duplex audio for Mac in beta due to a few bugs, and can't do that until this is uplifted.

[Describe test coverage new/current, TreeHerder]: Not tested apparently, since our tests use synthetic sources for audio and camera data - testing with real hardware inputs (or OS-level fake drivers) is only partially available.

[Risks and why]: Very low risk - removes an if() from the call to Init(), and Init() is safe to call when already Inited.  This is an effective 1-liner, though the actual patch removes the no-longer-needed boolean and adds a comment

[String/UUID change made/needed]: none
Attachment #8778017 - Flags: approval-mozilla-beta?
Attachment #8778017 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e1ff3a7826a3
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1292500
Comment on attachment 8778017 [details] [diff] [review]
Always Init() the VoiceEngine when enumerating audio inputs

Review of attachment 8778017 [details] [diff] [review]:
-----------------------------------------------------------------

This patch fixes a issue that users can't open audio input more than once. Take it in 49 beta and aurora.
Attachment #8778017 - Flags: approval-mozilla-beta?
Attachment #8778017 - Flags: approval-mozilla-beta+
Attachment #8778017 - Flags: approval-mozilla-aurora?
Attachment #8778017 - Flags: approval-mozilla-aurora+
This needs to be verified by QE. Should be in 49 beta 2.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.