RTCRtpSender.setParameters() does set active parameter
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Dec 2018 02:14:31 +0000 (02:14 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Dec 2018 02:14:31 +0000 (02:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192848

Reviewed by Eric Carlson.

Source/WebCore:

Covered by updated test.

* Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp:
(WebCore::updateRTCRtpSendParameters):
The routine was updating the local value, not the out parameter.

LayoutTests:

* webrtc/video.html:
Add a check for active value.
Test video freezing through canvas instead of stats.

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

LayoutTests/ChangeLog
LayoutTests/webrtc/video.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp

index d94c455..e4c2fd3 100644 (file)
@@ -1,3 +1,14 @@
+2018-12-21  Youenn Fablet  <youenn@apple.com>
+
+        RTCRtpSender.setParameters() does set active parameter
+        https://bugs.webkit.org/show_bug.cgi?id=192848
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/video.html:
+        Add a check for active value.
+        Test video freezing through canvas instead of stats.
+
 2018-12-21  Justin Michaud  <justin_michaud@apple.com>
 
         CSS variables don't work for colors in "border" property
index f75a460..c4180e9 100644 (file)
 video = document.getElementById("video");
 canvas = document.getElementById("canvas");
 
-function testImage()
+function grabFrameData(x, y, w, h)
 {
     canvas.width = video.videoWidth;
     canvas.height = video.videoHeight;
-    canvas.getContext('2d').drawImage(video, 0, 0, canvas.width, canvas.height);
 
-    imageData = canvas.getContext('2d').getImageData(10, 325, 250, 1);
-    data = imageData.data;
+    canvas.getContext('2d').drawImage(video, x, y, w, h, x, y, w, h);
+    return canvas.getContext('2d').getImageData(x, y, w, h).data;
+}
+
+function testImage()
+{
+    const data = grabFrameData(10, 325, 250, 1);
 
     var index = 20;
     assert_true(data[index] < 100);
@@ -72,69 +76,50 @@ promise_test(async (test) => {
     testImage();
 }, "Basic video exchange");
 
-async function getInboundRTPStatsNumberOfDecodedFrames(connection)
+function getCircleImageData()
 {
-    var report = await connection.getStats();
-    var framesDecoded;
-    report.forEach((statItem) => {
-        if (statItem.type === "inbound-rtp")
-            framesDecoded = statItem.framesDecoded;
-    });
-    return framesDecoded;
+    return grabFrameData(450, 100, 150, 100);
 }
 
-async function testFrameDecodedIncreased(connection, count, previousFramesNumber)
+async function checkVideoIsUpdated(shouldBeUpdated, count, referenceData)
 {
-    if (previousFramesNumber === undefined) {
-        let number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
-        await waitFor(1000);
-        return testFrameDecodedIncreased(connection, 0, number);
-    }
-
-    var number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
-    if (previousFramesNumber && number > previousFramesNumber)
-        return;
+    if (count === undefined)
+        count = 0;
+    else if (count >= 20)
+        return Promise.reject("checkVideoIsUpdated timed out :" + shouldBeUpdated + " " + count);
 
-    if (count >= 20)
-        return Promise.reject("test increasing frame encoded timed out");
+    if (referenceData === undefined)
+        referenceData = getCircleImageData();
 
-    await waitFor(1000);
-    return testFrameDecodedIncreased(connection, ++count, previousFramesNumber);
-}
+    await waitFor(200);
+    const newData = getCircleImageData();
 
-async function testFrameDecodedDidNotIncreased(connection, count, previousFramesNumber)
-{
-    if (previousFramesNumber === undefined) {
-        let number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
-        await waitFor(100);
-        return testFrameDecodedDidNotIncreased(connection, 0, number);
-    }
-
-    var number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
-    if (previousFramesNumber && number == previousFramesNumber)
+    if (shouldBeUpdated === (JSON.stringify(referenceData) !== JSON.stringify(newData)))
         return;
 
-    if (count >= 20)
-        return Promise.reject("test increasing frame encoded timed out");
-
-    await waitFor(100);
-    return testFrameDecodedIncreased(connection, ++count, number);
+    await checkVideoIsUpdated(shouldBeUpdated, ++count, newData);
 }
 
 promise_test(async (test) => {
-   let p = pc1.getSenders()[0].getParameters();
-   p.encodings[0].active = false;
-   await pc1.getSenders()[0].setParameters(p);
+    const sender = pc1.getSenders()[0];
+    let p = sender.getParameters();
+    p.encodings[0].active = false;
+    await sender.setParameters(p);
+
+    assert_false(sender.getParameters().encodings[0].active, "encodings[0].active should be false");
 
-   await testFrameDecodedDidNotIncreased(pc2);
+    await checkVideoIsUpdated(false);
 }, "Call setParameters to disable sending a given encoding");
 
 promise_test(async (test) => {
-   let p = pc1.getSenders()[0].getParameters();
-   p.encodings[0].active = true;
-   await pc1.getSenders()[0].setParameters(p);
+    const sender = pc1.getSenders()[0];
+    let p = sender.getParameters();
+    p.encodings[0].active = true;
+    await sender.setParameters(p);
+
+    assert_true(sender.getParameters().encodings[0].active, "encodings[0].active should be true");
 
-   await testFrameDecodedIncreased(pc2);
+    await checkVideoIsUpdated(true);
 }, "Call setParameters to reenable sending a given encoding");
         </script>
     </body>
index 5bb8de0..24291cc 100644 (file)
@@ -1,3 +1,16 @@
+2018-12-21  Youenn Fablet  <youenn@apple.com>
+
+        RTCRtpSender.setParameters() does set active parameter
+        https://bugs.webkit.org/show_bug.cgi?id=192848
+
+        Reviewed by Eric Carlson.
+
+        Covered by updated test.
+
+        * Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp:
+        (WebCore::updateRTCRtpSendParameters):
+        The routine was updating the local value, not the out parameter.
+
 2018-12-21  Eric Carlson  <eric.carlson@apple.com>
 
         'ended' Event doesn't fire on MediaStreamTrack when a USB camera is unplugged
index ddbb5f0..fa91dc6 100644 (file)
@@ -160,10 +160,8 @@ RTCRtpSendParameters toRTCRtpSendParameters(const webrtc::RtpParameters& rtcPara
     return parameters;
 }
 
-void updateRTCRtpSendParameters(const RTCRtpSendParameters& parameters, webrtc::RtpParameters& currentParameters)
+void updateRTCRtpSendParameters(const RTCRtpSendParameters& parameters, webrtc::RtpParameters& rtcParameters)
 {
-    webrtc::RtpParameters rtcParameters = currentParameters;
-
     rtcParameters.transaction_id = parameters.transactionId.utf8().data();
 
     if (parameters.encodings.size() != rtcParameters.encodings.size()) {