RTCPeerConnection: Support remote MediaStreamTrack being added and removed from MediaStream and pc/sender/receiver |
||||||||||||
Issue descriptionA track can be added or removed from a stream, independently of streams being added or removed. RTCPeerConnection.addTrack and removeTrack should be implemented. RTCPeerConnection and its handler only listen to remote streams being added or removed, and does not keep a record of tracks independent of streams. Streams and tracks are being created on a remote stream being added, but if a track is detached or added to an existing stream (which can happen on the webrtc layer) this is not reflected in the content and blink layer. We could end up with webrtc tracks that don't have a corresponding content/blink object, that are said to be on the incorrect stream. This is the cause of subtle bugs and a blocker for RTCRtpSender.replaceTrack.
,
Mar 28 2017
,
Mar 28 2017
webrtc land -> ... -> RTCPeerConnectionHandler::Observer::OnAddStream creating content streams/tracks: https://cs.chromium.org/chromium/src/content/renderer/media/rtc_peer_connection_handler.cc?sq=package:chromium&dr&l=993 -> ... -> RTCPeerConnection::didAddRemoteStream https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp?sq=package:chromium&dr&l=1303
,
Mar 28 2017
The WebRTC has dropped the concept of "mediastream attached to PeerConnection". Instead the new API promotes pc.addTrack and pc.ontrack. I tell this because Issue 415501 has been merged into this one, but once the new WebRTC API is implemented I don't think there is any relationship between them.
,
Mar 28 2017
You're right. We should listen to tracks being added or removed from the peerconnection and from senders/receivers. Until streams are deprecated we should listen to streams being added and removed too, but keep this as separated from tracks being added/removed as possible.
,
Mar 29 2017
Does this issue cover the APIs addTrack and removeTrack?
,
Mar 29 2017
,
Mar 29 2017
,
Mar 29 2017
,
Mar 29 2017
#6 Yes it should, I updated the description.
,
Apr 5 2017
Allowing tracks without streams might expose a related bug where the stats collector can crash involving tracks that don't belong to a stream.
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e3037fb7ebef14980926800a971f590486d21fd commit 8e3037fb7ebef14980926800a971f590486d21fd Author: hbos <hbos@chromium.org> Date: Tue May 30 12:29:50 2017 WebRtcMediaStreamTrackAdapter, maps 1 webrtc and 1 blink track. RemoteMediaStreamImpl creates and owns Remote[Audio/Video]TrackAdapter for remote audio and video tracks. These are moved into its own file so that they can be used independently of remote streams. Similarly, WebRtcMediaStreamAdapter creates and owns WebRtcAudioSink/MediaStreamVideoWebRtcSink for local audio and video tracks. By creating a new adapter for all kinds of tracks (local/remote x audio/video) that can be initialized and uninitialized independently from streams we achieve: 1) An abstraction, we can reference a track adapter without having different code depending on track type. 2) The ability to handle track adapters independently of stream adapters. This is important in the decoupling of streams and tracks. The RTP Media API[1]'s addTrack, removeTrack and replaceTrack will allow tracks to move between streams and belong to zero or multiple streams. In this CL, the new WebRtcMediaStreamTrackAdapter is only used in unittests. The plan is to update RemoteMediaStreamImpl and WebRtcMediaStreamAdapter to reference instances of WebRtcMediaStreamTrackAdapter, having the lifetime of tracks be independent of their membership of a particular stream. [1] https://w3c.github.io/webrtc-pc/#rtp-media-api BUG= 705901 , 700916 Review-Url: https://codereview.chromium.org/2883023002 Cr-Commit-Position: refs/heads/master@{#475499} [modify] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/BUILD.gn [modify] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/media/remote_media_stream_impl.cc [add] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/media/remote_media_stream_track_adapter.cc [add] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/media/remote_media_stream_track_adapter.h [modify] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/media/webrtc/webrtc_media_stream_adapter.cc [add] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/media/webrtc/webrtc_media_stream_track_adapter.cc [add] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/media/webrtc/webrtc_media_stream_track_adapter.h [add] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_unittest.cc [modify] https://crrev.com/8e3037fb7ebef14980926800a971f590486d21fd/content/test/BUILD.gn
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7e5d04b9bd8981b6cabd916203aa32423f41d493 commit 7e5d04b9bd8981b6cabd916203aa32423f41d493 Author: hbos <hbos@chromium.org> Date: Mon Jun 05 15:24:56 2017 Make RTCPeerConnectionHandlerTest use the webrtc signaling thread. The MockPeerConnectionDependecyFactory creates a new thread to act as the webrtc signaling thread, but the RTCPeerConnectionHandlerTest overrode its signaling_thread() helper method to yield the current thread. This meant that different components used different threads as their webrtc signaling thread in RTCPeerConnectionHandlerTest. This causes problems if you introduce code that relies on the main and and the webrtc signaling thread being different threads or if you DCHECK which thread you belong to depending on which component you ask for the webrtc signaling thread. Code updated to use the same thread as the webrtc signaling thread in all cases. base::RunLoop().RunUntilIdle() has been replaced by a helper method RunMessageLoopsUntilIdle() which makes sure both webrtc and main thread message loops has a chance to execute. BUG= 705901 Review-Url: https://codereview.chromium.org/2924563002 Cr-Commit-Position: refs/heads/master@{#476987} [modify] https://crrev.com/7e5d04b9bd8981b6cabd916203aa32423f41d493/content/renderer/media/rtc_peer_connection_handler.h [modify] https://crrev.com/7e5d04b9bd8981b6cabd916203aa32423f41d493/content/renderer/media/rtc_peer_connection_handler_unittest.cc
,
Jun 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e96b2036f3956bdf7bfc180044b90b71496a454 commit 9e96b2036f3956bdf7bfc180044b90b71496a454 Author: hbos <hbos@chromium.org> Date: Mon Jun 12 09:01:49 2017 TrackObserver DCHECK not on main thread, tests updated. The test was using a ThreadChecker to check that OnChanged is invoked on the webrtc signaling thread. The problem is it is attached to whatever thread first calls OnChanged, which in several tests was the main thread. The DCHECK is changed to "not on main thread", which should imply its invoked on the webrtc signaling thread. This is good enough, and avoids refactoring and changing initialization order that would be necessary to pass the webrtc signaling thread for the sake of a single DCHECK. The DCHECK showed that RTCPeerConnectionHandlerTest and MediaStreamRemoteVideoSourceTest was using the TrackObserver on the wrong thread - tests are updated. BUG= 705901 Review-Url: https://codereview.chromium.org/2924033002 Cr-Commit-Position: refs/heads/master@{#478569} [modify] https://crrev.com/9e96b2036f3956bdf7bfc180044b90b71496a454/content/renderer/media/rtc_peer_connection_handler_unittest.cc [modify] https://crrev.com/9e96b2036f3956bdf7bfc180044b90b71496a454/content/renderer/media/webrtc/media_stream_remote_video_source_unittest.cc [modify] https://crrev.com/9e96b2036f3956bdf7bfc180044b90b71496a454/content/renderer/media/webrtc/track_observer.cc
,
Jun 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ebab8e6156335da0a2f87cfaea1ad8da7f5e47e1 commit ebab8e6156335da0a2f87cfaea1ad8da7f5e47e1 Author: hbos <hbos@chromium.org> Date: Mon Jun 12 10:17:09 2017 WebRtcMediaStreamTrackMap added. This maps id <-> WebRtcMediaStreamTrackAdapter and handles creating track adapters if they don't already exist. Reference counting ensures adapters exists for as long as they are referenced, and that they are disposed and removed from the map when no longer needed. This can in a follow-up be used by WebRtcMediaStreamAdapter ("local streams") and RemoteMediaStreamImpl ("remote streams") to get references to track adapters "owned" by the map instead of directly creating and owning the sinks/adapters that a WebRtcMediaStreamTrackAdapter consists of. This would allow local and remote streams to reference tracks that already exist. The map can also be used to create and own tracks that are not associated with any streams. BUG= 705901 , 700916 Review-Url: https://codereview.chromium.org/2887403003 Cr-Commit-Position: refs/heads/master@{#478580} [modify] https://crrev.com/ebab8e6156335da0a2f87cfaea1ad8da7f5e47e1/content/renderer/BUILD.gn [add] https://crrev.com/ebab8e6156335da0a2f87cfaea1ad8da7f5e47e1/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.cc [add] https://crrev.com/ebab8e6156335da0a2f87cfaea1ad8da7f5e47e1/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.h [add] https://crrev.com/ebab8e6156335da0a2f87cfaea1ad8da7f5e47e1/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map_unittest.cc [modify] https://crrev.com/ebab8e6156335da0a2f87cfaea1ad8da7f5e47e1/content/test/BUILD.gn
,
Jun 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49effc1da26d29e677a79f49cf20ca85be501ee4 commit 49effc1da26d29e677a79f49cf20ca85be501ee4 Author: hbos <hbos@chromium.org> Date: Mon Jun 12 11:44:47 2017 WebRtcMediaStreamAdapter using WebRtcMediaStreamTrackMap. Make the WebRtcMediaStreamAdapter, which represent local streams, use the WebRtcMediaStreamTrackMap and AdapterRef classes for the handling of initializing, getting and uninitializing local tracks. WebRtcMediaStreamTrackAdapter is updated to allow local tracks from non-local sources (removed DCHECKs) which happens when redirecting remote sources as local streams. This is one step closer to decoupling streams and tracks. The same thing will be done in a follow-up CL for the remote streams case, RemoteMediaStreamImpl. BUG= 705901 , 700916 Review-Url: https://codereview.chromium.org/2897603004 Cr-Commit-Position: refs/heads/master@{#478592} [modify] https://crrev.com/49effc1da26d29e677a79f49cf20ca85be501ee4/content/renderer/media/peer_connection_tracker_unittest.cc [modify] https://crrev.com/49effc1da26d29e677a79f49cf20ca85be501ee4/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/49effc1da26d29e677a79f49cf20ca85be501ee4/content/renderer/media/rtc_peer_connection_handler.h [modify] https://crrev.com/49effc1da26d29e677a79f49cf20ca85be501ee4/content/renderer/media/webrtc/webrtc_media_stream_adapter.cc [modify] https://crrev.com/49effc1da26d29e677a79f49cf20ca85be501ee4/content/renderer/media/webrtc/webrtc_media_stream_adapter.h [modify] https://crrev.com/49effc1da26d29e677a79f49cf20ca85be501ee4/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc [modify] https://crrev.com/49effc1da26d29e677a79f49cf20ca85be501ee4/content/renderer/media/webrtc/webrtc_media_stream_track_adapter.cc
,
Jun 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ff812ea2691302e2307ff0e5c645cce989d420b commit 7ff812ea2691302e2307ff0e5c645cce989d420b Author: hbos <hbos@chromium.org> Date: Tue Jun 13 09:06:57 2017 RemoteMediaStreamImpl using WebRtcMediaStreamTrackMap. Make the RemoteMediaStreamImpl, which represent remote streams, use the WebRtcMediaStreamTrackMap and AdapterRef classes for the handling of initializing, getting and uninitializing remote tracks. This is one step closer to decoupling streams and tracks. PeerConnectionDependencyFactory's GetWebRtcSignalingThread is only callable from the main thread, and during initialization of the map this thread does not exist yet. Since the webrtc signaling thread was only used for DCHECKs and getting ahold of the webrtc signaling thread would require significant refactoring, the DCHECKs are updated to check that we are not on the main thread instead of checking that we are on the webrtc signaling thread. This is good enough for our purposes. BUG= 705901 , 700916 Review-Url: https://codereview.chromium.org/2902733003 Cr-Commit-Position: refs/heads/master@{#478943} [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/remote_media_stream_impl.cc [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/remote_media_stream_impl.h [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/webrtc/webrtc_media_stream_track_adapter.cc [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.cc [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.h [modify] https://crrev.com/7ff812ea2691302e2307ff0e5c645cce989d420b/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map_unittest.cc
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6144f20a4e3e525bc3e52d097a6c20c90d615ed commit d6144f20a4e3e525bc3e52d097a6c20c90d615ed Author: hbos <hbos@chromium.org> Date: Mon Jun 26 15:04:04 2017 WebRtcMediaStreamAdapterMap added. This will take care of creating and owning stream adapters, the glue between blink and webrtc layer media streams, independent of any one particular component using a stream. A stream adapter exists for as long as a component is using the stream (as long as it is holding on to a adapter reference). This is the stream equivalent of WebRtcMediaStreamTrackAdapterMap. Initial PS only cares about local streams, a TODO is added to take care of remote streams in the future. This will allow an RTCRtpSender to reference streams that are not part of the local stream set (added with addStream). This will unblock addTrack. The RTCPeerConnectionHandler will make use of this map in a small follow-up CL. BUG=700916, 705901 Review-Url: https://codereview.chromium.org/2946063003 Cr-Commit-Position: refs/heads/master@{#482276} [modify] https://crrev.com/d6144f20a4e3e525bc3e52d097a6c20c90d615ed/content/renderer/BUILD.gn [modify] https://crrev.com/d6144f20a4e3e525bc3e52d097a6c20c90d615ed/content/renderer/media/webrtc/webrtc_media_stream_adapter.cc [add] https://crrev.com/d6144f20a4e3e525bc3e52d097a6c20c90d615ed/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.cc [add] https://crrev.com/d6144f20a4e3e525bc3e52d097a6c20c90d615ed/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h [add] https://crrev.com/d6144f20a4e3e525bc3e52d097a6c20c90d615ed/content/renderer/media/webrtc/webrtc_media_stream_adapter_map_unittest.cc [modify] https://crrev.com/d6144f20a4e3e525bc3e52d097a6c20c90d615ed/content/test/BUILD.gn
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72f065897118a459b71ed840e5b9b22883ddaf5d commit 72f065897118a459b71ed840e5b9b22883ddaf5d Author: hbos <hbos@chromium.org> Date: Mon Jun 26 16:38:05 2017 RTCPeerConnectionHandler using WebRtcMediaStreamAdapterMap. Instead of local_streams_ owning the adapters, the map owns them and local_streams_ references stream adapters. This makes it possible for local stream adapters to exist that are not part of the local_streams_ vector. This unblocks addTrack, which will require senders to reference streams (stream adapters) that may not necessarily be part of local_streams_. BUG=700916, 705901 Review-Url: https://codereview.chromium.org/2953513003 Cr-Commit-Position: refs/heads/master@{#482292} [modify] https://crrev.com/72f065897118a459b71ed840e5b9b22883ddaf5d/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/72f065897118a459b71ed840e5b9b22883ddaf5d/content/renderer/media/rtc_peer_connection_handler.h
,
Jul 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/93aa0e383f74ed32072fedd74baf7a486dba5702 commit 93aa0e383f74ed32072fedd74baf7a486dba5702 Author: hbos <hbos@chromium.org> Date: Mon Jul 10 14:32:27 2017 RTCPeerConnection.addTrack and removeTrack added (behind flag). This allows a track to be added to a peerconnection with 0 or 1 associated streams. The multiple stream case is not yet supported. This allows tracks and streams to be attached to a peerconnection independently of add/removeStream and is a major milestone of the RTP Media API. The ontrack and onended events will be added/fire on the remote end in follow-up CLs. Spec: https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-addtrack https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-removetrack BUG=700916, 705901 Review-Url: https://codereview.chromium.org/2951713002 Cr-Commit-Position: refs/heads/master@{#485264} [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/chrome/browser/media/webrtc/webrtc_browsertest_base.cc [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/chrome/browser/media/webrtc/webrtc_browsertest_base.h [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/chrome/browser/media/webrtc/webrtc_rtp_browsertest.cc [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/chrome/test/data/webrtc/munge_sdp.js [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/chrome/test/data/webrtc/peerconnection.js [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/chrome/test/data/webrtc/peerconnection_rtp.js [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/content/renderer/media/rtc_peer_connection_handler.h [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/content/renderer/media/webrtc/rtc_rtp_sender.cc [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/content/renderer/media/webrtc/rtc_rtp_sender.h [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/content/shell/test_runner/mock_webrtc_peer_connection_handler.cc [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/content/shell/test_runner/mock_webrtc_peer_connection_handler.h [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-idl-expected.txt [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/LayoutTests/external/wpt/webrtc/interfaces-expected.txt [add] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-AddRemoveTrack.html [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-getSenders.html [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.cpp [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.h [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/Source/platform/testing/TestingPlatformSupportWithWebRTC.cpp [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/Source/platform/testing/TestingPlatformSupportWithWebRTC.h [modify] https://crrev.com/93aa0e383f74ed32072fedd74baf7a486dba5702/third_party/WebKit/public/platform/WebRTCPeerConnectionHandler.h
,
Aug 23 2017
,
Aug 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cdb2e7347e272e46ff67f64460991bf38eb90461 commit cdb2e7347e272e46ff67f64460991bf38eb90461 Author: Henrik Boström <hbos@chromium.org> Date: Fri Aug 25 10:20:50 2017 [Local/Remote]WebRtcMediaStreamAdapter Just like WebRtcMediaStreamTrackAdapters takes care of both local and remote tracks, WebRtcMediaStreamAdapter is updated to take care of both local and remote streams. The previously existing local stream adapter implementation is moved into subclass LocalWebRtcMediaStreamAdapter of interface WebRtcMediaStreamAdapter. The previously existing RemoteMediaStreamImpl is moved and refactored into RemoteWebRtcMediaStreamAdapter. Unittests added to cover the remote stream adapter (there was previosuly no test coverage for RemoteMediaStreamImpl). RemoteMediaStreamImpl is made a thin wrapper for the new adapter, with a TODO to remove this class completely in favor of the new adapter. Bug: 705901 Change-Id: I6bd7e5c02e2d44c52e286bf7fd5eeec5e17ee1ca Reviewed-on: https://chromium-review.googlesource.com/623714 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#497371} [modify] https://crrev.com/cdb2e7347e272e46ff67f64460991bf38eb90461/content/renderer/media/remote_media_stream_impl.cc [modify] https://crrev.com/cdb2e7347e272e46ff67f64460991bf38eb90461/content/renderer/media/remote_media_stream_impl.h [modify] https://crrev.com/cdb2e7347e272e46ff67f64460991bf38eb90461/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/cdb2e7347e272e46ff67f64460991bf38eb90461/content/renderer/media/webrtc/webrtc_media_stream_adapter.cc [modify] https://crrev.com/cdb2e7347e272e46ff67f64460991bf38eb90461/content/renderer/media/webrtc/webrtc_media_stream_adapter.h [modify] https://crrev.com/cdb2e7347e272e46ff67f64460991bf38eb90461/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.cc [modify] https://crrev.com/cdb2e7347e272e46ff67f64460991bf38eb90461/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc
,
Sep 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0e086e622f909ccd9bc013743e61fcd90dcfee9 commit f0e086e622f909ccd9bc013743e61fcd90dcfee9 Author: Henrik Boström <hbos@chromium.org> Date: Tue Sep 05 14:51:08 2017 WebRtcMediaStreamAdapterMap supporting both local and remote adapters. In a follow-up CL, the RemoteMediaStreamImpl will be removed in favor of using the map and its adapters. Bug: 705901 Change-Id: I167586a0ab8b686a9204db37729521fbb15d86dc Reviewed-on: https://chromium-review.googlesource.com/628496 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#499629} [modify] https://crrev.com/f0e086e622f909ccd9bc013743e61fcd90dcfee9/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.cc [modify] https://crrev.com/f0e086e622f909ccd9bc013743e61fcd90dcfee9/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h [modify] https://crrev.com/f0e086e622f909ccd9bc013743e61fcd90dcfee9/content/renderer/media/webrtc/webrtc_media_stream_adapter_map_unittest.cc
,
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 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/edd101586f8afe0fc7157f34b15ba44718aaa7a9 commit edd101586f8afe0fc7157f34b15ba44718aaa7a9 Author: Henrik Boström <hbos@chromium.org> Date: Mon Sep 18 15:11:04 2017 WebRtcMediaStreamAdapterMap using TwoKeysAdapterMap and UniqueId. The adapter is the glue between a blink and webrtc layer stream. With this change, the map's lookup is based on blink and webrtc streams rather than their ID, which is not necessarily unique (for example, if a remote stream is removed and then added again new objects are surfaced). webrtc::MediaStreamInterface are uniquely identified by pointer. blink::WebMediaStream are wrappers of objects uniquely identified by an int UniqueId. This unblocks switching from stream-based callbacks to track-based callbacks with regards to setRemoteDescription, as fully implementing the track-based RTP Media APIs necessitates, including supporting remote tracks that don't belong to any stream. This is a follow-up CL to https://chromium-review.googlesource.com/c/chromium/src/+/641870 which introduced the concept of TwoKeysAdapterMap and UniqueId for usage with the WebRtcMediaStreamTrackAdapterMap. Bug: 758850, 705901 Change-Id: I9f2827cd070e9164ebb6354a9fa58f1c0e240b4d Reviewed-on: https://chromium-review.googlesource.com/666923 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#502579} [modify] https://crrev.com/edd101586f8afe0fc7157f34b15ba44718aaa7a9/content/renderer/media/webrtc/two_keys_adapter_map.h [modify] https://crrev.com/edd101586f8afe0fc7157f34b15ba44718aaa7a9/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.cc [modify] https://crrev.com/edd101586f8afe0fc7157f34b15ba44718aaa7a9/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h [modify] https://crrev.com/edd101586f8afe0fc7157f34b15ba44718aaa7a9/content/renderer/media/webrtc/webrtc_media_stream_adapter_map_unittest.cc [modify] https://crrev.com/edd101586f8afe0fc7157f34b15ba44718aaa7a9/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.h
,
Sep 25 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 13 2017
There's a leftover bug related to this https://crbug.com/769743 but with the above CL I consider this Fixed. We do keep a record of tracks and streams independent of each other now, and the lifecycle of streams and tracks are independent of each other, with receivers keeping adapters alive with reference counting.
,
Nov 14 2017
,
Nov 14 2017
Issue 783433 has been merged into this issue.
,
Nov 14 2017
,
Dec 11 2017
Is this still blocked on webrtc:7445 (which is open as of writing this comment)? If yes, how come it's marked Verified? :)
,
Dec 13 2017
,
Dec 15 2017
,
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 |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by hbos@chromium.org
, Mar 28 2017