1443294 - Wrong camera/mic picked from non-exact deviceId constraint, unless permissions are persisted

Status: RESOLVED FIXED

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

Description

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

We get a different device chosen by deviceId constraints depending on whether permission is persisted or not.

STRs:
 1. Have two cameras on your system (e.g. one USB and one built-in)
 2. Open https://jsfiddle.net/jib1/bxgszem4/ and do nothing.
 3. If capture auto-starts, click red blinking camera, revoke permission with "×" in dropdown, refresh.
 4. Click "Allow" in both prompts that appear.

Expected result:

  A B A B

Actual result:

  A A A B

Workaround:
  1. Check "☑ Remember this decision" in the camera permission prompt, refresh.

We want the same behavior regardless of permission (A B A B).

Comment 1

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

Unless its hand is forced (by exact constraints) Firefox will return a camera that already has permission over triggering another prompt.

This was done for all constraints, not just deviceId — though why it's wrong is perhaps most obvious with that constraint.

Firefox is alone among browsers in supporting per-device permission, and I believe this was done intentionally at some point, out of fear that sites calling getUserMedia a lot (instead of keeping tracks around and using applyConstraints on them) might otherwise trigger a shift in cameras that would prompt only in Firefox.

In hindsight, I think that fear was overblown, and that it's more important to respect application constraints. meaning: we should fix this, but we might want to keep an eye on web compat if we do.

Comment 2

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

This bug just got a lot worse with bug 1697284, since per-device permissions now have a 60 minute grace period.

This makes trying to switch to a second camera using deviceId: id instead of deviceId: {exact: id} impossible even after the first camera track has been stopped. We'll need to escalate a fix.

[Tracking Requested - why for this release]: Might break camera switching on some sites (that don't use exact constraints in camera switcher under ⚙️ settings) without persistent permission.

Comment 3

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

This being an old bug, I've set recent tracking flags based on the comment 2 regression in behavior from bug 1697284.

Comment 4

[Pascal Chevrel:pascalc]

We are at the end of the beta cycle with one beta left before RC, Jan-Ivar is that something that is important enough to justify a fix before the last beta? Would that be a dot-release driver? If you have a fix, would that be risk free for an uplift this late in the cycle? Thanks

Comment 5

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

Attachment: Bug 1443294 - Remove getUserMedia's preference for less than ideal cameras/mics that already have permission (minimal patch). r?pbz

The checkRequestAllowed() function that determines whether we can bypass the
permission prompt and fulfill immediately with a device, is given a list of devices
sorted by fitness distance from most ideal to least. — It used to grant any
device on that list that was already active, even if less than ideal (not
first in the list).

This caused problems with sites using ideal constraints to switch between
devices because this function has to fail before Firefox prompts for a new
device, and the new grace periods added in bug 1697284 extended "active" to
include any device active within the last 60 minutes, making it impossible
to pick anything else using ideal constraints alone.

The fix is to only look at the first device in the list (per kind). If that
device is not active, we should prompt for that device, not return a
different one, which means failing this function.

Comment 6

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

(In reply to Pascal Chevrel:pascalc from comment #4)

We are at the end of the beta cycle with one beta left before RC, Jan-Ivar is that something that is important enough to justify a fix before the last beta?

Probably not. It would be nice to get in, considering 89 is when we bumped the grace period from 50 seconds to 60 minutes, which is what made this much worse. That being said:

• This would only affect users with two or more cameras, or two or more microphones, trying to switch between them.
• This would only affect sites using ideal (deviceId: id) constraints to do device selection, something they shouldn't be doing (and most sites aren't). We've reached out to at least one major service and they have already corrected to use deviceId: {exact: id} in Firefox as the spec recommends for device switching.

If you have a fix, would that be risk free for an uplift this late in the cycle? Thanks

The fix is trivial, but unfortunately, we don't have a way to test the specific case this fixes, because we lack test infrastructure for testing multiple devices of the same kind. — All we can test is this fix doesn't break permission for obtaining a single device per kind.

Comment 7

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

Workaround

For those arriving here looking for a workaround, the spec recommends:

1. using ideal deviceId: id when users come to the site, and
2. using exact deviceId: {exact: id} from your site's camera/mic picker (e.g. typically under a site's ⚙️ settings)

See Example 6 (use ideal on revisit) and Example 7 (use exact in device picker) respectively. This gives the best user experience in all browsers.

Benefits in all browsers:

• (Option 1) On revisits to your site, it's common to try to use the camera the user used last time, but using ideal constrains (e.g. deviceId: id) here is better, because this allows it to fall back to a different device if the user has removed their device since then, or (in cases where constraints other than deviceId are used) added another more fit device.
• (Option 2) assumes getUserMedia is called in direct response to a user selecting a specific camera or microphone in your interface. It makes sense to use deviceId: {exact: id} to control that you only get that particular device successfully, or an error. This avoids accidentally showing the wrong combination of preview and label. — If the request fails you may want to give an error to the user (NotAllowedError = no permission, other errors = this camera is not working), and depending on the specifics of your device switching UI, leave the preview blank or go back to an earlier device or not.

Benefits specific to Firefox which grants per-device permission by default:

• Option 1 gives users who haven't opted out of prompting a chance to use a different camera or microphone this visit, and won't wonder why they're not seeing all their devices listed in the prompt.
• Option 2 gives sites our simplified permission prompt appearance in Firefox 88+, meant to not compete with in-content device selection.

Comment 8

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

Paul, any tips on how to test this?

Comment 9

[Emma Zühlcke [:emz]]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #8)

Paul, any tips on how to test this?

I haven't worked with our webrtc tests that much yet so I'm not sure how exactly the test environment handles audio / video devices.
However, if you wanted to test checkRequestAllowed specifically, perhaps you could mock a webrtc:Request call which includes multiple cameras or mics? For example by calling WebRTCParent#receiveMessage directly.

Comment 10

[Pascal Chevrel:pascalc]

Marking as fix-optional for 89 in case we have a fix we could include in a potential dot release as a ride-along patch.

Comment 11

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

Johann, could you have a look at this please? I'd ideally like to get this landed before soft code freeze tomorrow, even doing a follow-up test if necessary.

Comment 12

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

Existing tests pass: https://treeherder.mozilla.org/jobs?repo=try&revision=8b04e8c66f7525f2af43f1871641df378ffda7bb

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a31940f60b7
Remove getUserMedia's preference for less than ideal cameras/mics that already have permission (minimal patch). r=pbz

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6a31940f60b7

Comment 15

[Johann Hofmann [:johannh]]

Sorry for the delay, seems fine retroactively.