Intent to Implement and Ship: Partial RTCRtpSynchronizationSource

65 views
Skip to first unread message

Zach Stein

unread,
Nov 8, 2017, 4:17:37 PM11/8/17
to blink-dev

Contact emails

zst...@chromium.org, hb...@chromium.org


Spec

https://w3c.github.io/webrtc-pc/#dom-rtcrtpreceiver

https://w3c.github.io/webrtc-pc/#dom-rtcrtpsynchronizationsource

Summary

This intent is to implement and ship more of the receiving part of the RTP Media API (old intent to implement and ship), specifically RTCRtpSynchronizationSource. The implemented portion will be the same as currently exists for RTCRtpContributingSource (intent to implement and ship).


RTCRtpSender extension:

- sequence<RTCRtpSynchronizationSource> getSynchronizationSources();


RTCRtpSynchronizationSource support:

- DOMHighRestTimeStamp timestamp;

- unsigned long source;

- byte audioLevel;


Motivation

The W3C WebRTC working group has consensus around adding this API.


Compatibility Risk
Low risk. There is consensus but it is not implemented in the other browsers yet.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.


Is this feature fully tested by web-platform-tests?

The tests are already upstream: https://github.com/w3c/web-platform-tests/blob/master/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html


Link to entry on the feature dashboard

Previous intents link to these features:

RTCRtpSender and RTCRtpReceiver extensions to webkitRTCPeerConnection (https://www.chromestatus.com/feature/5347809238712320)

Partial RTCRtpReceiver and RTCRtpContributingSource support (https://www.chromestatus.com/feature/5715393821802496)


I could either extend the scope of one of those, or add a new feature specific to RTCRtpSynchronizationSource. Please advise.


Requesting approval to ship?

Yes.

Joe Medley

unread,
Nov 9, 2017, 12:42:22 PM11/9/17
to Zach Stein, blink-dev
Zach,

Do you have a tracking bug so that I can follow its progress?

Joe

Joe Medley | Technical Writer, Chrome DevRel | jme...@google.com | 816-678-7195
If an API's not documented it doesn't exist.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJVzgw9bhJcC4C7UovjiWyaQnQft%3Dqag-TfFsgdzf5T_RAb0pw%40mail.gmail.com.

Zach Stein

unread,
Nov 10, 2017, 12:04:37 AM11/10/17
to Joe Medley, blink-dev

Joe Medley

unread,
Nov 10, 2017, 10:53:54 AM11/10/17
to Zach Stein, blink-dev
I should have looked at this more carefully. I think you need a separate status entry.

Joe Medley | Technical Writer, Chrome DevRel | jme...@google.com | 816-678-7195
If an API's not documented it doesn't exist.

Zach Stein

unread,
Nov 10, 2017, 12:51:07 PM11/10/17
to Joe Medley, blink-dev

Chris Harrelson

unread,
Nov 10, 2017, 1:56:31 PM11/10/17
to Zach Stein, Joe Medley, blink-dev

Philip Jägenstedt

unread,
Nov 14, 2017, 4:48:28 AM11/14/17
to Chris Harrelson, Zach Stein, Joe Medley, blink-dev
LGTM2 for shipping this, but after discussing with hbos@ I also have a few questions.

We have already shipped RTCRtpContributingSource and that also has an audioLevel attribute, so I wonder if it would be best to add support for both at the same time? (I'm guessing that people will write code that looks at one or the other depending on some condition.)

Also, I wonder if "If the RFC 6464 extension header is not present, the browser will compute a value for audioLevel as if it had come from RFC 6464." is really a good idea. How will it be computed, and can tests be written for this? Maybe audioLevel should be nullable in both places?

Finally, note that the existing tests in web-platform-tests are likely very surface-level since they were written with no implementation to run them against, so many more tests will likely be needed.

P.S. I'm assuming that voiceActivityFlag is being omitted deliberately.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

Rick Byers

unread,
Nov 14, 2017, 10:55:03 AM11/14/17
to Philip Jägenstedt, Chris Harrelson, Zach Stein, Joe Medley, blink-dev
LGTM3

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw-PBWaS%2BbRms7-59CurFeOZiDoRMstLgQVX5C9LTaPNEQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

Zach Stein

unread,
Nov 14, 2017, 5:16:47 PM11/14/17
to Philip Jägenstedt, Chris Harrelson, Zach Stein, Joe Medley, blink-dev
Thanks for taking a look.

We expect the two audioLevel fields to be used for two different use cases. RTCRtpSynchronizationSource.audioLevel is described in RFC 6464:

   an audio mixer or forwarder receives audio streams from
   many or all of the conference participants.  It then selectively
   forwards some of them to other participants in the conference.  In
   large conferences, it is possible that such a server might be
   receiving a large number of streams, of which only a few are intended
   to be forwarded to the other conference participants.

RTCRtpContributingSource.audioLevel assumes a different architecture as described in RFC 6465:

   "[The mixer is] responsible for combining
   the media streams that make up a conference, and generating one or
   more output streams that are delivered to recipients".  Every
   participant would hence receive, in a flat single stream, media
   originating from all the others.

   [...] The flat nature of the streams that a
   mixer would output and send to participants makes it difficult for
   users to identify the original source of what they are hearing.

Here, the audioLevel could be used by a receiving client to infer e.g. the current speaker.

So I think it is reasonable to implement them separately. RTCRtpContributingSource.audioLevel will also require some additional work in WebRTC before it can be implemented here.

Audio level is optional for contributing sources because it isn't computable by the browser if it isn't included in the header extension (the browser only gets one flat stream, so it can't tell how much of the audio came from various senders).
Synchronization sources don't have this problem, so the audio level is always computable. That being said, the initial implementation will make the field nullable and only include it if the header extension is present (like you suggest).

I will review the web-platform-tests before submitting.

voiceActivityFlag is deliberately omitted for now (though we would like to add it in the future).

Philip Jägenstedt

unread,
Nov 15, 2017, 4:17:56 AM11/15/17
to Zach Stein, Chris Harrelson, Zach Stein, Joe Medley, blink-dev
Thanks for the additional detail, Zach.

It makes sense to me to just make RTCRtpSynchronizationSource's audioLevel nullable, and if that is what you intend to ship, the spec should allow for it as well. Can you file an issue on https://github.com/w3c/webrtc-pc to make it so? Any tests where audioLevel is null would need to be tentative until such a spec change is merged.

Boris Zbarsky

unread,
Dec 8, 2017, 12:04:48 PM12/8/17
to blink-dev
On 11/8/17 4:17 PM, Zach Stein wrote:
> Compatibility Risk
> Low risk. There is consensus but it is not implemented in the other
> browsers yet.

As part of the Firefox intent-to-ship for this, I've raised several
issues[1] on the spec that probably need to get sorted out to avoid
compat problems. In particular, the nullability thing the spec does is
a bit questionable; I think it would make more sense to not put the
members into the dictionaries at all if they're not available.

-Boris

[1] <https://github.com/w3c/webrtc-pc/issues/1688>,
<https://github.com/w3c/webrtc-pc/issues/1689>,
<https://github.com/w3c/webrtc-pc/issues/1690>

Philip Jägenstedt

unread,
Dec 20, 2017, 7:50:50 AM12/20/17
to Boris Zbarsky, zst...@chromium.org, blink-dev
+Zach Stein, can you take a look at each of these issues?

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

zst...@google.com

unread,
Dec 21, 2017, 2:45:44 PM12/21/17
to blink-dev, bzba...@mit.edu
Thanks for bringing this up. It looks like RTCRtpSynchronizationSource was changed from an interface to a dictionary on 11/20, so not including audioLevel when it is not available is an option now. I have been looking in to making audioLevel always available to match the spec and hope to have an update soon.
Reply all
Reply to author
Forward
0 new messages