Define RTCPeerConnection.addStream/removeStream in terms of addTrack/removeTrack |
|||||
Issue description
When addTrack/removeTrack is available, define addStream/removeStream in terms of addTrack/removeTrack and nuke all the then unnecessary code.
This requires getLocalStreams() to not rely on behavior of addStream/removeStream that is not covered by addTrack/removeTrack.
RTCPeerConnection::addStream(stream) {
for each track in stream
addTrack(track, stream);
listen to tracks being added to/removed stream and...
OnStreamAddTrack: addTrack(track, stream)
OnStreamRemoveTrack: removeTrack(sender for track and stream);
}
RTCPeerConnection::removeStream(stream) {
for each track in stream {
sender = sender with track == track
removeTrack(sender);
}
stop listening to tracks being added/removed from stream;
}
,
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
,
Jan 17 2018
,
Jan 30 2018
,
Feb 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55a6f822232bb39ce9296fe9de7baae0df12b244 commit 55a6f822232bb39ce9296fe9de7baae0df12b244 Author: Henrik Boström <hbos@chromium.org> Date: Thu Feb 01 14:38:12 2018 Define legacy getLocalStreams() on top of track-based API. getLocalStreams() is redefined from a list of streams modified by addStream() and removeStream() to "all streams of all senders". When using the stream-based APIs, getLocalStreams() behave exactly like it did before this CL. Because senders are added or removed by addTrack() and removeTrack(), this CL makes it possible for spec-compliant APIs to modify the legacy getLocalStreams(), which was not possible before this CL. In follow-up CLs addStream() and removeStream() will also be implemented on top of the track-based APIs and RTCPeerConnectionHandler code will be removed. This CL adds LayoutTests for expected behavior of legacy stream-based APIs when fully implemented on top of track-based APIs. This includes the first tests for interaction between legacy and non-legacy APIs. Design doc: https://docs.google.com/document/d/1Obbeg-B4_04twVctHdf7C7vkUQrGlLvOaV0C8J5_-Gs/edit?usp=sharing Bug: 803021 , 738929 Change-Id: Ibab3f8329809437b6058c3350cd11e747054efd2 Reviewed-on: https://chromium-review.googlesource.com/893385 Reviewed-by: Harald Alvestrand <hta@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#533656} [modify] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https-expected.txt [modify] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-AddRemoveTrack.html [add] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-legacy-stream-APIs-expected.txt [add] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-legacy-stream-APIs.html [modify] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp [modify] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h [modify] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.cpp [modify] https://crrev.com/55a6f822232bb39ce9296fe9de7baae0df12b244/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.h
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d739276a42a3288fec6c6a08d2c63fa96311f73 commit 3d739276a42a3288fec6c6a08d2c63fa96311f73 Author: Henrik Boström <hbos@chromium.org> Date: Wed Feb 07 16:06:58 2018 Legacy getStats: Don't rely on stream APIs for selector. This is necessary in order to define addStream() and removeStream() on top of addTrack() and removeTrack(). We won't be able to use local_streams() and remote_streams() going forward. Bug: 738929 Change-Id: Ie9928ee1f49052cd39c0079e327d1dccd3b22b18 Reviewed-on: https://chromium-review.googlesource.com/904522 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Harald Alvestrand <hta@chromium.org> Cr-Commit-Position: refs/heads/master@{#535017} [modify] https://crrev.com/3d739276a42a3288fec6c6a08d2c63fa96311f73/content/renderer/media/peer_connection_tracker.cc [modify] https://crrev.com/3d739276a42a3288fec6c6a08d2c63fa96311f73/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/3d739276a42a3288fec6c6a08d2c63fa96311f73/content/renderer/media/rtc_peer_connection_handler.h [modify] https://crrev.com/3d739276a42a3288fec6c6a08d2c63fa96311f73/content/renderer/media/rtc_peer_connection_handler_unittest.cc
,
Feb 8 2018
,
Feb 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/85000e8aea2a9b14b0618cf8351ffbcd5110e123 commit 85000e8aea2a9b14b0618cf8351ffbcd5110e123 Author: Henrik Boström <hbos@chromium.org> Date: Mon Feb 12 10:52:40 2018 Redefine legacy addStream()/removeStream() on top of track-based APIs. Implementation: - addStream() is defined in terms of addTrack() and adding a listener of tracks being added/removed from the stream. The listener uses addTrack()/removeTrack(). - removeStream() is defined in terms of removeTrack() and removing the listener of tracks being added/removed from the stream. This fixes: https://crbug.com/738929 Related bug fixes: - By getting rid of local_streams_, this also fixes the issue where createDTMFSender() could only be called for tracks added with the legacy APIs. https://crbug.com/806875 - By adding code checking if a stream was the first/last stream added/removed, the chrome://webrtc-internals events related to adding or removing a stream are triggered by addTrack/removeTrack, fixing the bug where addTrack-streams did not appear in chrome://webrtc-internals. https://crbug.com/801093 Old code no longer exercised is removed. MockPeerConnectionImpl is updated to work with track-based APIs. We should get rid of this class in future CLs, it's a maintenance burden. Design doc: https://docs.google.com/document/d/1Obbeg-B4_04twVctHdf7C7vkUQrGlLvOaV0C8J5_-Gs/edit?usp=sharing Bug: 738929 , 803021 , 806875 , 801093 Change-Id: I77541a3dce180e1cf8b311cb7803047215b0af7b Reviewed-on: https://chromium-review.googlesource.com/899346 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Harald Alvestrand <hta@chromium.org> Cr-Commit-Position: refs/heads/master@{#536052} [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/content/renderer/media/webrtc/mock_peer_connection_impl.cc [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/content/renderer/media/webrtc/mock_peer_connection_impl.h [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/content/renderer/media/webrtc/rtc_peer_connection_handler.cc [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/content/renderer/media/webrtc/rtc_peer_connection_handler.h [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/content/renderer/media/webrtc/rtc_peer_connection_handler_unittest.cc [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/content/renderer/media/webrtc/rtc_rtp_sender.cc [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/content/renderer/media/webrtc/rtc_rtp_sender.h [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-AddRemoveTrack.html [delete] https://crrev.com/b7e3a91cb94a9f47a8ebbef7a887d31069c18aac/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-legacy-stream-APIs-expected.txt [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/third_party/WebKit/Source/platform/testing/TestingPlatformSupportWithWebRTC.cpp [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/third_party/WebKit/Source/platform/testing/TestingPlatformSupportWithWebRTC.h [modify] https://crrev.com/85000e8aea2a9b14b0618cf8351ffbcd5110e123/third_party/WebKit/public/platform/WebRTCPeerConnectionHandler.h
,
Feb 12 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Sep 8 2017