Add rtpTimestamp to RTCRtpContributingSource #2178
Conversation
webrtc.html
Outdated
| "idlMemberType">unsigned long</span> | ||
| <dd> | ||
| <p>The RTP timestamp of the latest played out media that arrived | ||
| in an RTP packet originating from this source.</p> |
There was a problem hiding this comment.
Can we refer this to the variable above, as "RTP timestamp of the media played out at "timestamp", to make it clear that the two timestamps are required to be from the same packet?
There was a problem hiding this comment.
I agree with @alvestrand that this needs to correlate with the same packet, not the "latest".
See also #2172 (comment) wrt this API—at least in Firefox—returns data no sooner than at time of playout.
There was a problem hiding this comment.
Yes, that makes sense. I think this change should be coordinated with #2172 .
| @@ -7295,6 +7295,7 @@ <h2>Methods</h2> | |||
| required DOMHighResTimeStamp timestamp; | |||
| required unsigned long source; | |||
| double audioLevel; | |||
| unsigned long rtpTimestamp; | |||
There was a problem hiding this comment.
Note that RTCRtpSynchronizationSource is derived from RTCRtpContributingSource. Did you mean to add rtpTimestamp to both? Just making sure it's not done by accident.
There was a problem hiding this comment.
I don't know the details of this well enough to tell which is the correct place to put it. To me it makes sense put it where you have the other timestamp in contributing source though.
There was a problem hiding this comment.
I think it belongs here. Both the SSRC and CSRC dictionary contains information from a particular RTP packet. This is more information about that RTP packet, so it can be added to both. (For the intended use case it would be enough to have it in the SSRC dictionary, but it doesn't hurt to have it here too.)
|
@jan-ivar to get implementers to comment |
|
@na-g can you comment on how this fits with our implementation? My concern is that this API is geared toward being "the audio bar API" in many respects, principally in that, at the time you poll the method, you get data related to playout right now. This means you won't get rtpTimestamp until playout time. Is this sufficient for the use-cases suggested, or does this require a new API? |
webrtc.html
Outdated
| <dt><dfn data-idl><code>rtpTimestamp</code></dfn> of type | ||
| <span class="idlMemberType">unsigned long</span></dt> | ||
| <dd> | ||
| <p>The RTP timestamp of the latest played out media that arrived |
There was a problem hiding this comment.
This RTP timestamp will be from a remote clock, right? Do we need language here defining this similar to remoteTimestamp?
There was a problem hiding this comment.
Let's reference [RFC3550] section 5.1
|
@jan-ivar @aboba @alvestrand Please have a look at the updated PR. |
Fixes #2177
Preview | Diff