Make webrtc replace track tests less flaky
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Sep 2017 18:10:56 +0000 (18:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Sep 2017 18:10:56 +0000 (18:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172645
<rdar://problem/34118218>

Patch by Youenn Fablet <youenn@apple.com> on 2017-09-20
Reviewed by Eric Carlson.

Improving stability of tests by retrying tests every 50 ms instead of once.
Doing some refactoring to put more helper routines in routine.js
Relaxing black pixel check as compression may introduce some uncertainty.
It is now considered black if RGB < (30, 30, 30).

* TestExpectations: Marking video-mute.html as Pass/Fail instead of Fail.
* webrtc/routines.js:
* webrtc/video-mute.html:
* webrtc/video-replace-muted-track.html:
* webrtc/video-replace-track-expected.txt:
* webrtc/video-replace-track-to-null.html:
* webrtc/video-replace-track.html:

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/webrtc/routines.js
LayoutTests/webrtc/video-mute.html
LayoutTests/webrtc/video-replace-muted-track.html
LayoutTests/webrtc/video-replace-track-expected.txt
LayoutTests/webrtc/video-replace-track-to-null.html
LayoutTests/webrtc/video-replace-track.html

index d66f061..779c7ba 100644 (file)
@@ -1,3 +1,24 @@
+2017-09-20  Youenn Fablet  <youenn@apple.com>
+
+        Make webrtc replace track tests less flaky
+        https://bugs.webkit.org/show_bug.cgi?id=172645
+        <rdar://problem/34118218>
+
+        Reviewed by Eric Carlson.
+
+        Improving stability of tests by retrying tests every 50 ms instead of once.
+        Doing some refactoring to put more helper routines in routine.js
+        Relaxing black pixel check as compression may introduce some uncertainty.
+        It is now considered black if RGB < (30, 30, 30).
+
+        * TestExpectations: Marking video-mute.html as Pass/Fail instead of Fail.
+        * webrtc/routines.js:
+        * webrtc/video-mute.html:
+        * webrtc/video-replace-muted-track.html:
+        * webrtc/video-replace-track-expected.txt:
+        * webrtc/video-replace-track-to-null.html:
+        * webrtc/video-replace-track.html:
+
 2017-09-20  Chris Dumez  <cdumez@apple.com>
 
         Video errors should be instances of Error
index e7dd6eb..b9f6a5f 100644 (file)
@@ -798,7 +798,7 @@ media/session [ Skip ]
 webrtc/datachannel/multiple-connections.html [ Slow ]
 webrtc/video-replace-track.html [ Pass Failure ]
 webkit.org/b/170178 webrtc/video-replace-track-to-null.html [ Pass Failure ]
-webkit.org/b/170704 webrtc/video-mute.html [ Failure ]
+webkit.org/b/170704 webrtc/video-mute.html [ Pass Failure ]
 webkit.org/b/170701 webrtc/datachannel/bufferedAmountLowThreshold-default.html [ Pass Failure ]
 webkit.org/b/173486 webrtc/video-replace-muted-track.html [ Pass Failure Timeout ]
 
index 4135aba..8d8cc63 100644 (file)
@@ -161,3 +161,41 @@ async function doHumAnalysis(stream, expected)
     await context.close();
     return false;
 }
+
+function isVideoBlack(canvas, video, startX, startY, grabbedWidth, grabbedHeight)
+{
+    canvas.width = video.videoWidth;
+    canvas.height = video.videoHeight;
+    if (!grabbedHeight) {
+        startX = 0;
+        startY = 0;
+        grabbedWidth = canvas.width;
+        grabbedHeight = canvas.height;
+    }
+
+    canvas.getContext('2d').drawImage(video, 0, 0, canvas.width, canvas.height);
+
+    imageData = canvas.getContext('2d').getImageData(startX, startY, grabbedWidth, grabbedHeight);
+    data = imageData.data;
+    for (var cptr = 0; cptr < grabbedWidth * grabbedHeight; ++cptr) {
+        // Approximatively black pixels.
+        if (data[4 * cptr] > 30 || data[4 * cptr + 1] > 30 || data[4 * cptr + 2] > 30)
+            return false;
+    }
+    return true;
+}
+
+async function checkVideoBlack(expected, canvas, video, errorMessage, counter)
+{
+    if (isVideoBlack(canvas, video) === expected)
+        return Promise.resolve();
+
+    if (counter > 50) {
+        if (!errorMessage)
+            errorMessage = "checkVideoBlack timed out expecting " + expected;
+        return Promise.reject(errorMessage);
+    }
+
+    await waitFor(50);
+    return checkVideoBlack(expected, canvas, video, errorMessage, ++counter);
+}
index a03b219..53cdeb4 100644 (file)
         <canvas id="canvas3" width="320" height="240"></canvas>
         <script src ="routines.js"></script>
         <script>
-function isVideoBlack(id)
-{
-    var canvas = document.getElementById(id);
-    canvas.width = video.videoWidth;
-    canvas.height = video.videoHeight;
-    canvas.getContext('2d').drawImage(video, 0, 0, canvas.width, canvas.height);
-
-    imageData = canvas.getContext('2d').getImageData(0, 0, canvas.width, canvas.height);
-    data = imageData.data;
-    for (var cptr = 0; cptr < canvas.width * canvas.height; ++cptr) {
-        // Approximatively black pixels.
-        if (data[4 * cptr] > 10 || data[4 * cptr + 1] > 10 || data[4 * cptr + 2] > 10)
-            return false;
-    }
-    return true;
-}
-
-function pollVideoBlackCheck(expected, id, resolve)
-{
-    if (isVideoBlack(id) === expected) {
-        resolve();
-        return;
-    }
-
-    setTimeout(() => pollVideoBlackCheck(expected, id, resolve), 50);
-}
-
-function checkVideoBlack(expected, id)
-{
-    return new Promise((resolve, reject) => {
-       pollVideoBlackCheck(expected, id, resolve);
-        setTimeout(() => reject("checkVideoBlack timed out for " + id + " expected " + expected), 5000);
-    });
-}
-
 var track;
 var remoteTrack;
 var receivingConnection;
@@ -91,12 +56,12 @@ promise_test(() => {
 }, "Ensuring connection state is connected");
 
 promise_test((test) => {
-    return checkVideoBlack(false, "canvas1");
+    return checkVideoBlack(false, canvas1, video);
 }, "Track is enabled, video should not be black");
 
 promise_test((test) => {
     track.enabled = false;
-    return checkVideoBlack(true, "canvas2");
+    return checkVideoBlack(true, canvas2, video);
 }, "Track is disabled, video should be black");
 
 promise_test((test) => {
@@ -116,7 +81,7 @@ promise_test((test) => {
 
 promise_test((test) => {
     track.enabled = true;
-    return checkVideoBlack(true, "canvas2");
+    return checkVideoBlack(false, canvas2, video);
 }, "Track is enabled, video should not be black");
 
         </script>
index 390d974..e357bfe 100644 (file)
         <canvas id="canvas" width="640" height="480"></canvas>
         <script src ="routines.js"></script>
         <script>
-function isVideoBlack()
-{
-    canvas.width = video.videoWidth;
-    canvas.height = video.videoHeight;
-    canvas.getContext('2d').drawImage(video, 0, 0, canvas.width, canvas.height);
-
-    imageData = canvas.getContext('2d').getImageData(0, 0, canvas.width, canvas.height);
-    data = imageData.data;
-    for (var cptr = 0; cptr < canvas.width * canvas.height; ++cptr) {
-        // Approximatively black pixels.
-        if (data[4 * cptr] > 10 || data[4 * cptr + 1] > 10 || data[4 * cptr + 2] > 10)
-            return false;
-    }
-    return true;
-}
-
-var stopPolling = false;
-function pollVideoBlackCheck(expected, resolve, delay)
-{
-    if (isVideoBlack() === expected || stopPolling) {
-        resolve(true);
-        return;
-    }
-
-    setTimeout(() => pollVideoBlackCheck(expected, resolve), delay);
-}
-
-async function checkVideoBlack(expected, duration, id, delay)
-{
-    stopPolling = false;
-    return new Promise((resolve, reject) => {
-        pollVideoBlackCheck(expected, resolve, delay);
-        setTimeout(() => {
-            resolve(false);
-            stopPolling = true;
-        }, duration);
-    });
-}
-
-function grabImagePixels()
-{
-    canvas.width = video.videoWidth;
-    canvas.height = video.videoHeight;
-    canvas.getContext('2d').drawImage(video, 0, 0, canvas.width, canvas.height);
-
-    imageData = canvas.getContext('2d').getImageData(20, 20, 2, 2);
-    return imageData.data;
-}
-
-async function checkForNoBlackFrames()
-{
-    for (var cptr = 0; cptr < 10; cptr++) {
-        if (!(await checkVideoBlack(true, 1500, "test3", 50)))
-            return true;
-    }
-    return false;
-}
-
 promise_test((test) => {
     if (window.testRunner)
         testRunner.setUserMediaPermission(true);
@@ -94,22 +36,15 @@ promise_test((test) => {
         return video.play();
     }).then(() => {
         firstTrack.enabled = false;
-        return waitFor(200);
+        return checkVideoBlack(true, canvas, video, "Check we receive black frames after disabling the track");
     }).then(() => {
         return navigator.mediaDevices.getUserMedia({ video: { facingMode: { exact: ["environment"] } } });
     }).then((stream) => {
         return sender.replaceTrack(stream.getVideoTracks()[0]);
     }).then(() => {
-        return waitFor(200);
-    }).then(() => {
-        return checkVideoBlack(false, 5000, "test2", 200);
-    }).then((result) => {
-        assert_true(result, "expect to receive some non black frames");
+        return checkVideoBlack(false, canvas, video, "Check we receive some non black frames");
     }).then(() => {
-        // Let's ensure there are no black frames being received.
-        return checkForNoBlackFrames();
-    }).then((result) => {
-        assert_true(result, "check no black frame");
+        return checkVideoBlack(true, canvas, video, "Check we do not receive any black frame", 40).then(assert_unreached, () => { });
     });
 }, "Switching from disabled to enabled track");
         </script>
index 4e1efda..6758221 100644 (file)
@@ -2,10 +2,4 @@
 PASS Switching from front to back camera 
 PASS Switching from front to back camera, with lower resolution 
 PASS Switching from front to back camera, with higher resolution 
-PASS testFrontCameraImage test1 
-PASS testBackCameraImage test1 
-PASS testFrontCameraImage test2 
-PASS testBackCameraImage test2 
-PASS testFrontCameraImage test3 
-PASS testBackCameraImage test4 
 
index 78787bf..d0a7bd5 100644 (file)
 video = document.getElementById("video");
 canvas = document.getElementById("canvas");
 
-function grabImagePixels()
+async function getOutboundRTPStatsNumberOfEncodedFrames(connection)
 {
-    canvas.width = video.videoWidth;
-    canvas.height = video.videoHeight;
-    canvas.getContext('2d').drawImage(video, 0, 0, canvas.width, canvas.height);
-
-    imageData = canvas.getContext('2d').getImageData(20, 20, 100, 100);
-    return imageData.data;
- }
-
-var firstFrameData;
-
-function storeFrame()
-{
-    firstFrameData = grabImagePixels();
+    var report = await connection.getStats();
+    var framesEncoded;
+    report.forEach((statItem) => {
+        if (statItem.type === "outbound-rtp") {
+            framesEncoded = statItem.framesEncoded;
+        }
+    });
+    return framesEncoded;
 }
 
-function checkCameraImageIsDifferent()
+async function testFrameEncodedStarted(connection, count, previousFramesNumber)
 {
-    data = grabImagePixels();
+    var framesEncodedNumber = await getOutboundRTPStatsNumberOfEncodedFrames(connection);
+    if (previousFramesNumber && framesEncodedNumber > previousFramesNumber)
+        return;
 
-    assert_true(data[0] < 20);
-    assert_true(data[1] < 20);
-    assert_true(data[2] < 20);
+    if (count === undefined)
+        count = 0;
 
-    var same = true;
-    for (var cptr = 0; cptr < data.length; ++cptr) {
-        if (data[cptr] != firstFrameData[cptr]) {
-            same = false;
-            break;
-        }
-    }
-    return !same;
+    if (count >= 20)
+        return Promise.reject("test increasing frame encoded timed out");
+
+    await waitFor(100);
+    return testFrameEncodedStarted(connection, ++count, framesEncodedNumber);
 }
 
-function testCameraImage(resolve, reject, counter)
+async function testFrameEncodedStopped(connection, count, previousFramesNumber)
 {
-    if (!counter)
-        counter = 0;
-
-    if (checkCameraImageIsDifferent()) {
-        resolve();
+    var framesEncodedNumber = await getOutboundRTPStatsNumberOfEncodedFrames(connection);
+    if (previousFramesNumber && framesEncodedNumber === previousFramesNumber)
         return;
-    }
-    if (++counter === 40) {
-        reject("testCameraImage timed out");
-        return;
-    }
-    setTimeout(() => testCameraImage(resolve, reject, counter), 100);
-}
 
-function testStoppedImage()
-{
-    assert_array_equals(grabImagePixels(), firstFrameData);
+    if (count === undefined)
+        count = 0;
+
+    if (count === 20)
+        return Promise.reject("test that number of encoded frames stopped increasing timed out");
+
+    await waitFor(100);
+    return testFrameEncodedStopped(connection, ++count, framesEncodedNumber);
 }
 
 promise_test((test) => {
@@ -77,11 +65,13 @@ promise_test((test) => {
 
     var sender;
     var frontStream;
+    var connection;
     return navigator.mediaDevices.getUserMedia({ video: true }).then((stream) => {
         frontStream = stream;
     }).then(() => {
         return new Promise((resolve, reject) => {
             createConnections((firstConnection) => {
+                connection = firstConnection;
                 sender = firstConnection.addTrack(frontStream.getVideoTracks()[0], frontStream);
             }, (secondConnection) => {
                 secondConnection.ontrack = (trackEvent) => {
@@ -94,25 +84,14 @@ promise_test((test) => {
         video.srcObject = remoteStream;
         return video.play();
     }).then(() => {
-        return waitFor(100);
+        return testFrameEncodedStarted(connection);
     }).then(() => {
-        storeFrame();
-        return waitFor(100);
-    }).then(() => {
-        return new Promise((resolve, reject) => {
-            testCameraImage(resolve, reject);
-        });
     }).then(() => {
         promise = sender.replaceTrack(null);
         assert_true(!!sender.track);
         return promise;
     }).then(() => {
-        return waitFor(100);
-    }).then(() => {
-        storeFrame();
-        return waitFor(100);
-    }).then(() => {
-        testStoppedImage();
+        return testFrameEncodedStopped(connection);
     });
 }, "Stopping sending video using replaceTrack");
         </script>
index 4f37958..71739aa 100644 (file)
@@ -5,6 +5,7 @@
         <title>Testing basic video exchange from offerer to receiver</title>
         <script src="../resources/testharness.js"></script>
         <script src="../resources/testharnessreport.js"></script>
+        <script src="routines.js"></script>
     </head>
     <body>
 <div id="log"></div>
@@ -25,30 +26,41 @@ function grabImagePixels()
     return imageData.data;
  }
 
-function testFrontCameraImage(testName)
+async function testFrontCameraImage(counter)
 {
-    test(() => {
-        data = grabImagePixels();
+    if (!counter)
+        counter = 0;
 
-        assert_true(data[0] < 20, "1");
-        assert_true(data[1] < 20, "2");
-        assert_true(data[2] < 20, "3");
-    }, "testFrontCameraImage " + testName);
+    if (isVideoBlack(canvas, video, 20, 20, 2 ,2))
+        return;
+
+    if (counter >= 20)
+        return Promise.reject("testFrontCameraImage timed out");
+
+    await waitFor(50);
+    return testFrontCameraImage(++counter);
 }
 
-function testBackCameraImage(testName)
+function isBetween100And200(value)
 {
-    test(() => {
-        data = grabImagePixels();
+     return data[0] > 100 && data[0] < 200;
+}
+
+async function testBackCameraImage(counter)
+{
+    if (!counter)
+        counter = 0;
+
+    data = grabImagePixels();
+
+    if(isBetween100And200(data[0]) && isBetween100And200(data[1]) && isBetween100And200(data[2]))
+        return;
 
-        assert_true(data[0] > 100, "1");
-        assert_true(data[1] > 100, "2");
-        assert_true(data[2] > 100, "3");
+    if (counter >= 20)
+        return Promise.reject("testFrontCameraImage timed out");
 
-        assert_true(data[0] < 200, "4");
-        assert_true(data[1] < 200, "5");
-        assert_true(data[2] < 200, "6");
-    }, "testBackCameraImage " + testName);
+    await waitFor(50);
+    return testBackCameraImage(++counter);
 }
 
 promise_test((test) => {
@@ -74,7 +86,8 @@ promise_test((test) => {
         video.srcObject = remoteStream;
         return video.play();
     }).then(() => {
-        testFrontCameraImage("test1");
+        return testFrontCameraImage();
+    }).then(() => {
         return navigator.mediaDevices.getUserMedia({ video: { facingMode: { exact: ["environment"] } } });
     }).then((stream) => {
         backStream = stream;
@@ -84,9 +97,7 @@ promise_test((test) => {
         return promise;
     }).then(() => {
         assert_true(sender.track === backStream.getVideoTracks()[0]);
-        return waitFor(500);
-    }).then(() => {
-        testBackCameraImage("test1");
+        return testBackCameraImage();
     });
 }, "Switching from front to back camera");
 
@@ -115,16 +126,15 @@ promise_test((test) => {
         video.srcObject = remoteStream;
         return video.play();
     }).then(() => {
-        testFrontCameraImage("test2");
+        return testFrontCameraImage();
+    }).then(() => {
         return navigator.mediaDevices.getUserMedia({ video: { width: 320, height: 240, facingMode: { exact: ["environment"] } } });
     }).then((stream) => {
         backStream = stream;
         assert_true(backStream.getVideoTracks()[0].getSettings().height === 240, "backStream should be small");
         return sender.replaceTrack(backStream.getVideoTracks()[0]);
     }).then(() => {
-        return waitFor(500);
-    }).then(() => {
-        testBackCameraImage("test2");
+        return testBackCameraImage();
     });
 }, "Switching from front to back camera, with lower resolution");
 
@@ -154,16 +164,15 @@ promise_test((test) => {
         video.srcObject = remoteStream;
         return video.play();
     }).then(() => {
-        testFrontCameraImage("test3");
+        return testFrontCameraImage();
+    }).then(() => {
         return navigator.mediaDevices.getUserMedia({ video: { width: 640, height: 480 , facingMode: { exact: ["environment"] } } });
     }).then((stream) => {
         backStream = stream;
         assert_true(backStream.getVideoTracks()[0].getSettings().height === 480, "back stream should be big");
         return sender.replaceTrack(backStream.getVideoTracks()[0]);
     }).then(() => {
-        return waitFor(500);
-    }).then(() => {
-        testBackCameraImage("test4");
+        return testBackCameraImage();
     });
 
 }, "Switching from front to back camera, with higher resolution");