Title: [237910] trunk
Revision
237910
Author
you...@apple.com
Date
2018-11-06 19:23:59 -0800 (Tue, 06 Nov 2018)

Log Message

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.

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:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (237909 => 237910)


--- trunk/LayoutTests/ChangeLog	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/LayoutTests/ChangeLog	2018-11-07 03:23:59 UTC (rev 237910)
@@ -1,5 +1,15 @@
 2018-11-06  Youenn Fablet  <you...@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  <you...@apple.com>
+
         Make mDNS ICE Candidate an experimental flag again
         https://bugs.webkit.org/show_bug.cgi?id=191262
 

Modified: trunk/LayoutTests/webrtc/video-addTransceiver-expected.txt (237909 => 237910)


--- trunk/LayoutTests/webrtc/video-addTransceiver-expected.txt	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/LayoutTests/webrtc/video-addTransceiver-expected.txt	2018-11-07 03:23:59 UTC (rev 237910)
@@ -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 
 

Modified: trunk/LayoutTests/webrtc/video-addTransceiver.html (237909 => 237910)


--- trunk/LayoutTests/webrtc/video-addTransceiver.html	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/LayoutTests/webrtc/video-addTransceiver.html	2018-11-07 03:23:59 UTC (rev 237910)
@@ -39,6 +39,24 @@
     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 @@
             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);
     });

Modified: trunk/Source/WebCore/ChangeLog (237909 => 237910)


--- trunk/Source/WebCore/ChangeLog	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/Source/WebCore/ChangeLog	2018-11-07 03:23:59 UTC (rev 237910)
@@ -1,3 +1,26 @@
+2018-11-06  Youenn Fablet  <you...@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  <cdu...@apple.com>
 
         Post too much text to iFrame could crash webkit

Modified: trunk/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp (237909 => 237910)


--- trunk/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp	2018-11-07 03:23:59 UTC (rev 237910)
@@ -92,6 +92,7 @@
         return;
     }
 
+    // FIXME: This whole function should be executed as part of the RTCPeerConnection operation queue.
     m_backend->replaceTrack(context, *this, WTFMove(withTrack), WTFMove(promise));
 }
 

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


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2018-11-07 03:23:59 UTC (rev 237910)
@@ -519,13 +519,14 @@
     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,10 +540,22 @@
         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)
 {
     for (auto& transceiver : m_backend->GetTransceivers()) {

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


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h	2018-11-07 03:23:59 UTC (rev 237910)
@@ -110,6 +110,8 @@
     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 @@
         : 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; }

Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp (237909 => 237910)


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp	2018-11-07 03:23:59 UTC (rev 237910)
@@ -471,6 +471,11 @@
     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());

Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h (237909 => 237910)


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h	2018-11-07 03:23:59 UTC (rev 237910)
@@ -37,6 +37,7 @@
 
 class LibWebRTCMediaEndpoint;
 class LibWebRTCProvider;
+class LibWebRTCRtpSenderBackend;
 class LibWebRTCRtpTransceiverBackend;
 class RTCRtpReceiver;
 class RTCRtpReceiverBackend;
@@ -93,6 +94,7 @@
 
     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>&&);

Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp (237909 => 237910)


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp	2018-11-07 03:12:18 UTC (rev 237909)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp	2018-11-07 03:23:59 UTC (rev 237910)
@@ -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 @@
     }
     }
 
+    // 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,14 +90,24 @@
 
         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();
     });
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to