res_rtp_asterisk: Fix one way audio problems with hold/unhold when using ICE

ICE sessions will now be restarted if sessions are changed to use new sets of remote candidates. (closes issue ASTERISK-22911) Reported by: Vytis Valentinavičius Review: https://reviewboard.asterisk.org/r/3275/ ........ Merged revisions 409565 from http://svn.asterisk.org/svn/asterisk/branches/11 ........ Merged revisions 409570 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@409587 65c4cc65-6c06-0410-ace0-fbb531ad65f3
2025-09-04 20:04:50 +00:00 · 2014-03-04 17:22:32 +00:00
parent 6204ea6c1a
commit a0fff439ab
1 changed files with 177 additions and 66 deletions
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -66,6 +66,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/unaligned.h"
 #include "asterisk/module.h"
 #include "asterisk/rtp_engine.h"
+#include "asterisk/test.h"

 #define MAX_TIMESTAMP_SKEW	640

@@ -263,7 +264,7 @@ struct ast_rtp {
 	pj_turn_sock *turn_rtcp;    /*!< RTCP TURN relay */
 	pj_turn_state_t turn_state; /*!< Current state of the TURN relay session */
 	unsigned int passthrough:1; /*!< Bit to indicate that the received packet should be passed through */
-	unsigned int ice_started:1; /*!< Bit to indicate ICE connectivity checks have started */
+	unsigned int ice_port;      /*!< Port that ICE was started with if it was previously started */

 	char remote_ufrag[256];  /*!< The remote ICE username */
 	char remote_passwd[256]; /*!< The remote ICE password */
@@ -271,8 +272,10 @@ struct ast_rtp {
 	char local_ufrag[256];  /*!< The local ICE username */
 	char local_passwd[256]; /*!< The local ICE password */

-	struct ao2_container *local_candidates;   /*!< The local ICE candidates */
-	struct ao2_container *remote_candidates;  /*!< The remote ICE candidates */
+	struct ao2_container *ice_local_candidates;           /*!< The local ICE candidates */
+	struct ao2_container *ice_active_remote_candidates;   /*!< The remote ICE candidates */
+	struct ao2_container *ice_proposed_remote_candidates; /*!< Incoming remote ICE candidates for new session */
+	struct ast_sockaddr ice_original_rtp_addr;            /*!< rtp address that ICE started on first session */
 #endif

 #ifdef HAVE_OPENSSL_SRTP
@@ -446,17 +449,32 @@ static void ast_rtp_ice_set_authentication(struct ast_rtp_instance *instance, co
 	}
 }

+static int ice_candidate_cmp(void *obj, void *arg, int flags)
+{
+	struct ast_rtp_engine_ice_candidate *candidate1 = obj, *candidate2 = arg;
+
+	if (strcmp(candidate1->foundation, candidate2->foundation) ||
+			candidate1->id != candidate2->id ||
+			ast_sockaddr_cmp(&candidate1->address, &candidate2->address) ||
+			candidate1->type != candidate1->type) {
+		return 0;
+	}
+
+	return CMP_MATCH | CMP_STOP;
+}
+
 static void ast_rtp_ice_add_remote_candidate(struct ast_rtp_instance *instance, const struct ast_rtp_engine_ice_candidate *candidate)
 {
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
 	struct ast_rtp_engine_ice_candidate *remote_candidate;

-	if (!rtp->remote_candidates && !(rtp->remote_candidates = ao2_container_alloc(1, NULL, NULL))) {
+	if (!rtp->ice_proposed_remote_candidates &&
+			!(rtp->ice_proposed_remote_candidates = ao2_container_alloc(1, NULL, ice_candidate_cmp))) {
 		return;
 	}

 	/* If this is going to exceed the maximum number of ICE candidates don't even add it */
-	if (ao2_container_count(rtp->remote_candidates) == PJ_ICE_MAX_CAND) {
+	if (ao2_container_count(rtp->ice_proposed_remote_candidates) == PJ_ICE_MAX_CAND) {
 		return;
 	}

@@ -472,7 +490,7 @@ static void ast_rtp_ice_add_remote_candidate(struct ast_rtp_instance *instance,
 	ast_sockaddr_copy(&remote_candidate->relay_address, &candidate->relay_address);
 	remote_candidate->type = candidate->type;

-	ao2_link(rtp->remote_candidates, remote_candidate);
+	ao2_link(rtp->ice_proposed_remote_candidates, remote_candidate);
 	ao2_ref(remote_candidate, -1);
 }

@@ -501,6 +519,58 @@ static void pj_thread_register_check(void)
 	return;
 }

+static int ice_create(struct ast_rtp_instance *instance, struct ast_sockaddr *addr,
+	int port, int replace);
+
+static void ast_rtp_ice_stop(struct ast_rtp_instance *instance)
+{
+	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
+
+	if (!rtp->ice) {
+		return;
+	}
+
+	pj_thread_register_check();
+
+	pj_ice_sess_destroy(rtp->ice);
+	rtp->ice = NULL;
+}
+
+static int ice_reset_session(struct ast_rtp_instance *instance)
+{
+	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
+
+	ast_rtp_ice_stop(instance);
+	return ice_create(instance, &rtp->ice_original_rtp_addr, rtp->ice_port, 1);
+}
+
+static int ice_candidates_compare(struct ao2_container *left, struct ao2_container *right)
+{
+	struct ao2_iterator i;
+	struct ast_rtp_engine_ice_candidate *right_candidate;
+
+	if (ao2_container_count(left) != ao2_container_count(right)) {
+		return -1;
+	}
+
+	i = ao2_iterator_init(right, 0);
+	while ((right_candidate = ao2_iterator_next(&i))) {
+		struct ast_rtp_engine_ice_candidate *left_candidate = ao2_find(left, right_candidate, OBJ_POINTER);
+
+		if (!left_candidate) {
+			ao2_ref(right_candidate, -1);
+			ao2_iterator_destroy(&i);
+			return -1;
+		}
+
+		ao2_ref(left_candidate, -1);
+		ao2_ref(right_candidate, -1);
+	}
+	ao2_iterator_destroy(&i);
+
+	return 0;
+}
+
 static void ast_rtp_ice_start(struct ast_rtp_instance *instance)
 {
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
@@ -510,13 +580,32 @@ static void ast_rtp_ice_start(struct ast_rtp_instance *instance)
 	struct ast_rtp_engine_ice_candidate *candidate;
 	int cand_cnt = 0;

-	if (!rtp->ice || !rtp->remote_candidates || rtp->ice_started) {
+	if (!rtp->ice || !rtp->ice_proposed_remote_candidates) {
+		return;
+	}
+
+	/* Check for equivalence in the lists */
+	if (rtp->ice_active_remote_candidates &&
+			!ice_candidates_compare(rtp->ice_proposed_remote_candidates, rtp->ice_active_remote_candidates)) {
+		ao2_cleanup(rtp->ice_proposed_remote_candidates);
+		rtp->ice_proposed_remote_candidates = NULL;
+		return;
+	}
+
+	/* Out with the old, in with the new */
+	ao2_cleanup(rtp->ice_active_remote_candidates);
+	rtp->ice_active_remote_candidates = rtp->ice_proposed_remote_candidates;
+	rtp->ice_proposed_remote_candidates = NULL;
+
+	/* Reset the ICE session. Is this going to work? */
+	if (ice_reset_session(instance)) {
+		ast_log(LOG_NOTICE, "Failed to create replacement ICE session\n");
 		return;
 	}

 	pj_thread_register_check();

-	i = ao2_iterator_init(rtp->remote_candidates, 0);
+	i = ao2_iterator_init(rtp->ice_active_remote_candidates, 0);

 	while ((candidate = ao2_iterator_next(&i)) && (cand_cnt < PJ_ICE_MAX_CAND)) {
 		pj_str_t address;
@@ -546,42 +635,31 @@ static void ast_rtp_ice_start(struct ast_rtp_instance *instance)
 		}

 		cand_cnt++;
+		ao2_ref(candidate, -1);
 	}

 	ao2_iterator_destroy(&i);

-	if (pj_ice_sess_create_check_list(rtp->ice, &ufrag, &passwd, ao2_container_count(rtp->remote_candidates), &candidates[0]) == PJ_SUCCESS) {
+	if (pj_ice_sess_create_check_list(rtp->ice, &ufrag, &passwd, ao2_container_count(rtp->ice_active_remote_candidates), &candidates[0]) == PJ_SUCCESS) {
+		ast_test_suite_event_notify("ICECHECKLISTCREATE", "Result: SUCCESS");
 		pj_ice_sess_start_check(rtp->ice);
 		pj_timer_heap_poll(timerheap, NULL);
-		rtp->ice_started = 1;
 		rtp->strict_rtp_state = STRICT_RTP_OPEN;
 		return;
 	}

+	ast_test_suite_event_notify("ICECHECKLISTCREATE", "Result: FAILURE");
+
 	/* even though create check list failed don't stop ice as
 	   it might still work */
 	ast_debug(1, "Failed to create ICE session check list\n");
 	/* however we do need to reset remote candidates since
 	   this function may be re-entered */
-	ao2_ref(rtp->remote_candidates, -1);
-	rtp->remote_candidates = NULL;
+	ao2_ref(rtp->ice_active_remote_candidates, -1);
+	rtp->ice_active_remote_candidates = NULL;
 	rtp->ice->rcand_cnt = rtp->ice->clist.count = 0;
 }

-static void ast_rtp_ice_stop(struct ast_rtp_instance *instance)
-{
-	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
-
-	if (!rtp->ice) {
-		return;
-	}
-
-	pj_thread_register_check();
-
-	pj_ice_sess_destroy(rtp->ice);
-	rtp->ice = NULL;
-}
-
 static const char *ast_rtp_ice_get_ufrag(struct ast_rtp_instance *instance)
 {
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
@@ -600,11 +678,11 @@ static struct ao2_container *ast_rtp_ice_get_local_candidates(struct ast_rtp_ins
 {
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);

-	if (rtp->local_candidates) {
-		ao2_ref(rtp->local_candidates, +1);
+	if (rtp->ice_local_candidates) {
+		ao2_ref(rtp->ice_local_candidates, +1);
 	}

-	return rtp->local_candidates;
+	return rtp->ice_local_candidates;
 }

 static void ast_rtp_ice_lite(struct ast_rtp_instance *instance)
@@ -620,20 +698,6 @@ static void ast_rtp_ice_lite(struct ast_rtp_instance *instance)
 	pj_ice_sess_change_role(rtp->ice, PJ_ICE_SESS_ROLE_CONTROLLING);
 }

-static int ice_candidate_cmp(void *obj, void *arg, int flags)
-{
-	struct ast_rtp_engine_ice_candidate *candidate1 = obj, *candidate2 = arg;
-
-	if ((strcmp(candidate1->foundation, candidate2->foundation)) ||
-	    (candidate1->id != candidate2->id) ||
-	    (ast_sockaddr_cmp(&candidate1->address, &candidate2->address)) ||
-	    (candidate1->type != candidate1->type)) {
-		return 0;
-	}
-
-	return CMP_MATCH | CMP_STOP;
-}
-
 static void ast_rtp_ice_add_cand(struct ast_rtp *rtp, unsigned comp_id, unsigned transport_id, pj_ice_cand_type type, pj_uint16_t local_pref,
 					const pj_sockaddr_t *addr, const pj_sockaddr_t *base_addr, const pj_sockaddr_t *rel_addr, int addr_len)
 {
@@ -645,7 +709,7 @@ static void ast_rtp_ice_add_cand(struct ast_rtp *rtp, unsigned comp_id, unsigned

 	pj_ice_calc_foundation(rtp->ice->pool, &foundation, type, addr);

-	if (!rtp->local_candidates && !(rtp->local_candidates = ao2_container_alloc(1, NULL, ice_candidate_cmp))) {
+	if (!rtp->ice_local_candidates && !(rtp->ice_local_candidates = ao2_container_alloc(1, NULL, ice_candidate_cmp))) {
 		return;
 	}

@@ -673,7 +737,7 @@ static void ast_rtp_ice_add_cand(struct ast_rtp *rtp, unsigned comp_id, unsigned
 		candidate->type = AST_RTP_ICE_CANDIDATE_TYPE_RELAYED;
 	}

-	if ((existing = ao2_find(rtp->local_candidates, candidate, OBJ_POINTER))) {
+	if ((existing = ao2_find(rtp->ice_local_candidates, candidate, OBJ_POINTER))) {
 		ao2_ref(existing, -1);
 		ao2_ref(candidate, -1);
 		return;
@@ -687,7 +751,7 @@ static void ast_rtp_ice_add_cand(struct ast_rtp *rtp, unsigned comp_id, unsigned
 	/* By placing the candidate into the ICE session it will have produced the priority, so update the local candidate with it */
 	candidate->priority = rtp->ice->lcand[rtp->ice->lcand_cnt - 1].prio;

-	ao2_link(rtp->local_candidates, candidate);
+	ao2_link(rtp->ice_local_candidates, candidate);
 	ao2_ref(candidate, -1);
 }

@@ -1767,16 +1831,68 @@ static unsigned int calc_txstamp(struct ast_rtp *rtp, struct timeval *delivery)
 	return (unsigned int) ms;
 }

+#ifdef HAVE_PJPROJECT
+/*!
+ * \internal
+ * \brief Creates an ICE session. Can be used to replace a destroyed ICE session.
+ *
+ * \param instance RTP instance for which the ICE session is being replaced
+ * \param addr ast_sockaddr to use for adding RTP candidates to the ICE session
+ * \param port port to use for adding RTP candidates to the ICE session
+ * \param replace 0 when creating a new session, 1 when replacing a destroyed session
+ *
+ * \retval 0 on success
+ * \retval -1 on failure
+ */
+static int ice_create(struct ast_rtp_instance *instance, struct ast_sockaddr *addr,
+	int port, int replace)
+{
+	pj_stun_config stun_config;
+	pj_str_t ufrag, passwd;
+	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
+
+	ao2_cleanup(rtp->ice_local_candidates);
+	rtp->ice_local_candidates = NULL;
+
+	pj_thread_register_check();
+
+	pj_stun_config_init(&stun_config, &cachingpool.factory, 0, ioqueue, timerheap);
+
+	ufrag = pj_str(rtp->local_ufrag);
+	passwd = pj_str(rtp->local_passwd);
+
+	/* Create an ICE session for ICE negotiation */
+	if (pj_ice_sess_create(&stun_config, NULL, PJ_ICE_SESS_ROLE_UNKNOWN, 2,
+			&ast_rtp_ice_sess_cb, &ufrag, &passwd, NULL, &rtp->ice) == PJ_SUCCESS) {
+		/* Make this available for the callbacks */
+		rtp->ice->user_data = rtp;
+
+		/* Add all of the available candidates to the ICE session */
+		rtp_add_candidates_to_ice(instance, rtp, addr, port, AST_RTP_ICE_COMPONENT_RTP,
+			TRANSPORT_SOCKET_RTP, &ast_rtp_turn_rtp_sock_cb, &rtp->turn_rtp);
+
+		/* Only add the RTCP candidates to ICE when replacing the session. New sessions
+		 * handle this in a separate part of the setup phase */
+		if (replace && rtp->rtcp) {
+			rtp_add_candidates_to_ice(instance, rtp, &rtp->rtcp->us,
+				ast_sockaddr_port(&rtp->rtcp->us), AST_RTP_ICE_COMPONENT_RTCP,
+				TRANSPORT_SOCKET_RTCP, &ast_rtp_turn_rtcp_sock_cb, &rtp->turn_rtcp);
+		}
+
+		return 0;
+	}
+
+	return -1;
+
+}
+#endif
+
 static int ast_rtp_new(struct ast_rtp_instance *instance,
 		       struct ast_sched_context *sched, struct ast_sockaddr *addr,
 		       void *data)
 {
 	struct ast_rtp *rtp = NULL;
 	int x, startplace;
-#ifdef HAVE_PJPROJECT
-	pj_stun_config stun_config;
-	pj_str_t ufrag, passwd;
-#endif

 	/* Create a new RTP structure to hold all of our data */
 	if (!(rtp = ast_calloc(1, sizeof(*rtp)))) {
@@ -1835,24 +1951,19 @@ static int ast_rtp_new(struct ast_rtp_instance *instance,
 	}

 #ifdef HAVE_PJPROJECT
-	pj_thread_register_check();
-
-	pj_stun_config_init(&stun_config, &cachingpool.factory, 0, ioqueue, timerheap);
-
 	generate_random_string(rtp->local_ufrag, sizeof(rtp->local_ufrag));
-	ufrag = pj_str(rtp->local_ufrag);
 	generate_random_string(rtp->local_passwd, sizeof(rtp->local_passwd));
-	passwd = pj_str(rtp->local_passwd);
 #endif
 	ast_rtp_instance_set_data(instance, rtp);
 #ifdef HAVE_PJPROJECT
 	/* Create an ICE session for ICE negotiation */
-	if (icesupport && pj_ice_sess_create(&stun_config, NULL, PJ_ICE_SESS_ROLE_UNKNOWN, 2, &ast_rtp_ice_sess_cb, &ufrag, &passwd, NULL, &rtp->ice) == PJ_SUCCESS) {
-		/* Make this available for the callbacks */
-		rtp->ice->user_data = rtp;
-
-		/* Add all of the available candidates to the ICE session */
-		rtp_add_candidates_to_ice(instance, rtp, addr, x, AST_RTP_ICE_COMPONENT_RTP, TRANSPORT_SOCKET_RTP, &ast_rtp_turn_rtp_sock_cb, &rtp->turn_rtp);
+	if (icesupport) {
+		if (ice_create(instance, addr, x, 0)) {
+			ast_log(LOG_NOTICE, "Failed to start ICE session\n");
+		} else {
+			rtp->ice_port = x;
+			ast_sockaddr_copy(&rtp->ice_original_rtp_addr, addr);
+		}
 	}
 #endif

@@ -1918,12 +2029,12 @@ static int ast_rtp_destroy(struct ast_rtp_instance *instance)
 	}

 	/* Destroy any candidates */
-	if (rtp->local_candidates) {
-		ao2_ref(rtp->local_candidates, -1);
+	if (rtp->ice_local_candidates) {
+		ao2_ref(rtp->ice_local_candidates, -1);
 	}

-	if (rtp->remote_candidates) {
-		ao2_ref(rtp->remote_candidates, -1);
+	if (rtp->ice_active_remote_candidates) {
+		ao2_ref(rtp->ice_active_remote_candidates, -1);
 	}
 #endif