Ticket #467: fixed issues with RTP/AVP vs RTP/SAVP negotiation

git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@1810 74dad513-b988-da41-8d7b-12977e46ad98

8 files changed, 195 insertions, 36 deletions

diff --git a/pjmedia/include/pjmedia/sdp.h b/pjmedia/include/pjmedia/sdp.h
index 8e284256..768af9f1 100644
--- a/pjmedia/include/pjmedia/sdp.h
+++ b/pjmedia/include/pjmedia/sdp.h
@@ -493,6 +493,30 @@ PJ_DECL(pj_status_t) pjmedia_sdp_media_cmp(const pjmedia_sdp_media *sd1,
 					   unsigned option);
 
 
+/**
+ * Compare two media transports for compatibility.
+ *
+ * @param t1	    The first media transport to compare.
+ * @param t2	    The second media transport to compare.
+ *
+ * @return	    PJ_SUCCESS when both media transports are compatible,
+ *		    otherwise returns PJMEDIA_SDP_ETPORTNOTEQUAL.
+ */
+PJ_DECL(pj_status_t) pjmedia_sdp_transport_cmp(const pj_str_t *t1,
+					       const pj_str_t *t2);
+
+
+/**
+ * Deactivate SDP media.
+ *
+ * @param m	    The SDP media to deactivate.
+ *
+ * @return	    PJ_SUCCESS when SDP media successfully deactivated,
+ *		    otherwise appropriate status code returned.
+ */
+PJ_DECL(pj_status_t) pjmedia_sdp_media_deactivate(pj_pool_t *pool,
+						  pjmedia_sdp_media *m);
+
 
 /* **************************************************************************
  * SDP SESSION DESCRIPTION
diff --git a/pjmedia/include/pjmedia/transport.h b/pjmedia/include/pjmedia/transport.h
index b2b16d5e..2629cf87 100644
--- a/pjmedia/include/pjmedia/transport.h
+++ b/pjmedia/include/pjmedia/transport.h
@@ -212,6 +212,21 @@ PJ_BEGIN_DECL
  */
 typedef struct pjmedia_transport pjmedia_transport;
 
+/**
+ * This enumeration specifies the general behaviour of media processing
+ */
+typedef enum pjmedia_tranport_media_option
+{
+    /**
+     * When this flag is specified, the transport will not perform media
+     * transport validation, this is useful when transport is stacked with
+     * other transport, for example when transport UDP is stacked under
+     * transport SRTP, media transport validation only need to be done by 
+     * transport SRTP.
+     */
+    PJMEDIA_TPMED_NO_TRANSPORT_CHECKING = 1
+
+} pjmedia_tranport_media_option;
 
 /**
  * This structure describes the operations for the stream transport.
@@ -292,6 +307,7 @@ struct pjmedia_transport_op
      */
     pj_status_t (*media_create)(pjmedia_transport *tp,
 				pj_pool_t *pool,
+				unsigned options,
 				pjmedia_sdp_session *sdp_local,
 				const pjmedia_sdp_session *sdp_remote,
 				unsigned media_index);
@@ -508,6 +524,7 @@ PJ_INLINE(pj_status_t) pjmedia_transport_send_rtcp(pjmedia_transport *tp,
  *
  * @param tp		The media transport.
  * @param pool		The memory pool.
+ * @param option	Option flags, from #pjmedia_tranport_media_option
  * @param sdp_local	Local SDP.
  * @param sdp_remote	Remote SDP.
  * @param media_index	Media index in SDP.
@@ -516,11 +533,12 @@ PJ_INLINE(pj_status_t) pjmedia_transport_send_rtcp(pjmedia_transport *tp,
  */
 PJ_INLINE(pj_status_t) pjmedia_transport_media_create(pjmedia_transport *tp,
 				    pj_pool_t *pool,
+				    unsigned options,
 				    pjmedia_sdp_session *sdp_local,
 				    const pjmedia_sdp_session *sdp_remote,
 				    unsigned media_index)
 {
-    return (*tp->op->media_create)(tp, pool, sdp_local, sdp_remote, 
+    return (*tp->op->media_create)(tp, pool, options, sdp_local, sdp_remote, 
 				   media_index);
 }
 
@@ -540,6 +558,7 @@ PJ_INLINE(pj_status_t) pjmedia_transport_media_create(pjmedia_transport *tp,
  *
  * @param tp		The media transport.
  * @param pool		The memory pool.
+ * @param option	The media transport option.
  * @param sdp_local	Local SDP.
  * @param sdp_remote	Remote SDP.
  * @param media_index	Media index to start.
diff --git a/pjmedia/src/pjmedia/sdp.c b/pjmedia/src/pjmedia/sdp.c
index be677292..a3f436f4 100644
--- a/pjmedia/src/pjmedia/sdp.c
+++ b/pjmedia/src/pjmedia/sdp.c
@@ -1281,4 +1281,37 @@ PJ_DEF(pj_status_t) pjmedia_sdp_validate(const pjmedia_sdp_session *sdp)
     return PJ_SUCCESS;
 }
 
+PJ_DEF(pj_status_t) pjmedia_sdp_transport_cmp( const pj_str_t *t1,
+					       const pj_str_t *t2)
+{
+    static const pj_str_t ID_RTP_AVP  = { "RTP/AVP", 7 };
+    static const pj_str_t ID_RTP_SAVP = { "RTP/SAVP", 8 };
+
+    /* Exactly equal? */
+    if (pj_stricmp(t1, t2) == 0)
+	return PJ_SUCCESS;
 
+    /* Compatible? */
+    if ((!pj_stricmp(t1, &ID_RTP_AVP) || !pj_stricmp(t1, &ID_RTP_SAVP)) &&
+        (!pj_stricmp(t2, &ID_RTP_AVP) || !pj_stricmp(t2, &ID_RTP_SAVP)))
+	return PJ_SUCCESS;
+
+    return PJMEDIA_SDP_ETPORTNOTEQUAL;
+}
+
+
+PJ_DEF(pj_status_t) pjmedia_sdp_media_deactivate(pj_pool_t *pool,
+						 pjmedia_sdp_media *m)
+{
+    pjmedia_sdp_attr *attr;
+    static const pj_str_t ID_INACTIVE = { "inactive", 8 };
+
+    attr = pjmedia_sdp_attr_create(pool, ID_INACTIVE.ptr, NULL);
+    if (NULL == attr)
+	return PJ_ENOMEM;
+
+    m->attr[m->attr_count++] = attr;
+    m->desc.port = 0;
+
+    return PJ_SUCCESS;
+}
diff --git a/pjmedia/src/pjmedia/sdp_neg.c b/pjmedia/src/pjmedia/sdp_neg.c
index 1b6c3e45..1a6d0a1d 100644
--- a/pjmedia/src/pjmedia/sdp_neg.c
+++ b/pjmedia/src/pjmedia/sdp_neg.c
@@ -479,15 +479,19 @@ static pj_status_t process_m_answer( pj_pool_t *pool,
     }
 
 
-    /* Chec that transport in the answer match our offer. */
+    /* Check that transport in the answer match our offer. */
 
-    if (pj_strcmp(&answer->desc.transport, 
-		  &offer->desc.transport)!=0) 
+    /* At this point, transport type must be compatible, 
+     * the transport instance will do more validation later.
+     */
+    if (pjmedia_sdp_transport_cmp(&answer->desc.transport, 
+				  &offer->desc.transport) 
+	!= PJ_SUCCESS)
     {
-	/* The transport in the answer is different than the offer! */
 	return PJMEDIA_SDPNEG_EINVANSTP;
     }
 
+
     /* Check if remote has rejected our offer */
     
     if (answer->desc.port == 0) {
diff --git a/pjmedia/src/pjmedia/session.c b/pjmedia/src/pjmedia/session.c
index 06b6b8c1..5b83bf0c 100644
--- a/pjmedia/src/pjmedia/session.c
+++ b/pjmedia/src/pjmedia/session.c
@@ -190,13 +190,13 @@ PJ_DEF(pj_status_t) pjmedia_stream_info_from_sdp(
 
     /* Transport protocol */
 
-    /* Transport type must be equal */
-    if (pj_stricmp(&rem_m->desc.transport, 
-		   &local_m->desc.transport) != 0) 
-    {
-	si->type = PJMEDIA_TYPE_UNKNOWN;
+    /* At this point, transport type must be compatible, 
+     * the transport instance will do more validation later.
+     */
+    status = pjmedia_sdp_transport_cmp(&rem_m->desc.transport, 
+				       &local_m->desc.transport);
+    if (status != PJ_SUCCESS)
 	return PJMEDIA_SDPNEG_EINVANSTP;
-    }
 
     if (pj_stricmp(&local_m->desc.transport, &ID_RTP_AVP) == 0) {
 
diff --git a/pjmedia/src/pjmedia/transport_ice.c b/pjmedia/src/pjmedia/transport_ice.c
index 6c3b1dcf..8a6124f3 100644
--- a/pjmedia/src/pjmedia/transport_ice.c
+++ b/pjmedia/src/pjmedia/transport_ice.c
@@ -24,11 +24,14 @@
 
 #define THIS_FILE   "transport_ice.c"
 
+static const pj_str_t ID_RTP_AVP  = { "RTP/AVP", 7 };
+
 struct transport_ice
 {
     pjmedia_transport	 base;
     pj_ice_strans	*ice_st;
     pjmedia_ice_cb	 cb;
+    unsigned		 media_option;
 
     pj_time_val		 start_ice;
     
@@ -74,6 +77,7 @@ static pj_status_t transport_send_rtcp(pjmedia_transport *tp,
 				       pj_size_t size);
 static pj_status_t transport_media_create(pjmedia_transport *tp,
 				       pj_pool_t *pool,
+				       unsigned options,
 				       pjmedia_sdp_session *sdp_local,
 				       const pjmedia_sdp_session *sdp_remote,
 				       unsigned media_index);
@@ -258,6 +262,7 @@ PJ_DEF(pj_status_t) pjmedia_ice_init_ice(pjmedia_transport *tp,
  */
 static pj_status_t transport_media_create(pjmedia_transport *tp,
 				       pj_pool_t *pool,
+				       unsigned options,
 				       pjmedia_sdp_session *sdp_local,
 				       const pjmedia_sdp_session *sdp_remote,
 				       unsigned media_index)
@@ -270,6 +275,24 @@ static pj_status_t transport_media_create(pjmedia_transport *tp,
     unsigned i, cand_cnt;
     pj_status_t status;
 
+    tp_ice->media_option = options;
+
+    /* Validate media transport */
+    /* By now, this transport only support RTP/AVP transport */
+    if ((tp_ice->media_option & PJMEDIA_TPMED_NO_TRANSPORT_CHECKING) == 0) {
+	pjmedia_sdp_media *m_rem, *m_loc;
+
+	m_rem = sdp_remote? sdp_remote->media[media_index] : NULL;
+	m_loc = sdp_local->media[media_index];
+
+	if (pj_stricmp(&m_loc->desc.transport, &ID_RTP_AVP) ||
+	   (m_rem && pj_stricmp(&m_rem->desc.transport, &ID_RTP_AVP)))
+	{
+	    pjmedia_sdp_media_deactivate(pool, m_loc);
+	    return PJMEDIA_SDP_EINPROTO;
+	}
+    }
+
     /* Init ICE */
     ice_role = (sdp_remote==NULL ? PJ_ICE_SESS_ROLE_CONTROLLING : 
 				   PJ_ICE_SESS_ROLE_CONTROLLED);
@@ -499,13 +522,27 @@ static pj_status_t transport_media_start(pjmedia_transport *tp,
     pj_str_t uname, pass;
     pj_status_t status;
 
-    PJ_UNUSED_ARG(sdp_local);
-
     PJ_ASSERT_RETURN(tp && pool && sdp_remote, PJ_EINVAL);
     PJ_ASSERT_RETURN(media_index < sdp_remote->media_count, PJ_EINVAL);
 
     sdp_med = sdp_remote->media[media_index];
 
+    /* Validate media transport */
+    /* By now, this transport only support RTP/AVP transport */
+    if ((tp_ice->media_option & PJMEDIA_TPMED_NO_TRANSPORT_CHECKING) == 0) {
+	pjmedia_sdp_media *m_rem, *m_loc;
+
+	m_rem = sdp_remote->media[media_index];
+	m_loc = sdp_local->media[media_index];
+
+	if (pj_stricmp(&m_loc->desc.transport, &ID_RTP_AVP) ||
+	   (pj_stricmp(&m_rem->desc.transport, &ID_RTP_AVP)))
+	{
+	    pjmedia_sdp_media_deactivate(pool, m_loc);
+	    return PJMEDIA_SDP_EINPROTO;
+	}
+    }
+
     /* Get the SDP connection for the media stream.
      * We'll verify later if the SDP connection address is specified 
      * as one of the candidate.
diff --git a/pjmedia/src/pjmedia/transport_srtp.c b/pjmedia/src/pjmedia/transport_srtp.c
index 1109325b..5dadb207 100644
--- a/pjmedia/src/pjmedia/transport_srtp.c
+++ b/pjmedia/src/pjmedia/transport_srtp.c
@@ -35,11 +35,7 @@
 /* Maximum size of packet */
 #define MAX_BUFFER_LEN	1500
 #define MAX_KEY_LEN	32
-#define DEACTIVATE_MEDIA(pool, m) {\
-	    attr = pjmedia_sdp_attr_create(pool, ID_INACTIVE.ptr, NULL); \
-	    m->attr[m->attr_count++] = attr; \
-	    m->desc.port = 0; \
-	}
+#define DEACTIVATE_MEDIA(pool, m) pjmedia_sdp_media_deactivate(pool, m)
 
 static const pj_str_t ID_RTP_AVP  = { "RTP/AVP", 7 };
 static const pj_str_t ID_RTP_SAVP = { "RTP/SAVP", 8 };
@@ -83,8 +79,9 @@ typedef struct transport_srtp
     pj_pool_t		*pool;
     pj_lock_t		*mutex;
     char		 tx_buffer[MAX_BUFFER_LEN];
-
     pjmedia_srtp_setting setting;
+    unsigned		 media_option;
+
     /* SRTP policy */
     pj_bool_t		 session_inited;
     pj_bool_t		 offerer_side;
@@ -150,6 +147,7 @@ static pj_status_t transport_send_rtcp(pjmedia_transport *tp,
 				       pj_size_t size);
 static pj_status_t transport_media_create(pjmedia_transport *tp,
 				       pj_pool_t *pool,
+				       unsigned options,
 				       pjmedia_sdp_session *sdp_local,
 				       const pjmedia_sdp_session *sdp_remote,
 				       unsigned media_index);
@@ -924,6 +922,7 @@ static pj_status_t parse_attr_crypto(pj_pool_t *pool,
 
 static pj_status_t transport_media_create(pjmedia_transport *tp,
 				          pj_pool_t *pool,
+					  unsigned options,
 				          pjmedia_sdp_session *sdp_local,
 				          const pjmedia_sdp_session *sdp_remote,
 					  unsigned media_index)
@@ -937,8 +936,12 @@ static pj_status_t transport_media_create(pjmedia_transport *tp,
     pjmedia_sdp_attr *attr;
     pj_str_t attr_value;
     int i, j;
+    unsigned member_tp_option;
 
     PJ_ASSERT_RETURN(tp && pool && sdp_local, PJ_EINVAL);
+    
+    srtp->media_option = options;
+    member_tp_option = options | PJMEDIA_TPMED_NO_TRANSPORT_CHECKING;
 
     pj_bzero(&srtp->rx_policy, sizeof(srtp->tx_policy));
     pj_bzero(&srtp->tx_policy, sizeof(srtp->rx_policy));
@@ -1130,10 +1133,12 @@ static pj_status_t transport_media_create(pjmedia_transport *tp,
 
 BYPASS_SRTP:
     srtp->bypass_srtp = PJ_TRUE;
+    member_tp_option &= ~PJMEDIA_TPMED_NO_TRANSPORT_CHECKING;
 
 PROPAGATE_MEDIA_CREATE:
-    return pjmedia_transport_media_create(srtp->real_tp, pool, sdp_local, 
-					   sdp_remote, media_index);
+    return pjmedia_transport_media_create(srtp->real_tp, pool, 
+			    member_tp_option,
+			    sdp_local, sdp_remote, media_index);
 }
 
 
@@ -1147,7 +1152,6 @@ static pj_status_t transport_media_start(pjmedia_transport *tp,
     struct transport_srtp *srtp = (struct transport_srtp*) tp;
     pjmedia_sdp_media *m_rem, *m_loc;
     pj_status_t status;
-    pjmedia_sdp_attr *attr;
     int i;
 
     PJ_ASSERT_RETURN(tp && pool && sdp_local && sdp_remote, PJ_EINVAL);
@@ -1171,10 +1175,14 @@ static pj_status_t transport_media_start(pjmedia_transport *tp,
 	    }
 	    goto BYPASS_SRTP;
 	} else if (srtp->setting.use == PJMEDIA_SRTP_OPTIONAL) {
-	    if (pj_stricmp(&m_rem->desc.transport, &m_loc->desc.transport)) {
-		DEACTIVATE_MEDIA(pool, m_loc);
-		return PJMEDIA_SDP_EINPROTO;
-	    }
+	    // Regardless the answer's transport type (RTP/AVP or RTP/SAVP),
+	    // the answer must be processed through in optional mode.
+	    // Please note that at this point transport type is ensured to be 
+	    // RTP/AVP or RTP/SAVP, see transport_media_create()
+	    //if (pj_stricmp(&m_rem->desc.transport, &m_loc->desc.transport)) {
+		//DEACTIVATE_MEDIA(pool, m_loc);
+		//return PJMEDIA_SDP_EINPROTO;
+	    //}
 	} else if (srtp->setting.use == PJMEDIA_SRTP_MANDATORY) {
 	    if (pj_stricmp(&m_rem->desc.transport, &ID_RTP_SAVP)) {
 		DEACTIVATE_MEDIA(pool, m_loc);
diff --git a/pjmedia/src/pjmedia/transport_udp.c b/pjmedia/src/pjmedia/transport_udp.c
index b86151ba..a341d44a 100644
--- a/pjmedia/src/pjmedia/transport_udp.c
+++ b/pjmedia/src/pjmedia/transport_udp.c
@@ -35,6 +35,8 @@
 /* Maximum pending write operations */
 #define MAX_PENDING 4
 
+static const pj_str_t ID_RTP_AVP  = { "RTP/AVP", 7 };
+
 /* Pending write buffer */
 typedef struct pending_write
 {
@@ -49,6 +51,7 @@ struct transport_udp
 
     pj_pool_t	       *pool;		/**< Memory pool		    */
     unsigned		options;	/**< Transport options.		    */
+    unsigned		media_options;	/**< Transport media options.	    */
     void	       *user_data;	/**< Only valid when attached	    */
     pj_bool_t		attached;	/**< Has attachment?		    */
     pj_sockaddr		rem_rtp_addr;	/**< Remote RTP address		    */
@@ -120,6 +123,7 @@ static pj_status_t transport_send_rtcp(pjmedia_transport *tp,
 				       pj_size_t size);
 static pj_status_t transport_media_create(pjmedia_transport *tp,
 				       pj_pool_t *pool,
+				       unsigned options,
 				       pjmedia_sdp_session *sdp_local,
 				       const pjmedia_sdp_session *sdp_remote,
 				       unsigned media_index);
@@ -764,15 +768,31 @@ static pj_status_t transport_send_rtcp(pjmedia_transport *tp,
 
 static pj_status_t transport_media_create(pjmedia_transport *tp,
 				  pj_pool_t *pool,
+				  unsigned options,
 				  pjmedia_sdp_session *sdp_local,
 				  const pjmedia_sdp_session *sdp_remote,
 				  unsigned media_index)
 {
-    PJ_UNUSED_ARG(tp);
-    PJ_UNUSED_ARG(pool);
-    PJ_UNUSED_ARG(sdp_local);
-    PJ_UNUSED_ARG(sdp_remote);
-    PJ_UNUSED_ARG(media_index);
+    struct transport_udp *udp = (struct transport_udp*)tp;
+
+    PJ_ASSERT_RETURN(tp && pool && sdp_local, PJ_EINVAL);
+    udp->media_options = options;
+
+    /* Validate media transport */
+    /* By now, this transport only support RTP/AVP transport */
+    if ((udp->media_options & PJMEDIA_TPMED_NO_TRANSPORT_CHECKING) == 0) {
+	pjmedia_sdp_media *m_rem, *m_loc;
+
+	m_rem = sdp_remote? sdp_remote->media[media_index] : NULL;
+	m_loc = sdp_local->media[media_index];
+
+	if (pj_stricmp(&m_loc->desc.transport, &ID_RTP_AVP) ||
+	   (m_rem && pj_stricmp(&m_rem->desc.transport, &ID_RTP_AVP)))
+	{
+	    pjmedia_sdp_media_deactivate(pool, m_loc);
+	    return PJMEDIA_SDP_EINPROTO;
+	}
+    }
 
     return PJ_SUCCESS;
 }
@@ -783,11 +803,25 @@ static pj_status_t transport_media_start(pjmedia_transport *tp,
 				  const pjmedia_sdp_session *sdp_remote,
 				  unsigned media_index)
 {
-    PJ_UNUSED_ARG(tp);
-    PJ_UNUSED_ARG(pool);
-    PJ_UNUSED_ARG(sdp_local);
-    PJ_UNUSED_ARG(sdp_remote);
-    PJ_UNUSED_ARG(media_index);
+    struct transport_udp *udp = (struct transport_udp*)tp;
+
+    PJ_ASSERT_RETURN(tp && pool && sdp_local, PJ_EINVAL);
+
+    /* Validate media transport */
+    /* By now, this transport only support RTP/AVP transport */
+    if ((udp->media_options & PJMEDIA_TPMED_NO_TRANSPORT_CHECKING) == 0) {
+	pjmedia_sdp_media *m_rem, *m_loc;
+
+	m_rem = sdp_remote->media[media_index];
+	m_loc = sdp_local->media[media_index];
+
+	if (pj_stricmp(&m_loc->desc.transport, &ID_RTP_AVP) ||
+	    pj_stricmp(&m_rem->desc.transport, &ID_RTP_AVP))
+	{
+	    pjmedia_sdp_media_deactivate(pool, m_loc);
+	    return PJMEDIA_SDP_EINPROTO;
+	}
+    }
 
     return PJ_SUCCESS;
 }