Closed
Bug 927358
Opened 11 years ago
Closed 11 years ago
getUserMedia() in Firefox Beta effectively fails all mandatory constraints, which is overly harsh and no longer the direction we want
Categories
(Core :: WebRTC: Audio/Video, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla28
People
(Reporter: dominik.steiner, Assigned: jib)
Details
Attachments
(2 files, 1 obsolete file)
4.02 KB,
patch
|
jib
:
review+
bajaj
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
5.50 KB,
text/html
|
Details |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_5) AppleWebKit/536.30.1 (KHTML, like Gecko) Version/6.0.5 Safari/536.30.1 Steps to reproduce: Call navigator.mozGetUserMedia() in FF Nightly with the following constraints (which is working in FF 24) { audio: true, video: { mandatory: { maxWidth: 320, maxHeight: 240 }}} Actual results: This will produce an error navigator.getUserMedia error: NOT_SUPPORTED_ERR: maxWidth Expected results: Investigating into the src code the reason seems to be in the changes to MediaStreamTrack.webidl https://github.com/mozilla/mozilla-central/blob/FIREFOX_24_0_RELEASE/dom/webidl/MediaStreamTrack.webidl https://github.com/mozilla/mozilla-central/blob/master/dom/webidl/MediaStreamTrack.webidl Is there any plans to add validation of minWidth/maxWidth/minHeight/maxHeight to the media constraints? Or is there any reason for having removed the possibility to set those in mozGetUserMedia?
Component: Untriaged → WebRTC: Audio/Video
Product: Firefox → Core
Version: Trunk → 27 Branch
Comment 1•11 years ago
|
||
ran into the same issue, the spec now lists a new syntax: { audio: true, video: { mandatory: { width: {max: 320}, height: {max: 240} }}} but thats also throwing an error
Comment 2•11 years ago
|
||
I'm using this currently and it works in Chrome, does not cause an error in Firefox Nightly but doesn't do anything except 4:3: { audio: false, video: { optional: [ { minHeight: 180 }, { maxHeight: 180 }, { minWidth: 320 }, { maxWidth: 320 } ] } }
Comment 3•11 years ago
|
||
We don't support the resolution constraints yet in Firefox. Jib? We probably should dup or invalid this bug.
Flags: needinfo?(jib)
Assignee | ||
Comment 4•11 years ago
|
||
Correct, we don't support resolution constraints yet, that is Bug 907352. Before I close as a dup, should we be concerned that as of Firefox 26 this (correctly) produces an error callback, whereas in earlier versions these constraints were ignored, which means at least it played? This might be foreshadowing problems with the 'mandatory' constraint. When would someone ever use mandatory? A question for the list perhaps.
Flags: needinfo?(jib)
Assignee | ||
Comment 5•11 years ago
|
||
We decided on Tuesday to pull back this new behavior before it goes to Beta, so I'm taking it to work on. Having mandatory constraints fail when most constraints are not implemented is too harsh and will cause complaints. Rather than revert the change, I plan to change mandatory to only work on implemented constraints (facingMode), as outlined in part A in this proposal http://lists.w3.org/Archives/Public/public-media-capture/2013Nov/0161.html pending resolution to this problem in the spec.
Assignee: nobody → jib
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 6•11 years ago
|
||
I've renamed the subject to clarify the immediate problem. I say "Firefox Beta" because this started with Bug 882145 which landed right at the end of mozilla26. I say "effectively all mandatory constraints" because facingMode, the only implemented one, only really works on phones and OSX cameras for now.
Severity: normal → critical
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
Summary: getUserMedia constraints change in FireFox Nightly → getUserMedia() in Firefox Beta effectively fails all mandatory constraints, which is overly harsh and no longer the direction we want
Version: 27 Branch → 26 Branch
Assignee | ||
Comment 7•11 years ago
|
||
No longer fires error-callback or bails if unknown keys are detected in aRawConstraints. Silently ignore them instead, like we used to. Try - https://tbpl.mozilla.org/?tree=Try&rev=9ae6242b570a
Attachment #8336879 -
Flags: review?(rjesup)
Assignee | ||
Comment 8•11 years ago
|
||
Actually, "Too soon" is misleading, I regret phrasing it that way. Instead it's more likely we don't want to go in this direction at all, which is why I'm hoping to get this super-safe change all the way to Beta, so things don't stop working and programmers aren't forced to remove all their mandatory constraints.
Updated•11 years ago
|
Attachment #8336879 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 9•11 years ago
|
||
No change. New commit message. Carrying forward r+ from jesup.
Attachment #8336879 -
Attachment is obsolete: true
Attachment #8337024 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 8337024 [details] [diff] [review] Was mistake to begin failing on unsupported mandatory constraints. r=jesup [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 882145 User impact if declined: getUserMedia() stops working in Firefox for any web-site that specifies mandatory constraints such as width and height, things that Chrome supports and we used to ignore before (which at least made things work, albeit in the wrong size). This patch prevents things from stopping by restoring the old behavior of ignoring unsupported mandatory constraints, hopefully in time (aiming for Beta). We now think it was wrong-headed in first place to be this strict, and we have decided to go a new direction that only constrains by properties the browser knows, and light of this the original behavior which this patch brings back is correct, and desirable. Testing completed (on m-c, etc.): Successful try with updated mochitest. Works locally. Risk to taking this patch (and alternatives if risky): Extremely low risk. The code used to bail for no technical reason if an unknown key was detected in a raw object. All subsequent code uses the normalized dictionary copy that the object got copied into, which is clean of any unknown keys since dictionaries don't support unknown keys. String or IDL/UUID changes made by this patch: None
Attachment #8337024 -
Flags: approval-mozilla-aurora?
Comment 11•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/f0a7ca5a20a8
Keywords: checkin-needed
Target Milestone: --- → mozilla28
https://hg.mozilla.org/mozilla-central/rev/f0a7ca5a20a8
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 13•11 years ago
|
||
Comment on attachment 8337024 [details] [diff] [review] Was mistake to begin failing on unsupported mandatory constraints. r=jesup Approving on aurora given we have some time and this can get baked/verified.
Attachment #8337024 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
status-firefox25:
--- → unaffected
status-firefox26:
--- → affected
status-firefox27:
--- → affected
status-firefox28:
--- → fixed
tracking-firefox26:
--- → ?
tracking-firefox27:
--- → +
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 14•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/8f1a8bfca0df
Keywords: checkin-needed
Assignee | ||
Comment 15•11 years ago
|
||
Comment on attachment 8337024 [details] [diff] [review] Was mistake to begin failing on unsupported mandatory constraints. r=jesup [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): String or IDL/UUID changes made by this patch: [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 882145 User impact if declined: getUserMedia() stops working in Firefox for any web-site that specifies mandatory constraints such as width and height, things that Chrome supports and we used to ignore before (which at least made things work, albeit in the wrong size). This patch prevents things from stopping by restoring the old behavior of ignoring unsupported mandatory constraints, hopefully in time (aiming for Beta). We now think it was wrong-headed in first place to be this strict, and we have decided to go a new direction that only constrains by properties the browser knows, and light of this the original behavior which this patch brings back is correct, and desirable. Testing completed (on m-c, etc.): Successful try with updated mochitest. Works locally. Landed on nightly and Aurora. Risk to taking this patch (and alternatives if risky): Extremely low risk. The code used to bail for no technical reason if an unknown key was detected in a raw object. All subsequent code uses the normalized dictionary copy that the object got copied into, which is clean of any unknown keys since dictionaries don't support unknown keys. This patch simply removes the code that bails, effectively ignoring that there ever was an unknown key in the origin raw object. This effectively neutralizes the validation code that was added, in the safest possible way. String or IDL/UUID changes made by this patch: None
Attachment #8337024 -
Flags: approval-mozilla-beta?
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #8337024 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 16•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/0c2f6fb212aa Bug 917298 landed on 27 and wasn't uplifted, so the changes to test_getUserMedia_constraints.html were ignored.
Keywords: checkin-needed
Updated•11 years ago
|
status-b2g-v1.2:
--- → fixed
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:27.0) Gecko/20100101 Firefox/27.0, User Agent 20131016030202 I've tried to reproduce the initial issue using the Nightly build from 16th October, where I set a pref as in Comment 0 (or Comment 1) and then went to several websites such as https://apprtc.webrtc.org, https://apprtc.appspot.com/, http://shinydemos.com/cracked/ but I didn't saw the error message. Sometimes on apprtc websites I get the error "Failed to get access to local media. Error code was undefined.", but it's intermittently and it's happening also on latest beta version (8). Is there something else I should do? Thank you.
Flags: needinfo?(jib)
Assignee | ||
Comment 19•11 years ago
|
||
(In reply to petruta.rasa from comment #18) > I've tried to reproduce the initial issue using the Nightly build from 16th > October, where I set a pref as in Comment 0 (or Comment 1) and then went to > several websites such as https://apprtc.webrtc.org, > https://apprtc.appspot.com/, http://shinydemos.com/cracked/ but I didn't saw > the error message. What pref are you using? I'm not aware of a pref for this. Constraints need to be specified in the mozGetUserMedia() call in the JS. See attached test case.
Flags: needinfo?(jib)
Thanks for explaining. I manage to reproduce the initial issue and I verified that it is fixed on Win 7 64-bit, Ubuntu 32-bit and Mac OS X 10.8.5 using Firefox 26 beta 8 (20131125215016), latest Aurora (20131127004001) and latest Nightly (20131127030201).
QA Contact: petruta.rasa
You need to log in
before you can comment on or make changes to this bug.
Description
•