New issue
Advanced search Search tips

Issue 810708 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 803021



Sign in to add a comment

Use AddTrack with stream labels

Project Member Reported by hbos@chromium.org, Feb 9 2018

Issue description

Currently RTCPeerConnectionHandler uses AddTrack() taking a list of webrtc::MediaStreamInterface*.

We should replace this by AddTrack() taking std::string stream labels instead.

Code used to keep track of mapping blink::WebMediaStream <-> webrtc::MediaStreamInterface* might become obsolete, since the blink::WebMediaStream knows of its ID.

If multiple streams have the same ID, this change could have noticeable consequences.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e95ed83d6fd0bc275f8fded449c61374c147f9eb

commit e95ed83d6fd0bc275f8fded449c61374c147f9eb
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Jun 28 15:20:18 2018

Update PeerConnectionTracker to track transceiver updates.

The PeerConnectionTracker logs events relating to an RTCPeerConnection
(APIs called, events firing, tracks added or removed) and lists them in
chrome://webrtc-internals.

Ever since the days of addStream()/removeStream(), the tracker has
listed a local or remote stream being added or removed as an event.
These APIs are no longer in the spec and are in fact implemented on top
of the spec-compliant addTrack()/removeTrack().

Prior to this CL, the tracker reported a stream being added by any means
as "addStream" and removed as "removeStream", which is misleading
because they may have been added through other APIs.

In preparation for Unified Plan, which expresses media sections in terms
of transceivers (sender-receiver pairs), the PeerConnectionTracker is
updated to more accurately express what is going on. In Unified Plan you
may for example add a transceiver that does not have a track yet or
receive a track by reusing an already existing transceiver. You may also
send the same track multiple times, or send/receive tracks that don't
belong to any stream or that belongs to multiple streams. This is all
described by the sender/receiver, not by any set of "local" or "remote
streams". In Unified Plan, because receiving tracks are created
alongside transceivers, often before being used if ever used, speaking
of tracks being "added" or "removed" is confusing.

Instead of "addStream"/"removeStream" we get
"[sender/receiver/transceiver][Added/Modified/Removed]".
The chrome://webrtc-internals log shows state of the sender or receiver
(TODO: or in Unified Plan, the transceiver) that was added, updated or
removed.

This will continue to make sense moving forward, and removes the
dependency on streams, which is a RTCRtpTransceiver blocker because
Unified Plan uses stream IDs, not streams.

Bug: 810708,  777617 
Change-Id: I47dad2cb0b754c4ab8fb47ca5c7da3e0f33c6072
Reviewed-on: https://chromium-review.googlesource.com/1117063
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571129}
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/content/browser/resources/media/peer_connection_update_table.js
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/content/renderer/media/webrtc/peer_connection_tracker.cc
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/content/renderer/media/webrtc/peer_connection_tracker.h
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/content/renderer/media/webrtc/rtc_peer_connection_handler_unittest.cc
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/content/renderer/media/webrtc/rtc_rtp_sender.h
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/third_party/blink/public/platform/web_rtc_rtp_sender.h
[modify] https://crrev.com/e95ed83d6fd0bc275f8fded449c61374c147f9eb/third_party/blink/renderer/platform/testing/testing_platform_support_with_web_rtc.cc

Comment 2 by hbos@chromium.org, Jun 28 2018

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af5c23c75d00f6513d9e823c46a3a3ddfdc949e4

commit af5c23c75d00f6513d9e823c46a3a3ddfdc949e4
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Jul 04 09:18:48 2018

MediaStreamTrackMetrics simplified to observe tracks, not streams.

Prior to this CL, MediaStreamTrackMetrics hooked up to streams, tracking
not only all of the streams' tracks (CONNECTED/DISCONNECTED) but also
any tracks that might be added or removed from the stream.

This has not been necessary to do for a long time, since the RTP Media
APIs are track-based, not stream-based, and the legacy
addStream()-streams are shimmed on top of addTrack(). Adding a track to
such a stream causes addTrack(), and there is no need for the
MediaStreamTrackMetrics class to observe the streams.

With this CL, MediaStreamTrackMetrics only care about tracks (direction,
kind and id). This also covers the Unified Plan use case of tracks not
necessarily belonging to any stream.

There is room for improvements in this area, such as counting tracks
added through replaceTrack(), but that should be handled separately.
This CL removes the dependency on streams which unblocks
RTCRtpTransceiver/Unified Plan work.

Bug: 810708,  777617 
Change-Id: I347e729560717be83b7b1a3571f25a8497d65e46
Reviewed-on: https://chromium-review.googlesource.com/1117691
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572521}
[modify] https://crrev.com/af5c23c75d00f6513d9e823c46a3a3ddfdc949e4/content/renderer/media/webrtc/media_stream_track_metrics.cc
[modify] https://crrev.com/af5c23c75d00f6513d9e823c46a3a3ddfdc949e4/content/renderer/media/webrtc/media_stream_track_metrics.h
[modify] https://crrev.com/af5c23c75d00f6513d9e823c46a3a3ddfdc949e4/content/renderer/media/webrtc/media_stream_track_metrics_unittest.cc
[modify] https://crrev.com/af5c23c75d00f6513d9e823c46a3a3ddfdc949e4/content/renderer/media/webrtc/rtc_peer_connection_handler.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f23ab64af3c7e426c11973385ef6f0d1380d12c3

commit f23ab64af3c7e426c11973385ef6f0d1380d12c3
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Jul 05 08:04:44 2018

RtpSenderState: Surfacing states between threads in a separate class.

In Unified Plan, it will be necessary to surface sender, receiver and
transceiver states from the signaling thread to the main thread both
synchronously and asynchronously. To make ownership clear, unify code
paths and allow decoupling of surfacing state information possible, the
states are put in a separate class.

RtpReceiverState, RtpTransceiverState and "state surfacer" helper class
will be added in follow-up CLs. This will allow both synchronous APIs
(e.g. addTrack/addTransceiver) which use blocking calls and
asynchronous APIs (e.g. setRemoteDescription) which use PostTask to get
the latest sender-receiver-transceiver states and initialize them
(including explicit initialization of track adapters) in time for
exposing them in blink (JavaScript).

This is outlined here:
https://docs.google.com/document/d/1HuNGPQuRM9uDFt4Oh3QdxmkEHvfHo1DogkD-pVTuOJM/edit?usp=sharing

This CL introduces RtpSenderState, and refactors
RTCRtpSender::RTCRtpSenderInternal to use it. The follow-up,
RtpReceiverState, will use the same design pattern.

Additionally, this CL makes content senders stop relying on streams and
instead uses stream IDs. Updated RTCPeerConnectionHandler to use the
stream ID-based AddTrack(), allowing third_party/webrtc to remove the
old AddTrack() API that still used streams.

Bug:  777617 , 810708
Change-Id: I3ac8dabe160e7eb1f9c3b3ac1882dc77b0be2a2e
Reviewed-on: https://chromium-review.googlesource.com/1118558
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572733}
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/BUILD.gn
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/mock_peer_connection_impl.cc
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/mock_peer_connection_impl.h
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/peer_connection_tracker.cc
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/rtc_peer_connection_handler_unittest.cc
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/rtc_rtp_sender.h
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/content/renderer/media/webrtc/rtc_rtp_sender_unittest.cc
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/third_party/blink/public/platform/web_rtc_rtp_sender.h
[modify] https://crrev.com/f23ab64af3c7e426c11973385ef6f0d1380d12c3/third_party/blink/renderer/platform/testing/testing_platform_support_with_web_rtc.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505

commit ae070c77aae47dabe7bd1cf2ee14e68f53f5f505
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Jul 05 08:39:39 2018

RtpReceiverState and Chrome to stop relying on WebRTC MediaStreams.

RtpReceiverState allows surfacing states between third_party/webrtc on
the signaling thread and conten/blink on the main thread in a separate
helper class that will enable doing this more easily in different code
paths, both synchronous and asynchronous ones. It unblocks transceiver
work and will make onion souping away the content receiver layer easy.
  See RtpSenderState for more information, this is the same thing but
for receivers:
https://chromium-review.googlesource.com/c/chromium/src/+/1118558

Additionally, this CL removes our dependency on third_party/webrtc's
webrtc::MediaStreamInterface, isolating stream creation and management
to the blink layer. This simplifies things greatly, and will allow us
to remove the WebRtcMediaStreamAdapter[Map] in follow-up CLs and makes
the updating of receiver's streams much easier and straight-forward.
No more logic spread out over different callbacks with regards to
streams.

MediaStream::[Add/Remove]RemoteTrack() added, allowing adding/removing
tracks using blink-layer tracks as argument rather than web components,
allowing us to move away from this old web wiring. When a track is
added/removed as "remote" it means it was not added by the
addTrack()/removeTrack() JavaScript APIs, so associated events (e.g.
"onaddtrack", "onremovetrack") should fire on the stream.

This CL also does misc drive-by changes, like renaming
Did[Add/Remove]RemoteTrack to the more accurately named
Did[Add/Remove]Receiver, and removing some already-disabled
RTCPeerConnectionHandler unittests that we don't want to maintain
(https://crbug.com/788659).

Bug:  777617 , 810708, 788659
Change-Id: I5c187c6346472aaecf6bc6fe4710e415b2a44902
Reviewed-on: https://chromium-review.googlesource.com/1122216
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572738}
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/mock_web_rtc_peer_connection_handler_client.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/mock_web_rtc_peer_connection_handler_client.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/peer_connection_tracker.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/peer_connection_tracker.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_peer_connection_handler.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_peer_connection_handler_unittest.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_rtp_receiver.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_rtp_receiver.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_rtp_receiver_unittest.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/webrtc_set_remote_description_observer.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/webrtc_set_remote_description_observer.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/webrtc_set_remote_description_observer_unittest.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/public/platform/web_rtc_peer_connection_handler_client.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/public/platform/web_rtc_rtp_receiver.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/mediastream/media_stream.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/mediastream/media_stream.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h
[modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc

Sign in to add a comment