Title: [240152] trunk
Revision
240152
Author
you...@apple.com
Date
2019-01-18 09:19:26 -0800 (Fri, 18 Jan 2019)

Log Message

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 Eric Carlson.

Source/WebCore:

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):

LayoutTests:

* webrtc/video-setDirection-expected.txt: Added.
* webrtc/video-setDirection.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (240151 => 240152)


--- trunk/LayoutTests/ChangeLog	2019-01-18 16:54:23 UTC (rev 240151)
+++ trunk/LayoutTests/ChangeLog	2019-01-18 17:19:26 UTC (rev 240152)
@@ -1,3 +1,14 @@
+2019-01-18  Youenn Fablet  <you...@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 Eric Carlson.
+
+        * webrtc/video-setDirection-expected.txt: Added.
+        * webrtc/video-setDirection.html: Added.
+
 2019-01-18  Jonathan Bedard  <jbed...@apple.com>
 
         webkitpy: Implement device type specific expected results (Part 2)

Added: trunk/LayoutTests/webrtc/video-setDirection-expected.txt (0 => 240152)


--- trunk/LayoutTests/webrtc/video-setDirection-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/webrtc/video-setDirection-expected.txt	2019-01-18 17:19:26 UTC (rev 240152)
@@ -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]"
+

Added: trunk/LayoutTests/webrtc/video-setDirection.html (0 => 240152)


--- trunk/LayoutTests/webrtc/video-setDirection.html	                        (rev 0)
+++ trunk/LayoutTests/webrtc/video-setDirection.html	2019-01-18 17:19:26 UTC (rev 240152)
@@ -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=""
+        <script src=""
+    </head>
+    <body>
+        <video id="video" autoplay=""></video>
+        <canvas id="canvas" width="640" height="480"></canvas>
+        <script src =""
+        <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 = "" 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>

Modified: trunk/Source/WebCore/ChangeLog (240151 => 240152)


--- trunk/Source/WebCore/ChangeLog	2019-01-18 16:54:23 UTC (rev 240151)
+++ trunk/Source/WebCore/ChangeLog	2019-01-18 17:19:26 UTC (rev 240152)
@@ -1,3 +1,25 @@
+2019-01-18  Youenn Fablet  <you...@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 Eric Carlson.
+
+        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-18  Charlie Turner  <ctur...@igalia.com>
 
         [GStreamer][EME][ClearKey] Request keys from CDMInstance rather than passing via bus messages

Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp (240151 => 240152)


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2019-01-18 16:54:23 UTC (rev 240151)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2019-01-18 17:19:26 UTC (rev 240152)
@@ -413,21 +413,24 @@
     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)

Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h (240151 => 240152)


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h	2019-01-18 16:54:23 UTC (rev 240151)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h	2019-01-18 17:19:26 UTC (rev 240152)
@@ -143,7 +143,7 @@
     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&&);

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm (240151 => 240152)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm	2019-01-18 16:54:23 UTC (rev 240151)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm	2019-01-18 17:19:26 UTC (rev 240152)
@@ -359,8 +359,6 @@
 
 void MediaPlayerPrivateMediaStreamAVFObjC::enqueueVideoSample(MediaStreamTrackPrivate& track, MediaSample& sample)
 {
-    ASSERT(m_videoTrackMap.contains(track.id()));
-
     if (&track != m_mediaStreamPrivate->activeVideoTrack())
         return;
 
@@ -414,6 +412,11 @@
 
         [m_sampleBufferDisplayLayer stopRequestingMediaData];
 
+        if (!m_activeVideoTrack) {
+            m_pendingVideoSampleQueue.clear();
+            return;
+        }
+
         while (!m_pendingVideoSampleQueue.isEmpty()) {
             if (![m_sampleBufferDisplayLayer isReadyForMoreMediaData]) {
                 requestNotificationWhenReadyForVideoData();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to