RTCPeerConnection should not rely on remote stream added/remove events |
||||||||
Issue descriptionaddTrack supports stream-less streams, these should be signaled ( https://crbug.com/webrtc/7933 ) as such, meaning we can no longer rely on all remote tracks belonging to a remote stream. If we move over to only listening to receivers being added or removed, we'll be able to define getRemoteStreams() in terms of receivers' streams ( https://crbug.com/741618 ).
,
Aug 29 2017
,
Aug 30 2017
,
Aug 30 2017
,
Sep 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13fd38073fc397678c4076dd4e0893db705f544d commit 13fd38073fc397678c4076dd4e0893db705f544d Author: Henrik Boström <hbos@chromium.org> Date: Fri Sep 08 10:56:06 2017 Remove RemoteMediaStreamImpl in favor of WebRtcMediaStreamAdapter. RTCPeerConnectionHandler keeps track of remote streams using AdapterRefs from a WebRtcMediaStreamAdapterMap. Follow-up CLs will allow receivers to hold AdapterRefs, enabling remote streams to exist independently of the remote_streams_ vector. Ultimately we want to get rid of the remote_streams_ vector, listen to receivers being added and removed (allowing streamless remote tracks) and define getRemoteStreams() as the set "for each receiver, for each stream of receiver". Bug: 705901 , 741619 , 741618 , 738929 Change-Id: I7179626ee20090528a63d0b5a0655fc07e243bd0 Reviewed-on: https://chromium-review.googlesource.com/628524 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#500562} [modify] https://crrev.com/13fd38073fc397678c4076dd4e0893db705f544d/content/renderer/BUILD.gn [delete] https://crrev.com/482ec872f8974245c8649ac2fecf60782f907b0e/content/renderer/media/remote_media_stream_impl.cc [delete] https://crrev.com/482ec872f8974245c8649ac2fecf60782f907b0e/content/renderer/media/remote_media_stream_impl.h [modify] https://crrev.com/13fd38073fc397678c4076dd4e0893db705f544d/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/13fd38073fc397678c4076dd4e0893db705f544d/content/renderer/media/rtc_peer_connection_handler.h [modify] https://crrev.com/13fd38073fc397678c4076dd4e0893db705f544d/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h
,
Sep 8 2017
,
Sep 28 2017
,
Sep 28 2017
,
Sep 28 2017
,
Oct 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09343dfc9e9911662d68a400d339f11c7f4854a4 commit 09343dfc9e9911662d68a400d339f11c7f4854a4 Author: Henrik Boström <hbos@chromium.org> Date: Fri Oct 13 19:53:05 2017 Decoupling remote MediaStream and MediaStreamTrack creation. The MediaStream::Create taking MediaStreamDescriptor* as an argument used to unconditionally create MediaStreamTracks from the associated MediaStreamComponents. This CL makes it possible to create a MediaStream from a MediaStreamDescriptor and MediaStreamTrackVectors. DCHECKs ensure the descriptor and tracks match. This makes it possible to create MediaStream containing pre-existing tracks by adding the tracks to the stream yourself. This is required for the track-based APIs. I exercise creating streams and tracks separately this way in DidAddRemoteStream even though tracks are always created, yielding exactly the same result as calling the previous Create function. Bug: 741619 , 769743 Change-Id: Ia0d361546a95b23c9a5275b7657460c7ab4ba73e Reviewed-on: https://chromium-review.googlesource.com/684297 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#508788} [modify] https://crrev.com/09343dfc9e9911662d68a400d339f11c7f4854a4/third_party/WebKit/Source/modules/mediastream/MediaStream.cpp [modify] https://crrev.com/09343dfc9e9911662d68a400d339f11c7f4854a4/third_party/WebKit/Source/modules/mediastream/MediaStream.h [modify] https://crrev.com/09343dfc9e9911662d68a400d339f11c7f4854a4/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
,
Oct 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/101ae86567ba231c612af756e2bb5925a127ea30 commit 101ae86567ba231c612af756e2bb5925a127ea30 Author: Henrik Boström <hbos@chromium.org> Date: Fri Oct 13 20:38:59 2017 PC.setRemoteDescription: Use track-based callbacks over stream-based. In RTP Media API, the processing of remote MediaStreamTracks works with the "addition" or "removal" of a remote track, i.e. of a receiver. The receiver has a track and an associated set of streams. https://w3c.github.io/webrtc-pc/#processing-remote-mediastreamtracks I refer to the new set of APIs as the track-based APIs, because they're centered around tracks, tracks being added to streams is a side-effect to the processing of tracks. This is different from the stream-based APIs, centered around remote streams being added or removed. While tracks could be added or removed from streams in this API there was always the assumption that a stream owned its track. As such, when a stream was added or removed, all of its tracks would be created or destroyed. Switching over to track-based callbacks, the ownership of tracks changes. A track can be added or removed to zero, one or multiple streams. On DidAddRemoteTrack we check if a stream or track exist before we create it and add the track to the stream. In this CL: - The receiver's set of associated streams is exposed (C++ layers). - Stream added/removed callbacks replaced by track added/removed, creating or getting streams/tracks based on the receivers. - Updating mocks and unittests. All usage cases that worked with addStream/removeStream() should continue to work with the new callbacks, and the new callbacks will enable new RTP Media API behavior not previously supported. Additionally, this fixes https://crbug.com/773613 because it makes ontrack fire for new tracks, even if the new tracks are surfaced as belonging to an existing stream. Follow-up CLs: - Add unittests for moving existing tracks between streams and make sure we don't end up with multiple tracks. https://crbug.com/769743 - With receivers added/removed based on callbacks, define getReceivers() as "return rtp_receivers_". http://crbug.com/755166 Future CLs: - All associated events are fired according to spec and in the right order. https://crbug.com/760107 - Define getRemoteStreams() in terms of "all streams of all receivers". https://crbug.com/741618 - Define addStream/removeStream() in terms of addTrack()/removeTrack(). https://crbug.com/738929 - Add usage metrics for add/remove tracks. https://crbug.com/765170 Resolving all these bugs will reduce the technical dept. Bug: 773613 , 741619 , 705901 , 769743 , 760107 , 741618 , 738929 , 765170 , webrtc:7933 Change-Id: I75c18b889e1d4d5827c53bf6dd627f495ca22b51 Reviewed-on: https://chromium-review.googlesource.com/657639 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#508805} [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/mock_peer_connection_impl.cc [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/mock_peer_connection_impl.h [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/mock_web_rtc_peer_connection_handler_client.cc [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/mock_web_rtc_peer_connection_handler_client.h [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/rtc_peer_connection_handler.h [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/rtc_peer_connection_handler_unittest.cc [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/webrtc/rtc_rtp_receiver.cc [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/content/renderer/media/webrtc/rtc_rtp_receiver.h [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/third_party/WebKit/Source/modules/peerconnection/RTCRtpReceiver.cpp [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/third_party/WebKit/Source/modules/peerconnection/RTCRtpReceiver.h [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/third_party/WebKit/public/platform/WebRTCPeerConnectionHandlerClient.h [modify] https://crrev.com/101ae86567ba231c612af756e2bb5925a127ea30/third_party/WebKit/public/platform/WebRTCRtpReceiver.h
,
Oct 13 2017
The above CL resolves this issue.
,
Oct 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9da69b5c188c2bc1996bdf09d93e49fc23febe3 commit d9da69b5c188c2bc1996bdf09d93e49fc23febe3 Author: Henrik Boström <hbos@chromium.org> Date: Tue Oct 17 18:46:20 2017 Simplify getReceivers(). Now that RTCPeerConnection and RTCPeerConnectionHandler listens to tracks being added/removed, not streams, we know when receivers are added or removed and keep track of them. This simplifies getReceivers() to "return rtp_receivers_;" instead of querying the lower layers. Bug: 741619 Change-Id: I84c43df31259bce8e31b9c34945ed8d972e45ca7 Reviewed-on: https://chromium-review.googlesource.com/707798 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Reviewed-by: Taylor Brandstetter <deadbeef@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#509453} [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/content/renderer/media/rtc_peer_connection_handler.h [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/content/renderer/media/rtc_peer_connection_handler_unittest.cc [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/third_party/WebKit/Source/platform/testing/TestingPlatformSupportWithWebRTC.cpp [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/third_party/WebKit/Source/platform/testing/TestingPlatformSupportWithWebRTC.h [modify] https://crrev.com/d9da69b5c188c2bc1996bdf09d93e49fc23febe3/third_party/WebKit/public/platform/WebRTCPeerConnectionHandler.h |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by hbos@chromium.org
, Aug 23 2017