getUserMedia with an ideal deviceId constraint doesn't always select the correct...
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Feb 2019 01:25:39 +0000 (01:25 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Feb 2019 01:25:39 +0000 (01:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193614

Source/WebCore:

Reviewed by Eric Carlson.

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):

LayoutTests:

Reviewed by Eric Carlson.

* fast/mediastream/get-user-media-device-id-expected.txt:
* fast/mediastream/get-user-media-device-id.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241489 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/mediastream/get-user-media-constraints-expected.txt [new file with mode: 0644]
LayoutTests/fast/mediastream/get-user-media-constraints.html [new file with mode: 0644]
LayoutTests/fast/mediastream/get-user-media-device-id-expected.txt
LayoutTests/fast/mediastream/get-user-media-device-id.html
Source/WebCore/ChangeLog
Source/WebCore/platform/mediastream/MediaConstraints.cpp
Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
Source/WebCore/platform/mediastream/RealtimeMediaSource.h
Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp

index 15a3d8d..f900e4e 100644 (file)
@@ -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 Eric Carlson.
+
+        * fast/mediastream/get-user-media-device-id-expected.txt:
+        * fast/mediastream/get-user-media-device-id.html:
+
 2019-02-13  Eric Carlson  <eric.carlson@apple.com>
 
         [iOS] Add a hack to work around buggy video control library
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 (file)
index 0000000..856fcc5
--- /dev/null
@@ -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 (file)
index 0000000..552cdeb
--- /dev/null
@@ -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>
index 6831a16..b498911 100644 (file)
@@ -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 
 
index d02069f..f9621ae 100644 (file)
@@ -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");
         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>
index d05b130..7197ad9 100644 (file)
@@ -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 Eric Carlson.
+
+        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  Eric Carlson  <eric.carlson@apple.com>
 
         [iOS] Add a hack to work around buggy video control library
index 60dea71..7c40e23 100644 (file)
@@ -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);
 }
     
 }
index ce0f9aa..c83ca74 100644 (file)
@@ -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);
index ab9ed87..afe1977 100644 (file)
@@ -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 };
index 290b311..3706fad 100644 (file)
@@ -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;