WebKit Bugzilla
Attachment 361934 Details for
Bug 193614
: getUserMedia with an ideal deviceId constraint doesn't always select the correct device
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193614-20190213135002.patch (text/plain), 17.79 KB, created by
youenn fablet
on 2019-02-13 13:50:03 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2019-02-13 13:50:03 PST
Size:
17.79 KB
patch
obsolete
>Subversion Revision: 241453 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 22adac6207dfa3690eae59a963fa1db62847e746..0c83cbc89a67b979ec525c6dda19d8c9ff255540 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,36 @@ >+2019-02-13 Eric Carlson <eric.carlson@apple.com> and Youenn Fablet <youenn@apple.com> >+ >+ getUserMedia with an ideal deviceId constraint doesn't always select the correct device >+ https://bugs.webkit.org/show_bug.cgi?id=193614 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Compute a fitness score based on constraints. >+ For each constraint, a fitness score is computed from the distance. >+ The smaller the distance, the higher the score. >+ Fitness scores are then summed to give a device fitness score. >+ Matching devices are then sorted according the fitness score. >+ >+ For important constraints, deviceId and facingMode, add a more important weight. >+ This ensures that should any of these ideal constraints are set, they will be respected. >+ >+ Restrict our automatic setting of default constraints to not add a default ideal facingMode in case of existing deviceId constraint. >+ Do not set a default ideal frameRate if width and height are already set. >+ >+ Covered by updated test. >+ >+ * platform/mediastream/MediaConstraints.cpp: >+ (WebCore::FlattenedConstraint::set): >+ (WebCore::MediaConstraints::setDefaultVideoConstraints): >+ * platform/mediastream/RealtimeMediaSource.cpp: >+ (WebCore::RealtimeMediaSource::fitnessDistance): >+ (WebCore::RealtimeMediaSource::selectSettings): >+ (WebCore::RealtimeMediaSource::supportsConstraints): >+ (WebCore::RealtimeMediaSource::applyConstraints): >+ * platform/mediastream/RealtimeMediaSource.h: >+ * platform/mediastream/RealtimeMediaSourceCenter.cpp: >+ (WebCore::RealtimeMediaSourceCenter::validateRequestConstraints): >+ > 2019-02-13 John Wilander <wilander@apple.com> > > Store Ad Click Attribution requests in the network process >diff --git a/Source/WebCore/platform/mediastream/MediaConstraints.cpp b/Source/WebCore/platform/mediastream/MediaConstraints.cpp >index 60dea71a2f99b518033e1e81d1d8827ea86b5ed3..7c40e23db29c407ec66f8bb39db843695d2a5301 100644 >--- a/Source/WebCore/platform/mediastream/MediaConstraints.cpp >+++ b/Source/WebCore/platform/mediastream/MediaConstraints.cpp >@@ -123,10 +123,8 @@ void StringConstraint::merge(const MediaConstraint& other) > > void FlattenedConstraint::set(const MediaConstraint& constraint) > { >- for (auto& variant : m_variants) { >- if (variant.constraintType() == constraint.constraintType()) >- return; >- } >+ if (find(constraint.constraintType())) >+ return; > > append(constraint); > } >@@ -391,23 +389,20 @@ bool MediaConstraints::isConstraintSet(const WTF::Function<bool(const MediaTrack > > void MediaConstraints::setDefaultVideoConstraints() > { >- // 640x480, 30fps, font-facing camera >- bool hasFrameRateConstraints = isConstraintSet([](const MediaTrackConstraintSetMap& constraint) { >- return !!constraint.frameRate(); >+ // 640x480, 30fps, front-facing camera >+ bool needsFrameRateConstraints = !isConstraintSet([](const MediaTrackConstraintSetMap& constraint) { >+ return !!constraint.frameRate() || !!constraint.width() || !!constraint.height(); > }); > >- bool hasSizeConstraints = isConstraintSet([](const MediaTrackConstraintSetMap& constraint) { >+ bool needsSizeConstraints = !isConstraintSet([](const MediaTrackConstraintSetMap& constraint) { > return !!constraint.width() || !!constraint.height(); > }); > >- bool hasFacingModeConstraints = isConstraintSet([](const MediaTrackConstraintSetMap& constraint) { >- return !!constraint.facingMode(); >+ bool needsFacingModeConstraints = !isConstraintSet([](const MediaTrackConstraintSetMap& constraint) { >+ return !!constraint.facingMode() || !!constraint.deviceId(); > }); > >- if (hasFrameRateConstraints && hasSizeConstraints && hasFacingModeConstraints) >- return; >- >- addDefaultVideoConstraints(mandatoryConstraints, !hasFrameRateConstraints, !hasSizeConstraints, !hasFacingModeConstraints); >+ addDefaultVideoConstraints(mandatoryConstraints, needsFrameRateConstraints, needsSizeConstraints, needsFacingModeConstraints); > } > > } >diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp b/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp >index ce0f9aaa7922e024b5be716e54a762ac2b6221c0..c83ca74cbef3567eb71bfa47252f936066d28b90 100644 >--- a/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp >+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp >@@ -413,7 +413,8 @@ double RealtimeMediaSource::fitnessDistance(const MediaConstraint& constraint) > } > > case MediaConstraintType::DeviceId: >- ASSERT_NOT_REACHED(); >+ ASSERT(!m_hashedID.isEmpty()); >+ return downcast<StringConstraint>(constraint).fitnessDistance(m_hashedID); > break; > > case MediaConstraintType::GroupId: { >@@ -569,9 +570,9 @@ void RealtimeMediaSource::applyConstraint(const MediaConstraint& constraint) > } > } > >-bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, FlattenedConstraint& candidates, String& failedConstraint, SelectType type) >+bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, FlattenedConstraint& candidates, String& failedConstraint) > { >- m_fitnessScore = std::numeric_limits<double>::infinity(); >+ double minimumDistance = std::numeric_limits<double>::infinity(); > > // https://w3c.github.io/mediacapture-main/#dfn-selectsettings > // >@@ -596,7 +597,7 @@ bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, Fl > > // Check width, height and frame rate jointly, because while they may be supported individually the combination may not be supported. > double distance = std::numeric_limits<double>::infinity(); >- if (!supportsSizeAndFrameRate(constraints.mandatoryConstraints.width(), constraints.mandatoryConstraints.height(), constraints.mandatoryConstraints.frameRate(), failedConstraint, m_fitnessScore)) >+ if (!supportsSizeAndFrameRate(constraints.mandatoryConstraints.width(), constraints.mandatoryConstraints.height(), constraints.mandatoryConstraints.frameRate(), failedConstraint, minimumDistance)) > return false; > > constraints.mandatoryConstraints.filter([&](const MediaConstraint& constraint) { >@@ -608,25 +609,6 @@ bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, Fl > return false; > } > >- // The deviceId can't be changed, and the constraint value is the hashed device ID, so verify that the >- // device's unique ID hashes to the constraint value but don't include the constraint in the flattened >- // constraint set. >- if (constraint.constraintType() == MediaConstraintType::DeviceId) { >- if (type == SelectType::ForApplyConstraints) >- return false; >- >- ASSERT(constraint.isString()); >- ASSERT(!m_hashedID.isEmpty()); >- >- double constraintDistance = downcast<StringConstraint>(constraint).fitnessDistance(m_hashedID); >- if (std::isinf(constraintDistance)) { >- failedConstraint = constraint.name(); >- return true; >- } >- >- return false; >- } >- > double constraintDistance = fitnessDistance(constraint); > if (std::isinf(constraintDistance)) { > failedConstraint = constraint.name(); >@@ -641,7 +623,7 @@ bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, Fl > if (!failedConstraint.isEmpty()) > return false; > >- m_fitnessScore = distance; >+ minimumDistance = distance; > > // 4. If candidates is empty, return undefined as the result of the SelectSettings() algorithm. > if (candidates.isEmpty()) >@@ -677,7 +659,7 @@ bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, Fl > supported = true; > }); > >- m_fitnessScore = std::min(m_fitnessScore, constraintDistance); >+ minimumDistance = std::min(minimumDistance, constraintDistance); > > // 5.2 If the fitness distance is finite for one or more settings dictionaries in candidates, keep those > // settings dictionaries in candidates, discarding others. >@@ -690,7 +672,7 @@ bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, Fl > // The UA should use the one with the smallest fitness distance, as calculated in step 3. > if (!supportedConstraints.isEmpty()) { > supportedConstraints.removeAllMatching([&](const std::pair<double, MediaTrackConstraintSetMap>& pair) -> bool { >- return std::isinf(pair.first) || pair.first > m_fitnessScore; >+ return std::isinf(pair.first) || pair.first > minimumDistance; > }); > > if (!supportedConstraints.isEmpty()) { >@@ -699,7 +681,7 @@ bool RealtimeMediaSource::selectSettings(const MediaConstraints& constraints, Fl > candidates.merge(constraint); > }); > >- m_fitnessScore = std::min(m_fitnessScore, supportedConstraints[0].first); >+ minimumDistance = std::min(minimumDistance, supportedConstraints[0].first); > } > } > >@@ -788,9 +770,35 @@ bool RealtimeMediaSource::supportsConstraints(const MediaConstraints& constraint > ASSERT(constraints.isValid); > > FlattenedConstraint candidates; >- if (!selectSettings(constraints, candidates, invalidConstraint, SelectType::ForSupportsConstraints)) >+ if (!selectSettings(constraints, candidates, invalidConstraint)) > return false; >- >+ >+ m_fitnessScore = 0; >+ for (auto& variant : candidates) { >+ double distance = fitnessDistance(variant); >+ switch (variant.constraintType()) { >+ case MediaConstraintType::DeviceId: >+ case MediaConstraintType::FacingMode: >+ m_fitnessScore += distance ? 1 : 32; >+ break; >+ >+ case MediaConstraintType::Width: >+ case MediaConstraintType::Height: >+ case MediaConstraintType::FrameRate: >+ case MediaConstraintType::AspectRatio: >+ case MediaConstraintType::Volume: >+ case MediaConstraintType::SampleRate: >+ case MediaConstraintType::SampleSize: >+ case MediaConstraintType::EchoCancellation: >+ case MediaConstraintType::GroupId: >+ case MediaConstraintType::DisplaySurface: >+ case MediaConstraintType::LogicalSurface: >+ case MediaConstraintType::Unknown: >+ m_fitnessScore += distance ? 1 : 2; >+ break; >+ } >+ } >+ > return true; > } > >@@ -849,7 +857,7 @@ Optional<RealtimeMediaSource::ApplyConstraintsError> RealtimeMediaSource::applyC > > FlattenedConstraint candidates; > String failedConstraint; >- if (!selectSettings(constraints, candidates, failedConstraint, SelectType::ForApplyConstraints)) >+ if (!selectSettings(constraints, candidates, failedConstraint)) > return ApplyConstraintsError { failedConstraint, "Constraint not supported"_s }; > > applyConstraints(candidates); >diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSource.h b/Source/WebCore/platform/mediastream/RealtimeMediaSource.h >index ab9ed87d44bd5cd5c3bd707ae7c80940a907cb2c..afe1977908d2137a4c6dcee649073bd71bb8b451 100644 >--- a/Source/WebCore/platform/mediastream/RealtimeMediaSource.h >+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSource.h >@@ -188,8 +188,7 @@ protected: > virtual void beginConfiguration() { } > virtual void commitConfiguration() { } > >- enum class SelectType { ForApplyConstraints, ForSupportsConstraints }; >- bool selectSettings(const MediaConstraints&, FlattenedConstraint&, String&, SelectType); >+ bool selectSettings(const MediaConstraints&, FlattenedConstraint&, String&); > double fitnessDistance(const MediaConstraint&); > void applyConstraint(const MediaConstraint&); > void applyConstraints(const FlattenedConstraint&); >@@ -234,7 +233,7 @@ private: > double m_volume { 1 }; > double m_sampleRate { 0 }; > double m_sampleSize { 0 }; >- double m_fitnessScore { std::numeric_limits<double>::infinity() }; >+ double m_fitnessScore { 0 }; > RealtimeMediaSourceSettings::VideoFacingMode m_facingMode { RealtimeMediaSourceSettings::User}; > > bool m_pendingSettingsDidChangeNotification { false }; >diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp b/Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp >index 290b311815037415506f5789c961ffd4abb11e0e..3706fad7ffc03f7ecce6ef38c41b7ae3d2d35338 100644 >--- a/Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp >+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp >@@ -225,7 +225,7 @@ void RealtimeMediaSourceCenter::validateRequestConstraints(ValidConstraintsHandl > struct { > bool operator()(const DeviceInfo& a, const DeviceInfo& b) > { >- return a.fitnessScore < b.fitnessScore; >+ return a.fitnessScore > b.fitnessScore; > } > } sortBasedOnFitnessScore; > >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 5b043279ca759f9f5d4773871d9c2099da99ad47..524addfb32992103a0df633fc30eb8cc82261f92 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,13 @@ >+2019-02-13 Youenn Fablet <youenn@apple.com> >+ >+ getUserMedia with an ideal deviceId constraint doesn't always select the correct device >+ https://bugs.webkit.org/show_bug.cgi?id=193614 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * fast/mediastream/get-user-media-device-id-expected.txt: >+ * fast/mediastream/get-user-media-device-id.html: >+ > 2019-02-13 John Wilander <wilander@apple.com> > > Store Ad Click Attribution requests in the network process >diff --git a/LayoutTests/fast/mediastream/get-user-media-constraints-expected.txt b/LayoutTests/fast/mediastream/get-user-media-constraints-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..856fcc5f9efa99acaeec1c64dc54998e2af7c353 >--- /dev/null >+++ b/LayoutTests/fast/mediastream/get-user-media-constraints-expected.txt >@@ -0,0 +1,4 @@ >+ >+ >+FAIL Ideal deviceId constraints promise_test: Unhandled rejection with value: object "Error: Invalid constraint" >+ >diff --git a/LayoutTests/fast/mediastream/get-user-media-constraints.html b/LayoutTests/fast/mediastream/get-user-media-constraints.html >new file mode 100644 >index 0000000000000000000000000000000000000000..552cdebdcef0f6f92ca05e88889846c041bd9c37 >--- /dev/null >+++ b/LayoutTests/fast/mediastream/get-user-media-constraints.html >@@ -0,0 +1,24 @@ >+<!DOCTYPE html> >+<html> >+<head> >+ <meta charset="utf-8"> >+ <title>Test passing constraints to getUserMedia</title> >+ <script src="../../resources/testharness.js"></script> >+ <script src="../../resources/testharnessreport.js"></script> >+</head> >+<body> >+ <video autoplay playsinline id="localVideo"></video> >+ <script> >+ if (window.testRunner) >+ testRunner.setUserMediaPermission(true); >+ >+ promise_test(async (test) => { >+ localVideo.srcObject = await navigator.mediaDevices.getUserMedia({video: {width: {exact: 300}}}); >+ await localVideo.play(); >+ >+ assert_equals(localVideo.videoWidth, 300); >+ }, "Ideal deviceId constraints"); >+ >+ </script> >+</body> >+</html> >diff --git a/LayoutTests/fast/mediastream/get-user-media-device-id-expected.txt b/LayoutTests/fast/mediastream/get-user-media-device-id-expected.txt >index 6831a167d9a4a0019d6c2251029610c8752bba2d..b49891192811231fc8a1c212eed4e40987167aad 100644 >--- a/LayoutTests/fast/mediastream/get-user-media-device-id-expected.txt >+++ b/LayoutTests/fast/mediastream/get-user-media-device-id-expected.txt >@@ -4,4 +4,5 @@ PASS Collect device IDs > PASS Pass device IDs as exact constraints > PASS Pass device IDs as optional constraints > PASS Exact device IDs with special values: empty string, null, undefined >+PASS Ideal deviceId constraints > >diff --git a/LayoutTests/fast/mediastream/get-user-media-device-id.html b/LayoutTests/fast/mediastream/get-user-media-device-id.html >index d02069fec7a2e143a04e7d1b8aa46a9d4a701e68..f9621ae5123c48cb3c77a19da82f66138fd39378 100644 >--- a/LayoutTests/fast/mediastream/get-user-media-device-id.html >+++ b/LayoutTests/fast/mediastream/get-user-media-device-id.html >@@ -65,7 +65,7 @@ > return navigator.mediaDevices.getUserMedia(constraints) > .then((stream) => { > assert_equals(stream.getAudioTracks().length, 1, "correct number of audio tracks"); >- assert_equals(stream.getVideoTracks().length, 1, "correct number of audio tracks"); >+ assert_equals(stream.getVideoTracks().length, 1, "correct number of video tracks"); > }) > > }, "Pass device IDs as optional constraints"); >@@ -79,6 +79,20 @@ > await navigator.mediaDevices.getUserMedia({ audio: {deviceId: {exact: "test"}}}).then(assert_unreached, () => { }); > }, "Exact device IDs with special values: empty string, null, undefined"); > >+ promise_test(async (test) => { >+ await navigator.mediaDevices.getUserMedia({video: true}); >+ const devices = await navigator.mediaDevices.enumerateDevices(); >+ for (let device of devices) { >+ if (device.kind === "audioinput") { >+ const stream = await navigator.mediaDevices.getUserMedia({audio: {deviceId: device.deviceId}}); >+ assert_equals(stream.getAudioTracks()[0].getSettings().deviceId, device.deviceId, "Matching audio device id"); >+ } else { >+ const stream = await navigator.mediaDevices.getUserMedia({video: {deviceId: device.deviceId}}); >+ assert_equals(stream.getVideoTracks()[0].getSettings().deviceId, device.deviceId, "Matching video device id"); >+ } >+ } >+ }, "Ideal deviceId constraints"); >+ > </script> > </head> > <body>
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=361934">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193614
:
361932
| 361934