WebRTC: Make MediaEndpointPeerConnection handle remotely assigned mids correctly
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Oct 2016 06:39:15 +0000 (06:39 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Oct 2016 06:39:15 +0000 (06:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163202

Patch by Adam Bergkvist  <adam.bergkvist@ericsson.com> and Alejandro G. Castro <alex@igalia.com> on 2016-10-10
Reviewed by Eric Carlson.

Source/WebCore:

An RTCRtpTransceiver has a null mid until it's been associated with a
media description (with a mid) [1]. During that time, it's identified by
a provisional mid that might become its real mid, but the transceiver
can also get its mid assigned by a remote media description. In the
second case, the mid value is initially unknown. A transceiver's
RTCRtpSender must directly (synchronously in the script) provide a muted
remote source that is playable by, for example, a media element. This
source is initially registered in the MediaEndpoint (WebRTC backend)
with the transceiver's provisional mid. So, if the real mid is set by a
remote description, the registered mid must be updated to preserve the
association between the registered source and the transceiver.

[1] https://w3c.github.io/webrtc-pc/archives/20160913/webrtc.html#dom-rtcrtptransceiver-mid

Test: fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html

* Modules/mediastream/MediaEndpointPeerConnection.cpp:
Don't break after finding the first transceiver in the loop that builds the send source map.
Update the mid used to register the muted remote source if the a transceiver's mid gets
assigned by a remote media description.
(WebCore::createSourceMap):
(WebCore::MediaEndpointPeerConnection::setRemoteDescriptionTask):
* platform/mediastream/MediaEndpoint.cpp:
* platform/mediastream/MediaEndpoint.h:
* platform/mock/MockMediaEndpoint.cpp:
(WebCore::MockMediaEndpoint::MockMediaEndpoint):
(WebCore::MockMediaEndpoint::updateReceiveConfiguration):
(WebCore::MockMediaEndpoint::updateSendConfiguration):
(WebCore::MockMediaEndpoint::createMutedRemoteSource):
(WebCore::MockMediaEndpoint::replaceMutedRemoteSourceMid):
(WebCore::MockMediaEndpoint::emulatePlatformEvent):
Add "unmute-remote-sources-by-mid" action that emulates data arriving on media descriptions
which unmutes the remote sources.
(WebCore::MockMediaEndpoint::updateConfigurationMids):
(WebCore::MockMediaEndpoint::unmuteRemoteSourcesByMid):
(WebCore::MockMediaEndpoint::unmuteTimerFired):
* platform/mock/MockMediaEndpoint.h:

LayoutTests:

Test the case when an RTCRtpTransceiver gets its mid assigned from a remote session
description.

* fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid-expected.txt: Added.
* fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html: Added.
* platform/mac/TestExpectations:
Skip above test until the Mac port builds with WEB_RTC.

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

LayoutTests/ChangeLog
LayoutTests/fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid-expected.txt [new file with mode: 0644]
LayoutTests/fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html [new file with mode: 0644]
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp
Source/WebCore/platform/mediastream/MediaEndpoint.cpp
Source/WebCore/platform/mediastream/MediaEndpoint.h
Source/WebCore/platform/mock/MockMediaEndpoint.cpp
Source/WebCore/platform/mock/MockMediaEndpoint.h

index ec51a67..4103971 100644 (file)
@@ -1,3 +1,18 @@
+2016-10-10  Adam Bergkvist  <adam.bergkvist@ericsson.com> and Alejandro G. Castro <alex@igalia.com>
+
+        WebRTC: Make MediaEndpointPeerConnection handle remotely assigned mids correctly
+        https://bugs.webkit.org/show_bug.cgi?id=163202
+
+        Reviewed by Eric Carlson.
+
+        Test the case when an RTCRtpTransceiver gets its mid assigned from a remote session
+        description.
+
+        * fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid-expected.txt: Added.
+        * fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html: Added.
+        * platform/mac/TestExpectations:
+        Skip above test until the Mac port builds with WEB_RTC.
+
 2016-10-10  Gyuyoung Kim  <gyuyoung.kim@navercorp.com>
 
         [EFL] Skip imported/w3c/web-platform-tests for a while
diff --git a/LayoutTests/fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid-expected.txt b/LayoutTests/fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid-expected.txt
new file mode 100644 (file)
index 0000000..ed270fa
--- /dev/null
@@ -0,0 +1,26 @@
+Test the case where an RTCRtpTransceiver gets a remotely assigned mid and also unmute the associated source
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pcA.addTrack(stream.getAudioTracks()[0], stream)
+A's transceiver is not yet associated with a media description and should have a null mid
+PASS pcA.getTransceivers()[0].mid is null
+A: local offer set (mid gets defined)
+midAssignedByA = pcA.getTransceivers()[0].mid
+PASS midAssignedByA !== null is true
+pcB.addTrack(stream.getAudioTracks()[0], stream)
+B's transceiver is not yet associated with a media description and should have a null mid
+PASS pcB.getTransceivers()[0].mid is null
+PASS B: got remote track event
+PASS event.track is an instance of MediaStreamTrack
+B: remote offer set (mid gets defined)
+B's transceiver should get its mid from the remote side (A)
+PASS pcB.getTransceivers()[0].mid is midAssignedByA
+PASS A: got remote track event
+PASS Offer/answer dialog completed
+PASS B: remote track unmute event
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html b/LayoutTests/fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html
new file mode 100644 (file)
index 0000000..0117d08
--- /dev/null
@@ -0,0 +1,96 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+    <head>
+        <script src="../../resources/js-test-pre.js"></script>
+        <script src="resources/promise-utils.js"></script>
+    </head>
+    <body>
+        <script>
+            let stream;
+            let midAssignedByA;
+            let event;
+
+            description("Test the case where an RTCRtpTransceiver gets a remotely assigned mid and also unmute the associated source");
+
+            if (window.testRunner)
+                testRunner.setUserMediaPermission(true);
+            else {
+                debug("This test can not be run without the testRunner");
+                finishJSTest();
+            }
+
+            const pcA = new webkitRTCPeerConnection({iceServers:[{urls:'stun:foo.com'}]});
+            const pcB = new webkitRTCPeerConnection({iceServers:[{urls:'stun:foo.com'}]});
+
+            pcA.ontrack = function (evt) {
+                testPassed("A: got remote track event");
+            };
+
+            pcB.ontrack = function (e) {
+                event = e;
+
+                testPassed("B: got remote track event");
+                shouldBeType("event.track", "MediaStreamTrack");
+
+                event.track.onunmute = function () {
+                    testPassed("B: remote track unmute event");
+                    finishJSTest();
+                };
+            };
+
+            navigator.mediaDevices.getUserMedia({ "audio": true })
+            .then(function (s) {
+                stream = s;
+
+                evalAndLog("pcA.addTrack(stream.getAudioTracks()[0], stream)");
+                debug("A's transceiver is not yet associated with a media description and should have a null mid");
+                shouldBeNull("pcA.getTransceivers()[0].mid");
+
+                return pcA.createOffer();
+            })
+            .then(function (offer) {
+                return pcA.setLocalDescription(offer);
+            })
+            .then(function () {
+                debug("A: local offer set (mid gets defined)");
+
+                evalAndLog("midAssignedByA = pcA.getTransceivers()[0].mid");
+                shouldBeTrue("midAssignedByA !== null");
+
+                evalAndLog("pcB.addTrack(stream.getAudioTracks()[0], stream)");
+                debug("B's transceiver is not yet associated with a media description and should have a null mid");
+                shouldBeNull("pcB.getTransceivers()[0].mid");
+
+                return pcB.setRemoteDescription(pcA.localDescription);
+            })
+            .then(function () {
+                debug("B: remote offer set (mid gets defined)");
+
+                debug("B's transceiver should get its mid from the remote side (A)");
+                shouldBe("pcB.getTransceivers()[0].mid", "midAssignedByA");
+
+                return pcB.createAnswer();
+            })
+            .then(function (answer) {
+                return pcB.setLocalDescription(answer);
+            })
+            .then(function () {
+                return pcA.setRemoteDescription(pcB.localDescription);
+            })
+            .then(function () {
+                testPassed("Offer/answer dialog completed")
+
+                window.internals.emulateRTCPeerConnectionPlatformEvent(pcB, "unmute-remote-sources-by-mid");
+            })
+            .catch(function (error) {
+                testFailed("Error in promise chain: " + error);
+                finishJSTest();
+            });
+
+            window.jsTestIsAsync = true;
+            window.successfullyParsed = true;
+
+        </script>
+        <script src="../../resources/js-test-post.js"></script>
+    </body>
+</html>
index ee30708..8102e6c 100644 (file)
@@ -208,6 +208,7 @@ fast/mediastream/RTCPeerConnection-media-setup-callbacks-single-dialog.html
 fast/mediastream/RTCPeerConnection-legacy-stream-based-api.html
 fast/mediastream/RTCPeerConnection-icecandidate-event.html
 fast/mediastream/RTCPeerConnection-iceconnectionstatechange-event.html
+fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html
 
 # Asserts in debug.
 [ Debug ] fast/images/large-size-image-crash.html [ Skip ]
index cd302b5..9e115e1 100644 (file)
@@ -1,3 +1,48 @@
+2016-10-10  Adam Bergkvist  <adam.bergkvist@ericsson.com> and Alejandro G. Castro <alex@igalia.com>
+
+        WebRTC: Make MediaEndpointPeerConnection handle remotely assigned mids correctly
+        https://bugs.webkit.org/show_bug.cgi?id=163202
+
+        Reviewed by Eric Carlson.
+
+        An RTCRtpTransceiver has a null mid until it's been associated with a
+        media description (with a mid) [1]. During that time, it's identified by
+        a provisional mid that might become its real mid, but the transceiver
+        can also get its mid assigned by a remote media description. In the
+        second case, the mid value is initially unknown. A transceiver's
+        RTCRtpSender must directly (synchronously in the script) provide a muted
+        remote source that is playable by, for example, a media element. This
+        source is initially registered in the MediaEndpoint (WebRTC backend)
+        with the transceiver's provisional mid. So, if the real mid is set by a
+        remote description, the registered mid must be updated to preserve the
+        association between the registered source and the transceiver.
+
+        [1] https://w3c.github.io/webrtc-pc/archives/20160913/webrtc.html#dom-rtcrtptransceiver-mid
+
+        Test: fast/mediastream/RTCPeerConnection-remotely-assigned-transceiver-mid.html
+
+        * Modules/mediastream/MediaEndpointPeerConnection.cpp:
+        Don't break after finding the first transceiver in the loop that builds the send source map.
+        Update the mid used to register the muted remote source if the a transceiver's mid gets
+        assigned by a remote media description.
+        (WebCore::createSourceMap):
+        (WebCore::MediaEndpointPeerConnection::setRemoteDescriptionTask):
+        * platform/mediastream/MediaEndpoint.cpp:
+        * platform/mediastream/MediaEndpoint.h:
+        * platform/mock/MockMediaEndpoint.cpp:
+        (WebCore::MockMediaEndpoint::MockMediaEndpoint):
+        (WebCore::MockMediaEndpoint::updateReceiveConfiguration):
+        (WebCore::MockMediaEndpoint::updateSendConfiguration):
+        (WebCore::MockMediaEndpoint::createMutedRemoteSource):
+        (WebCore::MockMediaEndpoint::replaceMutedRemoteSourceMid):
+        (WebCore::MockMediaEndpoint::emulatePlatformEvent):
+        Add "unmute-remote-sources-by-mid" action that emulates data arriving on media descriptions
+        which unmutes the remote sources.
+        (WebCore::MockMediaEndpoint::updateConfigurationMids):
+        (WebCore::MockMediaEndpoint::unmuteRemoteSourcesByMid):
+        (WebCore::MockMediaEndpoint::unmuteTimerFired):
+        * platform/mock/MockMediaEndpoint.h:
+
 2016-10-10  Darin Adler  <darin@apple.com>
 
         Move audio module off of legacy exceptions
index 4444a3a..f0ba4e2 100644 (file)
@@ -303,7 +303,6 @@ static RealtimeMediaSourceMap createSourceMap(const MediaDescriptionVector& remo
         if (transceiver) {
             if (transceiver->hasSendingDirection() && transceiver->sender()->track())
                 sourceMap.set(transceiver->mid(), &transceiver->sender()->track()->source());
-            break;
         }
     }
 
@@ -500,9 +499,12 @@ void MediaEndpointPeerConnection::setRemoteDescriptionTask(RefPtr<RTCSessionDesc
                     return !current.stopped() && current.mid().isNull() && current.sender()->trackKind() == mediaDescription->type();
                 });
 
-                if (transceiver)
+                if (transceiver) {
+                    // This transceiver was created locally with a provisional mid. Its real mid will now be set by the remote
+                    // description so we need to update the mid of the transceiver's muted source to preserve the association.
                     transceiver->setMid(mediaDescription->mid());
-                else
+                    m_mediaEndpoint->replaceMutedRemoteSourceMid(transceiver->provisionalMid(), mediaDescription->mid());
+                } else
                     receiveOnlyFlag = true;
             }
 
index 3d37fcd..cdafc45 100644 (file)
@@ -71,6 +71,7 @@ public:
 
     Ref<RealtimeMediaSource> createMutedRemoteSource(const String&, RealtimeMediaSource::Type) override { return EmptyRealtimeMediaSource::create(); }
     void replaceSendSource(RealtimeMediaSource&, const String&) override { }
+    void replaceMutedRemoteSourceMid(const String&, const String&) override { };
 
     void stop() override { }
 };
index d8d57ca..df42555 100644 (file)
@@ -77,6 +77,7 @@ public:
 
     virtual Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) = 0;
     virtual void replaceSendSource(RealtimeMediaSource&, const String& mid) = 0;
+    virtual void replaceMutedRemoteSourceMid(const String& oldMid, const String& newMid) = 0;
 
     virtual void stop() = 0;
 
index ae18dd1..57ed4e5 100644 (file)
@@ -37,6 +37,7 @@
 #include "MediaPayload.h"
 #include "MockRealtimeAudioSource.h"
 #include "MockRealtimeVideoSource.h"
+#include "RealtimeMediaSource.h"
 #include <wtf/MainThread.h>
 
 namespace WebCore {
@@ -53,6 +54,7 @@ MockMediaEndpoint::MockMediaEndpoint(MediaEndpointClient& client)
     : m_client(client)
     , m_iceCandidateTimer(*this, &MockMediaEndpoint::iceCandidateTimerFired)
     , m_iceTransportTimer(*this, &MockMediaEndpoint::iceTransportTimerFired)
+    , m_unmuteTimer(*this, &MockMediaEndpoint::unmuteTimerFired)
 {
 }
 
@@ -163,20 +165,16 @@ MediaEndpoint::UpdateResult MockMediaEndpoint::updateReceiveConfiguration(MediaE
 {
     UNUSED_PARAM(isInitiator);
 
-    Vector<String> mids;
-    for (const RefPtr<PeerMediaDescription>& mediaDescription : configuration->mediaDescriptions())
-        mids.append(mediaDescription->mid());
-    m_mids.swap(mids);
-
+    updateConfigurationMids(*configuration);
     return UpdateResult::Success;
 }
 
 MediaEndpoint::UpdateResult MockMediaEndpoint::updateSendConfiguration(MediaEndpointSessionConfiguration* configuration, const RealtimeMediaSourceMap& sendSourceMap, bool isInitiator)
 {
-    UNUSED_PARAM(configuration);
     UNUSED_PARAM(sendSourceMap);
     UNUSED_PARAM(isInitiator);
 
+    updateConfigurationMids(*configuration);
     return UpdateResult::Success;
 }
 
@@ -188,13 +186,19 @@ void MockMediaEndpoint::addRemoteCandidate(IceCandidate& candidate, const String
     UNUSED_PARAM(password);
 }
 
-Ref<RealtimeMediaSource> MockMediaEndpoint::createMutedRemoteSource(const String&, RealtimeMediaSource::Type type)
+Ref<RealtimeMediaSource> MockMediaEndpoint::createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type type)
 {
-    if (type == RealtimeMediaSource::Audio)
-        return MockRealtimeAudioSource::createMuted("remote audio");
+    RefPtr<RealtimeMediaSource> source;
+
+    switch (type) {
+    case RealtimeMediaSource::Audio: source = MockRealtimeAudioSource::createMuted("remote audio"); break;
+    case RealtimeMediaSource::Video: source = MockRealtimeVideoSource::createMuted("remote video"); break;
+    case RealtimeMediaSource::None:
+        ASSERT_NOT_REACHED();
+    }
 
-    ASSERT(type == RealtimeMediaSource::Video);
-    return MockRealtimeVideoSource::createMuted("remote video");
+    m_mutedRemoteSources.set(mid, source);
+    return *source;
 }
 
 void MockMediaEndpoint::replaceSendSource(RealtimeMediaSource& newSource, const String& mid)
@@ -203,6 +207,12 @@ void MockMediaEndpoint::replaceSendSource(RealtimeMediaSource& newSource, const
     UNUSED_PARAM(mid);
 }
 
+void MockMediaEndpoint::replaceMutedRemoteSourceMid(const String& oldMid, const String& newMid)
+{
+    RefPtr<RealtimeMediaSource> remoteSource = m_mutedRemoteSources.take(oldMid);
+    m_mutedRemoteSources.set(newMid, WTFMove(remoteSource));
+}
+
 void MockMediaEndpoint::stop()
 {
 }
@@ -213,6 +223,16 @@ void MockMediaEndpoint::emulatePlatformEvent(const String& action)
         dispatchFakeIceCandidates();
     else if (action == "step-ice-transport-states")
         stepIceTransportStates();
+    else if (action == "unmute-remote-sources-by-mid")
+        unmuteRemoteSourcesByMid();
+}
+
+void MockMediaEndpoint::updateConfigurationMids(const MediaEndpointSessionConfiguration& configuration)
+{
+    Vector<String> mids;
+    for (const RefPtr<PeerMediaDescription>& mediaDescription : configuration.mediaDescriptions())
+        mids.append(mediaDescription->mid());
+    m_mids.swap(mids);
 }
 
 void MockMediaEndpoint::dispatchFakeIceCandidates()
@@ -316,6 +336,31 @@ void MockMediaEndpoint::iceTransportTimerFired()
     m_iceTransportTimer.startOneShot(0);
 }
 
+void MockMediaEndpoint::unmuteRemoteSourcesByMid()
+{
+    if (m_mids.isEmpty())
+        return;
+
+    // Looking up each source by its mid, instead of simply iterating over the list of muted sources,
+    // emulates remote media arriving on a media description with a specific mid (RTCRtpTransceiver).
+
+    // Copy values in reverse order to maintain the original order while using takeLast()
+    for (int i = m_mids.size() - 1; i >= 0; --i)
+        m_midsOfSourcesToUnmute.append(m_mids[i]);
+
+    m_unmuteTimer.startOneShot(0);
+}
+
+void MockMediaEndpoint::unmuteTimerFired()
+{
+    RefPtr<RealtimeMediaSource> source = m_mutedRemoteSources.get(m_midsOfSourcesToUnmute.takeLast());
+    if (source)
+        source->setMuted(false);
+
+    if (!m_midsOfSourcesToUnmute.isEmpty())
+        m_unmuteTimer.startOneShot(0);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(WEB_RTC)
index faf91f8..101344f 100644 (file)
@@ -59,26 +59,36 @@ public:
 
     Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) override;
     void replaceSendSource(RealtimeMediaSource&, const String& mid) override;
+    void replaceMutedRemoteSourceMid(const String& oldMid, const String& newMid) override;
 
     void stop() override;
 
     void emulatePlatformEvent(const String& action) override;
 
 private:
+    void updateConfigurationMids(const MediaEndpointSessionConfiguration&);
+
     void dispatchFakeIceCandidates();
     void iceCandidateTimerFired();
 
     void stepIceTransportStates();
     void iceTransportTimerFired();
 
+    void unmuteRemoteSourcesByMid();
+    void unmuteTimerFired();
+
     MediaEndpointClient& m_client;
     Vector<String> m_mids;
+    HashMap<String, RefPtr<RealtimeMediaSource>> m_mutedRemoteSources;
 
     Vector<RefPtr<IceCandidate>> m_fakeIceCandidates;
     Timer m_iceCandidateTimer;
 
     Vector<std::pair<String, MediaEndpoint::IceTransportState>> m_iceTransportStateChanges;
     Timer m_iceTransportTimer;
+
+    Vector<String> m_midsOfSourcesToUnmute;
+    Timer m_unmuteTimer;
 };
 
 } // namespace WebCore