New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 7 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 701330
issue 883289
issue 893160
issue 893172


Show other hotlists

Hotlists containing this issue:
WebRTC-1.0-Spec-Compliance


Sign in to add a comment
link

Issue 893158: Implement getSynchronizationSources()

Reported by hbos@chromium.org, Oct 8 Project Member

Issue description

Implement RTCRtpReceiver.getSynchronizationSources() and the RTCRtpSynchronizationSource dictionary.
In doing so, also update RTCRtpContributingSource to be a dictionary. When it was implemented the spec defined it as an interface.
https://w3c.github.io/webrtc-pc/#dom-rtcrtpreceiver-getsynchronizationsources
 

Comment 1 by hbos@chromium.org, Oct 8

Blocking: 893159

Comment 2 by hbos@chromium.org, Oct 8

Blocking: 701330

Comment 3 by hbos@chromium.org, Oct 8

Blocking: 893160

Comment 5 by hbos@chromium.org, Dec 11

Cc: ovelius@google.com zstein@chromium.org hbos@chromium.org
 Issue 883287  has been merged into this issue.

Comment 6 by hbos@chromium.org, Dec 11

Labels: -M-71
This is for both audio and video.

Comment 7 by hbos@chromium.org, Jan 8

Blocking: 893172

Comment 8 by bugdroid1@chromium.org, Jan 8

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

commit da94bf9e3346524cbe0059e9aa6d58ac80041ff2
Author: Henrik Boström <hbos@chromium.org>
Date: Tue Jan 08 18:06:59 2019

Make RTCRtpContributingSource a dictionary.

Before this CL it was an interface, but the spec changed this to a
dictionary a long time ago.

This is mostly refactoring efforts in preparation for
getSynchronizationSources(), but there is one behavioral change: Because
it is now a dictionary, every call to getContributingSources() will
produce a new set of objects. In this comparison...
  r.getContributingSources()[0] == r.getContributingSources()[0]
... the objects will be value-equal, but not reference-equal.

We should add more test coverage for getContributingSources() but this
is currently not testable without fakes/mocks.

Bug:  893158 
Change-Id: If589c3fd3bea663e4b1284a16d5a0d6a5fa72231
Reviewed-on: https://chromium-review.googlesource.com/c/1398084
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@{#620791}
[modify] https://crrev.com/da94bf9e3346524cbe0059e9aa6d58ac80041ff2/third_party/blink/renderer/modules/modules_idl_files.gni
[modify] https://crrev.com/da94bf9e3346524cbe0059e9aa6d58ac80041ff2/third_party/blink/renderer/modules/peerconnection/BUILD.gn
[delete] https://crrev.com/27ac4fc7a586c8a86aaae687dd15c6c57a11e76d/third_party/blink/renderer/modules/peerconnection/rtc_rtp_contributing_source.cc
[delete] https://crrev.com/27ac4fc7a586c8a86aaae687dd15c6c57a11e76d/third_party/blink/renderer/modules/peerconnection/rtc_rtp_contributing_source.h
[modify] https://crrev.com/da94bf9e3346524cbe0059e9aa6d58ac80041ff2/third_party/blink/renderer/modules/peerconnection/rtc_rtp_contributing_source.idl
[modify] https://crrev.com/da94bf9e3346524cbe0059e9aa6d58ac80041ff2/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc
[modify] https://crrev.com/da94bf9e3346524cbe0059e9aa6d58ac80041ff2/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.h
[modify] https://crrev.com/da94bf9e3346524cbe0059e9aa6d58ac80041ff2/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/da94bf9e3346524cbe0059e9aa6d58ac80041ff2/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

Comment 9 by hbos@google.com, Jan 8

Labels: M-73
Status: Started (was: Assigned)

Comment 10 by hbos@chromium.org, Jan 8

Blocking: -893159

Comment 11 by hbos@chromium.org, Jan 9

Blockedon: 883289

Comment 12 by hbos@chromium.org, Jan 9

Blockedon: -883289
Blocking: 883289

Comment 13 by bugdroid1@chromium.org, Jan 10

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

commit ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Jan 10 13:05:13 2019

RTCRtpReceiver.getSynchronizationSources() added.

This CL:
- Adds getSynchronizationSources() for both audio and video.
- Adds proper test coverage, including test coverage for unsupported
  members audioLevel and voiceActivityFlag. It turns out that our
  timestamps are not comparable to performance.now(), this should be
  fixed separately (this must already have been the case for
  getContributingSources() but not noticed until now).

Intent to Ship:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/wTJMXOwcV1A/uszhuWsGCQAJ

Bug:  893158 ,  893160 
Change-Id: Ic90344af7fa48c3cd0b94929cc9453deb9dc7f9f
Reviewed-on: https://chromium-review.googlesource.com/c/1401068
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@{#621558}
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/renderer/modules/modules_idl_files.gni
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.h
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.idl
[add] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/renderer/modules/peerconnection/rtc_rtp_synchronization_source.idl
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/web_tests/external/wpt/webrtc/RTCRtpReceiver-getSynchronizationSources.https-expected.txt
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/web_tests/external/wpt/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/web_tests/external/wpt/webrtc/idlharness.https.window-expected.txt
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/web_tests/virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/idlharness.https.window-expected.txt
[modify] https://crrev.com/ea54e65cc79e2ec87ce021f9acbdd2d51b1f2a36/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

Comment 14 by hbos@chromium.org, Jan 10

Status: Verified (was: Started)

Comment 15 by bugdroid1@chromium.org, Jan 10

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2

commit 8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Jan 10 18:43:28 2019

Rename WebRTCRtpContributingSource to WebRTCRtpSource.

This is to account for the fact that this class is used both for CSRCs
(contributing sources) and SSRCs (synchronization sources), not just
CSRCs.

Also moving WebRTCRtpContributingSourceType to WebRTCRtpSource::Type
and adding the "k" prefix to its enum labels.

TBR=foolip@chromium.org

Bug:  893158 
Change-Id: I53fb7e6ee0ad46d08e63e6344eb2cf5bac1f900f
Reviewed-on: https://chromium-review.googlesource.com/c/1400835
Reviewed-by: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621665}
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/content/renderer/BUILD.gn
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/content/renderer/media/webrtc/fake_rtc_rtp_transceiver.cc
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/content/renderer/media/webrtc/fake_rtc_rtp_transceiver.h
[delete] https://crrev.com/6a0a09909f0f5bec51931c659da6acbf19dd2b17/content/renderer/media/webrtc/rtc_rtp_contributing_source.cc
[delete] https://crrev.com/6a0a09909f0f5bec51931c659da6acbf19dd2b17/content/renderer/media/webrtc/rtc_rtp_contributing_source.h
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/content/renderer/media/webrtc/rtc_rtp_receiver.cc
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/content/renderer/media/webrtc/rtc_rtp_receiver.h
[add] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/content/renderer/media/webrtc/rtc_rtp_source.cc
[add] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/content/renderer/media/webrtc/rtc_rtp_source.h
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/third_party/blink/public/BUILD.gn
[delete] https://crrev.com/6a0a09909f0f5bec51931c659da6acbf19dd2b17/third_party/blink/public/platform/web_rtc_rtp_contributing_source.h
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/third_party/blink/public/platform/web_rtc_rtp_receiver.h
[add] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/third_party/blink/public/platform/web_rtc_rtp_source.h
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.h
[modify] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/third_party/blink/renderer/platform/BUILD.gn
[delete] https://crrev.com/6a0a09909f0f5bec51931c659da6acbf19dd2b17/third_party/blink/renderer/platform/exported/web_rtc_rtp_contributing_source.cc
[add] https://crrev.com/8cb8cb2ea6340b0c15c722d5a3b8d2fe7ecef4b2/third_party/blink/renderer/platform/exported/web_rtc_rtp_source.cc

Comment 16 by bugdroid1@chromium.org, Jan 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66a94294e79fe5790b2d5514679200d48ff4673e

commit 66a94294e79fe5790b2d5514679200d48ff4673e
Author: Henrik Boström <hbos@chromium.org>
Date: Fri Jan 11 11:26:23 2019

Verify getContributingSources() does not return any CSRCs.

The previous test was, and always had been, completely broken. In a
loopback call scenario such as in WPT tests, no CSRCs are being
received so we should not expect them to. In order to properly test this
method one would need fakes, mocks or a server to receive CSRCs from.
This test is updated to test the only thing that we can test in Web
Platform Tests: in a loopback call, getContributingSources() returns an
empty list.

Bug:  893158 
Change-Id: I18782ea7912001c012c98145500d8b3c5e95a7ba
Reviewed-on: https://chromium-review.googlesource.com/c/1404266
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621972}
[delete] https://crrev.com/199c1000a62aebb35ce42e94fd10e959cd5c7b1a/third_party/blink/web_tests/external/wpt/webrtc/RTCRtpReceiver-getContributingSources.https-expected.txt
[modify] https://crrev.com/66a94294e79fe5790b2d5514679200d48ff4673e/third_party/blink/web_tests/external/wpt/webrtc/RTCRtpReceiver-getContributingSources.https.html

Sign in to add a comment