Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Split getContributingSources into two methods, for CSRCs and SSRCs. #1133

Merged
merged 1 commit into from May 4, 2017
Merged

Split getContributingSources into two methods, for CSRCs and SSRCs. #1133

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
109 changes: 78 additions & 31 deletions webrtc.html
View file
Open in desktop
Expand Up @@ -6450,7 +6450,8 @@ <h3><dfn>RTCRtpReceiver</dfn> Interface</h3>
readonly attribute RTCDtlsTransport? rtcpTransport; // Feature at risk
static RTCRtpCapabilities getCapabilities (DOMString kind);
RTCRtpParameters getParameters ();
sequence&lt;RTCRtpContributingSource&gt; getContributingSources ();
sequence&lt;RTCRtpContributingSource&gt; getContributingSources ();
sequence&lt;RTCRtpSynchronizationSource&gt; getSynchronizationSources ();
Promise&lt;RTCStatsReport&gt; getStats();
};</pre>
<section>
Expand Down Expand Up @@ -6557,8 +6558,8 @@ <h2>Methods</h2>
<dt><dfn><code>getContributingSources</code></dfn></dt>
<dd>
<p>Returns an <code><a>RTCRtpContributingSource</a></code> for
each unique CSRC or SSRC received by this RTCRtpReceiver in the
last 10 seconds.</p>
each unique CSRC identifier received by this RTCRtpReceiver in
the last 10 seconds.</p>
<div>
<em>No parameters.</em>
</div>
Expand All @@ -6567,6 +6568,19 @@ <h2>Methods</h2>
<code>sequence&lt;RTCRtpContributingSource&gt;</code>
</div>
</dd>
<dt><dfn><code>getSynchronizationSources</code></dfn></dt>
<dd>
<p>Returns an <code><a>RTCRtpSynchronizationSource</a></code> for
each unique SSRC identifier received by this RTCRtpReceiver in
the last 10 seconds.</p>
<div>
<em>No parameters.</em>
</div>
<div>
<em>Return type:</em>
<code>sequence&lt;RTCRtpSynchronizationSource&gt;</code>
</div>
</dd>
<dt><code>getStats</code></dt>
<dd>
<p>Gathers stats for this receiver only and reports the result
Expand Down Expand Up @@ -6611,24 +6625,25 @@ <h2>Methods</h2>
</dl>
</section>
</div>
<p>The <dfn>RTCRtpContributingSource</dfn> objects contain information
about a given contributing source, including the most recent time a
<p>The <dfn>RTCRtpContributingSource</dfn> and
<dfn>RTCRtpSynchronizationSource</dfn> objects contain information about
a given contributing source (CSRC) or synchronization source (SSRC),
including the most recent time a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: weird line-break

Copy link
Contributor Author

@taylor-b taylor-b Apr 20, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this to make the diff cleaner, will rely on auto-formatting tool at some point in the future.

packet that the source contributed to was played out. The browser MUST
keep information from RTP packets received in the previous 10 seconds.
When the first audio frame contained in an RTP packet is delivered to the
<code><a>RTCRtpReceiver</a></code>'s <code><a>MediaStreamTrack</a></code>
for playout, the relevant <code><a>RTCRtpContributingSource</a></code>
objects are updated. The
<code><a>RTCRtpContributingSource</a></code> object corresponding to the
SSRC is always updated, and if the RTP packet contains CSRCs, then the
<code><a>RTCRtpContributingSource</a></code> objects corresponding to
those CSRCs are also updated.</p>
and <code><a>RTCRtpSynchronizationSource</a></code> objects are updated. The
<code><a>RTCRtpSynchronizationSource</a></code> object corresponding to the
SSRC identifier is always updated, and if the RTP packet contains CSRC
identifiers, then the <code><a>RTCRtpContributingSource</a></code>
objects corresponding to those CSRC identifiers are also updated.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSRC and CSRC numbers are independent of each other? You could have one RTP packet (ssrc:1, csrc:[1,...]) and then another one (ssrc:2, csrc:[1,...])? Does it make sense to have a reference to the most recent RTCRtpSynchronizationSource from an RTCRtpContributingSource?

Hmm actually that could be misleading because the SSRC could be updated and so a CSRC's SSRC could represent a different state and time than the CSRC. But what about a source id? Or is this not useful?

It would seem the SSRC's audioLevel would be enough (CSRC's audioLevel being a copy) if it wasn't for the reference representing a different state and time. Copy needed to have audioLevel for that state/time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could have one RTP packet (ssrc:1, csrc:[1,...]) and then another one (ssrc:2, csrc:[1,...])?

I don't think this is a scenario we need to worry about. Technically the IDs might share the same space ("CSRC identifier" is just shorthand for "SSRC identifier of the CSRC") but I can't imagine how a CSRC could also be a primary SSRC in the same session.

The intention is basically that RTCRtpContributingSource represents "information from CSRC list and RFC 6465 extension" and RTCRtpSynchronizationSource represents "information from SSRC field and RFC 6464 extension".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I wasn't thinking about CSRC and SSRC namespaces colliding but rather if different SSRCs transport the same CSRCs at different points in time. I'll update the example to:

"You could have one RTP packet (ssrc:1, csrc:[10,...]) and then another one (ssrc:2, csrc:[10,...])?"

Would the CSRC information be copies of the latest SSRC packet information that contained that CSRC?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your question now, sorry for my mixup.

Anyway: I was thinking of each RTCRtpContributingSource object as being tied to a particular RTCRtpReceiver. That seems more sensible given how the API is expected to be used.

So, if the same source is contributing to two different streams, I'd suggest having two different RTCRtpContributingSource objects, one for each stream it's contributing to.

If the application really does need to know "which stream did this source most recently contribute to?," it still can do this by comparing timestamps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that sounds good to me.

<div>
<pre class="idl">interface RTCRtpContributingSource {
readonly attribute DOMHighResTimeStamp timestamp;
readonly attribute unsigned long source;
readonly attribute byte? audioLevel;
readonly attribute boolean? voiceActivityFlag;
};</pre>
<section>
<h2>Attributes</h2>
Expand All @@ -6645,35 +6660,67 @@ <h2>Attributes</h2>
<dt><dfn><code>source</code></dfn> of type <span class=
"idlAttrType"><a>unsigned long</a></span>, readonly</dt>
<dd>
<p>The CSRC or SSRC value of the contributing source.</p>
<p>The CSRC identifier of the contributing source.</p>
</dd>
<dt><dfn><code>audioLevel</code></dfn> of type <span class=
"idlAttrType"><a>byte</a></span>, readonly , nullable</dt>
<dd>
<p>The audio level contained in the last RTP packet played from
this source. <code>audioLevel</code> will be the level value
defined in [[!RFC6465]] if the RFC 6465 header extension is
present, and otherwise <code>null</code>. RFC 6465 defines the
level as a integral value from 0 to 127 representing the audio
level in negative decibels relative to the loudest signal that
the system could possibly encode. Thus, 0 represents the loudest
signal the system could possibly encode, and 127 represents
silence.</p>
</dd>
</dl>
</section>
</div>
<div>
<pre class="idl">interface RTCRtpSynchronizationSource {
readonly attribute DOMHighResTimeStamp timestamp;
readonly attribute unsigned long source;
readonly attribute byte audioLevel;
readonly attribute boolean? voiceActivityFlag;
};</pre>
<section>
<h2>Attributes</h2>
<dl data-link-for="RTCRtpSynchronizationSource" data-dfn-for=
"RTCRtpSynchronizationSource" class="attributes">
<dt><dfn><code>timestamp</code></dfn> of type <span class=
"idlAttrType"><a>DOMHighResTimeStamp</a></span>, readonly</dt>
<dd>
<p>The timestamp of type DOMHighResTimeStamp [[!HIGHRES-TIME]],
indicating the most recent time of playout of an RTP packet
from the source. The timestamp is defined in
[[!HIGHRES-TIME]] and corresponds to a local clock.</p>
</dd>
<dt><dfn><code>source</code></dfn> of type <span class=
"idlAttrType"><a>unsigned long</a></span>, readonly</dt>
<dd>
<p>The SSRC identifier of the synchronization source.</p>
</dd>
<dt><dfn><code>audioLevel</code></dfn> of type <span class=
"idlAttrType"><a>byte</a></span>, readonly , nullable</dt>
<dd>
<p>The audio level contained in the last RTP packet played from
this source. If the source corresponds to an SSRC,
<code>audioLevel</code> will be the level value defined in
[[!RFC6464]], if the RFC 6464 header extension is present. If the
RFC 6464 extension header is not present, the browser will
compute a value for <code>audioLevel</code> as if it had come
from RFC 6464. If the source corresponds to a CSRC,
<code>audioLevel</code> will be the level value defined in
[[!RFC6465]] if the RFC 6465 header extension is present, and
otherwise <code>null</code>. RFC 6464 and 6465 define the level
as a integral value from 0 to 127 representing the audio level in
negative decibels relative to the loudest signal that the system
could possibly encode. Thus, 0 represents the loudest signal the
system could possibly encode, and 127 represents silence.</p>
this source. <code>audioLevel</code> will be the level value
defined in [[!RFC6464]], if the RFC 6464 header extension is
present. If the RFC 6464 extension header is not present, the
browser will compute a value for <code>audioLevel</code> as if it
had come from RFC 6464.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a significant cost to making computing audioLevel non-optional per RTP packet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully not, since it's already computed for stats. Though I'm not changing that in this PR, just moving it around.

</dd>
<dt><dfn><code>voiceActivityFlag</code></dfn> of type <span class=
"idlAttrType"><a>boolean</a></span>, readonly , nullable</dt>
<dd>
<p>Whether the last RTP packet received from this source contains
voice activity (true) or not (false). Since the "V" bit is
supported in [[!RFC6464]] but not [[!RFC6465]], the
<var>voiceActivityFlag</var> attribute will only be set for RTP
packets received from peers enabling the client-mixer header
extension with the "vad" extension set to "on".</p>
<p>Whether the last RTP packet played from this source contains
voice activity (true) or not (false). If the RFC 6464 extension
header was not present, or if the peer has signaled that it is
not using the V bit by setting the "vad" extension attribute to
"off", as described in [[!RFC6464]], Section 4,
<code>voiceActivityFlag</code> will be <code>null</code>.</p>
</dd>
</dl>
</section>
Expand Down