Media loss after 5 minutes when using ICE+TURN (pjsip#2503)

- Assign unique local preferences for candidates with the same type.
- Update component's valid pair condition, instead of just the highest priority, also consider 'nominated' flag.
- Add new compile-time setting PJ_ICE_ST_USE_TURN_PERMANENT_PERM, if set, TURN client session will automatically renew permission for all remote candidates.
- Update local preference for peer reflexive candidate.
- Also update PRIORITY field value in performing connectivity check: use unique local pref (with peer-reflexive candidate type).
- Fix local preference mask for non-standard ICE prio calculation.

pjnath/include/pjnath/config.h

@@ -457,6 +457,17 @@
 #endif
 
 
+/**
+ * This constant specifies whether ICE stream transport should allow TURN
+ * client session to automatically renew permission for all remote candidates.
+ *
+ * Default: PJ_FALSE
+ */
+#ifndef PJ_ICE_ST_USE_TURN_PERMANENT_PERM
+#   define PJ_ICE_ST_USE_TURN_PERMANENT_PERM	    PJ_FALSE
+#endif
+
+
 /** ICE session pool initial size. */
 #ifndef PJNATH_POOL_LEN_ICE_SESS
 #   define PJNATH_POOL_LEN_ICE_SESS		    512

pjnath/include/pjnath/ice_session.h

@@ -237,6 +237,11 @@ typedef struct pj_ice_msg_data
  */
 typedef struct pj_ice_sess_cand
 {
+    /**
+     * The candidate ID.
+     */
+    unsigned		 id;
+
     /**
      * The candidate type, as described in #pj_ice_cand_type enumeration.
      */

pjnath/src/pjnath/ice_session.c

@@ -686,9 +686,9 @@ static pj_uint32_t CALC_CAND_PRIO(pj_ice_sess *ice,
 	   (((256 - comp_id) & 0xFF) << 0);
 #else
     enum {
-	type_mask   = ((2 << PJ_ICE_CAND_TYPE_PREF_BITS) - 1),
-	local_mask  = ((2 << PJ_ICE_LOCAL_PREF_BITS) - 1),
-	comp_mask   = ((2 << PJ_ICE_COMP_BITS) - 1),
+	type_mask   = ((1 << PJ_ICE_CAND_TYPE_PREF_BITS) - 1),
+	local_mask  = ((1 << PJ_ICE_LOCAL_PREF_BITS) - 1),
+	comp_mask   = ((1 << PJ_ICE_COMP_BITS) - 1),
 
 	comp_shift  = 0,
 	local_shift = (PJ_ICE_COMP_BITS),
@@ -737,10 +737,12 @@ PJ_DEF(pj_status_t) pj_ice_sess_add_cand(pj_ice_sess *ice,
     }
 
     lcand = &ice->lcand[ice->lcand_cnt];
+    lcand->id = ice->lcand_cnt;
     lcand->comp_id = (pj_uint8_t)comp_id;
     lcand->transport_id = (pj_uint8_t)transport_id;
     lcand->type = type;
     pj_strdup(ice->pool, &lcand->foundation, foundation);
+    lcand->local_pref = local_pref;
     lcand->prio = CALC_CAND_PRIO(ice, type, local_pref, lcand->comp_id);
     pj_sockaddr_cp(&lcand->addr, addr);
     pj_sockaddr_cp(&lcand->base_addr, base_addr);
@@ -768,7 +770,7 @@ PJ_DEF(pj_status_t) pj_ice_sess_add_cand(pj_ice_sess *ice,
     LOG4((ice->obj_name, 
 	 "Candidate %d added: comp_id=%d, type=%s, foundation=%.*s, "
 	 "addr=%s:%d, base=%s:%d, prio=0x%x (%u)",
-	 ice->lcand_cnt, 
+	 lcand->id,
 	 lcand->comp_id, 
 	 cand_type_names[lcand->type],
 	 (int)lcand->foundation.slen,
@@ -780,7 +782,7 @@ PJ_DEF(pj_status_t) pj_ice_sess_add_cand(pj_ice_sess *ice,
 	 lcand->prio, lcand->prio));
 
     if (p_cand_id)
-	*p_cand_id = ice->lcand_cnt;
+	*p_cand_id = lcand->id;
 
     ++ice->lcand_cnt;
 
@@ -1297,7 +1299,23 @@ static void update_comp_check(pj_ice_sess *ice, unsigned comp_id,
     if (comp->valid_check == NULL) {
 	comp->valid_check = check;
     } else {
-	if (CMP_CHECK_PRIO(comp->valid_check, check) < 0)
+	pj_bool_t update = PJ_FALSE;
+
+	/* Update component's valid check with conditions:
+	 * - it is the first nominated check, or
+	 * - it has higher prio, as long as nomination status is NOT degraded
+	 *   (existing is nominated -> new is not-nominated).
+	 */
+	if (!comp->nominated_check && check->nominated)
+	{
+	    update = PJ_TRUE;
+	} else if (CMP_CHECK_PRIO(comp->valid_check, check) < 0 &&
+		   (!comp->nominated_check || check->nominated))
+	{
+	    update = PJ_TRUE;
+	}
+
+	if (update)
 	    comp->valid_check = check;
     }
 
@@ -1676,7 +1694,7 @@ PJ_DEF(pj_status_t) pj_ice_sess_create_check_list(
 
 	pj_memcpy(cn, &rem_cand[i], sizeof(pj_ice_sess_cand));
 	pj_strdup(ice->pool, &cn->foundation, &rem_cand[i].foundation);
-	ice->rcand_cnt++;
+	cn->id = ice->rcand_cnt++;
     }
 
     /* Generate checklist */
@@ -1807,10 +1825,11 @@ static pj_status_t perform_check(pj_ice_sess *ice,
 
     /* Add PRIORITY */
 #if PJNATH_ICE_PRIO_STD
-    prio = CALC_CAND_PRIO(ice, PJ_ICE_CAND_TYPE_PRFLX, 65535, 
+    prio = CALC_CAND_PRIO(ice, PJ_ICE_CAND_TYPE_PRFLX, 65535 - lcand->id,
 			  lcand->comp_id);
 #else
-    prio = CALC_CAND_PRIO(ice, PJ_ICE_CAND_TYPE_PRFLX, 0, 
+    prio = CALC_CAND_PRIO(ice, PJ_ICE_CAND_TYPE_PRFLX,
+			  ((1 << PJ_ICE_LOCAL_PREF_BITS) - 1) - lcand->id,
 			  lcand->comp_id);
 #endif
     pj_stun_msg_add_uint_attr(check->tdata->pool, check->tdata->msg, 
@@ -2406,7 +2425,13 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
 	status = pj_ice_sess_add_cand(ice, check->lcand->comp_id, 
 				      msg_data->transport_id,
 				      PJ_ICE_CAND_TYPE_PRFLX,
-				      65535, &foundation,
+#if PJNATH_ICE_PRIO_STD
+				      65535 - ice->lcand_cnt,
+#else
+				      ((1 << PJ_ICE_LOCAL_PREF_BITS) - 1) -
+				      ice->lcand_cnt,
+#endif
+				      &foundation,
 				      &xaddr->sockaddr, 
 				      &check->lcand->base_addr, 
 				      &check->lcand->base_addr,

pjnath/src/pjnath/ice_strans.c

@@ -64,9 +64,9 @@ enum tp_type
 #   define HOST_PREF   65535
 #   define RELAY_PREF  65535
 #else
-#   define SRFLX_PREF  0
-#   define HOST_PREF   0
-#   define RELAY_PREF  0
+#   define SRFLX_PREF  ((1 << PJ_ICE_LOCAL_PREF_BITS) - 1)
+#   define HOST_PREF   ((1 << PJ_ICE_LOCAL_PREF_BITS) - 1)
+#   define RELAY_PREF  ((1 << PJ_ICE_LOCAL_PREF_BITS) - 1)
 #endif
 
 
@@ -418,7 +418,7 @@ static pj_status_t add_update_turn(pj_ice_strans *ice_st,
 	cand = &comp->cand_list[comp->cand_cnt];
 	cand->type = PJ_ICE_CAND_TYPE_RELAYED;
 	cand->status = PJ_EPENDING;
-	cand->local_pref = RELAY_PREF;
+	cand->local_pref = (pj_uint16_t)(RELAY_PREF - idx);
 	cand->transport_id = tp_id;
 	cand->comp_id = (pj_uint8_t) comp->comp_id;
 	new_cand = PJ_TRUE;
@@ -480,7 +480,8 @@ static pj_bool_t ice_cand_equals(pj_ice_sess_cand *lcand,
         || lcand->status != rcand->status
         || lcand->comp_id != rcand->comp_id
         || lcand->transport_id != rcand->transport_id
-        || lcand->local_pref != rcand->local_pref
+	// local pref is no longer a constant, so it may be different
+        //|| lcand->local_pref != rcand->local_pref
         || lcand->prio != rcand->prio
         || pj_sockaddr_cmp(&lcand->addr, &rcand->addr) != 0
         || pj_sockaddr_cmp(&lcand->base_addr, &rcand->base_addr) != 0)
@@ -539,7 +540,7 @@ static pj_status_t add_stun_and_host(pj_ice_strans *ice_st,
     cand = &comp->cand_list[comp->cand_cnt];
     cand->type = PJ_ICE_CAND_TYPE_SRFLX;
     cand->status = PJ_EPENDING;
-    cand->local_pref = SRFLX_PREF;
+    cand->local_pref = (pj_uint16_t)(SRFLX_PREF - idx);
     cand->transport_id = CREATE_TP_ID(TP_STUN, idx);
     cand->comp_id = (pj_uint8_t) comp->comp_id;
 
@@ -679,7 +680,7 @@ static pj_status_t add_stun_and_host(pj_ice_strans *ice_st,
 
 	    cand->type = PJ_ICE_CAND_TYPE_HOST;
 	    cand->status = PJ_SUCCESS;
-	    cand->local_pref = HOST_PREF;
+	    cand->local_pref = (pj_uint16_t)(HOST_PREF - cand_cnt);
 	    cand->transport_id = CREATE_TP_ID(TP_STUN, idx);
 	    cand->comp_id = (pj_uint8_t) comp->comp_id;
 	    pj_sockaddr_cp(&cand->addr, addr);
@@ -1503,8 +1504,9 @@ PJ_DEF(pj_status_t) pj_ice_strans_start_ice( pj_ice_strans *ice_st,
 	    }
 
 	    if (count && !comp->turn[n].err_cnt && comp->turn[n].sock) {
-		status = pj_turn_sock_set_perm(comp->turn[n].sock, count,
-					       addrs, 0);
+		status = pj_turn_sock_set_perm(
+				    comp->turn[n].sock, count,
+				    addrs, PJ_ICE_ST_USE_TURN_PERMANENT_PERM);
 		if (status != PJ_SUCCESS) {
 		    pj_ice_strans_stop_ice(ice_st);
 		    return status;