Slightly more clever RTP transport remote address switch: transmission won't switch as long as we're receiving RTP/RTCP packets from the "correct" remote address, to avoid constantly switching to a new source address. Also when the remote peer address is in probation, RT(C)P packets from that address will be silently discarded, to avoid playback confusion. This closes #1366.

git-svn-id: http://svn.pjsip.org/repos/pjproject/branches/1.x@3745 74dad513-b988-da41-8d7b-12977e46ad98

3 files changed, 90 insertions, 41 deletions

diff --git a/pjmedia/include/pjmedia/config.h b/pjmedia/include/pjmedia/config.h
index 6b7fc9b4..b0c3d6f7 100644
--- a/pjmedia/include/pjmedia/config.h
+++ b/pjmedia/include/pjmedia/config.h
@@ -387,7 +387,7 @@
 
 
 /**
- * Number of packets received from different source IP address from the
+ * Number of RTP packets received from different source IP address from the
  * remote address required to make the stream switch transmission
  * to the source address.
  */
@@ -397,6 +397,16 @@
 
 
 /**
+ * Number of RTCP packets received from different source IP address from the
+ * remote address required to make the stream switch RTCP transmission
+ * to the source address.
+ */
+#ifndef PJMEDIA_RTCP_NAT_PROBATION_CNT
+#  define PJMEDIA_RTCP_NAT_PROBATION_CNT	3
+#endif
+
+
+/**
  * Specify whether RTCP should be advertised in SDP. This setting would
  * affect whether RTCP candidate will be added in SDP when ICE is used.
  * Application might want to disable RTCP advertisement in SDP to
diff --git a/pjmedia/src/pjmedia/transport_ice.c b/pjmedia/src/pjmedia/transport_ice.c
index df2a68ed..3e6db887 100644
--- a/pjmedia/src/pjmedia/transport_ice.c
+++ b/pjmedia/src/pjmedia/transport_ice.c
@@ -72,6 +72,7 @@ struct transport_ice
     pj_sockaddr		 rtp_src_addr;	/**< Actual source RTP address.	    */
     pj_sockaddr		 rtcp_src_addr;	/**< Actual source RTCP address.    */
     unsigned		 rtp_src_cnt;	/**< How many pkt from this addr.   */
+    unsigned		 rtcp_src_cnt;  /**< How many pkt from this addr.   */
 
     unsigned		 tx_drop_pct;	/**< Percent of tx pkts to drop.    */
     unsigned		 rx_drop_pct;	/**< Percent of rx pkts to drop.    */
@@ -1499,6 +1500,8 @@ static pj_status_t transport_get_info(pjmedia_transport *tp,
      */
     if (tp_ice->use_ice || tp_ice->rtp_src_cnt) {
 	info->src_rtp_name  = tp_ice->rtp_src_addr;
+    }
+    if (tp_ice->use_ice || tp_ice->rtcp_src_cnt) {
 	info->src_rtcp_name = tp_ice->rtcp_src_addr;
     }
 
@@ -1564,6 +1567,7 @@ static pj_status_t transport_attach  (pjmedia_transport *tp,
     tp_ice->rtp_src_addr = tp_ice->remote_rtp;
     tp_ice->rtcp_src_addr = tp_ice->remote_rtcp;
     tp_ice->rtp_src_cnt = 0;
+    tp_ice->rtcp_src_cnt = 0;
 
     return PJ_SUCCESS;
 }
@@ -1639,6 +1643,7 @@ static void ice_on_rx_data(pj_ice_strans *ice_st, unsigned comp_id,
 			   unsigned src_addr_len)
 {
     struct transport_ice *tp_ice;
+    pj_bool_t discard = PJ_FALSE;
 
     tp_ice = (struct transport_ice*) pj_ice_strans_get_user_data(ice_st);
 
@@ -1654,20 +1659,23 @@ static void ice_on_rx_data(pj_ice_strans *ice_st, unsigned comp_id,
 	    }
 	}
 
-	(*tp_ice->rtp_cb)(tp_ice->stream, pkt, size);
-
 	/* See if source address of RTP packet is different than the 
 	 * configured address, and switch RTP remote address to 
 	 * source packet address after several consecutive packets
 	 * have been received.
 	 */
 	if (!tp_ice->use_ice) {
+	    pj_bool_t enable_switch =
+		    ((tp_ice->options & PJMEDIA_ICE_NO_SRC_ADDR_CHECKING)==0);
 
-	    /* Increment counter and avoid zero */
-	    if (++tp_ice->rtp_src_cnt == 0) 
-		tp_ice->rtp_src_cnt = 1;
+	    if (!enable_switch ||
+		pj_sockaddr_cmp(&tp_ice->remote_rtp, src_addr) == 0)
+	    {
+		/* Don't switch while we're receiving from remote_rtp */
+		tp_ice->rtp_src_cnt = 0;
+	    } else {
 
-	    if (pj_sockaddr_cmp(&tp_ice->remote_rtp, src_addr) != 0) {
+		++tp_ice->rtp_src_cnt;
 
 		/* Check if the source address is recognized. */
 		if (pj_sockaddr_cmp(src_addr, &tp_ice->rtp_src_addr) != 0) {
@@ -1675,11 +1683,12 @@ static void ice_on_rx_data(pj_ice_strans *ice_st, unsigned comp_id,
 		    pj_sockaddr_cp(&tp_ice->rtp_src_addr, src_addr);
 		    /* Reset counter */
 		    tp_ice->rtp_src_cnt = 0;
+		    discard = PJ_TRUE;
 		}
 
-		if ((tp_ice->options & PJMEDIA_ICE_NO_SRC_ADDR_CHECKING)==0 &&
-		    tp_ice->rtp_src_cnt >= PJMEDIA_RTP_NAT_PROBATION_CNT) 
-		{
+		if (tp_ice->rtp_src_cnt < PJMEDIA_RTP_NAT_PROBATION_CNT) {
+		    discard = PJ_TRUE;
+		} else {
 		    char addr_text[80];
 
 		    /* Set remote RTP address to source address */
@@ -1708,7 +1717,8 @@ static void ice_on_rx_data(pj_ice_strans *ice_st, unsigned comp_id,
 			pj_sockaddr_set_port(&tp_ice->remote_rtcp, port);
 
 			PJ_LOG(4,(tp_ice->base.name,
-				  "Remote RTCP address switched to %s",
+				  "Remote RTCP address switched to predicted "
+				  "address %s",
 				  pj_sockaddr_print(&tp_ice->remote_rtcp, 
 						    addr_text,
 						    sizeof(addr_text), 3)));
@@ -1716,31 +1726,45 @@ static void ice_on_rx_data(pj_ice_strans *ice_st, unsigned comp_id,
 		}
 	    }
 	}
+
+	if (!discard)
+	    (*tp_ice->rtp_cb)(tp_ice->stream, pkt, size);
+
     } else if (comp_id==2 && tp_ice->rtcp_cb) {
-	(*tp_ice->rtcp_cb)(tp_ice->stream, pkt, size);
 
 	/* Check if RTCP source address is the same as the configured
 	 * remote address, and switch the address when they are
 	 * different.
 	 */
 	if (!tp_ice->use_ice &&
-	    pj_sockaddr_cmp(&tp_ice->remote_rtcp, src_addr) != 0)
+	    (tp_ice->options & PJMEDIA_ICE_NO_SRC_ADDR_CHECKING)==0)
 	{
-	    pj_sockaddr_cp(&tp_ice->rtcp_src_addr, src_addr);
-
-	    if ((tp_ice->options & PJMEDIA_ICE_NO_SRC_ADDR_CHECKING)==0) {
+	    if (pj_sockaddr_cmp(&tp_ice->remote_rtcp, src_addr) == 0) {
+		tp_ice->rtcp_src_cnt = 0;
+	    } else {
 		char addr_text[80];
 
-		pj_sockaddr_cp(&tp_ice->remote_rtcp, src_addr);
+		++tp_ice->rtcp_src_cnt;
+		if (tp_ice->rtcp_src_cnt < PJMEDIA_RTCP_NAT_PROBATION_CNT) {
+		    discard = PJ_TRUE;
+		} else {
+		    tp_ice->rtcp_src_cnt = 0;
+		    pj_sockaddr_cp(&tp_ice->rtcp_src_addr, src_addr);
+		    pj_sockaddr_cp(&tp_ice->remote_rtcp, src_addr);
 
-		pj_assert(tp_ice->addr_len == pj_sockaddr_get_len(src_addr));
+		    pj_assert(tp_ice->addr_len==pj_sockaddr_get_len(src_addr));
 
-		PJ_LOG(4,(tp_ice->base.name,
-			  "Remote RTCP address switched to %s",
-			  pj_sockaddr_print(&tp_ice->remote_rtcp, addr_text,
-					    sizeof(addr_text), 3)));
+		    PJ_LOG(4,(tp_ice->base.name,
+			      "Remote RTCP address switched to %s",
+			      pj_sockaddr_print(&tp_ice->remote_rtcp,
+			                        addr_text, sizeof(addr_text),
+			                        3)));
+		}
 	    }
 	}
+
+	if (!discard)
+	    (*tp_ice->rtcp_cb)(tp_ice->stream, pkt, size);
     }
 
     PJ_UNUSED_ARG(src_addr_len);
diff --git a/pjmedia/src/pjmedia/transport_udp.c b/pjmedia/src/pjmedia/transport_udp.c
index 59d2dc70..63178599 100644
--- a/pjmedia/src/pjmedia/transport_udp.c
+++ b/pjmedia/src/pjmedia/transport_udp.c
@@ -83,6 +83,7 @@ struct transport_udp
     pj_sock_t		rtcp_sock;	/**< RTCP socket		    */
     pj_sockaddr		rtcp_addr_name;	/**< Published RTCP address.	    */
     pj_sockaddr		rtcp_src_addr;	/**< Actual source RTCP address.    */
+    unsigned		rtcp_src_cnt;	/**< How many pkt from this addr.   */
     int			rtcp_addr_len;	/**< Length of RTCP src address.    */
     pj_ioqueue_key_t   *rtcp_key;	/**< RTCP socket key in ioqueue	    */
     pj_ioqueue_op_key_t rtcp_read_op;	/**< Pending read operation	    */
@@ -452,6 +453,7 @@ static void on_rx_rtp( pj_ioqueue_key_t *key,
     do {
 	void (*cb)(void*,void*,pj_ssize_t);
 	void *user_data;
+	pj_bool_t discard = PJ_FALSE;
 
 	cb = udp->rtp_cb;
 	user_data = udp->user_data;
@@ -462,14 +464,10 @@ static void on_rx_rtp( pj_ioqueue_key_t *key,
 		PJ_LOG(5,(udp->base.name, 
 			  "RX RTP packet dropped because of pkt lost "
 			  "simulation"));
-		goto read_next_packet;
+		discard = PJ_TRUE;
 	    }
 	}
 
-
-	if (udp->attached && cb)
-	    (*cb)(user_data, udp->rtp_pkt, bytes_read);
-
 	/* See if source address of RTP packet is different than the 
 	 * configured address, and switch RTP remote address to 
 	 * source packet address after several consecutive packets
@@ -478,11 +476,15 @@ static void on_rx_rtp( pj_ioqueue_key_t *key,
 	if (bytes_read>0 && 
 	    (udp->options & PJMEDIA_UDP_NO_SRC_ADDR_CHECKING)==0) 
 	{
-	    if (pj_sockaddr_cmp(&udp->rem_rtp_addr, &udp->rtp_src_addr) != 0) {
-
+	    if (pj_sockaddr_cmp(&udp->rem_rtp_addr, &udp->rtp_src_addr) == 0) {
+		/* We're still receiving from rem_rtp_addr. Don't switch. */
+		udp->rtp_src_cnt = 0;
+	    } else {
 		udp->rtp_src_cnt++;
 
-		if (udp->rtp_src_cnt >= PJMEDIA_RTP_NAT_PROBATION_CNT) {
+		if (udp->rtp_src_cnt < PJMEDIA_RTP_NAT_PROBATION_CNT) {
+		    discard = PJ_TRUE;
+		} else {
 		
 		    char addr_text[80];
 
@@ -516,7 +518,8 @@ static void on_rx_rtp( pj_ioqueue_key_t *key,
 				  sizeof(pj_sockaddr));
 
 			PJ_LOG(4,(udp->base.name,
-				  "Remote RTCP address switched to %s",
+				  "Remote RTCP address switched to predicted"
+				  " address %s",
 				  pj_sockaddr_print(&udp->rtcp_src_addr, 
 						    addr_text,
 						    sizeof(addr_text), 3)));
@@ -526,7 +529,9 @@ static void on_rx_rtp( pj_ioqueue_key_t *key,
 	    }
 	}
 
-read_next_packet:
+	if (!discard && udp->attached && cb)
+	    (*cb)(user_data, udp->rtp_pkt, bytes_read);
+
 	bytes_read = sizeof(udp->rtp_pkt);
 	udp->rtp_addrlen = sizeof(udp->rtp_src_addr);
 	status = pj_ioqueue_recvfrom(udp->rtp_key, &udp->rtp_read_op,
@@ -568,18 +573,27 @@ static void on_rx_rtcp(pj_ioqueue_key_t *key,
 	 * different.
 	 */
 	if (bytes_read>0 &&
-	    (udp->options & PJMEDIA_UDP_NO_SRC_ADDR_CHECKING)==0 &&
-	    pj_sockaddr_cmp(&udp->rem_rtcp_addr, &udp->rtcp_src_addr) != 0)
+	    (udp->options & PJMEDIA_UDP_NO_SRC_ADDR_CHECKING)==0)
 	{
-	    char addr_text[80];
+	    if (pj_sockaddr_cmp(&udp->rem_rtcp_addr, &udp->rtcp_src_addr) == 0) {
+		/* Still receiving from rem_rtcp_addr, don't switch */
+		udp->rtcp_src_cnt = 0;
+	    } else {
+		++udp->rtcp_src_cnt;
 
-	    pj_memcpy(&udp->rem_rtcp_addr, &udp->rtcp_src_addr,
-		      sizeof(pj_sockaddr));
+		if (udp->rtcp_src_cnt >= PJMEDIA_RTCP_NAT_PROBATION_CNT	) {
+		    char addr_text[80];
+
+		    udp->rtcp_src_cnt = 0;
+		    pj_memcpy(&udp->rem_rtcp_addr, &udp->rtcp_src_addr,
+			      sizeof(pj_sockaddr));
 
-	    PJ_LOG(4,(udp->base.name,
-		      "Remote RTCP address switched to %s",
-		      pj_sockaddr_print(&udp->rtcp_src_addr, addr_text,
-					sizeof(addr_text), 3)));
+		    PJ_LOG(4,(udp->base.name,
+			      "Remote RTCP address switched to %s",
+			      pj_sockaddr_print(&udp->rtcp_src_addr, addr_text,
+						sizeof(addr_text), 3)));
+		}
+	    }
 	}
 
 	bytes_read = sizeof(udp->rtcp_pkt);
@@ -677,6 +691,7 @@ static pj_status_t transport_attach(   pjmedia_transport *tp,
     pj_bzero(&udp->rtp_src_addr, sizeof(udp->rtp_src_addr));
     pj_bzero(&udp->rtcp_src_addr, sizeof(udp->rtcp_src_addr));
     udp->rtp_src_cnt = 0;
+    udp->rtcp_src_cnt = 0;
 
     /* Unlock keys */
     pj_ioqueue_unlock_key(udp->rtcp_key);