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.

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

Make NACK buffer cleanup on outgoing keyframe configurable (see #2401) #2402

Merged
merged 2 commits into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions conf/janus.jcfg.sample.in
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,17 @@ media: {
#twcc_period = 100
#dtls_timeout = 500

# Janus can do some optimizations on the NACK queue, specifically when
# keyframes are involved. Namely, you can configure Janus so that any
# time a keyframe is sent to a user, the NACK buffer for that connection
# is emptied. This allows Janus to ignore NACK requests for packets
# sent shortly before the keyframe was sent, since it can be assumed
# that the keyframe will restore a complete working image for the user
# anyway (which is the main reason why video retransmissions are typically
# required). While this optimization is known to work fine in most cases,
# it can backfire in some edge cases, and so is disabled by default.
#nack_optimizations = true

# If you need DSCP packet marking and prioritization, you can configure
# the 'dscp' property to a specific values, and Janus will try to
# set it on all outgoing packets using libnice. Normally, the specs
Expand Down
17 changes: 13 additions & 4 deletions ice.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ gboolean janus_ice_is_mdns_enabled(void) {
return janus_mdns_enabled;
}

/* IPv6 support (still mostly WIP) */
/* IPv6 support */
static gboolean janus_ipv6_enabled;
gboolean janus_ice_is_ipv6_enabled(void) {
return janus_ipv6_enabled;
Expand Down Expand Up @@ -499,6 +499,14 @@ static void janus_ice_free_queued_packet(janus_ice_queued_packet *pkt) {
/* Maximum ignore count after retransmission (200ms) */
#define MAX_NACK_IGNORE 200000

static gboolean nack_optimizations = FALSE;
void janus_set_nack_optimizations_enabled(gboolean optimize) {
nack_optimizations = optimize;
}
gboolean janus_is_nack_optimizations_enabled(void) {
return nack_optimizations;
}

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;
Expand Down Expand Up @@ -3449,7 +3457,7 @@ int janus_ice_setup_local(janus_ice_handle *handle, int offer, int audio, int vi
family = ifa->ifa_addr->sa_family;
if(family != AF_INET && family != AF_INET6)
continue;
/* We only add IPv6 addresses if support for them has been explicitly enabled (still WIP, mostly) */
/* We only add IPv6 addresses if support for them has been explicitly enabled */
if(family == AF_INET6 && !janus_ipv6_enabled)
continue;
/* Check the interface name first, we can ignore that as well: enforce list would be checked later */
Expand Down Expand Up @@ -4358,8 +4366,9 @@ static gboolean janus_ice_outgoing_traffic_handle(janus_ice_handle *handle, janu
if(g_atomic_int_get(&handle->dump_packets))
janus_text2pcap_dump(handle->text2pcap, JANUS_TEXT2PCAP_RTP, FALSE, pkt->data, pkt->length,
"[session=%"SCNu64"][handle=%"SCNu64"]", session->session_id, handle->handle_id);
/* If this is video, check if this is a keyframe: if so, we empty our retransmit buffer for incoming NACKs */
if(video && stream->video_is_keyframe) {
/* If this is video and NACK optimizations are enabled, check if this is
* a keyframe: if so, we empty our retransmit buffer for incoming NACKs */
if(video && nack_optimizations && stream->video_is_keyframe) {
int plen = 0;
char *payload = janus_rtp_payload(pkt->data, pkt->length, &plen);
if(stream->video_is_keyframe(payload, plen)) {
Expand Down
12 changes: 11 additions & 1 deletion ice.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ gboolean janus_ice_is_full_trickle_enabled(void);
/*! \brief Method to check whether mDNS resolution is enabled or not
* @returns true if mDNS resolution is enabled, false otherwise */
gboolean janus_ice_is_mdns_enabled(void);
/*! \brief Method to check whether IPv6 candidates are enabled/supported or not (still WIP)
/*! \brief Method to check whether IPv6 candidates are enabled/supported or not
* @returns true if IPv6 candidates are enabled/supported, false otherwise */
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)
Expand All @@ -134,6 +134,16 @@ 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 */
uint16_t janus_get_min_nack_queue(void);
/*! \brief Method to enable/disable the NACK optimizations on outgoing keyframes: when
* enabled, the NACK buffer for a PeerConnection is cleaned any time Janus sends a
* keyframe, as any missing packet won't be needed since the keyframe will allow the
* media recipient to still restore a complete image anyway. Since this optimization
* seems to cause some issues in some edge cases, it's disabled by default.
* @param[in] optimize Whether the opzimization should be enabled or disabled */
void janus_set_nack_optimizations_enabled(gboolean optimize);
/*! \brief Method to check whether NACK optimizations on outgoing keyframes are enabled or not
* @returns optimize if optimizations are enabled, false otherwise */
gboolean janus_is_nack_optimizations_enabled(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);
Expand Down
27 changes: 27 additions & 0 deletions janus.c
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ static struct janus_json_parameter colors_parameters[] = {
static struct janus_json_parameter mnq_parameters[] = {
{"min_nack_queue", JSON_INTEGER, JANUS_JSON_PARAM_REQUIRED | JANUS_JSON_PARAM_POSITIVE}
};
static struct janus_json_parameter nopt_parameters[] = {
{"nack_optimizations", JANUS_JSON_BOOL, JANUS_JSON_PARAM_REQUIRED}
};
static struct janus_json_parameter nmt_parameters[] = {
{"no_media_timer", JSON_INTEGER, JANUS_JSON_PARAM_REQUIRED | JANUS_JSON_PARAM_POSITIVE}
};
Expand Down Expand Up @@ -332,6 +335,7 @@ static json_t *janus_info(const char *transaction) {
json_object_set_new(info, "full-trickle", janus_ice_is_full_trickle_enabled() ? json_true() : json_false());
json_object_set_new(info, "mdns-enabled", janus_ice_is_mdns_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, "nack-optimizations", janus_is_nack_optimizations_enabled() ? json_true() : json_false());
json_object_set_new(info, "twcc-period", json_integer(janus_get_twcc_period()));
if(janus_get_dscp() > 0)
json_object_set_new(info, "dscp", json_integer(janus_get_dscp()));
Expand Down Expand Up @@ -1964,6 +1968,7 @@ int janus_process_incoming_admin_request(janus_request *request) {
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, "min_nack_queue", json_integer(janus_get_min_nack_queue()));
json_object_set_new(status, "nack-optimizations", janus_is_nack_optimizations_enabled() ? json_true() : json_false());
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);
Expand Down Expand Up @@ -2127,6 +2132,23 @@ 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_nack_optimizations")) {
/* Enable/disable NACK optimizations */
JANUS_VALIDATE_JSON_OBJECT(root, nopt_parameters,
error_code, error_cause, FALSE,
JANUS_ERROR_MISSING_MANDATORY_ELEMENT, JANUS_ERROR_INVALID_ELEMENT_TYPE);
if(error_code != 0) {
ret = janus_process_error_string(request, session_id, transaction_text, error_code, error_cause);
goto jsondone;
}
gboolean optimize = json_is_true(json_object_get(root, "nack_optimizations"));
janus_set_nack_optimizations_enabled(optimize);
/* Prepare JSON reply */
json_t *reply = janus_create_message("success", 0, transaction_text);
json_object_set_new(reply, "nack-optimizations", janus_is_nack_optimizations_enabled() ? json_true() : json_false());
/* Send the success reply */
ret = janus_process_success(request, reply);
goto jsondone;
} else if(!strcasecmp(message_text, "set_no_media_timer")) {
/* Change the current value for the no-media timer */
JANUS_VALIDATE_JSON_OBJECT(root, nmt_parameters,
Expand Down Expand Up @@ -4822,6 +4844,11 @@ gint main(int argc, char *argv[])
janus_set_min_nack_queue(mnq);
}
}
item = janus_config_get(config, config_media, janus_config_type_item, "nack_optimizations");
if(item && item->value) {
gboolean optimize = janus_is_true(item->value);
janus_set_nack_optimizations_enabled(optimize);
}
/* no-media timer */
item = janus_config_get(config, config_media, janus_config_type_item, "no_media_timer");
if(item && item->value) {
Expand Down