1191298 - getUserMedia fails for audio if constraints are specified

Status: VERIFIED FIXED

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

Description

[Pawel Domas]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.125 Safari/537.36

Steps to reproduce:

Calling getUserMedia with constraints for audio only stream fails:

{
  audio: true,
  video: false
}



Actual results:

MediaStreamError {
  name: "NotFoundError",
  message: "The object can not be found here.",
  constraintName: "",
  stack: "" 
}


Expected results:

Audio only media stream should be returned.

This happens with the latest nightly build 42.0a1 (2015-08-04). I have USB camera with integrated microphone, but it also fails on all of my laptops.

Calling with the constraints below works fine:
{
  audio: true,
  video: true
}

Comment 1

[Loic]

Could you attach a minimal testcase to reproduce the issue, please.

Comment 2

[Pawel Domas]

Sorry I should have done it in the first place: http://pastebin.com/SY1vU6uy

It turned out that it works when { audio: true }, but it does not allow to specify optional constratins like we do in the example above. It used to work until few days ago.

Comment 3

[Loic]

Attachment: getusermediatest.html

Comment 4

[Loic]

It's a recent regression:

good=2015-07-27
bad=2015-07-28
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d3228c82badd&tochange=33dc8a83cfc0

suspected: bug 1156472

Comment 5

[jen strickland :jenstrickland]

Tracked for 42, because this is a regression, and expectation would be that the audio would work consistently.

Comment 6

[Maire Reavy [:mreavy]]

Hey Paul, this looks like fallout from your audio capture landing.

Comment 7

[Paul Adenot (:padenot)]

(clearing NI, I'm on it).

Comment 8

[Paul Adenot (:padenot)]

After talking with Jan-Ivar, he thought of a better fix and knows the area better, so he's taking this one.

Comment 9

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

Attachment: MozReview Request: Bug 1191298 - don't fail on unknown audio constraints e.g. getUserMedia({ audio: {} }) (regression)

Bug 1191298 - don't fail on unknown audio constraints e.g. getUserMedia({ audio: {} }) (regression)

Comment 10

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

We don't support the (non-spec) constraints in comment 0, so they're ignored, but we fail even on an empty {}, which is a regression: http://jsfiddle.net/03d0frua

Comment 11

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

Comment on attachment 8645767 [details]
MozReview Request: Bug 1191298 - don't fail on unknown audio constraints e.g. getUserMedia({ audio: {} }) (regression)

Bug 1191298 - don't fail on unknown audio constraints e.g. getUserMedia({ audio: {} }) (regression)

Comment 12

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

Extending review to jesup. I'll take the first review I get.

Comment 13

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

Comment on attachment 8645767 [details]
MozReview Request: Bug 1191298 - don't fail on unknown audio constraints e.g. getUserMedia({ audio: {} }) (regression)

https://reviewboard.mozilla.org/r/15547/#review13953

Ship It!

Comment 14

[Paul Adenot (:padenot)]

Looks good, but this needs tests, let me know if you want me to write some.

Comment 15

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

I'm a bit unsure how to test this since our fake devices don't really have constraints yet, so there's nothing to work against for a test. Even the linux loopback devices don't have this AFAIK?

test_getUserMedia_constraints.html only really exercises the code and tests known failure paths for any device. The success path in it doesn't really do anything other than test that the syntax passes.

If you have tests in mind for this then that would be great!

Comment 16

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

It's hard/impossible to test this in automation currently (and this affects other aspects of constraints).  Let's manually test this, and file a bug for automated tests, and another bug (blocking the test bug, and likely some others (or others we should file)) for adding support for constraints to the fake devices and/or the loopback devices.  (Loopback is probably better, as it tests more of the real code paths, but for constraints testing this may not matter.)

Lack of being able to test these has bitten us before, so this should be a high P2 at least.

Comment 17

[Paul Adenot (:padenot)]

It might be hard to write integration tests for this because it indeed relies on devices enumeration, but that sounds like something that should be mocked and tested with unit tests.

Comment 18

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

Filed the bug (and hooked to the bug for making the fake/loopback devices more real)

Comment 19

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

I'm good with c++ unit tests or updating the fake/loopback devices.  This is an important-enough regression (and too hard to back out the regressing code) I'd like to manually test this, land it, and then backfill with tests.  I'm good with making tests a P1 (and there are less hurdles in getting a c++ unit test added compared to changes to the loopback devices, though perhaps more code to write).

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a170e64625c

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0a170e64625c

Comment 22

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

Comment on attachment 8645767 [details]
MozReview Request: Bug 1191298 - don't fail on unknown audio constraints e.g. getUserMedia({ audio: {} }) (regression)

Approval Request Comment
[Feature/regressing bug #]: Bug 1156472
[User impact if declined]:

Users will no longer get prompted to share their cameras or microphones on sites that specify audio-constraints of any kind. Even if they provide zero audio constraints e.g. they pass {audio: {}} instead of {audio:true} to getUserMedia, then getUserMedia will always fail immediately with NotFoundError instead of prompting the user.

[Describe test coverage new/current, TreeHerder]:

Tested locally. Landed on m-c a week ago.

[Risks and why]:

Very low risk, because the new code mirrors similar code that exists for video, but was missing for audio. Only code dealing directly with audio constraints is modified (plus a minor variable rename in video for symmetry).

[String/UUID change made/needed]: none

Comment 24

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8645767 [details]
MozReview Request: Bug 1191298 - don't fail on unknown audio constraints e.g. getUserMedia({ audio: {} }) (regression)

Fix a regression, taking it.

Comment 25

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 26

[Ada [:adalucinet]]

Reproduced with 42.0a1 (from 2015-08-05).
Verified fixed with 42.0b2 and 43.0a2 (from 2015-09-29), across platforms [1].

[1] Windows 7 64-bit, Ubuntu 14.04 64-bit and Mac OS X 10.10.4