audio: add rtp sequence number on incoming frames #154
Conversation
index.bs
Outdated
| @@ -311,6 +311,7 @@ dictionary RTCEncodedAudioFrameMetadata { | |||
| long synchronizationSource; | |||
| octet payloadType; | |||
| sequence<long> contributingSources; | |||
| short? sequenceNumber; // RTP sequence number on incoming frames. | |||
There was a problem hiding this comment.
Why is it nullable?
From an editorial perspective, we should probably have a section describing each metadata.
Can we add this section and add a description for sequenceNumber, like a link to RFC 3550 corresponding section.
There was a problem hiding this comment.
I'm avoiding answering the question how/if this is supposed to work on outbound packets. The encoder might not be concerned with the RTP sequence number (which starts at a random offset) yet. On the receiving end the sequence number is always known (but I think there will be gaps when receiving padding-only probes)
There was a problem hiding this comment.
Given sequenceNumber is not required, outgoing frames would have their sequenceNumber field be undefined.
It would be nice though to set sequenceNumber as required for incoming frames.
This seems to be another hint at exposing different APIs for outgoing frames vs. incoming frames.
There was a problem hiding this comment.
lets use #155 for the format of how to describe fields
There was a problem hiding this comment.
I removed the ? since dictionary members are optional by default.
We probably need to add some required in other places (see also #138)
There was a problem hiding this comment.
I agree that exposing this is information useful and necessary.
However I think we would do better to expose the index (https://www.rfc-editor.org/rfc/rfc3711#section-3.3.1).
In order to decrypt the packet we've already calculated the index correctly (i.e. factored in the ROC) - there is no reason to make the javascript developer do it again here.
The benefit is that for any sensible length calls you can assume a monotonically increasing index making packet drops and out-of-order packets simpler to detect and manage.
There was a problem hiding this comment.
I checked and at least libwebrtc rarely cares about the extended sequence number and does not expose it at a level needed for wiring it up here.
fippo
force-pushed
the
incoming-audio-sequencenumber
branch
from
80906e8
to
73472cd
Compare
where it is easily available.
fippo
force-pushed
the
incoming-audio-sequencenumber
branch
from
73472cd
to
b4916b8
Compare
|
@youennf , @alvestrand , are there any extra changes that you believe are required prior to merging this? Having access to sequence numbers enables applications to build custom audio receive pipelines that operate on top of RTP. |
|
This is the PR where we have the discussion on the short sequence number vs the long sequence number. |
|
most codebases I've seen only expose the short sequence number, e.g. This, as well as the implications of sequence number arithmetic when comparing are well understood. Not pretty but... |
|
Ok, then perhaps the best compromise is to expose the short sequence number but include javascript reference code for converting to the long sequence number in the documentation, so fewer people get caught out. |
|
Added a note about serial number arithmetic (which got me pondering why it is not called sequence number arithmetic :-)) |
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short - remove unused contributing_sources vector on audio frame BUG=TBD Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short - remove unused contributing_sources vector on audio frame BUG=TBD Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short - remove unused contributing_sources vector on audio frame BUG=TBD Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf
SHA: 5c7ab84 Reason: push, by alvestrand Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short - remove unused contributing_sources vector on audio frame BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3925401 Reviewed-by: Harald Alvestrand <hta@chromium.org> Commit-Queue: Philipp Hancke <phancke@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1100372}
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3925401 Reviewed-by: Harald Alvestrand <hta@chromium.org> Commit-Queue: Philipp Hancke <phancke@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1100372}
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3925401 Reviewed-by: Harald Alvestrand <hta@chromium.org> Commit-Queue: Philipp Hancke <phancke@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1100372}
…ber to incoming audio metadata, a=testonly Automatic update from web-platform-tests insertable streams: add rtp sequence number to incoming audio metadata spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3925401 Reviewed-by: Harald Alvestrand <hta@chromium.org> Commit-Queue: Philipp Hancke <phancke@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1100372} -- wpt-commits: 7815492dc65bf1f07447668ab144a655cba44351 wpt-pr: 38057
…ber to incoming audio metadata, a=testonly Automatic update from web-platform-tests insertable streams: add rtp sequence number to incoming audio metadata spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3925401 Reviewed-by: Harald Alvestrand <hta@chromium.org> Commit-Queue: Philipp Hancke <phancke@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1100372} -- wpt-commits: 7815492dc65bf1f07447668ab144a655cba44351 wpt-pr: 38057
spec change w3c/webrtc-encoded-transform#154 drive-by: - fix idl of payloadType to be octet instead of short BUG=chromium:1411703 Change-Id: Ic8dc8dcebb035a8f79823a8ca70e02ac1d6788cf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3925401 Reviewed-by: Harald Alvestrand <hta@chromium.org> Commit-Queue: Philipp Hancke <phancke@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1100372}
where it is easily available.
This is useful in cases where one decodes audio using other means and just wants to grab the encoded frame (similar to what @alvestrand describes here as "alternative consumers")
Preview | Diff