New issue
Advanced search Search tips

Issue 806875 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 803021

Blocking:
issue 806872



Sign in to add a comment

createDTMFSender does not work with addTrack-tracks

Project Member Reported by hbos@chromium.org, Jan 29 2018

Issue description

This is legacy API that should be deprecated and removed (https://crbug.com/699846) in favor of spec-compliant RTCRtpSender.dtmf ( https://crbug.com/806872 ), but it is currently shipped.

It is based on |local_streams_|. It should be updated to rely on senders's tracks or "streams of senders" so that we can remove |local_streams_| and so that DTMF users can use spec-compliant addTrack() instead of legacy addStream().

This would also do some of the prerequisite work for RTCRtpSender.dtmf.
 

Comment 1 by hbos@chromium.org, Jan 31 2018

Blockedon: 803021
Part of fixing  https://crbug.com/803021  will move us away from |local_streams_| and fix this issue at the same time. I could do the "blocked on" in either direction, but putting it as blocked on on the legacy API issue which is the primary issue being worked on.
Project Member

Comment 2 by bugdroid1@chromium.org, 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

Comment 3 by hbos@chromium.org, Feb 12 2018

Labels: M-66
Status: Verified (was: Assigned)

Sign in to add a comment