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

Status: RESOLVED FIXED

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

Description

[Gian-Carlo Pascutto [:gcp]]

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"

Comment 1

[Gian-Carlo Pascutto [:gcp]]

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.

Comment 2

[Gian-Carlo Pascutto [:gcp]]

FWIW bug 874572 contains the UX discussion that decided the camera should be released when Firefox is backgrounded.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Always Init() the VoiceEngine when enumerating audio inputs

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

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)

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

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 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8778017 [details] [diff] [review]
Always Init() the VoiceEngine when enumerating audio inputs

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

lgtm.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1ff3a7826a3
Always Init() the VoiceEngine when enumerating audio inputs r=jib

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e1ff3a7826a3

Comment 11

[Gerry Chang [:gchang]]

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.

Comment 12

[Gerry Chang [:gchang]]

This needs to be verified by QE. Should be in 49 beta 2.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ce57d7b45b4a

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bfd404d86a62