Attachment 359426 Details for Bug 193554 – Patch

[patch] Patch

bug-193554-20190117165348.patch (text/plain), 10.79 KB, created by youenn fablet on 2019-01-17 16:53:49 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-01-17 16:53:49 PST

Size: 10.79 KB

patch

obsolete

>Subversion Revision: 240112
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b58a862f6b2e77ac10dac93ca371351f3e006bca..61d4a81ea42c4306b33576bad88f0c3930fcf707 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-01-17  Youenn Fablet  <youenn@apple.com>
>+
>+        A track source should be unmuted whenever reenabled after setDirection changes
>+        https://bugs.webkit.org/show_bug.cgi?id=193554
>+        <rdar://problem/47366196>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Ensure that track gets unmuted after being fired as part of track event.
>+        Test is triggering some existing issues with MediaPlayerPrivateMediaStreamAVFObjC.
>+        Given the enqueuing of samples happens in a different frame than the thread used to update media stream and the active video track,
>+        some enqueued samples might not be from the right active video track or there might be no active video track.
>+
>+        Test: webrtc/video-setDirection.html
>+
>+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
>+        (WebCore::LibWebRTCMediaEndpoint::fireTrackEvent):
>+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
>+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:
>+        (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::enqueueVideoSample):
>+        (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::requestNotificationWhenReadyForVideoData):
>+
> 2019-01-17  Jon Lee  <jonlee@apple.com>
> 
>         [EME] Remove Amazon Prime Video from quirks list
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>index ad820957c1a6a356e543d7f76b4c3ac52772dd2d..4c76ac34aeefe8ad21d572e0558331b550516ebe 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>@@ -407,21 +407,24 @@ void LibWebRTCMediaEndpoint::addRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiv
>     fireTrackEvent(receiver.releaseNonNull(), track, rtcStreams, nullptr);
> }
> 
>-void LibWebRTCMediaEndpoint::fireTrackEvent(Ref<RTCRtpReceiver>&& receiver, Ref<MediaStreamTrack>&& track, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>& rtcStreams, RefPtr<RTCRtpTransceiver>&& transceiver)
>+void LibWebRTCMediaEndpoint::fireTrackEvent(Ref<RTCRtpReceiver>&& receiver, MediaStreamTrack& track, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>& rtcStreams, RefPtr<RTCRtpTransceiver>&& transceiver)
> {
>     Vector<RefPtr<MediaStream>> streams;
>     for (auto& rtcStream : rtcStreams) {
>         auto& mediaStream = mediaStreamFromRTCStream(*rtcStream.get());
>         streams.append(&mediaStream);
>-        mediaStream.addTrackFromPlatform(track.get());
>+        mediaStream.addTrackFromPlatform(track);
>     }
>     auto streamIds = WTF::map(streams, [](auto& stream) -> String {
>         return stream->id();
>     });
>-    m_remoteStreamsFromRemoteTrack.add(track.ptr(), WTFMove(streamIds));
>+    m_remoteStreamsFromRemoteTrack.add(&track, WTFMove(streamIds));
> 
>     m_peerConnectionBackend.connection().fireEvent(RTCTrackEvent::create(eventNames().trackEvent,
>-        Event::CanBubble::No, Event::IsCancelable::No, WTFMove(receiver), WTFMove(track), WTFMove(streams), WTFMove(transceiver)));
>+        Event::CanBubble::No, Event::IsCancelable::No, WTFMove(receiver), &track, WTFMove(streams), WTFMove(transceiver)));
>+
>+    // FIXME: As per spec, we should set muted to 'false' when starting to receive the content from network.
>+    track.source().setMuted(false);
> }
> 
> static inline void setExistingReceiverSourceTrack(RealtimeMediaSource& existingSource, webrtc::RtpReceiverInterface& rtcReceiver)
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
>index 19e856c0887725f7a2fc0dc8f0318b8814eeca13..47a1c929ca465dd85ef51c6c9573c4e1453d8a74 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
>@@ -143,7 +143,7 @@ private:
>     void newTransceiver(rtc::scoped_refptr<webrtc::RtpTransceiverInterface>&&);
>     void removeRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>&&);
> 
>-    void fireTrackEvent(Ref<RTCRtpReceiver>&&, Ref<MediaStreamTrack>&&, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&, RefPtr<RTCRtpTransceiver>&&);
>+    void fireTrackEvent(Ref<RTCRtpReceiver>&&, MediaStreamTrack&, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&, RefPtr<RTCRtpTransceiver>&&);
> 
>     template<typename T>
>     Optional<Backends> createTransceiverBackends(T&&, const RTCRtpTransceiverInit&, LibWebRTCRtpSenderBackend::Source&&);
>diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm
>index 238a2ea3958c31b66bfd4867ddaad8a585d9a539..935b1bfc236e95af6d061baffb49cf2c4e7a540f 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm
>+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm
>@@ -359,8 +359,6 @@ void MediaPlayerPrivateMediaStreamAVFObjC::enqueueCorrectedVideoSample(MediaSamp
> 
> void MediaPlayerPrivateMediaStreamAVFObjC::enqueueVideoSample(MediaStreamTrackPrivate& track, MediaSample& sample)
> {
>-    ASSERT(m_videoTrackMap.contains(track.id()));
>-
>     if (&track != m_mediaStreamPrivate->activeVideoTrack())
>         return;
> 
>@@ -414,6 +412,11 @@ void MediaPlayerPrivateMediaStreamAVFObjC::requestNotificationWhenReadyForVideoD
> 
>         [m_sampleBufferDisplayLayer stopRequestingMediaData];
> 
>+        if (!m_activeVideoTrack) {
>+            m_pendingVideoSampleQueue.clear();
>+            return;
>+        }
>+
>         while (!m_pendingVideoSampleQueue.isEmpty()) {
>             if (![m_sampleBufferDisplayLayer isReadyForMoreMediaData]) {
>                 requestNotificationWhenReadyForVideoData();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 4229e6c17249ed48c33db5c0c5b68c89c886d5b0..3487065227aabc6c7cce1d657be9f7b03fe60dd4 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-01-17  Youenn Fablet  <youenn@apple.com>
>+
>+        A track source should be unmuted whenever reenabled after setDirection changes
>+        https://bugs.webkit.org/show_bug.cgi?id=193554
>+        <rdar://problem/47366196>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * webrtc/video-setDirection-expected.txt: Added.
>+        * webrtc/video-setDirection.html: Added.
>+
> 2019-01-17  Youenn Fablet  <youenn@apple.com>
> 
>         Add a new SPI to request for cache storage quota increase
>diff --git a/LayoutTests/webrtc/video-setDirection-expected.txt b/LayoutTests/webrtc/video-setDirection-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..4571b8408b76568caaee43764383228a56221ca8
>--- /dev/null
>+++ b/LayoutTests/webrtc/video-setDirection-expected.txt
>@@ -0,0 +1,5 @@
>+
>+
>+PASS Going from sendrecv to inactive and back to sendrecv 
>+FAIL The MediaStream should remain the same assert_equals: expected object "[object MediaStream]" but got object "[object MediaStream]"
>+
>diff --git a/LayoutTests/webrtc/video-setDirection.html b/LayoutTests/webrtc/video-setDirection.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f21c1c3c774ae674af4ee80554693f7fc72fa3d5
>--- /dev/null
>+++ b/LayoutTests/webrtc/video-setDirection.html
>@@ -0,0 +1,108 @@
>+<!doctype html>
>+<html>
>+    <head>
>+        <meta charset="utf-8">
>+        <title>Testing video exchange using setDirection with renegotiation 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>
>+video = document.getElementById("video");
>+canvas = document.getElementById("canvas");
>+
>+function grabFrameData(x, y, w, h)
>+{
>+    canvas.width = video.videoWidth;
>+    canvas.height = video.videoHeight;
>+
>+    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);
>+    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);
>+}
>+
>+var pc1, pc2;
>+async function renegotiate()
>+{
>+    let d = await pc1.createOffer();
>+    await pc1.setLocalDescription(d);
>+    await pc2.setRemoteDescription(d);
>+    d = await pc2.createAnswer();
>+    await pc1.setRemoteDescription(d);
>+    await pc2.setLocalDescription(d);
>+}
>+
>+promise_test(async (t) => {
>+    if (window.testRunner)
>+        testRunner.setUserMediaPermission(true);
>+
>+    const localStream = await navigator.mediaDevices.getUserMedia({video: true});
>+    const stream = await new Promise((resolve, reject) => {
>+        createConnections((firstConnection) => {
>+            pc1 = firstConnection;
>+            firstConnection.addTrack(localStream.getVideoTracks()[0], localStream);
>+        }, (secondConnection) => {
>+            pc2 = secondConnection;
>+            secondConnection.ontrack = (trackEvent) => { resolve(trackEvent.streams[0]); };
>+        });
>+        setTimeout(() => reject("Test timed out"), 5000);
>+    });
>+
>+    video.srcObject = stream;
>+    await video.play();
>+
>+    testImage();
>+
>+    let promise = new Promise((resolve) => {
>+        pc2.getReceivers()[0].track.onmute = resolve;
>+    });
>+
>+    pc1.getTransceivers()[0].direction = "inactive";
>+    await renegotiate();
>+    await promise;
>+
>+    promise = new Promise((resolve) => {
>+        pc2.getReceivers()[0].track.onunmute = resolve;
>+    });
>+
>+    pc1.getTransceivers()[0].direction = "sendrecv";
>+    const streamPromise = new Promise(resolve => {
>+        pc2.ontrack = (trackEvent) => { resolve (trackEvent.streams[0]); };
>+    });
>+
>+    await renegotiate();
>+    video.srcObject = await streamPromise;
>+    await promise;
>+
>+    test(() => {
>+        assert_equals(stream, video.srcObject);
>+    }, "The MediaStream should remain the same");
>+    await video.play();
>+
>+    testImage();
>+}, "Going from sendrecv to inactive and back to sendrecv");
>+        </script>
>+    </body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 193554: 359426