Attachment 349320 Details for Bug 189477 – Patch

[patch] Patch

bug-189477-20180910120007.patch (text/plain), 11.50 KB, created by youenn fablet on 2018-09-10 12:00:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-09-10 12:00:08 PDT

Size: 11.50 KB

patch

obsolete

>Subversion Revision: 235849
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index dd210ef1910c03d091ebbd781891c8c3e2512f31..83d94e1ebd0cef831954229a27dec2961f31aeb3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-09-10  Youenn Fablet  <youenn@apple.com>
>+
>+        ontrack events should be fired even if an existing transceiver exists
>+        https://bugs.webkit.org/show_bug.cgi?id=189477
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        In case of an OnTrack callback from libwebrtc, make sure the ontrack event is called even if a transceiver already exists for that track.
>+
>+        Covered by updated video-addTransceiver.html
>+        New test video-addLegacyTransceiver.html keeps testing the old transceiver behavior.
>+        Test: webrtc/video-addLegacyTransceiver.html
>+
>+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
>+        (WebCore::LibWebRTCMediaEndpoint::newTransceiver):
>+
> 2018-09-10  Alicia Boya GarcÃa  <aboya@igalia.com>
> 
>         [GStreamer] Fix overflow in buffered ranges
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>index 01b4aa2dc595fcc1dfe16e998e3c960a4851a0d7..3e07af5753a63df43a53a6e988627f8be155353f 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>@@ -427,7 +427,9 @@ void LibWebRTCMediaEndpoint::newTransceiver(rtc::scoped_refptr<webrtc::RtpTransc
>         return rtcTransceiver.get() == transceiverBackend.rtcTransceiver();
>     });
>     if (transceiver) {
>-        setExistingReceiverSourceTrack(transceiver->receiver().track().source(), *rtcTransceiver->receiver());
>+        auto rtcReceiver = rtcTransceiver->receiver();
>+        setExistingReceiverSourceTrack(transceiver->receiver().track().source(), *rtcReceiver);
>+        fireTrackEvent(makeRef(transceiver->receiver()), transceiver->receiver().track(), rtcReceiver->streams(), makeRef(*transceiver));
>         return;
>     }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 608c3d645bfb51ed981d24bf68ad0a4ac7cb5f1d..f27fbdb189dce1d30b6fe79147393958de7fcbdc 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-09-10  Youenn Fablet  <youenn@apple.com>
>+
>+        ontrack events should be fired even if an existing transceiver exists
>+        https://bugs.webkit.org/show_bug.cgi?id=189477
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * webrtc/video-addLegacyTransceiver-expected.txt: Added.
>+        * webrtc/video-addLegacyTransceiver.html: Copied from LayoutTests/webrtc/video-addTransceiver.html.
>+        * webrtc/video-addTransceiver.html:
>+
> 2018-09-10  Per Arne Vollan  <pvollan@apple.com>
> 
>         [Windows] Layout Test webanimations/accelerated-transition-interrupted-on-composited-element.html is failing
>diff --git a/LayoutTests/webrtc/video-addLegacyTransceiver-expected.txt b/LayoutTests/webrtc/video-addLegacyTransceiver-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..8790e6b8b4104da2260286290776d4330788d8c6
>--- /dev/null
>+++ b/LayoutTests/webrtc/video-addLegacyTransceiver-expected.txt
>@@ -0,0 +1,6 @@
>+
>+
>+PASS Setting up calls with addTransceiver but with no track 
>+PASS Setting up calls with addTransceiver with a track 
>+PASS Basic video exchange set up with addTransceiver 
>+
>diff --git a/LayoutTests/webrtc/video-addLegacyTransceiver.html b/LayoutTests/webrtc/video-addLegacyTransceiver.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..3ca754c6e692f8468909bd888ec8836a3ec4a388
>--- /dev/null
>+++ b/LayoutTests/webrtc/video-addLegacyTransceiver.html
>@@ -0,0 +1,101 @@
>+<!doctype html>
>+<html>
>+    <head>
>+        <meta charset="utf-8">
>+        <title>Testing basic video exchange from offerer to receiver</title>
>+        <script src="../resources/testharness.js"></script>
>+        <script src="../resources/testharnessreport.js"></script>
>+    </head>
>+    <body>
>+        <video id="video" autoplay=""></video>
>+        <canvas id="canvas" width="640" height="480"></canvas>
>+        <script src ="routines.js"></script>
>+        <script>
>+if (window.testRunner)
>+    testRunner.setWebRTCUnifiedPlanEnabled(false);
>+
>+promise_test((test) => {
>+    var pc = new RTCPeerConnection();
>+    pc.addTransceiver("video");
>+
>+    return pc.createOffer().then((offer) => {
>+        assert_true(offer.sdp.indexOf("mid:video") !== -1);
>+        assert_true(offer.sdp.indexOf("a=recvonly") !== -1);
>+
>+        pc.addTransceiver("audio");
>+        return pc.createOffer();
>+    }).then((offer) => {
>+        assert_true(offer.sdp.indexOf("mid:audio") !== -1);
>+    });
>+}, "Setting up calls with addTransceiver but with no track");
>+
>+promise_test((test) => {
>+    if (window.testRunner)
>+        testRunner.setUserMediaPermission(true);
>+
>+    return navigator.mediaDevices.getUserMedia({ video: true }).then((stream) => {
>+        var pc = new RTCPeerConnection();
>+        pc.addTransceiver("video");
>+        pc.getSenders()[0].replaceTrack(stream.getVideoTracks()[0]);
>+
>+        return pc.createOffer().then((offer) => {
>+            assert_true(offer.sdp.indexOf("mid:video") !== -1);
>+            // Replacing the track is not done yet so we still set it as a recvonly.
>+            assert_true(offer.sdp.indexOf("a=recvonly") !== -1);
>+        });
>+    });
>+}, "Setting up calls with addTransceiver with a track");
>+
>+function testImage()
>+{
>+    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;
>+
>+    var index = 20;
>+    assert_true(data[index] < 100);
>+    assert_true(data[index + 1] < 100);
>+    assert_true(data[index + 2] < 100);
>+
>+    index = 80;
>+    assert_true(data[index] > 200);
>+    assert_true(data[index + 1] > 200);
>+    assert_true(data[index + 2] > 200);
>+
>+    index += 80;
>+    assert_true(data[index] > 200);
>+    assert_true(data[index + 1] > 200);
>+    assert_true(data[index + 2] < 100);
>+}
>+
>+promise_test((test) => {
>+    if (window.testRunner)
>+        testRunner.setUserMediaPermission(true);
>+
>+    return navigator.mediaDevices.getUserMedia({ video: true}).then((stream) => {
>+        return new Promise((resolve, reject) => {
>+            createConnections((firstConnection) => {
>+                var track = stream.getVideoTracks()[0];
>+                firstConnection.addTransceiver("video");
>+                return firstConnection.getSenders()[0].replaceTrack(stream.getVideoTracks()[0]);
>+            }, (secondConnection) => {
>+                secondConnection.ontrack = (trackEvent) => {
>+                    resolve(trackEvent.streams[0]);
>+                };
>+            });
>+            setTimeout(() => reject("Test timed out"), 5000);
>+        });
>+    }).then((stream) => {
>+        video.srcObject = stream;
>+        return video.play();
>+    }).then(() => {
>+        testImage();
>+    });
>+}, "Basic video exchange set up with addTransceiver");
>+
>+        </script>
>+    </body>
>+</html>
>diff --git a/LayoutTests/webrtc/video-addTransceiver.html b/LayoutTests/webrtc/video-addTransceiver.html
>index 3ca754c6e692f8468909bd888ec8836a3ec4a388..42c7c993ee624f2ce23dbff09695a4ee54a6650e 100644
>--- a/LayoutTests/webrtc/video-addTransceiver.html
>+++ b/LayoutTests/webrtc/video-addTransceiver.html
>@@ -11,39 +11,32 @@
>         <canvas id="canvas" width="640" height="480"></canvas>
>         <script src ="routines.js"></script>
>         <script>
>-if (window.testRunner)
>-    testRunner.setWebRTCUnifiedPlanEnabled(false);
>-
>-promise_test((test) => {
>+promise_test(async (test) => {
>     var pc = new RTCPeerConnection();
>     pc.addTransceiver("video");
> 
>-    return pc.createOffer().then((offer) => {
>-        assert_true(offer.sdp.indexOf("mid:video") !== -1);
>-        assert_true(offer.sdp.indexOf("a=recvonly") !== -1);
>+    let offer = await pc.createOffer();
>+    assert_true(offer.sdp.indexOf("m=video") !== -1, "mid");
>+    assert_true(offer.sdp.indexOf("a=sendrecv") !== -1, "recvonly");
> 
>-        pc.addTransceiver("audio");
>-        return pc.createOffer();
>-    }).then((offer) => {
>-        assert_true(offer.sdp.indexOf("mid:audio") !== -1);
>-    });
>+    pc.addTransceiver("audio");
>+    offer = await pc.createOffer();
>+    assert_true(offer.sdp.indexOf("m=audio") !== -1, "m=audio");
> }, "Setting up calls with addTransceiver but with no track");
> 
>-promise_test((test) => {
>+promise_test(async (test) => {
>     if (window.testRunner)
>         testRunner.setUserMediaPermission(true);
> 
>-    return navigator.mediaDevices.getUserMedia({ video: true }).then((stream) => {
>-        var pc = new RTCPeerConnection();
>-        pc.addTransceiver("video");
>-        pc.getSenders()[0].replaceTrack(stream.getVideoTracks()[0]);
>+    const stream = await navigator.mediaDevices.getUserMedia({ video: true });
>+    var pc = new RTCPeerConnection();
>+    pc.addTransceiver("video", {direction:"recvonly"});
>+    pc.getSenders()[0].replaceTrack(stream.getVideoTracks()[0]);
> 
>-        return pc.createOffer().then((offer) => {
>-            assert_true(offer.sdp.indexOf("mid:video") !== -1);
>-            // Replacing the track is not done yet so we still set it as a recvonly.
>-            assert_true(offer.sdp.indexOf("a=recvonly") !== -1);
>-        });
>-    });
>+    const offer = await pc.createOffer();
>+    assert_true(offer.sdp.indexOf("m=video") !== -1, "m=video");
>+    // Replacing the track is not done yet so we still set it as a recvonly.
>+    assert_true(offer.sdp.indexOf("a=recvonly") !== -1, "a=recvonly");
> }, "Setting up calls with addTransceiver with a track");
> 
> function testImage()
>@@ -71,29 +64,27 @@ function testImage()
>     assert_true(data[index + 2] < 100);
> }
> 
>-promise_test((test) => {
>+promise_test(async (test) => {
>     if (window.testRunner)
>         testRunner.setUserMediaPermission(true);
> 
>-    return navigator.mediaDevices.getUserMedia({ video: true}).then((stream) => {
>-        return new Promise((resolve, reject) => {
>-            createConnections((firstConnection) => {
>-                var track = stream.getVideoTracks()[0];
>-                firstConnection.addTransceiver("video");
>-                return firstConnection.getSenders()[0].replaceTrack(stream.getVideoTracks()[0]);
>-            }, (secondConnection) => {
>-                secondConnection.ontrack = (trackEvent) => {
>-                    resolve(trackEvent.streams[0]);
>-                };
>-            });
>-            setTimeout(() => reject("Test timed out"), 5000);
>+    const stream = await navigator.mediaDevices.getUserMedia({ video: true});
>+    const track = await new Promise((resolve, reject) => {
>+        createConnections((firstConnection) => {
>+            var track = stream.getVideoTracks()[0];
>+            firstConnection.addTransceiver("video");
>+            return firstConnection.getSenders()[0].replaceTrack(stream.getVideoTracks()[0]);
>+        }, (secondConnection) => {
>+            secondConnection.addTransceiver("video");
>+            secondConnection.ontrack = (trackEvent) => {
>+                resolve(trackEvent.track);
>+            };
>         });
>-    }).then((stream) => {
>-        video.srcObject = stream;
>-        return video.play();
>-    }).then(() => {
>-        testImage();
>+        setTimeout(() => reject("Test timed out"), 5000);
>     });
>+    video.srcObject = new MediaStream([track]);
>+    await video.play();
>+    testImage();
> }, "Basic video exchange set up with addTransceiver");
> 
>         </script>

Actions: View | Formatted Diff | Diff

Attachments on bug 189477: 349320