Issue metadata
Sign in to add a comment
|
multiple optional MediaStreamTrack Constrains not satisfied as expected
Reported by
thembr...@gmail.com,
Mar 15 2016
|
||||||||||||||||||||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.75 Safari/537.36
Steps to reproduce the problem:
I noticed a change in behaviour how optional constraints are handled from M49 -> M50. These are the constraints and resolutions I get on a Macbook Pro with internal 720p cam.
In M49:
{"video":{"optional":[{"minHeight":1080},{"minHeight":720},{"minHeight":480}]}} yields {"width":1280,"height":720},
{"video":{"optional":[{"minHeight":720}]}} yields {"width":1280,"height":720},
M50/M51:
{"video":{"optional":[{"minHeight":1080},{"minHeight":720},{"minHeight":480}]}} yields {"width":640,"height":480},
{"video":{"optional":[{"minHeight":720}]}} yields {"width":1280,"height":720},
See https://output.jsbin.com/kodosib
What is the expected behavior?
What went wrong?
From the specs I'd expect the M49 behaviour. As it'd try to satisfy as many advanced/optional constraints as possible, thus yield the highest possible resolution.
Did this work before? Yes M49
Chrome version: 49.0.2623.75 Channel: n/a
OS Version:
Flash Version: Shockwave Flash 21.0 r0
See https://bugs.chromium.org/p/chromium/issues/detail?id=543997#c14
,
Mar 15 2016
Thanks for making the sample!
49 doesn't look "right" either. My result on the jsbin (HD-capable camera):
{"video":{"optional":[{"minHeight":1080},{"minHeight":720},{"minHeight":480}]}} yields {"width":1920,"height":1080},
{"video":{"optional":[{"minHeight":720}]}} yields {"width":960,"height":720},
chrome://webrtc-internals shows
optional: {minHeight:1080} for the first sample, 720 for the second.
,
Mar 15 2016
Maybe webrtc-internals is off here. Reversing the contraints order https://output.jsbin.com/nibopu on 49, with hd cam: {"video":{"optional":[{"minHeight":480},{"minHeight":720},{"minHeight":1080}]}} yields {"width":1920,"height":1080}, {"video":{"optional":[{"minHeight":720}]}} yields {"width":960,"height":720}, webrtc-internals just shows: optional: {minHeight:480} for the first sample
,
Mar 15 2016
In chrome 51, the chrome://webrtc-internals shows:
{advanced: [{height: {min: 480}}]}
It seems that 51 (and probably 50) preserves only the last in the sequence, not the first like 49 did. I'll get that fixed.
,
Mar 15 2016
What does Firefox do? What does the specification say? It makes sense - to me, anyway - to take the last.
,
Mar 15 2016
phistuck: The specification of 2 years ago said "evaluate each element of the optional array in turn, skip the ones you can't satisfy". I can't get the example to work in Firefox (yet). The specification currently talks about the "advanced" array with similar properties - the "optional" array is gone. but there is nothing in there that should force the browser to choose anything smaller than height 720.
,
Mar 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/20078f72c457d9612c1e12044e7e87fc721b4397 commit 20078f72c457d9612c1e12044e7e87fc721b4397 Author: hta <hta@chromium.org> Date: Thu Mar 17 16:13:04 2016 Constraints: Make advanced list be a list. Previous behavior: A list of optional constraints resulted in a single advanced element, with one field for each value. New behavior: A list of optional constraints results in a list of advanced elements, with one field set for each value. The difference is important when there are mulitple values with the same name. For constraints with distinct names, there should be no difference. BUG= 594929 Review URL: https://codereview.chromium.org/1808173002 Cr-Commit-Position: refs/heads/master@{#381724} [modify] https://crrev.com/20078f72c457d9612c1e12044e7e87fc721b4397/third_party/WebKit/Source/modules/mediastream/MediaConstraintsImpl.cpp
,
Mar 18 2016
Checked in today's Canary 51.0.2682.0 with https://output.jsbin.com/kodosib Attached are the constraints I see in jsbin and chrome://webrtc-internals hta@ - does it look right?
,
Mar 18 2016
btw, the test in #8 was done in Mac 10.11.3 using Logitech c930e camera
,
Mar 18 2016
Yep, this looks as I would expect after the change. I think the behavior is what the original poster wanted.
,
Mar 18 2016
hta@ - do you want your fix https://codereview.chromium.org/1808173002 merged to M-50?
,
Mar 21 2016
Yes, this should be merged to M50.
,
Mar 21 2016
Your change meets the bar and is auto-approved for M50 (branch: 2661)
,
Mar 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/557071c271897464f0c1eceb1e8a6590ffb13916 commit 557071c271897464f0c1eceb1e8a6590ffb13916 Author: Harald Alvestrand <hta@chromium.org> Date: Mon Mar 21 08:05:53 2016 Constraints: Make advanced list be a list. Previous behavior: A list of optional constraints resulted in a single advanced element, with one field for each value. New behavior: A list of optional constraints results in a list of advanced elements, with one field set for each value. The difference is important when there are mulitple values with the same name. For constraints with distinct names, there should be no difference. BUG= 594929 Review URL: https://codereview.chromium.org/1808173002 Cr-Commit-Position: refs/heads/master@{#381724} (cherry picked from commit 20078f72c457d9612c1e12044e7e87fc721b4397) Conflicts: third_party/WebKit/Source/modules/mediastream/MediaConstraintsImpl.cpp Review URL: https://codereview.chromium.org/1817943003 . Cr-Commit-Position: refs/branch-heads/2661@{#307} Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081} [modify] https://crrev.com/557071c271897464f0c1eceb1e8a6590ffb13916/third_party/WebKit/Source/modules/mediastream/MediaConstraintsImpl.cpp |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by hta@chromium.org
, Mar 15 2016