WebKit Bugzilla
Attachment 356227 Details for
Bug 192247
: Change of msid information in the SDP should trigger the corresponding track events
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-192247-20181130125738.patch (text/plain), 16.40 KB, created by
youenn fablet
on 2018-11-30 12:57:39 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2018-11-30 12:57:39 PST
Size:
16.40 KB
patch
obsolete
>Subversion Revision: 238644 >diff --git a/Source/ThirdParty/libwebrtc/ChangeLog b/Source/ThirdParty/libwebrtc/ChangeLog >index 0e100dd0fa4a992c533ec09a64645149bf12dcf1..69011112fa8b5ece660ce2f877e390cde00331de 100644 >--- a/Source/ThirdParty/libwebrtc/ChangeLog >+++ b/Source/ThirdParty/libwebrtc/ChangeLog >@@ -1,3 +1,12 @@ >+2018-11-30 Youenn Fablet <youenn@apple.com> >+ >+ Change of msid information in the SDP should trigger the corresponding track events >+ https://bugs.webkit.org/show_bug.cgi?id=192247 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * Source/webrtc/pc/peerconnection.cc: Set the recevier media streams whenever applying a remote description. >+ > 2018-11-27 Thibault Saunier <tsaunier@igalia.com> > > [GStreamer][WebRTC] Use LibWebRTC provided vp8 decoders and encoders >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index b177930ccc48ae6dbbfd69cabbc1448bbfe9f4f0..225d6ab9272f65f02cca2155ccf88cb2f9510c17 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,29 @@ >+2018-11-30 Youenn Fablet <youenn@apple.com> >+ >+ Change of msid information in the SDP should trigger the corresponding track events >+ https://bugs.webkit.org/show_bug.cgi?id=192247 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Implement https://w3c.github.io/webrtc-pc/#set-associated-remote-streams >+ by checking just before resolving the setRemoteDescription promise >+ whether libwebrtc streams have changed for each receiver. >+ If streams have been removed, remove the track from the stream, which >+ will fire a removetrack event. >+ If new streams are associated with the receiver, fire a track event. >+ >+ Test: imported/w3c/web-platform-tests/webrtc/RTCTrackEvent-fire.html >+ >+ * Modules/mediastream/MediaStream.cpp: >+ (WebCore::MediaStream::removeTrackFromPlatform): >+ * Modules/mediastream/MediaStream.h: >+ * Modules/mediastream/RTCPeerConnection.h: >+ * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp: >+ (WebCore::LibWebRTCMediaEndpoint::fireTrackEvent): >+ (WebCore::LibWebRTCMediaEndpoint::updateReceiverTracksAndStreams): >+ (WebCore::LibWebRTCMediaEndpoint::setRemoteSessionDescriptionSucceeded): >+ * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h: >+ > 2018-11-28 Youenn Fablet <youenn@apple.com> > > CSS subresource loads should not be observable from resource timing if the stylesheet is opaque >diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc b/Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc >index 6d67329242d75c1ca8256d07a4514c72d9f40287..dd316332acae32466a9aa1160273415b6067b49d 100644 >--- a/Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc >+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc >@@ -2380,10 +2380,9 @@ RTCError PeerConnection::ApplyRemoteDescription( > // The remote description has signaled the stream IDs. > stream_ids = media_desc->streams()[0].stream_ids(); > } >- if (RtpTransceiverDirectionHasRecv(local_direction) && >- (!transceiver->fired_direction() || >- !RtpTransceiverDirectionHasRecv(*transceiver->fired_direction()))) { >- RTC_LOG(LS_INFO) << "Processing the addition of a new track for MID=" >+ if (RtpTransceiverDirectionHasRecv(local_direction)) { >+ bool is_now_receiving_transceiver = !transceiver->fired_direction() || !RtpTransceiverDirectionHasRecv(*transceiver->fired_direction()); >+ RTC_LOG(LS_INFO) << "Processing the" << (is_now_receiving_transceiver ? " addition of a new " : " description of the ") << "track for MID=" > << content->name << " (added to " > << GetStreamIdsString(stream_ids) << ")."; > >@@ -2403,7 +2402,8 @@ RTCError PeerConnection::ApplyRemoteDescription( > // TODO(hbos): When we remove remote_streams(), use set_stream_ids() > // instead. https://crbug.com/webrtc/9480 > transceiver->internal()->receiver_internal()->SetStreams(media_streams); >- now_receiving_transceivers.push_back(transceiver); >+ if (is_now_receiving_transceiver) >+ now_receiving_transceivers.push_back(transceiver); > } > // 2.2.8.1.7: If direction is "sendonly" or "inactive", and transceiver's > // [[FiredDirection]] slot is either "sendrecv" or "recvonly", process the >diff --git a/Source/WebCore/Modules/mediastream/MediaStream.cpp b/Source/WebCore/Modules/mediastream/MediaStream.cpp >index 7e221502408fbc491e870669768679ebe0043462..e01b5555abb3d0458ea93057a311738751df5fc2 100644 >--- a/Source/WebCore/Modules/mediastream/MediaStream.cpp >+++ b/Source/WebCore/Modules/mediastream/MediaStream.cpp >@@ -212,6 +212,12 @@ void MediaStream::addTrackFromPlatform(Ref<MediaStreamTrack>&& track) > m_private->addTrack(privateTrack, MediaStreamPrivate::NotifyClientOption::Notify); > } > >+void MediaStream::removeTrackFromPlatform(MediaStreamTrack& track) >+{ >+ internalRemoveTrack(track.id(), StreamModifier::Platform); >+ m_private->removeTrack(track.privateTrack(), MediaStreamPrivate::NotifyClientOption::Notify); >+} >+ > bool MediaStream::internalAddTrack(Ref<MediaStreamTrack>&& trackToAdd, StreamModifier streamModifier) > { > auto result = m_trackSet.add(trackToAdd->id(), WTFMove(trackToAdd)); >diff --git a/Source/WebCore/Modules/mediastream/MediaStream.h b/Source/WebCore/Modules/mediastream/MediaStream.h >index 1c1ae11b80e00b573c33ec24c3945115866ea455..44987caf4475f364b434fd8a6823b552514b15e3 100644 >--- a/Source/WebCore/Modules/mediastream/MediaStream.h >+++ b/Source/WebCore/Modules/mediastream/MediaStream.h >@@ -105,6 +105,7 @@ public: > void removeObserver(Observer*); > > void addTrackFromPlatform(Ref<MediaStreamTrack>&&); >+ void removeTrackFromPlatform(MediaStreamTrack&); > > Document* document() const; > >diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.h b/Source/WebCore/Modules/mediastream/RTCPeerConnection.h >index d671d0544fcf6bebe46c787355c797f816a8ccfc..e18a69ee337055623b07a54162d7ef0b21cae0b6 100644 >--- a/Source/WebCore/Modules/mediastream/RTCPeerConnection.h >+++ b/Source/WebCore/Modules/mediastream/RTCPeerConnection.h >@@ -133,6 +133,7 @@ public: > const Vector<RefPtr<RTCRtpTransceiver>>& getTransceivers() const; > > const Vector<std::reference_wrapper<RTCRtpSender>>& currentSenders() const { return m_transceiverSet->senders(); } >+ const Vector<std::reference_wrapper<RTCRtpReceiver>>& currentReceivers() const { return m_transceiverSet->receivers(); } > const Vector<RefPtr<RTCRtpTransceiver>>& currentTransceivers() const { return m_transceiverSet->list(); } > > ExceptionOr<Ref<RTCRtpSender>> addTrack(Ref<MediaStreamTrack>&&, const Vector<std::reference_wrapper<MediaStream>>&); >diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp >index 3acb25c2dbe6fabaf51960c535add286079d5268..9656890f23a53ce9d6f5d67a07841bab3ca7c572 100644 >--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp >+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp >@@ -408,15 +408,17 @@ void LibWebRTCMediaEndpoint::addRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiv > > void LibWebRTCMediaEndpoint::fireTrackEvent(Ref<RTCRtpReceiver>&& receiver, Ref<MediaStreamTrack>&& track, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>& rtcStreams, RefPtr<RTCRtpTransceiver>&& transceiver) > { >+ HashSet<String> streamIds; > Vector<RefPtr<MediaStream>> streams; >+ streams.reserveInitialCapacity(rtcStreams.size()); > for (auto& rtcStream : rtcStreams) { > auto& mediaStream = mediaStreamFromRTCStream(*rtcStream.get()); >- streams.append(&mediaStream); >+ streams.uncheckedAppend(&mediaStream); > mediaStream.addTrackFromPlatform(track.get()); >+ >+ streamIds.add(mediaStream.id()); > } >- auto streamIds = WTF::map(streams, [](auto& stream) -> String { >- return stream->id(); >- }); >+ > m_remoteStreamsFromRemoteTrack.add(track.ptr(), WTFMove(streamIds)); > > m_peerConnectionBackend.connection().fireEvent(RTCTrackEvent::create(eventNames().trackEvent, >@@ -808,12 +810,56 @@ void LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed(ExceptionCode erro > }); > } > >+void LibWebRTCMediaEndpoint::updateReceiverTracksAndStreams() >+{ >+ if (!RuntimeEnabledFeatures::sharedFeatures().webRTCUnifiedPlanEnabled()) >+ return; >+ >+ // Handle https://w3c.github.io/webrtc-pc/#set-the-rtcsessiondescription step 2.2.8 to fire track events that have changed msids. >+ // FIXME: Refactor the code to fire all the track events in the order of the transceivers. >+ for (auto& rtcTransceiver : m_backend->GetTransceivers()) { >+ auto* existingTransceiver = m_peerConnectionBackend.existingTransceiver([&](auto& transceiverBackend) { >+ return rtcTransceiver.get() == transceiverBackend.rtcTransceiver(); >+ }); >+ if (!existingTransceiver) >+ continue; >+ >+ auto& receiver = existingTransceiver->receiver(); >+ auto& track = receiver.track(); >+ >+ auto streamIds = m_remoteStreamsFromRemoteTrack.get(&track); >+ auto rtcStreams = rtcTransceiver->receiver()->streams(); >+ >+ bool hasNewStream = false; >+ HashSet<String> rtcStreamIds; >+ for (auto& rtcStream : rtcStreams) { >+ auto streamId = fromStdString(rtcStream->id()); >+ if (!streamIds.contains(streamId)) >+ hasNewStream = true; >+ rtcStreamIds.add(WTFMove(streamId)); >+ } >+ >+ for (auto& streamId : streamIds) { >+ if (!rtcStreamIds.contains(streamId)) { >+ auto iterator = m_remoteStreamsById.find(streamId); >+ if (iterator != m_remoteStreamsById.end()) >+ iterator->value->removeTrackFromPlatform(track); >+ } >+ } >+ >+ if (hasNewStream) >+ fireTrackEvent(receiver, track, rtcStreams, existingTransceiver); >+ } >+} >+ > void LibWebRTCMediaEndpoint::setRemoteSessionDescriptionSucceeded() > { >- callOnMainThread([protectedThis = makeRef(*this)] { >+ callOnMainThread([this, protectedThis = makeRef(*this)] { > if (protectedThis->isStopped()) > return; >- protectedThis->m_peerConnectionBackend.setRemoteDescriptionSucceeded(); >+ >+ updateReceiverTracksAndStreams(); >+ m_peerConnectionBackend.setRemoteDescriptionSucceeded(); > }); > } > >diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h >index 41cd8847d36c0e78b48234d41b24519977b8a363..03020b688ab4b32919eaaac0a4481a261d0271b5 100644 >--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h >+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h >@@ -143,6 +143,7 @@ private: > void newTransceiver(rtc::scoped_refptr<webrtc::RtpTransceiverInterface>&&); > void removeRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>&&); > >+ void updateReceiverTracksAndStreams(); > void fireTrackEvent(Ref<RTCRtpReceiver>&&, Ref<MediaStreamTrack>&&, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&, RefPtr<RTCRtpTransceiver>&&); > > template<typename T> >@@ -190,7 +191,7 @@ private: > SetRemoteSessionDescriptionObserver<LibWebRTCMediaEndpoint> m_setRemoteSessionDescriptionObserver; > > HashMap<String, RefPtr<MediaStream>> m_remoteStreamsById; >- HashMap<MediaStreamTrack*, Vector<String>> m_remoteStreamsFromRemoteTrack; >+ HashMap<MediaStreamTrack*, HashSet<String>> m_remoteStreamsFromRemoteTrack; > > bool m_isInitiator { false }; > Timer m_statsLogTimer; >diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog >index e8e2b732ee1500c2ca8b58d958912d0272a8e03e..d9f1e0ca1491866c4b916c9aecb06559de996600 100644 >--- a/LayoutTests/imported/w3c/ChangeLog >+++ b/LayoutTests/imported/w3c/ChangeLog >@@ -1,3 +1,13 @@ >+2018-11-30 Youenn Fablet <youenn@apple.com> >+ >+ Change of msid information in the SDP should trigger the corresponding track events >+ https://bugs.webkit.org/show_bug.cgi?id=192247 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * web-platform-tests/webrtc/RTCTrackEvent-fire-expected.txt: Added. >+ * web-platform-tests/webrtc/RTCTrackEvent-fire.html: Added. >+ > 2018-11-28 Rob Buis <rbuis@igalia.com> > > [XHR] Document.lastModified doesn't work for non-rendered documents >diff --git a/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCTrackEvent-fire-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCTrackEvent-fire-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..faca5b1a0c29c722776da242d18e09bae135f40a >--- /dev/null >+++ b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCTrackEvent-fire-expected.txt >@@ -0,0 +1,4 @@ >+ >+PASS Applying a remote description with a new msid should trigger firing an event with populated streams >+PASS Applying a remote description with removed msid should trigger firing a removetrack event on the corresponding stream >+ >diff --git a/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCTrackEvent-fire.html b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCTrackEvent-fire.html >new file mode 100644 >index 0000000000000000000000000000000000000000..3f26912efb0c6b95a419d3aff506a5af2c75756a >--- /dev/null >+++ b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCTrackEvent-fire.html >@@ -0,0 +1,90 @@ >+<!doctype html> >+<meta charset=utf-8> >+<title>Change of msid in remote description should trigger related track events</title> >+<script src="/resources/testharness.js"></script> >+<script src="/resources/testharnessreport.js"></script> >+<script> >+const sdpBase =`v=0 >+o=- 5511237691691746 2 IN IP4 127.0.0.1 >+s=- >+t=0 0 >+a=group:BUNDLE 0 >+a=ice-options:trickle >+a=ice-lite >+a=msid-semantic:WMS * >+m=audio 9 UDP/TLS/RTP/SAVPF 111 103 9 102 0 8 105 13 110 113 126 >+c=IN IP6 :: >+a=rtcp:9 IN IP6 :: >+a=rtcp-mux >+a=mid:0 >+a=sendrecv >+a=ice-ufrag:z0i8R3C9C4hPRWls >+a=ice-pwd:O7bPpOFAqasqoidV4yxnFVbc >+a=ice-lite >+a=fingerprint:sha-256 B7:9C:0D:C9:D1:42:57:97:82:4D:F9:B7:93:75:49:C3:42:21:5A:DD:9C:B5:ED:53:53:F0:B4:C8:AE:88:7A:E7 >+a=setup:actpass >+a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level >+a=extmap:9 urn:ietf:params:rtp-hdrext:sdes:mid >+a=rtpmap:103 ISAC/16000 >+a=rtpmap:9 G722/8000 >+a=rtpmap:102 ILBC/8000 >+a=rtpmap:0 PCMU/8000 >+a=rtpmap:8 PCMA/8000 >+a=rtpmap:105 CN/16000 >+a=rtpmap:13 CN/8000 >+a=rtpmap:110 telephone-event/48000 >+a=rtpmap:113 telephone-event/16000 >+a=rtpmap:126 telephone-event/8000`; >+ >+const sdp0 = sdpBase + ` >+`; >+ >+const sdp1 = sdpBase + ` >+a=msid:1 1 >+a=ssrc:1 cname:1 >+a=ssrc:1 msid:1 1 >+`; >+ >+async function applyRemoteDescriptionAndReturnTrackEventStreams(test, pc, sdp) >+{ >+ let testTrackPromise = new Promise((resolve, reject) => { >+ pc.ontrack = (event) => { >+ resolve(event.streams ? event.streams : []); >+ } >+ test.step_timeout(() => { reject("No track event fired") }, 5000); >+ }); >+ await pc.setRemoteDescription({type: 'offer', 'sdp': sdp}); >+ return testTrackPromise; >+} >+ >+promise_test(async test => { >+ const pc = new RTCPeerConnection(); >+ >+ let streams = await applyRemoteDescriptionAndReturnTrackEventStreams(test, pc, sdp0); >+ >+ const answer = await pc.createAnswer(); >+ await pc.setLocalDescription(answer); >+ >+ streams = await applyRemoteDescriptionAndReturnTrackEventStreams(test, pc, sdp1); >+ assert_equals(streams.length, 1, "track event has a stream"); >+ assert_equals(streams[0].id, "1", "msid should match"); >+}, "Applying a remote description with a new msid should trigger firing an event with populated streams"); >+ >+promise_test(async test => { >+ const pc = new RTCPeerConnection(); >+ >+ let streams = await applyRemoteDescriptionAndReturnTrackEventStreams(test, pc, sdp1); >+ assert_equals(streams.length, 1, "track event has a stream"); >+ assert_equals(streams[0].id, "1", "msid should match"); >+ >+ const answer = await pc.createAnswer(); >+ await pc.setLocalDescription(answer); >+ >+ let testTrackPromise = new Promise((resolve, reject) => { >+ streams[0].onremovetrack = resolve; >+ test.step_timeout(() => { reject("No removetrack event fired") }, 5000); >+ }); >+ await pc.setRemoteDescription({type: 'offer', 'sdp': sdp0}); >+ await testTrackPromise; >+}, "Applying a remote description with removed msid should trigger firing a removetrack event on the corresponding stream"); >+</script>
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=356227">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192247
: 356227