From 685d4c485c8852752105bd4eb35758b7b38b51d2 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Sun, 17 Nov 2019 04:51:40 +0100 Subject: [PATCH 1/7] Dynamically update NACK queue size depending on RTT --- README.md | 4 ++-- conf/janus.jcfg.sample.in | 6 +++--- ice.c | 34 +++++++++++++++++++++------------- ice.h | 14 ++++++++------ janus.1 | 4 ++-- janus.c | 34 +++++++++++++++------------------- janus.ggo | 2 +- 7 files changed, 52 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index bfc846311a..197f9e8375 100644 --- a/README.md +++ b/README.md @@ -282,8 +282,8 @@ or on the command line: (default=off) -R, --rfc-4588 Whether to enable RFC4588 retransmissions support or not (default=off) - -q, --max-nack-queue=number Maximum size of the NACK queue (in ms) per user - for retransmissions + -q, --min-nack-queue=number Minimum size of the NACK queue (in ms) per user + for retransmissions, no matter the RTT -t, --no-media-timer=number Time (in s) that should pass with no media (audio or video) being received before Janus notifies you about this diff --git a/conf/janus.jcfg.sample.in b/conf/janus.jcfg.sample.in index ff396054e1..40685009cf 100644 --- a/conf/janus.jcfg.sample.in +++ b/conf/janus.jcfg.sample.in @@ -131,8 +131,8 @@ certificates: { # Media-related stuff: you can configure whether if you want # to enable IPv6 support, if RFC4588 support for retransmissions -# should be negotiated or not (off by default), the maximum size -# of the NACK queue (in milliseconds, defaults to 500ms) for retransmissions, the +# should be negotiated or not (off by default), the minimum size of the +# NACK queue (in ms, defaults to 200ms) for retransmissions no matter the RTT, the # range of ports to use for RTP and RTCP (by default, no range is envisaged), the # starting MTU for DTLS (1200 by default, it adapts automatically), # how much time, in seconds, should pass with no media (audio or @@ -148,7 +148,7 @@ certificates: { # you should pick a reasonable trade-off (usually 2*max expected RTT). media: { #ipv6 = true - #max_nack_queue = 500 + #min_nack_queue = 500 #rfc_4588 = true #rtp_port_range = "20000-40000" #dtls_mtu = 1200 diff --git a/ice.c b/ice.c index 1c1beb4d6a..0a0642c69c 100644 --- a/ice.c +++ b/ice.c @@ -468,29 +468,36 @@ static void janus_ice_free_queued_packet(janus_ice_queued_packet *pkt) { g_free(pkt); } -/* Maximum value, in milliseconds, for the NACK queue/retransmissions (default=500ms) */ -#define DEFAULT_MAX_NACK_QUEUE 500 +/* Minimum value, in milliseconds, for the NACK queue/retransmissions (default=200ms) */ +#define DEFAULT_MIN_NACK_QUEUE 200 /* Maximum ignore count after retransmission (200ms) */ #define MAX_NACK_IGNORE 200000 -static uint max_nack_queue = DEFAULT_MAX_NACK_QUEUE; -void janus_set_max_nack_queue(uint mnq) { - max_nack_queue = mnq; - if(max_nack_queue == 0) +static uint min_nack_queue = DEFAULT_MIN_NACK_QUEUE; +void janus_set_min_nack_queue(uint mnq) { + min_nack_queue = mnq; + if(min_nack_queue == 0) JANUS_LOG(LOG_VERB, "Disabling NACK queue\n"); else - JANUS_LOG(LOG_VERB, "Setting max NACK queue to %dms\n", max_nack_queue); + JANUS_LOG(LOG_VERB, "Setting min NACK queue to %dms\n", min_nack_queue); } -uint janus_get_max_nack_queue(void) { - return max_nack_queue; +uint janus_get_min_nack_queue(void) { + return min_nack_queue; } /* Helper to clean old NACK packets in the buffer when they exceed the queue time limit */ static void janus_cleanup_nack_buffer(gint64 now, janus_ice_stream *stream, gboolean audio, gboolean video) { if(stream && stream->component) { + /* Check the current RTT, to see if we need to update the size of the queue: we take + * the highest RTT (audio or video), double it, and add 100ms just to be conservative */ + uint32_t audio_rtt = janus_rtcp_context_get_rtt(stream->audio_rtcp_ctx), + video_rtt = janus_rtcp_context_get_rtt(stream->video_rtcp_ctx[0]); + stream->nack_queue_ms = 2*(audio_rtt > video_rtt ? audio_rtt : video_rtt) + 100; + if(stream->nack_queue_ms < min_nack_queue) + stream->nack_queue_ms = min_nack_queue; janus_ice_component *component = stream->component; if(audio && component->audio_retransmit_buffer) { janus_rtp_packet *p = (janus_rtp_packet *)g_queue_peek_head(component->audio_retransmit_buffer); - while(p && (!now || (now - p->created >= (gint64)max_nack_queue*1000))) { + while(p && (!now || (now - p->created >= (gint64)stream->nack_queue_ms*1000))) { /* Packet is too old, get rid of it */ g_queue_pop_head(component->audio_retransmit_buffer); /* Remove from hashtable too */ @@ -504,7 +511,7 @@ static void janus_cleanup_nack_buffer(gint64 now, janus_ice_stream *stream, gboo } if(video && component->video_retransmit_buffer) { janus_rtp_packet *p = (janus_rtp_packet *)g_queue_peek_head(component->video_retransmit_buffer); - while(p && (!now || (now - p->created >= (gint64)max_nack_queue*1000))) { + while(p && (!now || (now - p->created >= (gint64)stream->nack_queue_ms*1000))) { /* Packet is too old, get rid of it */ g_queue_pop_head(component->video_retransmit_buffer); /* Remove from hashtable too */ @@ -3353,6 +3360,7 @@ int janus_ice_setup_local(janus_ice_handle *handle, int offer, int audio, int vi stream->audio_payload_type = -1; stream->video_payload_type = -1; stream->video_rtx_payload_type = -1; + stream->nack_queue_ms = min_nack_queue; /* FIXME By default, if we're being called we're DTLS clients, but this may be changed by ICE... */ stream->dtls_role = offer ? JANUS_DTLS_ROLE_CLIENT : JANUS_DTLS_ROLE_ACTPASS; if(audio) { @@ -4114,7 +4122,7 @@ static gboolean janus_ice_outgoing_traffic_handle(janus_ice_handle *handle, janu } /* Before encrypting, check if we need to copy the unencrypted payload (e.g., for rtx/90000) */ janus_rtp_packet *p = NULL; - if(max_nack_queue > 0 && !pkt->retransmission && pkt->type == JANUS_ICE_PACKET_VIDEO && component->do_video_nacks && + if(stream->nack_queue_ms > 0 && !pkt->retransmission && pkt->type == JANUS_ICE_PACKET_VIDEO && component->do_video_nacks && janus_flags_is_set(&handle->webrtc_flags, JANUS_ICE_HANDLE_WEBRTC_RFC4588_RTX)) { /* Save the packet for retransmissions that may be needed later: start by * making room for two more bytes to store the original sequence number */ @@ -4221,7 +4229,7 @@ static gboolean janus_ice_outgoing_traffic_handle(janus_ice_handle *handle, janu if(rtcp_ctx) g_atomic_int_inc(&rtcp_ctx->sent_packets_since_last_rr); } - if(max_nack_queue > 0 && !pkt->retransmission) { + if(stream->nack_queue_ms > 0 && !pkt->retransmission) { /* Save the packet for retransmissions that may be needed later */ if((pkt->type == JANUS_ICE_PACKET_AUDIO && !component->do_audio_nacks) || (pkt->type == JANUS_ICE_PACKET_VIDEO && !component->do_video_nacks)) { diff --git a/ice.h b/ice.h index 74a35353ac..8f156f5b4e 100644 --- a/ice.h +++ b/ice.h @@ -122,12 +122,12 @@ gboolean janus_ice_is_full_trickle_enabled(void); /*! \brief Method to check whether IPv6 candidates are enabled/supported or not (still WIP) * @returns true if IPv6 candidates are enabled/supported, false otherwise */ gboolean janus_ice_is_ipv6_enabled(void); -/*! \brief Method to modify the max NACK value (i.e., the number of packets per handle to store for retransmissions) - * @param[in] mnq The new max NACK value */ -void janus_set_max_nack_queue(uint mnq); -/*! \brief Method to get the current max NACK value (i.e., the number of packets per handle to store for retransmissions) - * @returns The current max NACK value */ -uint janus_get_max_nack_queue(void); +/*! \brief Method to modify the min NACK value (i.e., the minimum time window of packets per handle to store for retransmissions) + * @param[in] mnq The new min NACK value */ +void janus_set_min_nack_queue(uint mnq); +/*! \brief Method to get the current min NACK value (i.e., the minimum time window of packets per handle to store for retransmissions) + * @returns The current min NACK value */ +uint janus_get_min_nack_queue(void); /*! \brief Method to modify the no-media event timer (i.e., the number of seconds where no media arrives before Janus notifies this) * @param[in] timer The new timer value, in seconds */ void janus_set_no_media_timer(uint timer); @@ -392,6 +392,8 @@ struct janus_ice_stream { janus_rtcp_context *audio_rtcp_ctx; /*! \brief RTCP context(s) for the video stream (may be simulcasting) */ janus_rtcp_context *video_rtcp_ctx[3]; + /*! \brief Size of the NACK queue (in ms), dynamically updated per the RTT */ + uint nack_queue_ms; /*! \brief Map(s) of the NACKed packets (to track retransmissions and avoid duplicates) */ GHashTable *rtx_nacked[3]; /*! \brief First received audio NTP timestamp */ diff --git a/janus.1 b/janus.1 index 062d3c872e..1b8c524b43 100644 --- a/janus.1 +++ b/janus.1 @@ -83,8 +83,8 @@ Whether to enable ICE-TCP or not (warning: only works with ICE Lite) (default=of .BR \-R ", " \-\-rfc-4588 Whether to enable RFC4588 retransmissions support or not (default=off) .TP -.BR \-q ", " \-\-max-nack-queue=\fInumber\fR -Maximum size of the NACK queue per user for retransmissions +.BR \-q ", " \-\-min-nack-queue=\fInumber\fR +Minimum size of the NACK queue (in ms) per user for retransmissions, no matter the RTT .TP .BR \-t ", " \-\-no-media-timer=\fInumber\fR Time (in s) that should pass with no media (audio or video) being received before Janus notifies you about this diff --git a/janus.c b/janus.c index 54e3e68b87..2da88be18b 100644 --- a/janus.c +++ b/janus.c @@ -129,7 +129,7 @@ static struct janus_json_parameter colors_parameters[] = { {"colors", JANUS_JSON_BOOL, JANUS_JSON_PARAM_REQUIRED} }; static struct janus_json_parameter mnq_parameters[] = { - {"max_nack_queue", JSON_INTEGER, JANUS_JSON_PARAM_REQUIRED | JANUS_JSON_PARAM_POSITIVE} + {"min_nack_queue", JSON_INTEGER, JANUS_JSON_PARAM_REQUIRED | JANUS_JSON_PARAM_POSITIVE} }; static struct janus_json_parameter nmt_parameters[] = { {"no_media_timer", JSON_INTEGER, JANUS_JSON_PARAM_REQUIRED | JANUS_JSON_PARAM_POSITIVE} @@ -290,6 +290,7 @@ static json_t *janus_info(const char *transaction) { json_object_set_new(info, "ice-tcp", janus_ice_is_ice_tcp_enabled() ? json_true() : json_false()); json_object_set_new(info, "full-trickle", janus_ice_is_full_trickle_enabled() ? json_true() : json_false()); json_object_set_new(info, "rfc-4588", janus_is_rfc4588_enabled() ? json_true() : json_false()); + json_object_set_new(info, "min-nack-queue", json_integer(janus_get_min_nack_queue())); json_object_set_new(info, "twcc-period", json_integer(janus_get_twcc_period())); if(janus_ice_get_stun_server() != NULL) { char server[255]; @@ -1781,7 +1782,7 @@ int janus_process_incoming_admin_request(janus_request *request) { json_object_set_new(status, "locking_debug", lock_debug ? json_true() : json_false()); json_object_set_new(status, "refcount_debug", refcount_debug ? json_true() : json_false()); json_object_set_new(status, "libnice_debug", janus_ice_is_ice_debugging_enabled() ? json_true() : json_false()); - json_object_set_new(status, "max_nack_queue", json_integer(janus_get_max_nack_queue())); + json_object_set_new(status, "min_nack_queue", json_integer(janus_get_min_nack_queue())); json_object_set_new(status, "no_media_timer", json_integer(janus_get_no_media_timer())); json_object_set_new(status, "slowlink_threshold", json_integer(janus_get_slowlink_threshold())); json_object_set_new(reply, "status", status); @@ -1927,8 +1928,8 @@ int janus_process_incoming_admin_request(janus_request *request) { /* Send the success reply */ ret = janus_process_success(request, reply); goto jsondone; - } else if(!strcasecmp(message_text, "set_max_nack_queue")) { - /* Change the current value for the max NACK queue */ + } else if(!strcasecmp(message_text, "set_min_nack_queue")) { + /* Change the current value for the min NACK queue */ JANUS_VALIDATE_JSON_OBJECT(root, mnq_parameters, error_code, error_cause, FALSE, JANUS_ERROR_MISSING_MANDATORY_ELEMENT, JANUS_ERROR_INVALID_ELEMENT_TYPE); @@ -1936,16 +1937,12 @@ int janus_process_incoming_admin_request(janus_request *request) { ret = janus_process_error_string(request, session_id, transaction_text, error_code, error_cause); goto jsondone; } - json_t *mnq = json_object_get(root, "max_nack_queue"); + json_t *mnq = json_object_get(root, "min_nack_queue"); int mnq_num = json_integer_value(mnq); - if(mnq_num < 0 || (mnq_num > 0 && mnq_num < 200)) { - ret = janus_process_error(request, session_id, transaction_text, JANUS_ERROR_INVALID_ELEMENT_TYPE, "Invalid element type (max_nack_queue, if provided, should be greater than 200)"); - goto jsondone; - } - janus_set_max_nack_queue(mnq_num); + janus_set_min_nack_queue(mnq_num); /* Prepare JSON reply */ json_t *reply = janus_create_message("success", 0, transaction_text); - json_object_set_new(reply, "max_nack_queue", json_integer(janus_get_max_nack_queue())); + json_object_set_new(reply, "min_nack_queue", json_integer(janus_get_min_nack_queue())); /* Send the success reply */ ret = janus_process_success(request, reply); goto jsondone; @@ -2706,6 +2703,7 @@ json_t *janus_admin_stream_summary(janus_ice_stream *stream) { if(stream->transport_wide_cc_ext_id > 0) json_object_set_new(bwe, "twcc-ext-id", json_integer(stream->transport_wide_cc_ext_id)); json_object_set_new(s, "bwe", bwe); + json_object_set_new(s, "nack-queue-ms", json_integer(stream->nack_queue_ms)); json_t *components = json_array(); if(stream->component) { json_t *c = janus_admin_component_summary(stream->component); @@ -3925,10 +3923,10 @@ gint main(int argc, char *argv[]) if(args_info.ipv6_candidates_given) { janus_config_add(config, config_media, janus_config_item_create("ipv6", "true")); } - if(args_info.max_nack_queue_given) { + if(args_info.min_nack_queue_given) { char mnq[20]; - g_snprintf(mnq, 20, "%d", args_info.max_nack_queue_arg); - janus_config_add(config, config_media, janus_config_item_create("max_nack_queue", mnq)); + g_snprintf(mnq, 20, "%d", args_info.min_nack_queue_arg); + janus_config_add(config, config_media, janus_config_item_create("min_nack_queue", mnq)); } if(args_info.no_media_timer_given) { char nmt[20]; @@ -4299,15 +4297,13 @@ gint main(int argc, char *argv[]) } } /* NACK related stuff */ - item = janus_config_get(config, config_media, janus_config_type_item, "max_nack_queue"); + item = janus_config_get(config, config_media, janus_config_type_item, "min_nack_queue"); if(item && item->value) { int mnq = atoi(item->value); if(mnq < 0) { - JANUS_LOG(LOG_WARN, "Ignoring max_nack_queue value as it's not a positive integer\n"); - } else if(mnq > 0 && mnq < 200) { - JANUS_LOG(LOG_WARN, "Ignoring max_nack_queue value as it's less than 200\n"); + JANUS_LOG(LOG_WARN, "Ignoring min_nack_queue value as it's not a positive integer\n"); } else { - janus_set_max_nack_queue(mnq); + janus_set_min_nack_queue(mnq); } } /* no-media timer */ diff --git a/janus.ggo b/janus.ggo index 4e8243658f..3b2e2f2259 100644 --- a/janus.ggo +++ b/janus.ggo @@ -21,7 +21,7 @@ option "full-trickle" f "Do full-trickle instead of half-trickle" flag off option "ice-lite" I "Whether to enable the ICE Lite mode or not" flag off option "ice-tcp" T "Whether to enable ICE-TCP or not (warning: only works with ICE Lite)" flag off option "rfc-4588" R "Whether to enable RFC4588 retransmissions support or not" flag off -option "max-nack-queue" q "Maximum size of the NACK queue (in ms) per user for retransmissions" int typestr="number" optional +option "min-nack-queue" q "Minimum size of the NACK queue (in ms) per user for retransmissions, no matter the RTT" int typestr="number" optional option "no-media-timer" t "Time (in s) that should pass with no media (audio or video) being received before Janus notifies you about this" int typestr="number" optional option "slowlink-threshold" W "Number of lost packets (per s) that should trigger a 'slowlink' Janus API event to users" int typestr="number" optional option "rtp-port-range" r "Port range to use for RTP/RTCP" string typestr="min-max" optional From 0b5523c4aa203c79af3f908dd5276701c6b35dfb Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Sun, 17 Nov 2019 07:11:23 +0100 Subject: [PATCH 2/7] Don't double RTT --- ice.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ice.c b/ice.c index 0a0642c69c..79f976a9e1 100644 --- a/ice.c +++ b/ice.c @@ -488,10 +488,10 @@ uint janus_get_min_nack_queue(void) { static void janus_cleanup_nack_buffer(gint64 now, janus_ice_stream *stream, gboolean audio, gboolean video) { if(stream && stream->component) { /* Check the current RTT, to see if we need to update the size of the queue: we take - * the highest RTT (audio or video), double it, and add 100ms just to be conservative */ + * the highest RTT (audio or video) and add 100ms just to be conservative */ uint32_t audio_rtt = janus_rtcp_context_get_rtt(stream->audio_rtcp_ctx), video_rtt = janus_rtcp_context_get_rtt(stream->video_rtcp_ctx[0]); - stream->nack_queue_ms = 2*(audio_rtt > video_rtt ? audio_rtt : video_rtt) + 100; + stream->nack_queue_ms = (audio_rtt > video_rtt ? audio_rtt : video_rtt) + 100; if(stream->nack_queue_ms < min_nack_queue) stream->nack_queue_ms = min_nack_queue; janus_ice_component *component = stream->component; From 88b0bcb255a1bee58684d938f4dd72778d086e86 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Mon, 18 Nov 2019 03:53:59 +0100 Subject: [PATCH 3/7] Updated docs --- mainpage.dox | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mainpage.dox b/mainpage.dox index 31badf6264..49cbf12b44 100644 --- a/mainpage.dox +++ b/mainpage.dox @@ -2264,7 +2264,7 @@ const token = getJanusToken('janus', ['janus.plugin.videoroom']), * the reference counters in Janus on the fly (useful if you're experiencing * memory leaks in the Janus structures and want to investigate them); * - \c set_libnice_debug: selectively enable/disable libnice debugging; - * - \c set_max_nack_queue: change the value of the max NACK queue window; + * - \c set_min_nack_queue: change the value of the min NACK queue window; * - \c set_no_media_timer: change the value of the no-media timer property; * - \c set_slowlink_threshold: change the value of the slowlink-threshold property. * From 5f6744a6d206d4036d1490b2df1b983863fd198f Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Tue, 19 Nov 2019 09:19:53 +0100 Subject: [PATCH 4/7] Added a cap to the NACK queue size, and only updated if it's growing --- ice.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/ice.c b/ice.c index 79f976a9e1..2d747f9002 100644 --- a/ice.c +++ b/ice.c @@ -468,14 +468,15 @@ static void janus_ice_free_queued_packet(janus_ice_queued_packet *pkt) { g_free(pkt); } -/* Minimum value, in milliseconds, for the NACK queue/retransmissions (default=200ms) */ +/* Minimum and maximum value, in milliseconds, for the NACK queue/retransmissions (default=200ms/1000ms) */ #define DEFAULT_MIN_NACK_QUEUE 200 +#define DEFAULT_MAX_NACK_QUEUE 1000 /* Maximum ignore count after retransmission (200ms) */ #define MAX_NACK_IGNORE 200000 static uint min_nack_queue = DEFAULT_MIN_NACK_QUEUE; void janus_set_min_nack_queue(uint mnq) { - min_nack_queue = mnq; + min_nack_queue = mnq < DEFAULT_MAX_NACK_QUEUE ? mnq : DEFAULT_MAX_NACK_QUEUE; if(min_nack_queue == 0) JANUS_LOG(LOG_VERB, "Disabling NACK queue\n"); else @@ -491,9 +492,13 @@ static void janus_cleanup_nack_buffer(gint64 now, janus_ice_stream *stream, gboo * the highest RTT (audio or video) and add 100ms just to be conservative */ uint32_t audio_rtt = janus_rtcp_context_get_rtt(stream->audio_rtcp_ctx), video_rtt = janus_rtcp_context_get_rtt(stream->video_rtcp_ctx[0]); - stream->nack_queue_ms = (audio_rtt > video_rtt ? audio_rtt : video_rtt) + 100; - if(stream->nack_queue_ms < min_nack_queue) - stream->nack_queue_ms = min_nack_queue; + uint nack_queue_ms = (audio_rtt > video_rtt ? audio_rtt : video_rtt) + 100; + if(nack_queue_ms > DEFAULT_MAX_NACK_QUEUE) + nack_queue_ms = DEFAULT_MAX_NACK_QUEUE; + else if(nack_queue_ms < min_nack_queue) + nack_queue_ms = min_nack_queue; + if(stream->nack_queue_ms < nack_queue_ms) + stream->nack_queue_ms = nack_queue_ms; janus_ice_component *component = stream->component; if(audio && component->audio_retransmit_buffer) { janus_rtp_packet *p = (janus_rtp_packet *)g_queue_peek_head(component->audio_retransmit_buffer); From 09309c769de1cdc7be8c28dc5616522392c0219f Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Wed, 27 Nov 2019 12:07:20 +0100 Subject: [PATCH 5/7] Renamed command line shortcut to avoid ambiguity with old setting --- README.md | 2 +- janus.1 | 2 +- janus.ggo | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 197f9e8375..8858a4e5b4 100644 --- a/README.md +++ b/README.md @@ -282,7 +282,7 @@ or on the command line: (default=off) -R, --rfc-4588 Whether to enable RFC4588 retransmissions support or not (default=off) - -q, --min-nack-queue=number Minimum size of the NACK queue (in ms) per user + -Q, --min-nack-queue=number Minimum size of the NACK queue (in ms) per user for retransmissions, no matter the RTT -t, --no-media-timer=number Time (in s) that should pass with no media (audio or video) being received before Janus diff --git a/janus.1 b/janus.1 index 1b8c524b43..f548d3b24c 100644 --- a/janus.1 +++ b/janus.1 @@ -83,7 +83,7 @@ Whether to enable ICE-TCP or not (warning: only works with ICE Lite) (default=of .BR \-R ", " \-\-rfc-4588 Whether to enable RFC4588 retransmissions support or not (default=off) .TP -.BR \-q ", " \-\-min-nack-queue=\fInumber\fR +.BR \-Q ", " \-\-min-nack-queue=\fInumber\fR Minimum size of the NACK queue (in ms) per user for retransmissions, no matter the RTT .TP .BR \-t ", " \-\-no-media-timer=\fInumber\fR diff --git a/janus.ggo b/janus.ggo index 3b2e2f2259..29fc890a37 100644 --- a/janus.ggo +++ b/janus.ggo @@ -21,7 +21,7 @@ option "full-trickle" f "Do full-trickle instead of half-trickle" flag off option "ice-lite" I "Whether to enable the ICE Lite mode or not" flag off option "ice-tcp" T "Whether to enable ICE-TCP or not (warning: only works with ICE Lite)" flag off option "rfc-4588" R "Whether to enable RFC4588 retransmissions support or not" flag off -option "min-nack-queue" q "Minimum size of the NACK queue (in ms) per user for retransmissions, no matter the RTT" int typestr="number" optional +option "min-nack-queue" Q "Minimum size of the NACK queue (in ms) per user for retransmissions, no matter the RTT" int typestr="number" optional option "no-media-timer" t "Time (in s) that should pass with no media (audio or video) being received before Janus notifies you about this" int typestr="number" optional option "slowlink-threshold" W "Number of lost packets (per s) that should trigger a 'slowlink' Janus API event to users" int typestr="number" optional option "rtp-port-range" r "Port range to use for RTP/RTCP" string typestr="min-max" optional From 7c23da901695df19ac8bb538c13a2431264d3a86 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Fri, 24 Jan 2020 17:28:20 +0100 Subject: [PATCH 6/7] Updated Admin demo page --- html/admin.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/html/admin.js b/html/admin.js index 8de8b89a61..9e4121099b 100644 --- a/html/admin.js +++ b/html/admin.js @@ -339,20 +339,20 @@ function updateSettings() { setLogLevel(result); }); }); - } else if(k === 'max_nack_queue') { - $('#'+k).append(''); + } else if(k === 'min_nack_queue') { + $('#'+k).append(''); $('#'+k + "_button").click(function() { - bootbox.prompt("Set the new desired max NACK queue (a positive integer, currently " + settings["max_nack_queue"] + ")", function(result) { + bootbox.prompt("Set the new desired min NACK queue (a positive integer, currently " + settings["min_nack_queue"] + ")", function(result) { if(isNaN(result)) { - bootbox.alert("Invalid max NACK queue (should be a positive integer)"); + bootbox.alert("Invalid min NACK queue (should be a positive integer)"); return; } result = parseInt(result); if(result < 0) { - bootbox.alert("Invalid max NACK queue (should be a positive integer)"); + bootbox.alert("Invalid min NACK queue (should be a positive integer)"); return; } - setMaxNackQueue(result); + setMinNackQueue(result); }); }); } else if(k === 'no_media_timer') { @@ -510,8 +510,8 @@ function setLibniceDebug(enable) { sendSettingsRequest(request); } -function setMaxNackQueue(queue) { - var request = { "janus": "set_max_nack_queue", "max_nack_queue": queue, "transaction": randomString(12), "admin_secret": secret }; +function setMinNackQueue(queue) { + var request = { "janus": "set_min_nack_queue", "min_nack_queue": queue, "transaction": randomString(12), "admin_secret": secret }; sendSettingsRequest(request); } From 19b9981923ec0c61aaeecf2aae46c818ca257871 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Fri, 24 Jan 2020 17:34:41 +0100 Subject: [PATCH 7/7] Use moving average to update queue size --- ice.c | 37 ++++++++++++++++++++++--------------- ice.h | 6 +++--- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/ice.c b/ice.c index 418e72bde6..44e98b8f86 100644 --- a/ice.c +++ b/ice.c @@ -465,31 +465,20 @@ static void janus_ice_free_queued_packet(janus_ice_queued_packet *pkt) { /* Maximum ignore count after retransmission (200ms) */ #define MAX_NACK_IGNORE 200000 -static uint min_nack_queue = DEFAULT_MIN_NACK_QUEUE; -void janus_set_min_nack_queue(uint mnq) { +static uint16_t min_nack_queue = DEFAULT_MIN_NACK_QUEUE; +void janus_set_min_nack_queue(uint16_t mnq) { min_nack_queue = mnq < DEFAULT_MAX_NACK_QUEUE ? mnq : DEFAULT_MAX_NACK_QUEUE; if(min_nack_queue == 0) JANUS_LOG(LOG_VERB, "Disabling NACK queue\n"); else JANUS_LOG(LOG_VERB, "Setting min NACK queue to %dms\n", min_nack_queue); } -uint janus_get_min_nack_queue(void) { +uint16_t janus_get_min_nack_queue(void) { return min_nack_queue; } /* Helper to clean old NACK packets in the buffer when they exceed the queue time limit */ static void janus_cleanup_nack_buffer(gint64 now, janus_ice_stream *stream, gboolean audio, gboolean video) { if(stream && stream->component) { - /* Check the current RTT, to see if we need to update the size of the queue: we take - * the highest RTT (audio or video) and add 100ms just to be conservative */ - uint32_t audio_rtt = janus_rtcp_context_get_rtt(stream->audio_rtcp_ctx), - video_rtt = janus_rtcp_context_get_rtt(stream->video_rtcp_ctx[0]); - uint nack_queue_ms = (audio_rtt > video_rtt ? audio_rtt : video_rtt) + 100; - if(nack_queue_ms > DEFAULT_MAX_NACK_QUEUE) - nack_queue_ms = DEFAULT_MAX_NACK_QUEUE; - else if(nack_queue_ms < min_nack_queue) - nack_queue_ms = min_nack_queue; - if(stream->nack_queue_ms < nack_queue_ms) - stream->nack_queue_ms = nack_queue_ms; janus_ice_component *component = stream->component; if(audio && component->audio_retransmit_buffer) { janus_rtp_packet *p = (janus_rtp_packet *)g_queue_peek_head(component->audio_retransmit_buffer); @@ -2758,10 +2747,28 @@ static void janus_ice_cb_nice_recv(NiceAgent *agent, guint stream_id, guint comp /* Let's process this RTCP (compound?) packet, and update the RTCP context for this stream in case */ rtcp_context *rtcp_ctx = video ? stream->video_rtcp_ctx[vindex] : stream->audio_rtcp_ctx; - if (janus_rtcp_parse(rtcp_ctx, buf, buflen) < 0) { + uint32_t rtt = rtcp_ctx ? rtcp_ctx->rtt : 0; + if(janus_rtcp_parse(rtcp_ctx, buf, buflen) < 0) { /* Drop the packet if the parsing function returns with an error */ return; } + if(rtcp_ctx && rtcp_ctx->rtt != rtt) { + /* Check the current RTT, to see if we need to update the size of the queue: we take + * the highest RTT (audio or video) and add 100ms just to be conservative */ + uint32_t audio_rtt = janus_rtcp_context_get_rtt(stream->audio_rtcp_ctx), + video_rtt = janus_rtcp_context_get_rtt(stream->video_rtcp_ctx[0]); + uint16_t nack_queue_ms = (audio_rtt > video_rtt ? audio_rtt : video_rtt) + 100; + if(nack_queue_ms > DEFAULT_MAX_NACK_QUEUE) + nack_queue_ms = DEFAULT_MAX_NACK_QUEUE; + else if(nack_queue_ms < min_nack_queue) + nack_queue_ms = min_nack_queue; + uint16_t mavg = rtt ? ((7*stream->nack_queue_ms + nack_queue_ms)/8) : nack_queue_ms; + if(mavg > DEFAULT_MAX_NACK_QUEUE) + mavg = DEFAULT_MAX_NACK_QUEUE; + else if(mavg < min_nack_queue) + mavg = min_nack_queue; + stream->nack_queue_ms = mavg; + } JANUS_LOG(LOG_HUGE, "[%"SCNu64"] Got %s RTCP (%d bytes)\n", handle->handle_id, video ? "video" : "audio", buflen); /* Now let's see if there are any NACKs to handle */ diff --git a/ice.h b/ice.h index 39c5e8fe0b..883a012bb7 100644 --- a/ice.h +++ b/ice.h @@ -124,10 +124,10 @@ gboolean janus_ice_is_full_trickle_enabled(void); gboolean janus_ice_is_ipv6_enabled(void); /*! \brief Method to modify the min NACK value (i.e., the minimum time window of packets per handle to store for retransmissions) * @param[in] mnq The new min NACK value */ -void janus_set_min_nack_queue(uint mnq); +void janus_set_min_nack_queue(uint16_t mnq); /*! \brief Method to get the current min NACK value (i.e., the minimum time window of packets per handle to store for retransmissions) * @returns The current min NACK value */ -uint janus_get_min_nack_queue(void); +uint16_t janus_get_min_nack_queue(void); /*! \brief Method to modify the no-media event timer (i.e., the number of seconds where no media arrives before Janus notifies this) * @param[in] timer The new timer value, in seconds */ void janus_set_no_media_timer(uint timer); @@ -387,7 +387,7 @@ struct janus_ice_stream { /*! \brief RTCP context(s) for the video stream (may be simulcasting) */ janus_rtcp_context *video_rtcp_ctx[3]; /*! \brief Size of the NACK queue (in ms), dynamically updated per the RTT */ - uint nack_queue_ms; + uint16_t nack_queue_ms; /*! \brief Map(s) of the NACKed packets (to track retransmissions and avoid duplicates) */ GHashTable *rtx_nacked[3]; /*! \brief First received audio NTP timestamp */