Attachment 361932 Details for Bug 193614 – Patch

[patch] Patch

bug-193614-20190213125051.patch (text/plain), 16.35 KB, created by youenn fablet on 2019-02-13 12:50:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-02-13 12:50:53 PST

Size: 16.35 KB

patch

obsolete

>Subversion Revision: 241123
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index bd4a49cd0e4e62202aa81b86976c3033ebddd58c..9f4b03207c5040776081208f46abb7b35b2be6ca 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-08  Youenn Fablet  <youenn@apple.com>
> 
>         Make Request constructor throw if FetchRequestInit.signal is not undefined, null or an AbortSignal object
>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 7f39db216da8abeec76344379e51b1a7e941852d..3742152f6859f603385ee5d612e2a1d766c28a80 100644
>--- a/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
>+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
>@@ -412,7 +412,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: {
>@@ -568,9 +569,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
>     //
>@@ -595,7 +596,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) {
>@@ -607,25 +608,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();
>@@ -640,7 +622,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())
>@@ -676,7 +658,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.
>@@ -689,7 +671,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()) {
>@@ -698,7 +680,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);
>         }
>     }
> 
>@@ -787,9 +769,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;
> }
> 
>@@ -848,7 +856,7 @@ Optional<std::pair<String, String>> RealtimeMediaSource::applyConstraints(const
> 
>     FlattenedConstraint candidates;
>     String failedConstraint;
>-    if (!selectSettings(constraints, candidates, failedConstraint, SelectType::ForApplyConstraints))
>+    if (!selectSettings(constraints, candidates, failedConstraint))
>         return { { failedConstraint, "Constraint not supported"_s } };
> 
>     applyConstraints(candidates);
>diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSource.h b/Source/WebCore/platform/mediastream/RealtimeMediaSource.h
>index 79a79e105886df5c990894105f2e10887ceb2d00..e862aacf7b50175dd28ff4084880b01f68ce6c1d 100644
>--- a/Source/WebCore/platform/mediastream/RealtimeMediaSource.h
>+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSource.h
>@@ -185,8 +185,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&);
>@@ -231,7 +230,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 6d634932e5a20b393d5132b68d8860356438702b..e9cf59ad258a714f86475410734d0aa902691bd7 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-08  Youenn Fablet  <youenn@apple.com>
> 
>         Make Request constructor throw if FetchRequestInit.signal is not undefined, null or an AbortSignal object
>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>

Actions: View | Formatted Diff | Diff

Attachments on bug 193614: 361932 | 361934