-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
Support for abs-send-time RTP extension #2721
Changes from 4 commits
9b2453d
6798684
f5712f4
ea92740
8db97b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4484,6 +4484,21 @@ static gboolean janus_ice_outgoing_traffic_handle(janus_ice_handle *handle, janu | |
/* ... but only if this isn't a retransmission (for those we already set it before) */ | ||
header->ssrc = htonl(video ? stream->video_ssrc : stream->audio_ssrc); | ||
} | ||
/* Set the abs-send-time value, if needed */ | ||
if(video && stream->abs_send_time_ext_id > 0) { | ||
struct timeval tv; | ||
gettimeofday(&tv, NULL); | ||
uint64_t s = tv.tv_sec + 2208988800u; | ||
uint32_t u = tv.tv_usec; | ||
uint32_t f = (u << 12) + (u << 8) - ((u * 3650) >> 6); | ||
uint64_t ntp_ts = (s << 32) + f; | ||
uint64_t abs24 = (ntp_ts >> 14) & 0x00ffffff; | ||
uint32_t abs_ts = abs24; | ||
if(janus_rtp_header_extension_set_abs_send_time(pkt->data, pkt->length, | ||
stream->abs_send_time_ext_id, abs_ts) < 0) { | ||
JANUS_LOG(LOG_ERR, "[%"SCNu64"] Error setting abs-send-time value...\n", handle->handle_id); | ||
} | ||
} | ||
/* Set the transport-wide sequence number, if needed */ | ||
if(video && stream->transport_wide_cc_ext_id > 0) { | ||
stream->transport_wide_cc_out_seq_num++; | ||
|
@@ -4774,7 +4789,8 @@ void janus_ice_relay_rtp(janus_ice_handle *handle, janus_plugin_rtp *packet) { | |
int origext = header->extension; | ||
header->extension = 0; | ||
/* Add core and plugin extensions, if any */ | ||
if((packet->video && handle->stream->transport_wide_cc_ext_id > 0) || handle->stream->mid_ext_id > 0 || | ||
if(handle->stream->mid_ext_id > 0 || (packet->video && handle->stream->abs_send_time_ext_id > 0) || | ||
(packet->video && handle->stream->transport_wide_cc_ext_id > 0) || | ||
(!packet->video && packet->extensions.audio_level != -1 && handle->stream->audiolevel_ext_id > 0) || | ||
(packet->video && packet->extensions.video_rotation != -1 && handle->stream->videoorientation_ext_id > 0)) { | ||
header->extension = 1; | ||
|
@@ -4784,6 +4800,14 @@ void janus_ice_relay_rtp(janus_ice_handle *handle, janus_plugin_rtp *packet) { | |
extheader->length = 0; | ||
/* Iterate on all extensions we need */ | ||
char *index = extensions + 4; | ||
/* Check if we need to add the abs-send-time extension */ | ||
if(packet->video && handle->stream->abs_send_time_ext_id > 0) { | ||
*index = (handle->stream->abs_send_time_ext_id << 4) + 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, the length in the protocol field is always There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case we're writing the first byte of the extension, so the ID of the extension and the length field:
|
||
/* We'll actually set the value later, when sending the packet */ | ||
memset(index+1, 0, 3); | ||
index += 4; | ||
extlen += 4; | ||
} | ||
/* Check if we need to add the transport-wide CC extension */ | ||
if(packet->video && handle->stream->transport_wide_cc_ext_id > 0) { | ||
*index = (handle->stream->transport_wide_cc_ext_id << 4) + 1; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo the documentation on webrtc is misleading here. It says something about the NTP timestamp but this is not what libwebrtc does:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/rtp_rtcp/source/rtp_sender_egress.cc;l=226;drc=6a0a55907bf29c7d6bfedb0f8aa4405ed3e2dcf5
It takes the current monotonic clock and then does some shifting with it:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h;l=42;drc=1f5325b50d27ed221d4a26530908c4b68b834507
i'd strongly recommend the same. Mistakes in this are subtle but may lead to overshooting the available bandwidth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fixed now 👍