New issue
Advanced search Search tips

Issue 680172 link

Starred by 2 users

Issue metadata

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

Blocking:
issue webrtc:7060



Sign in to add a comment

RTCPeerConnection.getStats selector argument

Project Member Reported by hbos@chromium.org, Jan 11 2017

Issue description

The new getStats API should take an optional selector argument of type MediaStreamTrack.

The current spec definition is a bit vague, saying we should return stats related to the track.

Its meaning is being properly specified in this pull request: https://github.com/w3c/webrtc-pc/pull/990

Implement!
 
Labels: -M-57 M-58
Bumping this to M58. Please correct if that's wrong.

Comment 2 by hbos@chromium.org, Jan 26 2017

Blocking: -627816 webrtc:7060

Comment 3 by q...@chromium.org, Feb 17 2017

Cc: q...@chromium.org
Pardon my ignorance / confusion from trying to follow the pull request, but does the selector select a specific track or can it select certain types of stat? Is there a way to only request a subset of the stats (but from all tracks) so that we can get these stats cheaper and more frequently.

Comment 4 by hbos@chromium.org, Feb 20 2017

There is not a consensus yet.

One proposal suggests to only return relevant RTCMediaStreamTrackStats, RTCInboundRTPStreamStats and RTCOutboundRTPStreamStats (ignoring references to other stats objects).
https://github.com/w3c/webrtc-pc/pull/990

Another suggests to use RTPSender/RTPReceiver as selectors (or track for backwards-compatibility, then taking the corresponding sender/receiver for that track) and get the corresponding RTCInboundRTPStreamStats or RTCOutboundRTPStreamStats - then recursively get any stats referenced by any stats object. This tree would include almost all stats dictionary types.
https://github.com/w3c/webrtc-pc/pull/1030

I know there have been discussions about more specific selectors to allow polling specific stats more frequently at low cost but I don't know of any proposal yet. There's a per-frame stat proposal here, not sure about status.
https://discourse.wicg.io/t/proposal-a-frame-level-event-logging-mechanism-for-webrtc/1780
Labels: -M-58 M-59
Labels: -M-59 M-60
Bumping to M60. Please correct if that's wrong.
Labels: -M-60 M-61
Labels: -M-61 M-62

Comment 9 by hbos@chromium.org, Mar 9 2018

Labels: -M-62 M-67
Started working on this now. (Status was lying earlier)

Comment 10 by hbos@chromium.org, Mar 9 2018

Since this bug was filed the spec has evolved, there is now also RTCRtpSender.getStats() and RTCRtpReceiver.getStats() and the RTCPeerConnection.getStats(MediaStreamTrack selectorArg) version picks "the sender or receiver for the selectorArg track if there is one and only one".

Comment 11 by hbos@chromium.org, Mar 9 2018

Note to self: Need to do an Intent to Implement & Ship.
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 12 2018

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

commit b619936dee2807ca99ae2ed76956997bd2b43d6f
Author: Henrik Boström <hbos@webrtc.org>
Date: Mon Mar 12 10:54:09 2018

Stats traversal algorithm added.

This is part of the work to add a selector argument to getStats().

Changes:
- TakeReferencedStats() added, which traverses the stats graph and takes
  any stats from the report that are directly or indirectly accessible
  from the starting stats objects in the stats graph. The result is
  returned as a stats report.
- GetStatsReferencedIds(), an efficient helper function for getting
  neighbor stats object IDs.
- RTCStatsReport::Take(), removed the stats object with the given ID and
  returns ownership of it (so that it can be added to another report).

TakeReferencedStats() is tested with a bunch of sample stats graphs.

GetStatsReferencedIds() is tested in the rtcstats_integrationttest.cc,
making sure the expected IDs are returned. The expected IDs are the
values of the stats object members with the "Id" or "Ids" suffix.

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

Bug:  chromium:680172 
Change-Id: I5da9da8250da0cb05adb864015901393a4290776
Reviewed-on: https://webrtc-review.googlesource.com/60869
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22381}
[modify] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/api/stats/rtcstatsreport.h
[modify] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/pc/BUILD.gn
[modify] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/pc/rtcstats_integrationtest.cc
[add] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/pc/rtcstatstraversal.cc
[add] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/pc/rtcstatstraversal.h
[add] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/pc/rtcstatstraversal_unittest.cc
[modify] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/stats/rtcstatsreport.cc
[modify] https://crrev.com/b619936dee2807ca99ae2ed76956997bd2b43d6f/stats/rtcstatsreport_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 19 2018

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

commit 5b3541f9afadf6418cc425169b1bdc27502598a2
Author: Henrik Boström <hbos@webrtc.org>
Date: Mon Mar 19 15:32:16 2018

RTCStatsCollector::GetStatsReport() with optional selector argument.

This implements the stats selection algorithm[1] in RTCStatsCollector by
obtaining the selector's inbound-rtp/outbound-rtp stats and performing
the stats traversal algorithm (TakeReferencedStats)[2] on a copy of the
cached report with the rtps as starting point.

Changes:
- RTCStatsCollector.GetStatsReport() with selector arguments added.
  - RequestInfo added, "callbacks_" is replaced by "requests_".
- RTCStatsReport.Copy() added.
- New test for sender selector and receiver selector,
  RTCStatsCollectorTest.GetStatsWithSelector.

[1] https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm
[2] https://cs.chromium.org/chromium/src/third_party/webrtc/pc/rtcstatstraversal.h

Bug:  chromium:680172 
Change-Id: I9eff00738a1f24c94c9c8ecd13c1304452e962cf
Reviewed-on: https://webrtc-review.googlesource.com/62141
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22499}
[modify] https://crrev.com/5b3541f9afadf6418cc425169b1bdc27502598a2/api/stats/rtcstatsreport.h
[modify] https://crrev.com/5b3541f9afadf6418cc425169b1bdc27502598a2/pc/rtcstatscollector.cc
[modify] https://crrev.com/5b3541f9afadf6418cc425169b1bdc27502598a2/pc/rtcstatscollector.h
[modify] https://crrev.com/5b3541f9afadf6418cc425169b1bdc27502598a2/pc/rtcstatscollector_unittest.cc
[modify] https://crrev.com/5b3541f9afadf6418cc425169b1bdc27502598a2/pc/rtcstatstraversal.cc
[modify] https://crrev.com/5b3541f9afadf6418cc425169b1bdc27502598a2/stats/rtcstatsreport.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 20 2018

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

commit 345d5f1b785f1d80d9d07d4b70811f61756b99d6
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Mar 20 14:02:47 2018

Roll src/third_party/webrtc/ d2c8332e2..9047dac75 (23 commits)

https://webrtc.googlesource.com/src.git/+log/d2c8332e2b03..9047dac7576d

$ git log d2c8332e2..9047dac75 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:None,chromium:None,chromium:None,chromium:822799,chromium:680172,chromium:None,chromium:755660,chromium:None,chromium:None,chromium:None,chromium:755660


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng;master.tryserver.chromium.win:win-msvc-dbg
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ia063873eca78ba81cec927cf4374423586bb338f
Reviewed-on: https://chromium-review.googlesource.com/970723
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#544347}
[modify] https://crrev.com/345d5f1b785f1d80d9d07d4b70811f61756b99d6/DEPS

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 26 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/1df1bf8551c8e01ee72e2ef336d27a3073060487

commit 1df1bf8551c8e01ee72e2ef336d27a3073060487
Author: Henrik Boström <hbos@webrtc.org>
Date: Mon Mar 26 12:08:20 2018

PeerConnectionInterface::GetStats() with selector argument added.

This exposes the stats selection algorithm[1] on the PeerConnection.

Per-spec, there are four flavors of getStats():
1. RTCPeerConnection.getStats().
2. RTCPeerConnection.getStats(MediaStreamTrack selector).
3. RTCRtpSender.getStats().
4. RTCRtpReceiver.getStats().

1) is the parameterless getStats() which is already shipped.
2) is the same as 3) and 4) except the track is used to look up the
corresponding sender/receiver to use as the selector.
3) and 4) perform stats collection with a filter, which is implemented
in RTCStatsCollector.GetStatsReport(selector).

For technical reasons, it is easier to place GetStats() on the
PeerConnection where the RTCStatsCollector lives than to place it on the
sender/receiver. Passing the selector as an argument or as a "this"
makes little difference other than style. Wiring Chrome up such that the
JavaScript APIs is like the spec is trivial after GetStats() is added to
PeerConnectionInterface.

This CL also adds comments documenting our intent to deprecate and
remove the legacy GetStats() APIs some time in the future.

[1] https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm

Bug:  chromium:680172 
Change-Id: I09316ba6f20b25d4f9c11785d0a1a1262d6062a1
Reviewed-on: https://webrtc-review.googlesource.com/62900
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22602}
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/api/peerconnectioninterface.h
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/api/peerconnectionproxy.h
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/api/stats/rtcstats_objects.h
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/pc/peerconnection.cc
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/pc/peerconnection.h
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/pc/rtcstats_integrationtest.cc
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/pc/test/fakepeerconnectionbase.h
[modify] https://crrev.com/1df1bf8551c8e01ee72e2ef336d27a3073060487/stats/rtcstats_objects.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Mar 27 2018

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

commit a1cb5235e016291ac2102a6fd1ea9dd7828268d0
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Mar 27 03:40:08 2018

Roll src/third_party/webrtc/ dc4737bbd..bb50ce5bb (14 commits)

https://webrtc.googlesource.com/src.git/+log/dc4737bbdacc..bb50ce5bb6d5

$ git log dc4737bbd..bb50ce5bb --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:799759,chromium:None,chromium:None,chromium:680172,chromium:None,chromium:775858,chromium:824706,chromium:775858,chromium:824706,chromium:775858,chromium:824706


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng;master.tryserver.chromium.win:win-msvc-dbg
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I8d505ada2d85afb907c8a08575e16c6b8ed11f48
Reviewed-on: https://chromium-review.googlesource.com/981605
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#545989}
[modify] https://crrev.com/a1cb5235e016291ac2102a6fd1ea9dd7828268d0/DEPS

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 27 2018

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

commit 12fa8b73250ca164464b307c658303dce4ab93b4
Author: Henrik Boström <hbos@chromium.org>
Date: Tue Mar 27 14:08:30 2018

Preparation CL for getStats() with selector argument.

Mostly moving things around, refactoring...

1. Move GetRTCStatsCallback from rtc_peer_connection_handler.cc to
   rtc_stats.[h/cc] where other blink/webrtc stats-related content-glue
   is placed, and rename it RTCStatsCollectorCallbackImpl.

   This allows it to be used by content's RTCRtpSender and
   RTCRtpReceiver in follow-up CLs.

2. Remove unused content::RTCRtpSender-constructor.

3. Move WebRTCStatsReportCallbackResolver from RTCPeerConnection.cpp
   to its own file.

   This allows it to be used by blink's RTCRtpSender and RTCRtpReceiver
   in follow-up CLs.

4. Implement new PeerConnectionInterface::GetStats() functions in
   MockPeerConnectionImpl (need to wait for
   https://webrtc-review.googlesource.com/c/src/+/62900 to land and roll
   into chromium).

Bug:  680172 
Change-Id: Ief18b2fb96ebc292149af606bd0edd87bac9e961
Reviewed-on: https://chromium-review.googlesource.com/973368
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546101}
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/content/renderer/media/webrtc/mock_peer_connection_impl.cc
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/content/renderer/media/webrtc/mock_peer_connection_impl.h
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/content/renderer/media/webrtc/rtc_rtp_sender.h
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/content/renderer/media/webrtc/rtc_stats.cc
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/content/renderer/media/webrtc/rtc_stats.h
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/third_party/WebKit/Source/modules/peerconnection/BUILD.gn
[modify] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[add] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/third_party/WebKit/Source/modules/peerconnection/WebRTCStatsReportCallbackResolver.cpp
[add] https://crrev.com/12fa8b73250ca164464b307c658303dce4ab93b4/third_party/WebKit/Source/modules/peerconnection/WebRTCStatsReportCallbackResolver.h

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 28 2018

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

commit 33802942258dc390231bd41bd57b3756938e7da0
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Mar 28 12:56:50 2018

RTCRtpSender.getStats() implemented in content.

This is the content layer wiring of getStats() with the sender selector
argument. A follow up CL will add blink layer wiring and expose it in
JavaScript.
https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm

Bug:  680172 
Change-Id: I2cb29c0d06a30995d9b468c70ba0fe1074afbf12
Reviewed-on: https://chromium-review.googlesource.com/973608
Reviewed-by: Mike West <mkwst@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@{#546463}
[modify] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/content/renderer/media/webrtc/rtc_rtp_sender.h
[modify] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/content/renderer/media/webrtc/rtc_rtp_sender_unittest.cc
[add] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/content/renderer/media/webrtc/test/webrtc_stats_report_obtainer.cc
[add] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/content/renderer/media/webrtc/test/webrtc_stats_report_obtainer.h
[modify] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/content/test/BUILD.gn
[modify] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/third_party/WebKit/Source/platform/testing/TestingPlatformSupportWithWebRTC.cpp
[modify] https://crrev.com/33802942258dc390231bd41bd57b3756938e7da0/third_party/WebKit/public/platform/WebRTCRtpSender.h

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 28 2018

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

commit 234ec636cba7bfc944bdc297da3fca2dd0dcc46b
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Mar 28 15:37:03 2018

RTCRtpSender.getStats() in blink added behind flag.

This exposes RTCRtpSender.getStats() in JavaScript (behind flag) which
implements the stats selection algorithm[1] for senders.

[1] https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm

Bug:  680172 
Change-Id: I8117c87f475d1c78fa3301fc2d821f0c3a21281f
Reviewed-on: https://chromium-review.googlesource.com/975306
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Taylor Brandstetter <deadbeef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546493}
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-track-stats.https-expected.txt
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-track-stats.https.html
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCRtpSender-getStats.https-expected.txt
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCRtpSender-getStats.https.html
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.cpp
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.h
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/Source/modules/peerconnection/RTCRtpSender.idl
[modify] https://crrev.com/234ec636cba7bfc944bdc297da3fca2dd0dcc46b/third_party/WebKit/Source/platform/runtime_enabled_features.json5

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 28 2018

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

commit 3628cfd2c1805650f0ba5ebd805cdd6315ab263f
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Mar 28 19:27:12 2018

Refactor content::RTCRtpReceiver and add unittests.

RTCRtpReceiver is a thin content layer wrapper for a webrtc layer
receiver. It is passed around with std::unique_ptr<> ownership (it is an
implementation of WebRTCRtpReceiver which is not reference counted).
It is possible for multiple RTCRtpReceivers to wrap around the same
webrtc layer receiver.

This ownership is limiting:
- Because wrappers are independent of one another even if they represent
  the same webrtc layer receiver, it is not possible to add any content
  layer states without worrying about different wrappers becoming
  out-of-sync.
- Because each wrapper uses std::unique_ptr<> it is not possible to add
  asynchronous methods to it without worrying that the RTCRtpReceiver is
  destroyed before the async op is finished.

This CL solves these problems by moving all of RTCRtpReceiver's
functionality into an internal ref-counted class RTCRtpReceiverInternal,
alling multiple wrappers to reference the same internal state and
allowing async operations to keep RTCRtpReceiverInternal alive with
references. (Same design as content::RTCRtpSender[Internal].)

This CL also adds rtc_rtp_receiver_unittest.cc.

The motivation for this CL is wanting to add async method getStats() to
RTCRtpReceiver and to test it in content_unittests. This will be done in
a follow-up CL.

TBR=guidou@chromium.org

Bug:  680172 
Change-Id: Ic74b08b5bc6f1f2a148be36f1f0ebef60ec80757
Reviewed-on: https://chromium-review.googlesource.com/975567
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546563}
[modify] https://crrev.com/3628cfd2c1805650f0ba5ebd805cdd6315ab263f/content/renderer/media/webrtc/rtc_rtp_receiver.cc
[modify] https://crrev.com/3628cfd2c1805650f0ba5ebd805cdd6315ab263f/content/renderer/media/webrtc/rtc_rtp_receiver.h
[add] https://crrev.com/3628cfd2c1805650f0ba5ebd805cdd6315ab263f/content/renderer/media/webrtc/rtc_rtp_receiver_unittest.cc
[modify] https://crrev.com/3628cfd2c1805650f0ba5ebd805cdd6315ab263f/content/test/BUILD.gn

Project Member

Comment 22 by bugdroid1@chromium.org, Mar 29 2018

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

commit 4caa570b0375c22f2009f66c3ac7f6f412e83d33
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Mar 29 07:38:50 2018

RTCRtpReceiver.getStats() implemented in content.

This is the content layer wiring of getStats() with the receiver
selector argument. A follow up CL will add blink layer wiring and expose
it in JavaScript.
https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm

(This CL is very similar to the RTCRtpSender.getStats() version:
https://chromium-review.googlesource.com/c/chromium/src/+/973608)

Bug:  680172 
Change-Id: I2f0cb0e997602c01ad6b1a2523663811c2e0ebe3
Reviewed-on: https://chromium-review.googlesource.com/975963
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Taylor Brandstetter <deadbeef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546760}
[modify] https://crrev.com/4caa570b0375c22f2009f66c3ac7f6f412e83d33/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/4caa570b0375c22f2009f66c3ac7f6f412e83d33/content/renderer/media/webrtc/rtc_rtp_receiver.cc
[modify] https://crrev.com/4caa570b0375c22f2009f66c3ac7f6f412e83d33/content/renderer/media/webrtc/rtc_rtp_receiver.h
[modify] https://crrev.com/4caa570b0375c22f2009f66c3ac7f6f412e83d33/content/renderer/media/webrtc/rtc_rtp_receiver_unittest.cc
[modify] https://crrev.com/4caa570b0375c22f2009f66c3ac7f6f412e83d33/third_party/WebKit/public/platform/WebRTCRtpReceiver.h

Project Member

Comment 23 by bugdroid1@chromium.org, Mar 29 2018

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

commit 81f83a5b7cb321a3c23c2678b804e5e568b45dbd
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Mar 29 13:14:34 2018

RTCRtpReceiver.getStats() in blink added behind flag.

This exposes RTCRtpReceiver.getStats() in JavaScript (behind flag) which
implements the stats selection algorithm[1] for receivers.

(This is similar to RTCRtpSender.getStats():
https://chromium-review.googlesource.com/c/chromium/src/+/975306)

[1] https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm

Bug:  680172 
Change-Id: I8049a52fcaa3c2bd51e5541c7149d9b3aee57e3d
Reviewed-on: https://chromium-review.googlesource.com/976121
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Taylor Brandstetter <deadbeef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546801}
[modify] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-track-stats.https-expected.txt
[modify] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-track-stats.https.html
[delete] https://crrev.com/392206a08a64ef572310d0ce432defa49778562f/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCRtpReceiver-getStats-expected.txt
[delete] https://crrev.com/392206a08a64ef572310d0ce432defa49778562f/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCRtpReceiver-getStats.html
[add] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCRtpReceiver-getStats.https-expected.txt
[add] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCRtpReceiver-getStats.https.html
[modify] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/Source/modules/peerconnection/RTCRtpReceiver.cpp
[modify] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/Source/modules/peerconnection/RTCRtpReceiver.h
[modify] https://crrev.com/81f83a5b7cb321a3c23c2678b804e5e568b45dbd/third_party/WebKit/Source/modules/peerconnection/RTCRtpReceiver.idl

Project Member

Comment 24 by bugdroid1@chromium.org, Mar 30 2018

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

commit 96d90f7a00a0bdd43be13a5446c6e4727ca53fb6
Author: Wez <wez@chromium.org>
Date: Fri Mar 30 02:58:03 2018

Disable newly-added RTCRtpReceiverTest.GetStats, which is flaky.

TBR: hbos
Bug:  827450 ,  680172 
Change-Id: I2d351e517dae3b011bd477c9482e756344df4fe2
Reviewed-on: https://chromium-review.googlesource.com/987394
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547089}
[modify] https://crrev.com/96d90f7a00a0bdd43be13a5446c6e4727ca53fb6/content/renderer/media/webrtc/rtc_rtp_receiver_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Apr 4 2018

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

commit ac7139070a15296cc490158eb9dee7d23f2165a4
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Apr 04 19:07:27 2018

RTCPeerConnection.getStats(MediaStreamTrack? selector = null) added.

Implements the RTCPeerConnection.getStats(MediaStreamTrack) version[1]
of the stats selection algorithm behind flag. This is the equivalent of
doing RTCRtpSender.getStats() or RTCRtpReceiver.getStats() for the
track's sender or receiver. This is tested in external/wpt/.

Due to limitations of the generated V8 bindings it is not possible to
express all versions of RTCPeerConnection.getStats() at the same time
in IDL. Thus this CL introduces a version of getStats() taking
"optional any callbacksOrSelector" as argument, with custom binding
logic performed in RTCPeerConnection.cpp. A fast/peerconnection/
LayoutTest is added to test all combinations.

[1] https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-getstats

Bug:  680172 
Change-Id: I8c64fc64c708d266c926dfa3eb3587c4cbb31210
Reviewed-on: https://chromium-review.googlesource.com/978128
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548149}
[modify] https://crrev.com/ac7139070a15296cc490158eb9dee7d23f2165a4/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-getStats.https-expected.txt
[modify] https://crrev.com/ac7139070a15296cc490158eb9dee7d23f2165a4/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-track-stats.https-expected.txt
[modify] https://crrev.com/ac7139070a15296cc490158eb9dee7d23f2165a4/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-track-stats.https.html
[add] https://crrev.com/ac7139070a15296cc490158eb9dee7d23f2165a4/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-getStats-overrides.https.html
[modify] https://crrev.com/ac7139070a15296cc490158eb9dee7d23f2165a4/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/ac7139070a15296cc490158eb9dee7d23f2165a4/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
[modify] https://crrev.com/ac7139070a15296cc490158eb9dee7d23f2165a4/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl

Project Member

Comment 26 by bugdroid1@chromium.org, Apr 9 2018

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

commit fe8a5e81a3b68c6b3c388728288591165a5a8c63
Author: Henrik Boström <hbos@chromium.org>
Date: Mon Apr 09 16:55:35 2018

Unflag the stats selection algorithm.

This ships:
- RTCPeerConnection.getStats(optional MediaStreamTrack? selector = null)
- RTCRtpSender.getStats()
- RTCRtpReceiver.getStats()

Spec:
https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm

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

Intent to Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/2mGY257k4-M

Bug:  680172 
Change-Id: I63c7991270489434cf67c9ebf7794a2648ceb6b8
Reviewed-on: https://chromium-review.googlesource.com/997554
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549201}
[modify] https://crrev.com/fe8a5e81a3b68c6b3c388728288591165a5a8c63/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/fe8a5e81a3b68c6b3c388728288591165a5a8c63/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
[modify] https://crrev.com/fe8a5e81a3b68c6b3c388728288591165a5a8c63/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.idl
[modify] https://crrev.com/fe8a5e81a3b68c6b3c388728288591165a5a8c63/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.idl
[modify] https://crrev.com/fe8a5e81a3b68c6b3c388728288591165a5a8c63/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.idl
[modify] https://crrev.com/fe8a5e81a3b68c6b3c388728288591165a5a8c63/third_party/blink/renderer/platform/runtime_enabled_features.json5

Comment 27 by hbos@chromium.org, Apr 10 2018

Status: Verified (was: Started)
Labels: -Type-Bug Type-Feature

Sign in to add a comment