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

Prevent generating huge opus files when received timestamp is 0 #2328

Closed
wants to merge 4 commits into from
Closed

Prevent generating huge opus files when received timestamp is 0 #2328

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f43ce1
Fixed timestamp=0 issue
Aug 18, 2020
4da04a9
Use context.a_last_time
Aug 18, 2020
789fa97
Added debug logs
Aug 18, 2020
790edce
Removed debug logs
Aug 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions rtp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,17 @@ int janus_rtp_skew_compensate_video(janus_rtp_header *header, janus_rtp_switchin
return exit_status;
}

static gint64 janus_get_audio_time_diff(uint16_t codec_type, gint64 a_last_time) {
gint64 time_diff = janus_get_monotonic_time() - a_last_time;
int akhz = 48;
if(codec_type == 0 || codec_type == 8 || codec_type == 9)
akhz = 8; /* We're assuming 48khz here (Opus), unless it's G.711/G.722 (8khz) */
time_diff = (time_diff*akhz)/1000;
if(time_diff == 0)
time_diff = 1;
return time_diff;
}

void janus_rtp_header_update(janus_rtp_header *header, janus_rtp_switching_context *context, gboolean video, int step) {
if(header == NULL || context == NULL)
return;
Expand Down Expand Up @@ -666,13 +677,7 @@ void janus_rtp_header_update(janus_rtp_header *header, janus_rtp_switching_conte
context->a_base_seq = seq;
/* How much time since the last audio RTP packet? We compute an offset accordingly */
if(context->a_last_time > 0) {
gint64 time_diff = janus_get_monotonic_time() - context->a_last_time;
int akhz = 48;
if(header->type == 0 || header->type == 8 || header->type == 9)
akhz = 8; /* We're assuming 48khz here (Opus), unless it's G.711/G.722 (8khz) */
time_diff = (time_diff*akhz)/1000;
if(time_diff == 0)
time_diff = 1;
gint64 time_diff = janus_get_audio_time_diff(header->type, context->a_last_time);
context->a_base_ts_prev += (guint32)time_diff;
context->a_prev_ts += (guint32)time_diff;
context->a_last_ts += (guint32)time_diff;
Expand All @@ -689,7 +694,12 @@ void janus_rtp_header_update(janus_rtp_header *header, janus_rtp_switching_conte
}
/* Compute a coherent timestamp and sequence number */
context->a_prev_ts = context->a_last_ts;
context->a_last_ts = (timestamp-context->a_base_ts) + context->a_base_ts_prev;
if(timestamp >= context->a_base_ts) {
Copy link
Member

@atoppi atoppi Aug 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is not correct.
Any packet received with a TS lesser than the base TS will have its TS replaced with a value estimated on the real time (janus_get_audio_time_diff).
E.g. a SSRC change imposes a new base ts with a high value (say 4000000000). Once the TS wraps around, any new packet until 4000000000 will have its TS replaced with a new estimated one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When SSRC is changed, context->a_base_ts is set to the current timestamp. So, it will not be replaced with a new estimated timestamp. But, this is irrelevant now because we will move this code to the SIP plugin. :)

Copy link
Member

Choose a reason for hiding this comment

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

The RTP TS will eventually wrap around (keeping the same SSRC). So after a certain amount of time you'll receive a TS lesser than the last known base TS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right, thanks.

context->a_last_ts = (timestamp - context->a_base_ts) + context->a_base_ts_prev;
} else {
gint64 time_diff = janus_get_audio_time_diff(header->type, context->a_last_time);
context->a_last_ts += time_diff;
}
context->a_prev_seq = context->a_last_seq;
context->a_last_seq = (seq-context->a_base_seq)+context->a_base_seq_prev+1;
/* Update the timestamp and sequence number in the RTP packet */
Expand Down