From 01483432d459b7fe5637929328a701e0c3f4fdd8 Mon Sep 17 00:00:00 2001 From: Nanang Izzuddin Date: Fri, 4 Jun 2010 13:41:34 +0000 Subject: 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 --- pjmedia/src/pjmedia/sdp.c | 46 ++++++++++++++++++++++++++++++--------- pjmedia/src/pjmedia/sdp_cmp.c | 4 ++++ pjmedia/src/pjmedia/sdp_neg.c | 48 ++++++++++++++++------------------------- pjmedia/src/test/sdp_neg_test.c | 33 +++++++++++++++------------- 4 files changed, 77 insertions(+), 54 deletions(-) (limited to 'pjmedia/src') 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; idesc.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" "", -- cgit v1.2.3