RTCPeerConnection.getRemoteStreams() should return all streams of all receivers |
|||||
Issue descriptionThe remote-side equivalent of https://bugs.chromium.org/p/chromium/issues/detail?id=738918.
,
Aug 23 2017
,
Aug 29 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
,
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 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd258a5bc5f6f9fe8b6fb830b9069236632f2aec commit cd258a5bc5f6f9fe8b6fb830b9069236632f2aec Author: Henrik Boström <hbos@chromium.org> Date: Thu Oct 19 00:30:50 2017 Redefine getRemoteStreams() as "get all streams of all receivers". getRemoteStreams() does not exist in the spec. This redefines the method on top of the RTP Media API and removes the need to keep track of a remote_streams_ list and some plumbing. This also makes the order of getReceivers() well-defined (receivers are iterated in the same order that they were added) so that the order of getRemoteStreams() remains well-defined and the same as before this CL. Bug: 741618 Change-Id: I78d25abf7e01ce8efd9dd085d304e778c9789bc4 Reviewed-on: https://chromium-review.googlesource.com/707693 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Reviewed-by: Taylor Brandstetter <deadbeef@chromium.org> Cr-Commit-Position: refs/heads/master@{#509944} [add] https://crrev.com/cd258a5bc5f6f9fe8b6fb830b9069236632f2aec/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-getRemoteStreams.html [modify] https://crrev.com/cd258a5bc5f6f9fe8b6fb830b9069236632f2aec/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp [modify] https://crrev.com/cd258a5bc5f6f9fe8b6fb830b9069236632f2aec/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h [modify] https://crrev.com/cd258a5bc5f6f9fe8b6fb830b9069236632f2aec/third_party/WebKit/Source/modules/peerconnection/RTCRtpReceiver.cpp [modify] https://crrev.com/cd258a5bc5f6f9fe8b6fb830b9069236632f2aec/third_party/WebKit/Source/modules/peerconnection/RTCRtpReceiver.h
,
Oct 19 2017
With the above CL this is fixed. When we have transceivers, the set of receivers will not decrease as streams are removed. If we want to retain the same behavior of getRemoteStreams()-streams disappearing I suppose we'll have to look at the state of the transceiver to see if it's active, and ignore inactive ones. WDYT deadbeef?
,
Oct 19 2017
That makes sense. You'd be enumerating transceivers and checking their direction instead of enumerating receivers.
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dd3c1dab5b9c8e92b9cb0ae3054f8513333e76a3 commit dd3c1dab5b9c8e92b9cb0ae3054f8513333e76a3 Author: Henrik Boström <hbos@chromium.org> Date: Wed Feb 21 14:48:01 2018 Remove RTCPeerConnectionHandler::remote_streams_. This is no longer needed because we already keep track of remote streams as "streams of receivers". Forgotten clean-up relating to the linked bugs (issues already closed). Bug: 741618 , 705901 Change-Id: I062d66d85aded03d013796cfe5e8c237b5d65832 Reviewed-on: https://chromium-review.googlesource.com/928725 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#538099} [modify] https://crrev.com/dd3c1dab5b9c8e92b9cb0ae3054f8513333e76a3/content/renderer/media/webrtc/rtc_peer_connection_handler.cc [modify] https://crrev.com/dd3c1dab5b9c8e92b9cb0ae3054f8513333e76a3/content/renderer/media/webrtc/rtc_peer_connection_handler.h
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37e317d0a7865fb5a31da742384743f2ef5ffc0f commit 37e317d0a7865fb5a31da742384743f2ef5ffc0f Author: Henrik Boström <hbos@chromium.org> Date: Fri Apr 20 10:42:26 2018 Remove RTCPeerConnectionHandler::remote_streams_ for reals. The previous CL that attempted to do this, https://chromium-review.googlesource.com/928725, removed all usages of it but forgot to remove the actual member. So doing that here. Bug: 741618 Change-Id: Ibc26479b5cf10b45b206d544fa68cb9eff625d31 Reviewed-on: https://chromium-review.googlesource.com/1021472 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#552300} [modify] https://crrev.com/37e317d0a7865fb5a31da742384743f2ef5ffc0f/content/renderer/media/webrtc/rtc_peer_connection_handler.h |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by hbos@chromium.org
, Jul 12 2017