Calling sender.replaceTrack() twice produces a new transceiver and its corresponding...
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2018 03:23:59 +0000 (03:23 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2018 03:23:59 +0000 (03:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191261

Reviewed by Eric Carlson.

Source/WebCore:

Handle the case of replacing a track in a sender that has no track.
In particular, do not create a new m-section as was implied by plan B implementation.
Instead, set the track directly on the rtc sender.
Covered by webrtc/video-addTransceiver.html.

* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::LibWebRTCMediaEndpoint::createSourceAndRTCTrack):
(WebCore::LibWebRTCMediaEndpoint::addTransceiver):
(WebCore::LibWebRTCMediaEndpoint::setSenderSourceFromTrack):
* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
* Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
(WebCore::LibWebRTCPeerConnectionBackend::setSenderSourceFromTrack):
* Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h:
* Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp:
(WebCore::LibWebRTCRtpSenderBackend::replaceTrack):

LayoutTests:

* webrtc/video-addTransceiver-expected.txt:
* webrtc/video-addTransceiver.html:

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

LayoutTests/ChangeLog
LayoutTests/webrtc/video-addTransceiver-expected.txt
LayoutTests/webrtc/video-addTransceiver.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/RTCRtpSender.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp

index 6db52eb..5f9b1a8 100644 (file)
@@ -1,5 +1,15 @@
 2018-11-06  Youenn Fablet  <youenn@apple.com>
 
+        Calling sender.replaceTrack() twice produces a new transceiver and its corresponding m= section
+        https://bugs.webkit.org/show_bug.cgi?id=191261
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/video-addTransceiver-expected.txt:
+        * webrtc/video-addTransceiver.html:
+
+2018-11-06  Youenn Fablet  <youenn@apple.com>
+
         Make mDNS ICE Candidate an experimental flag again
         https://bugs.webkit.org/show_bug.cgi?id=191262
 
index c2ce510..5be7eac 100644 (file)
@@ -2,6 +2,7 @@
 
 PASS Setting up calls with addTransceiver but with no track 
 PASS Setting up calls with addTransceiver with a track 
+PASS Setting up calls with addTransceiver with a track and use replaceTrack several times 
 PASS Basic video exchange set up with addTransceiver 
 PASS Testing synchronization sources 
 
index ccfa72a..e210638 100644 (file)
@@ -39,6 +39,24 @@ promise_test(async (test) => {
     assert_true(offer.sdp.indexOf("a=recvonly") !== -1, "a=recvonly");
 }, "Setting up calls with addTransceiver with a track");
 
+promise_test(async (test) => {
+    if (window.testRunner)
+        testRunner.setUserMediaPermission(true);
+
+    const stream = await navigator.mediaDevices.getUserMedia({ video: true });
+    var pc = new RTCPeerConnection();
+    pc.addTransceiver(stream.getVideoTracks()[0]);
+    await pc.getSenders()[0].replaceTrack(null);
+    await pc.getSenders()[0].replaceTrack(stream.getVideoTracks()[0]);
+
+    const offer = await pc.createOffer();
+
+    const sdpLines = offer.sdp.split('\r\n').filter(line => {
+        return line.startsWith("m=video");
+    });
+    assert_equals(sdpLines.length, 1, "There should be 1 video m section");
+}, "Setting up calls with addTransceiver with a track and use replaceTrack several times");
+
 function testImage()
 {
     canvas.width = video.videoWidth;
@@ -79,6 +97,13 @@ promise_test(async (test) => {
             secondConnection.ontrack = (trackEvent) => {
                 resolve(trackEvent.track);
             };
+        }, {
+            observeOffer : (desc) => {
+                const sdpLines = desc.sdp.split('\r\n').filter(line => {
+                    return line.startsWith("m=video");
+                });
+                assert_equals(sdpLines.length, 1, "There should be 1 video m section");
+            }
         });
         setTimeout(() => reject("Test timed out"), 5000);
     });
index 7579b0c..9f88e1e 100644 (file)
@@ -1,3 +1,26 @@
+2018-11-06  Youenn Fablet  <youenn@apple.com>
+
+        Calling sender.replaceTrack() twice produces a new transceiver and its corresponding m= section
+        https://bugs.webkit.org/show_bug.cgi?id=191261
+
+        Reviewed by Eric Carlson.
+
+        Handle the case of replacing a track in a sender that has no track.
+        In particular, do not create a new m-section as was implied by plan B implementation.
+        Instead, set the track directly on the rtc sender.
+        Covered by webrtc/video-addTransceiver.html.
+
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+        (WebCore::LibWebRTCMediaEndpoint::createSourceAndRTCTrack):
+        (WebCore::LibWebRTCMediaEndpoint::addTransceiver):
+        (WebCore::LibWebRTCMediaEndpoint::setSenderSourceFromTrack):
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
+        * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
+        (WebCore::LibWebRTCPeerConnectionBackend::setSenderSourceFromTrack):
+        * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h:
+        * Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp:
+        (WebCore::LibWebRTCRtpSenderBackend::replaceTrack):
+
 2018-11-06  Chris Dumez  <cdumez@apple.com>
 
         Post too much text to iFrame could crash webkit
index c7ca2b4..a18c02f 100644 (file)
@@ -92,6 +92,7 @@ void RTCRtpSender::replaceTrack(ScriptExecutionContext& context, RefPtr<MediaStr
         return;
     }
 
+    // FIXME: This whole function should be executed as part of the RTCPeerConnection operation queue.
     m_backend->replaceTrack(context, *this, WTFMove(withTrack), WTFMove(promise));
 }
 
index ac4e251..64e0cb6 100644 (file)
@@ -519,13 +519,14 @@ std::optional<LibWebRTCMediaEndpoint::Backends> LibWebRTCMediaEndpoint::addTrans
     return createTransceiverBackends(type, init, nullptr);
 }
 
-std::optional<LibWebRTCMediaEndpoint::Backends> LibWebRTCMediaEndpoint::addTransceiver(MediaStreamTrack& track, const RTCRtpTransceiverInit& init)
+std::pair<LibWebRTCRtpSenderBackend::Source, rtc::scoped_refptr<webrtc::MediaStreamTrackInterface>> LibWebRTCMediaEndpoint::createSourceAndRTCTrack(MediaStreamTrack& track)
 {
     LibWebRTCRtpSenderBackend::Source source;
     rtc::scoped_refptr<webrtc::MediaStreamTrackInterface> rtcTrack;
     switch (track.privateTrack().type()) {
     case RealtimeMediaSource::Type::None:
-        return std::nullopt;
+        ASSERT_NOT_REACHED();
+        break;
     case RealtimeMediaSource::Type::Audio: {
         auto audioSource = RealtimeOutgoingAudioSource::create(track.privateTrack());
         rtcTrack = m_peerConnectionFactory.CreateAudioTrack(track.id().utf8().data(), audioSource.ptr());
@@ -539,8 +540,20 @@ std::optional<LibWebRTCMediaEndpoint::Backends> LibWebRTCMediaEndpoint::addTrans
         break;
     }
     }
+    return std::make_pair(WTFMove(source), WTFMove(rtcTrack));
+}
 
-    return createTransceiverBackends(WTFMove(rtcTrack), init, WTFMove(source));
+std::optional<LibWebRTCMediaEndpoint::Backends> LibWebRTCMediaEndpoint::addTransceiver(MediaStreamTrack& track, const RTCRtpTransceiverInit& init)
+{
+    auto sourceAndTrack = createSourceAndRTCTrack(track);
+    return createTransceiverBackends(WTFMove(sourceAndTrack.second), init, WTFMove(sourceAndTrack.first));
+}
+
+void LibWebRTCMediaEndpoint::setSenderSourceFromTrack(LibWebRTCRtpSenderBackend& sender, MediaStreamTrack& track)
+{
+    auto sourceAndTrack = createSourceAndRTCTrack(track);
+    sender.setSource(WTFMove(sourceAndTrack.first));
+    sender.rtcSender()->SetTrack(WTFMove(sourceAndTrack.second));
 }
 
 std::unique_ptr<LibWebRTCRtpTransceiverBackend> LibWebRTCMediaEndpoint::transceiverBackendFromSender(LibWebRTCRtpSenderBackend& backend)
index c6994af..3bb0e73 100644 (file)
@@ -110,6 +110,8 @@ public:
     std::optional<Backends> addTransceiver(MediaStreamTrack&, const RTCRtpTransceiverInit&);
     std::unique_ptr<LibWebRTCRtpTransceiverBackend> transceiverBackendFromSender(LibWebRTCRtpSenderBackend&);
 
+    void setSenderSourceFromTrack(LibWebRTCRtpSenderBackend&, MediaStreamTrack&);
+
 private:
     LibWebRTCMediaEndpoint(LibWebRTCPeerConnectionBackend&, LibWebRTCProvider&);
 
@@ -163,6 +165,8 @@ private:
         : rtc::RefCountReleaseStatus::kDroppedLastRef;
     }
 
+    std::pair<LibWebRTCRtpSenderBackend::Source, rtc::scoped_refptr<webrtc::MediaStreamTrackInterface>> createSourceAndRTCTrack(MediaStreamTrack&);
+
 #if !RELEASE_LOG_DISABLED
     const Logger& logger() const final { return m_logger.get(); }
     const void* logIdentifier() const final { return m_logIdentifier; }
index 2cd9d37..6979ceb 100644 (file)
@@ -471,6 +471,11 @@ ExceptionOr<Ref<RTCRtpTransceiver>> LibWebRTCPeerConnectionBackend::addTransceiv
     return completeAddTransceiver(WTFMove(sender), init, track->id(), track->kind());
 }
 
+void LibWebRTCPeerConnectionBackend::setSenderSourceFromTrack(LibWebRTCRtpSenderBackend& sender, MediaStreamTrack& track)
+{
+    m_endpoint->setSenderSourceFromTrack(sender, track);
+}
+
 static inline LibWebRTCRtpTransceiverBackend& backendFromRTPTransceiver(RTCRtpTransceiver& transceiver)
 {
     return static_cast<LibWebRTCRtpTransceiverBackend&>(*transceiver.backend());
index 5ec7918..5e5f243 100644 (file)
@@ -37,6 +37,7 @@ namespace WebCore {
 
 class LibWebRTCMediaEndpoint;
 class LibWebRTCProvider;
+class LibWebRTCRtpSenderBackend;
 class LibWebRTCRtpTransceiverBackend;
 class RTCRtpReceiver;
 class RTCRtpReceiverBackend;
@@ -93,6 +94,7 @@ private:
 
     ExceptionOr<Ref<RTCRtpTransceiver>> addTransceiver(const String&, const RTCRtpTransceiverInit&) final;
     ExceptionOr<Ref<RTCRtpTransceiver>> addTransceiver(Ref<MediaStreamTrack>&&, const RTCRtpTransceiverInit&) final;
+    void setSenderSourceFromTrack(LibWebRTCRtpSenderBackend&, MediaStreamTrack&);
 
     RTCRtpTransceiver* existingTransceiver(WTF::Function<bool(LibWebRTCRtpTransceiverBackend&)>&&);
     RTCRtpTransceiver& newRemoteTransceiver(std::unique_ptr<LibWebRTCRtpTransceiverBackend>&&, Ref<RealtimeMediaSource>&&);
index 8990489..0f7d2b5 100644 (file)
@@ -31,6 +31,7 @@
 #include "LibWebRTCUtils.h"
 #include "RTCPeerConnection.h"
 #include "RTCRtpSender.h"
+#include "RuntimeEnabledFeatures.h"
 #include "ScriptExecutionContext.h"
 
 namespace WebCore {
@@ -76,6 +77,7 @@ void LibWebRTCRtpSenderBackend::replaceTrack(ScriptExecutionContext& context, RT
     }
     }
 
+    // FIXME: Remove this postTask once this whole function is executed as part of the RTCPeerConnection operation queue.
     context.postTask([protectedSender = makeRef(sender), promise = WTFMove(promise), track = WTFMove(track), this](ScriptExecutionContext&) mutable {
         if (protectedSender->isStopped())
             return;
@@ -88,13 +90,23 @@ void LibWebRTCRtpSenderBackend::replaceTrack(ScriptExecutionContext& context, RT
 
         bool hasTrack = protectedSender->track();
         protectedSender->setTrack(track.releaseNonNull());
-        if (!hasTrack) {
-            // FIXME: In case of unified plan, we should use m_rtcSender->SetTrack and no longer need m_peerConnectionBackend.
-            auto result = m_peerConnectionBackend->addTrack(*protectedSender->track(), { });
-            if (result.hasException()) {
-                promise.reject(result.releaseException());
-                return;
-            }
+
+        if (hasTrack) {
+            promise.resolve();
+            return;
+        }
+
+        if (RuntimeEnabledFeatures::sharedFeatures().webRTCUnifiedPlanEnabled()) {
+            m_source = nullptr;
+            m_peerConnectionBackend->setSenderSourceFromTrack(*this, *protectedSender->track());
+            promise.resolve();
+            return;
+        }
+
+        auto result = m_peerConnectionBackend->addTrack(*protectedSender->track(), { });
+        if (result.hasException()) {
+            promise.reject(result.releaseException());
+            return;
         }
         promise.resolve();
     });