New issue
Advanced search Search tips

Issue 790007 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature


Sign in to add a comment

RTCRtpSender.replaceTrack

Project Member Reported by hbos@chromium.org, Nov 30 2017

Issue description

Implement and expose RTCRtpSender.replaceTrack according to spec:
https://w3c.github.io/webrtc-pc/#dom-rtcrtpsender-replacetrack

 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 6 2017

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

commit a1356693554cd0bdb5b1cc4bc406896346fa3159
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Dec 06 12:07:17 2017

Implement WebRTCRtpSender with an internal ref counted class.

This is in preparation for ReplaceTrack. When we have ReplaceTrack, the
WebRTCRtpSender is modifiable. Having an internal ref counted class
ensures that if one WebRTCRtpSender is modified, this is reflected in
all shallow copies of the class.

In other words, this CL enables content::RTCRtpSender having internal
modifiable states separate from the webrtc::RtpSenderInterface.

Made some misc fly-by changes like renaming "webrtc_rtp_sender"
variables/methods to "webrtc_sender" and making
WebRTCRtpSender::Track() return by value instead of pointer (since
there is a "null" value).

Bug:  790007 
Change-Id: I3f11b2af7457b6c3b9e6de5d97c32085bf050280
Reviewed-on: https://chromium-review.googlesource.com/806161
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522069}
[modify] https://crrev.com/a1356693554cd0bdb5b1cc4bc406896346fa3159/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/a1356693554cd0bdb5b1cc4bc406896346fa3159/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/a1356693554cd0bdb5b1cc4bc406896346fa3159/content/renderer/media/webrtc/rtc_rtp_sender.h
[modify] https://crrev.com/a1356693554cd0bdb5b1cc4bc406896346fa3159/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/a1356693554cd0bdb5b1cc4bc406896346fa3159/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.cpp
[modify] https://crrev.com/a1356693554cd0bdb5b1cc4bc406896346fa3159/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.h
[modify] https://crrev.com/a1356693554cd0bdb5b1cc4bc406896346fa3159/third_party/WebKit/public/platform/WebRTCRtpSender.h

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 6 2017

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

commit 3b9232c5c2acd26e4a55b7d855e1eccd2bc6d598
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Dec 06 16:57:26 2017

Move RemoveTrack logic to RTCRtpSender::RTCRtpSenderInternal.

Refactoring. Will do the same pattern for ReplaceTrack.

Bug:  790007 
Change-Id: Iadd661a839fed870fd423692ca553ec506413f8b
Reviewed-on: https://chromium-review.googlesource.com/808284
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522108}
[modify] https://crrev.com/3b9232c5c2acd26e4a55b7d855e1eccd2bc6d598/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/3b9232c5c2acd26e4a55b7d855e1eccd2bc6d598/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/3b9232c5c2acd26e4a55b7d855e1eccd2bc6d598/content/renderer/media/webrtc/rtc_rtp_sender.h
[modify] https://crrev.com/3b9232c5c2acd26e4a55b7d855e1eccd2bc6d598/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp

Comment 3 by hbos@chromium.org, Dec 28 2017

Blockedon: 797876
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 8 2018

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

commit 2e34176b27b038796a731e9d9021d4c9cd1281f1
Author: Henrik Boström <hbos@chromium.org>
Date: Mon Jan 08 21:36:31 2018

WebRTCRtpSender::ReplaceTrack(), implementation and unittests added.

This implements replaceTrack()[1] in content::RTCRtpSender by jumping
from the main thread, invoking webrtc layer replaceTrack() on the webrtc
signaling thread, and back to the main thread to handle the result.

In order to get/create a track adapter for the replaced
blink::MediaStreamTrack, the sender now has a reference to the stream
and track adapter maps.

rtc_rtp_sender_unittest.cc added.

Design doc:
https://docs.google.com/document/d/1bpsYLC35cHAJnlnmiBJ3bhUQXfUBgKbTv7A6nkEs6OA/edit?usp=sharing

In a follow-up CL, blink layer sender/peer connection and
RTCPeerConnectionHandler can be wired up to to enable replaceTrack() in
JavaScript.

[1] https://w3c.github.io/webrtc-pc/#dom-rtcrtpsender-replacetrack

Bug:  790007 
Change-Id: I8ce3a7512083fc707e33453526356740d2536952
Reviewed-on: https://chromium-review.googlesource.com/833870
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527762}
[modify] https://crrev.com/2e34176b27b038796a731e9d9021d4c9cd1281f1/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/2e34176b27b038796a731e9d9021d4c9cd1281f1/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/2e34176b27b038796a731e9d9021d4c9cd1281f1/content/renderer/media/webrtc/rtc_rtp_sender.h
[add] https://crrev.com/2e34176b27b038796a731e9d9021d4c9cd1281f1/content/renderer/media/webrtc/rtc_rtp_sender_unittest.cc
[modify] https://crrev.com/2e34176b27b038796a731e9d9021d4c9cd1281f1/content/test/BUILD.gn
[modify] https://crrev.com/2e34176b27b038796a731e9d9021d4c9cd1281f1/third_party/WebKit/public/platform/WebRTCRtpSender.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 9 2018

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

commit 5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c
Author: Henrik Boström <hbos@chromium.org>
Date: Tue Jan 09 19:39:56 2018

RTCRtpSender.replaceTrack added behind flag.

Wires up replaceTrack[1] from the content layer implementation[2],
exposing it in JavaScript behind RuntimeEnabled experimental feature
"RTCRtpSenderReplaceTrack".

The meat of this CL are the tests. This includes making sure the track
is set asynchronously (tested with WPT) and that replacing a video
track causes different video to be sent (tested with browser_tests
instead of WPT due to bug https://crbug.com/793808).

Design doc:
https://docs.google.com/document/d/1bpsYLC35cHAJnlnmiBJ3bhUQXfUBgKbTv7A6nkEs6OA/edit?usp=sharing

[1] https://w3c.github.io/webrtc-pc/#dom-rtcrtpsender-replacetrack
[2] https://chromium-review.googlesource.com/c/chromium/src/+/833870/

Bug:  790007 
Change-Id: If8c3499c78fe664a496c5d90cfbfa7b48c1c036f
Reviewed-on: https://chromium-review.googlesource.com/810765
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Taylor Brandstetter <deadbeef@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528071}
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/chrome/browser/media/webrtc/webrtc_rtp_browsertest.cc
[add] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/chrome/test/data/webrtc/peerconnection_replacetrack.js
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/chrome/test/data/webrtc/webrtc_jsep01_test.html
[add] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-replaceTrack.https-expected.txt
[add] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-replaceTrack.https.html
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/LayoutTests/external/wpt/webrtc/interfaces.https-expected.txt
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.cpp
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.h
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.idl
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/Source/platform/runtime_enabled_features.json5
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/5ccc9382e7bd5c7dac9349fc1aae6fdc66a6ff4c/tools/metrics/histograms/enums.xml

Comment 6 by hbos@webrtc.org, Jan 11 2018

Blockedon: webrtc:8734

Comment 7 by hbos@chromium.org, Jan 11 2018

Blockedon: 801205

Comment 9 by hbos@chromium.org, Jan 17 2018

Blockedon: -webrtc:8734
Labels: M-65
Status: Verified (was: Started)
With the above CL, this is shipped in M65.

Removing blockedon bug that is tracked separately, not blocking.

Comment 10 by hbos@chromium.org, Jan 17 2018

Blocking: 699846

Comment 11 by hbos@chromium.org, Jan 17 2018

Blocking: 738929
Labels: -Type-Bug Type-Feature
Blocking: webrtc:2131
Project Member

Comment 14 by bugdroid1@chromium.org, Feb 12 2018

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

commit 849048b40d0ed8b43f107e41f189eb02db541f35
Author: Philipp Hancke <philipp.hancke@googlemail.com>
Date: Mon Feb 12 12:16:40 2018

Improve RTCRtpSender.replaceTrack tests' Firefox compatibility

The current test relies on addTrack(track) which is not the
API tested. Using addTrack(track, stream) works in Firefox
as well as Chrome.

BUG= 790007 

Change-Id: I098101cd6556620c42b4f51df98ef54b0a8ad1f7
Reviewed-on: https://chromium-review.googlesource.com/912289
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536063}
[modify] https://crrev.com/849048b40d0ed8b43f107e41f189eb02db541f35/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-replaceTrack.https.html

Sign in to add a comment