Re #668:

 - Fixed process_answer() of SDP negotiation, when no common format in a media, instead of returning error, it should just deactivate the media (offer & answer) and continue negotiating next media.
 - Generalized the way of deactivating media: set port to 0 and remove all attributes.
 - Added new API pjmedia_sdp_media_clone_deactivate() to clone media and deactivate the newly cloned media.
 - Updated PJMEDIA SDP negotiation test.



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

5 files changed, 89 insertions, 54 deletions

diff --git a/pjmedia/include/pjmedia/sdp.h b/pjmedia/include/pjmedia/sdp.h
index 433e3e53..c8eb902c 100644
--- a/pjmedia/include/pjmedia/sdp.h
+++ b/pjmedia/include/pjmedia/sdp.h
@@ -531,6 +531,18 @@ PJ_DECL(pj_status_t) pjmedia_sdp_media_deactivate(pj_pool_t *pool,
 						  pjmedia_sdp_media *m);
 
 
+/**
+ * Clone SDP media description and deactivate the new SDP media.
+ *
+ * @param rhs	    The SDP media to clone.
+ *
+ * @return	    New media descrption with deactivated indication.
+ */
+PJ_DECL(pjmedia_sdp_media*) pjmedia_sdp_media_clone_deactivate(
+						pj_pool_t *pool,
+						const pjmedia_sdp_media *rhs);
+
+
 /* **************************************************************************
  * SDP SESSION DESCRIPTION
  ****************************************************************************
diff --git a/pjmedia/src/pjmedia/sdp.c b/pjmedia/src/pjmedia/sdp.c
index 139630fc..f1d0f36c 100644
--- a/pjmedia/src/pjmedia/sdp.c
+++ b/pjmedia/src/pjmedia/sdp.c
@@ -1430,18 +1430,44 @@ PJ_DEF(pj_status_t) pjmedia_sdp_transport_cmp( const pj_str_t *t1,
 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 };
-
-    if (m->attr_count >= PJMEDIA_MAX_SDP_ATTR)
-	return PJ_ETOOMANY;
+    PJ_ASSERT_RETURN(m, PJ_EINVAL);
+    PJ_UNUSED_ARG(pool);
 
-    attr = pjmedia_sdp_attr_create(pool, ID_INACTIVE.ptr, NULL);
-    if (NULL == attr)
-	return PJ_ENOMEM;
-
-    pjmedia_sdp_media_add_attr(m, attr);
+    /* Set port to zero */
     m->desc.port = 0;
 
+    /* And remove attributes */
+    m->attr_count = 0;
+
     return PJ_SUCCESS;
 }
+
+
+PJ_DEF(pjmedia_sdp_media*) pjmedia_sdp_media_clone_deactivate(
+						pj_pool_t *pool,
+						const pjmedia_sdp_media *rhs)
+{
+    unsigned int i;
+    pjmedia_sdp_media *m;
+
+    PJ_ASSERT_RETURN(pool && rhs, NULL);
+
+    m = PJ_POOL_ZALLOC_T(pool, pjmedia_sdp_media);
+    pj_memcpy(m, rhs, sizeof(*m));
+
+    /* Clone the media line only */
+    pj_strdup (pool, &m->desc.media, &rhs->desc.media);
+    pj_strdup (pool, &m->desc.transport, &rhs->desc.transport);
+    for (i=0; i<rhs->desc.fmt_count; ++i)
+	pj_strdup(pool, &m->desc.fmt[i], &rhs->desc.fmt[i]);
+
+    if (rhs->conn) {
+	m->conn = pjmedia_sdp_conn_clone (pool, rhs->conn);
+	PJ_ASSERT_RETURN(m->conn != NULL, NULL);
+    }
+
+    /* And deactivate it */
+    pjmedia_sdp_media_deactivate(pool, m);
+
+    return m;
+}
diff --git a/pjmedia/src/pjmedia/sdp_cmp.c b/pjmedia/src/pjmedia/sdp_cmp.c
index ac5fde3c..cab7eac0 100644
--- a/pjmedia/src/pjmedia/sdp_cmp.c
+++ b/pjmedia/src/pjmedia/sdp_cmp.c
@@ -183,6 +183,10 @@ PJ_DEF(pj_status_t) pjmedia_sdp_media_cmp( const pjmedia_sdp_media *sd1,
     if (pj_strcmp(&sd1->desc.transport, &sd2->desc.transport) != 0)
 	return PJMEDIA_SDP_ETPORTNOTEQUAL;
 
+    /* For zeroed port media, stop comparing here */
+    if (sd1->desc.port == 0)
+	return PJ_SUCCESS;
+
     /* Compare number of formats. */
     if (sd1->desc.fmt_count != sd2->desc.fmt_count)
 	return PJMEDIA_SDP_EFORMATNOTEQUAL;
diff --git a/pjmedia/src/pjmedia/sdp_neg.c b/pjmedia/src/pjmedia/sdp_neg.c
index 8c33468a..3e6039ec 100644
--- a/pjmedia/src/pjmedia/sdp_neg.c
+++ b/pjmedia/src/pjmedia/sdp_neg.c
@@ -311,8 +311,7 @@ PJ_DEF(pj_status_t) pjmedia_sdp_neg_modify_local_offer( pj_pool_t *pool,
 	if (!found) {
 	    pjmedia_sdp_media *m;
 
-	    m = pjmedia_sdp_media_clone(pool, om);
-	    m->desc.port = 0;
+	    m = pjmedia_sdp_media_clone_deactivate(pool, om);
 
 	    pj_array_insert(new_offer->media, sizeof(new_offer->media[0]),
 			    new_offer->media_count++, oi, &m);
@@ -776,9 +775,9 @@ static pj_status_t process_m_answer( pj_pool_t *pool,
     if (answer->desc.port == 0) {
 	
 	/* Remote has rejected our offer. 
-	 * Set our port to zero too in active SDP.
+	 * Deactivate our media too.
 	 */
-	offer->desc.port = 0;
+	pjmedia_sdp_media_deactivate(pool, offer);
 
 	/* Don't need to proceed */
 	return PJ_SUCCESS;
@@ -1011,13 +1010,14 @@ static pj_status_t process_answer(pj_pool_t *pool,
 	    pjmedia_sdp_media *am;
 
 	    /* Generate matching-but-disabled-media for the answer */
-	    am = pjmedia_sdp_media_clone(pool, offer->media[omi]);
-	    am->desc.port = 0;
+	    am = pjmedia_sdp_media_clone_deactivate(pool, offer->media[omi]);
 	    answer->media[answer->media_count++] = am;
 	    ++ami;
 
+	    /* Deactivate our media offer too */
+	    pjmedia_sdp_media_deactivate(pool, offer->media[omi]);
+
 	    /* No answer media to be negotiated */
-	    offer->media[omi]->desc.port = 0;
 	    continue;
 	}
 
@@ -1026,12 +1026,18 @@ static pj_status_t process_answer(pj_pool_t *pool,
 
 	/* If media type is mismatched, just disable the media. */
 	if (status == PJMEDIA_SDPNEG_EINVANSMEDIA) {
-	    offer->media[omi]->desc.port = 0;
+	    pjmedia_sdp_media_deactivate(pool, offer->media[omi]);
 	    continue;
 	}
-
-	if (status != PJ_SUCCESS)
+	/* No common format in the answer media. */
+	else if (status == PJMEDIA_SDPNEG_EANSNOMEDIA) {
+	    pjmedia_sdp_media_deactivate(pool, offer->media[omi]);
+	    pjmedia_sdp_media_deactivate(pool, answer->media[ami]);
+	} 
+	/* Return the error code, for other errors. */
+	else if (status != PJ_SUCCESS) {
 	    return status;
+	}
 
 	if (offer->media[omi]->desc.port != 0)
 	    has_active = PJ_TRUE;
@@ -1064,11 +1070,9 @@ static pj_status_t match_offer(pj_pool_t *pool,
     const pjmedia_sdp_media *master, *slave;
     pj_str_t pt_amr_need_adapt = {NULL, 0};
 
-    /* If offer has zero port, just clone the offer and update direction */
+    /* If offer has zero port, just clone the offer */
     if (offer->desc.port == 0) {
-	answer = pjmedia_sdp_media_clone(pool, offer);
-	remove_all_media_directions(answer);
-	update_media_direction(pool, offer, answer);
+	answer = pjmedia_sdp_media_clone_deactivate(pool, offer);
 	*p_answer = answer;
 	return PJ_SUCCESS;
     }
@@ -1356,25 +1360,12 @@ static pj_status_t create_answer( pj_pool_t *pool,
 	    /* No matching media.
 	     * Reject the offer by setting the port to zero in the answer.
 	     */
-	    //pjmedia_sdp_attr *a;
-
 	    /* For simplicity in the construction of the answer, we'll
 	     * just clone the media from the offer. Anyway receiver will
 	     * ignore anything in the media once it sees that the port
 	     * number is zero.
 	     */
-	    am = pjmedia_sdp_media_clone(pool, om);
-	    am->desc.port = 0;
-
-	    // Just set port zero to disable stream without set it to inactive.
-	    /* Remove direction attribute, and replace with inactive */
-	    remove_all_media_directions(am);
-	    //a = pjmedia_sdp_attr_create(pool, "inactive", NULL);
-	    //pjmedia_sdp_media_add_attr(am, a);
-
-	    /* Then update direction */
-	    update_media_direction(pool, om, am);
-
+	    am = pjmedia_sdp_media_clone_deactivate(pool, om);
 	} else {
 	    /* The answer is in am */
 	    pj_assert(am != NULL);
@@ -1438,7 +1429,6 @@ PJ_DEF(pj_status_t) pjmedia_sdp_neg_negotiate( pj_pool_t *pool,
 	    /* Only update active SDPs when negotiation is successfull */
 	    neg->active_local_sdp = active;
 	    neg->active_remote_sdp = neg->neg_remote_sdp;
-
 	}
     } else {
 	pjmedia_sdp_session *answer = NULL;
diff --git a/pjmedia/src/test/sdp_neg_test.c b/pjmedia/src/test/sdp_neg_test.c
index 87317156..72d29963 100644
--- a/pjmedia/src/test/sdp_neg_test.c
+++ b/pjmedia/src/test/sdp_neg_test.c
@@ -96,7 +96,7 @@ static struct test
 	    "m=audio 49170 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=video 0 RTP/AVP 31\r\n"
-	    "a=rtpmap:31 H261/90000\r\n"
+	    //"a=rtpmap:31 H261/90000\r\n"	/* <-- this is not necessary (port 0) */
 	    "m=video 53000 RTP/AVP 32\r\n"
 	    "a=rtpmap:32 MPV/90000\r\n"
 	  },
@@ -129,12 +129,13 @@ static struct test
 	    "m=audio 49170 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=video 0 RTP/AVP 31\r\n"
-	    //"a=rtpmap:31 H261/90000\r\n"	/* <-- this is not necessary */
+	    //"a=rtpmap:31 H261/90000\r\n"	/* <-- this is not necessary (port 0) */
 	    "m=video 53000 RTP/AVP 32\r\n"
 	    "a=rtpmap:32 MPV/90000\r\n"
 	    "m=audio 0 RTP/AVP 110\r\n"
-	    "a=rtpmap:110 telephone-events/8000\r\n"
-	    "a=sendonly\r\n"
+	    /* <-- the following attributes are not necessary (port 0) */
+	    //"a=rtpmap:110 telephone-events/8000\r\n"
+	    //"a=sendonly\r\n"
 	  }
 	}
     },
@@ -555,8 +556,10 @@ static struct test
 	    "m=audio 49170 RTP/AVP 0 8\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "a=rtpmap:8 PCMA/8000\r\n"
-	    "m=video 0 RTP/AVP 31\r\n"
-	    "a=rtpmap:31 H261/90000\r\n"
+	    // By #1088, the formats won't be negotiated when the media has port 0.
+	    //"m=video 0 RTP/AVP 31\r\n"
+	    "m=video 0 RTP/AVP 31 32\r\n"
+	    //"a=rtpmap:31 H261/90000\r\n"  /* <-- this is not necessary (port 0) */
 	  },
 	  {
 	    LOCAL_OFFER,
@@ -589,7 +592,7 @@ static struct test
 	    "m=audio 51372 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=video 0 RTP/AVP 31\r\n"
-	    "a=rtpmap:31 H261/90000\r\n"
+	    //"a=rtpmap:31 H261/90000\r\n"  /* <-- this is not necessary (port 0) */
 	  }
 	}
     },
@@ -665,7 +668,7 @@ static struct test
 	    "m=audio 49172 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=video 0 RTP/AVP 31\r\n"
-	    "a=rtpmap:31 H261/90000\r\n"
+	    //"a=rtpmap:31 H261/90000\r\n"  /* <-- this is not necessary (port 0) */
 	  }
 	}
     },
@@ -816,7 +819,7 @@ static struct test
 	    "c=IN IP4 host.atlanta.example.com\r\n"
 	    "t=0 0\r\n"
 	    "m=audio 0 RTP/AVP 0\r\n"
-	    "a=rtpmap:0 PCMU/8000\r\n"
+	    //"a=rtpmap:0 PCMU/8000\r\n"	  /* <-- this is not necessary (port 0) */
 	    "m=audio 51372 RTP/AVP 97 101\r\n"
 	    "a=rtpmap:97 iLBC/8000\r\n"
 	    "a=rtpmap:101 telephone-event/8000\r\n"
@@ -870,7 +873,7 @@ static struct test
 	    "c=IN IP4 host.biloxi.example.com\r\n"
 	    "t=0 0\r\n"
 	    "m=audio 0 RTP/AVP 0\r\n"
-	    "a=rtpmap:0 PCMU/8000\r\n"
+	    //"a=rtpmap:0 PCMU/8000\r\n"      /* <-- this is not necessary (port 0) */
 	    "m=audio 49170 RTP/AVP 97 101\r\n"
 	    "a=rtpmap:97 iLBC/8000\r\n"
 	    "a=rtpmap:101 telephone-event/8000\r\n",
@@ -916,7 +919,7 @@ static struct test
 	    "m=audio 49170 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=video 0 RTP/AVP 31\r\n"
-	    "a=rtpmap:31 H261/90000\r\n"
+	    //"a=rtpmap:31 H261/90000\r\n"    /* <-- this is not necessary (port 0) */
 	    "",
 	  }
 	}
@@ -962,7 +965,7 @@ static struct test
 	    "m=audio 3000 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=video 0 RTP/AVP 31\r\n"
-	    "a=rtpmap:31 H261/90000\r\n"
+	    //"a=rtpmap:31 H261/90000\r\n"    /* <-- this is not necessary (port 0) */
 	    "",
 	  }
 	}
@@ -1052,9 +1055,9 @@ static struct test
 	    "m=audio 5200 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=audio 0 RTP/AVP 0\r\n"
-	    "a=rtpmap:0 PCMU/8000\r\n"
+	    //"a=rtpmap:0 PCMU/8000\r\n"      /* <-- this is not necessary (port 0) */
 	    "m=video 0 RTP/AVP 31\r\n"
-	    "a=rtpmap:31 H261/90000\r\n"
+	    //"a=rtpmap:31 H261/90000\r\n"    /* <-- this is not necessary (port 0) */
 	    "",
 	  }
 	}
@@ -1230,7 +1233,7 @@ static struct test
 	    "m=audio 7000 RTP/AVP 0\r\n"
 	    "a=rtpmap:0 PCMU/8000\r\n"
 	    "m=audio 0 RTP/AVP 0\r\n"
-	    "a=rtpmap:0 PCMU/8000\r\n"
+	    //"a=rtpmap:0 PCMU/8000\r\n"      /* <-- this is not necessary (port 0) */
 	    "m=video 5000 RTP/AVP 31\r\n"
 	    "a=rtpmap:31 H261/90000\r\n"
 	    "",