From cf3c435e0093cf2f1fb8add2298ee468f8c6afc5 Mon Sep 17 00:00:00 2001 From: Benny Prijono Date: Mon, 25 Sep 2006 13:40:12 +0000 Subject: Tests with other user agents revealed some bugs which have been fixed below: - some UAs sends "telephone-event/8000/1" instead of "telephone-event/8000", which caused SDP negotiation to fail. Fixed in sdp_neg.c. - codec name was (incorrectly) compared case-sensitively, causing negotiation to fail. Fixed in sdp_neg.c. - Also improved error reporting in SDP negotiation by introducing few more error codes. - Added Warning header in Not Acceptable response sent by pjsip_inv_session when SDP negotiation fails. - PJSUA-LIB will try to negotiate both SDPs before sending 100 response. - Fixed bug in iLBC codec when setting the mode to 30. Also: - Echo cancellation by default is disabled now since it doesn't seem to work. Further investigation needed. git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@738 74dad513-b988-da41-8d7b-12977e46ad98 --- pjmedia/include/pjmedia/errno.h | 15 ++++++++++++ pjmedia/src/pjmedia-codec/ilbc.c | 2 +- pjmedia/src/pjmedia/errno.c | 3 +++ pjmedia/src/pjmedia/sdp_neg.c | 49 +++++++++++++++++++++------------------- 4 files changed, 45 insertions(+), 24 deletions(-) (limited to 'pjmedia') diff --git a/pjmedia/include/pjmedia/errno.h b/pjmedia/include/pjmedia/errno.h index 03608f9f..af3467ed 100644 --- a/pjmedia/include/pjmedia/errno.h +++ b/pjmedia/include/pjmedia/errno.h @@ -198,6 +198,21 @@ PJ_BEGIN_DECL * No media is active after negotiation. */ #define PJMEDIA_SDPNEG_ENOMEDIA (PJMEDIA_ERRNO_START+48) /* 220048 */ +/** + * @hideinitializer + * No suitable codec for remote offer. + */ +#define PJMEDIA_SDPNEG_NOANSCODEC (PJMEDIA_ERRNO_START+49) /* 220049 */ +/** + * @hideinitializer + * No suitable telephone-event for remote offer. + */ +#define PJMEDIA_SDPNEG_NOANSTELEVENT (PJMEDIA_ERRNO_START+50) /* 220050 */ +/** + * @hideinitializer + * No suitable answer for unknown remote offer. + */ +#define PJMEDIA_SDPNEG_NOANSUNKNOWN (PJMEDIA_ERRNO_START+51) /* 220051 */ /************************************************************ diff --git a/pjmedia/src/pjmedia-codec/ilbc.c b/pjmedia/src/pjmedia-codec/ilbc.c index c6ec700a..da4bcec0 100644 --- a/pjmedia/src/pjmedia-codec/ilbc.c +++ b/pjmedia/src/pjmedia-codec/ilbc.c @@ -395,7 +395,7 @@ static pj_status_t ilbc_codec_open(pjmedia_codec *codec, attr->setting.penh); if (attr->setting.dec_fmtp_mode == 20) ilbc_codec->dec_frame_size = 38; - else if (attr->setting.dec_fmtp_mode == 20) + else if (attr->setting.dec_fmtp_mode == 30) ilbc_codec->dec_frame_size = 50; else { pj_assert(!"Invalid iLBC mode"); diff --git a/pjmedia/src/pjmedia/errno.c b/pjmedia/src/pjmedia/errno.c index 9305cd65..35441b9b 100644 --- a/pjmedia/src/pjmedia/errno.c +++ b/pjmedia/src/pjmedia/errno.c @@ -67,6 +67,9 @@ static const struct PJ_BUILD_ERR( PJMEDIA_SDPNEG_EINVANSTP, "SDP media transport type mismatch in offer/answer" ), PJ_BUILD_ERR( PJMEDIA_SDPNEG_EANSNOMEDIA, "No common SDP media payload in answer" ), PJ_BUILD_ERR( PJMEDIA_SDPNEG_ENOMEDIA, "No active media stream after negotiation" ), + PJ_BUILD_ERR( PJMEDIA_SDPNEG_NOANSCODEC, "No suitable codec for remote offer"), + PJ_BUILD_ERR( PJMEDIA_SDPNEG_NOANSTELEVENT, "No suitable telephone-event for remote offer"), + PJ_BUILD_ERR( PJMEDIA_SDPNEG_NOANSUNKNOWN, "No suitable answer for unknown remote offer"), /* SDP comparison results */ PJ_BUILD_ERR( PJMEDIA_SDP_EMEDIANOTEQUAL, "SDP media descriptor not equal" ), diff --git a/pjmedia/src/pjmedia/sdp_neg.c b/pjmedia/src/pjmedia/sdp_neg.c index 8b362ba9..8b65644d 100644 --- a/pjmedia/src/pjmedia/sdp_neg.c +++ b/pjmedia/src/pjmedia/sdp_neg.c @@ -533,9 +533,10 @@ static pj_status_t process_m_answer( pj_pool_t *pool, /* See if encoding name, clock rate, and channel * count match */ - if (!pj_strcmp(&or.enc_name, &ar.enc_name) && + if (!pj_stricmp(&or.enc_name, &ar.enc_name) && or.clock_rate == ar.clock_rate && - pj_strcmp(&or.param, &ar.param)==0) + (pj_stricmp(&or.param, &ar.param)==0 || + ar.param.slen==1 && *ar.param.ptr=='1')) { /* Match! */ break; @@ -628,10 +629,10 @@ static pj_status_t process_answer(pj_pool_t *pool, } /* Try to match offer with answer. */ -static pj_bool_t match_offer(pj_pool_t *pool, - const pjmedia_sdp_media *offer, - const pjmedia_sdp_media *local, - pjmedia_sdp_media **p_answer) +static pj_status_t match_offer(pj_pool_t *pool, + const pjmedia_sdp_media *offer, + const pjmedia_sdp_media *local, + pjmedia_sdp_media **p_answer) { unsigned i; pj_bool_t offer_has_codec = 0, @@ -697,7 +698,7 @@ static pj_bool_t match_offer(pj_pool_t *pool, &offer->desc.fmt[i]); if (!a) { pj_assert(!"Bug! Offer should have been validated"); - return PJ_FALSE; + return PJMEDIA_SDP_EMISSINGRTPMAP; } pjmedia_sdp_attr_get_rtpmap(a, &or); @@ -726,9 +727,10 @@ static pj_bool_t match_offer(pj_pool_t *pool, /* See if encoding name, clock rate, and * channel count match */ - if (!pj_strcmp(&or.enc_name, &lr.enc_name) && + if (!pj_stricmp(&or.enc_name, &lr.enc_name) && or.clock_rate == lr.clock_rate && - pj_strcmp(&or.param, &lr.param)==0) + (pj_strcmp(&or.param, &lr.param)==0 || + or.param.slen==1 && *or.param.ptr=='1')) { /* Match! */ if (is_codec) @@ -766,12 +768,16 @@ static pj_bool_t match_offer(pj_pool_t *pool, /* See if all types of offer can be matched. */ #if 1 - if ((offer_has_codec && !found_matching_codec) || - (offer_has_telephone_event && !found_matching_telephone_event) || - (offer_has_other && !found_matching_other)) - { - /* Some of the payload in the offer has no matching local sdp */ - return PJ_FALSE; + if (offer_has_codec && !found_matching_codec) { + return PJMEDIA_SDPNEG_NOANSCODEC; + } + + if (offer_has_telephone_event && !found_matching_telephone_event) { + return PJMEDIA_SDPNEG_NOANSTELEVENT; + } + + if (offer_has_other && !found_matching_other) { + return PJMEDIA_SDPNEG_NOANSUNKNOWN; } #else PJ_TODO(FULL_MATCHING_WITH_TELEPHONE_EVENTS); @@ -827,7 +833,7 @@ static pj_bool_t match_offer(pj_pool_t *pool, update_media_direction(pool, offer, answer); *p_answer = answer; - return PJ_TRUE; + return PJ_SUCCESS; } /* Create complete answer for remote's offer. */ @@ -836,7 +842,7 @@ static pj_status_t create_answer( pj_pool_t *pool, const pjmedia_sdp_session *offer, pjmedia_sdp_session **p_answer) { - pj_status_t status; + pj_status_t status = PJMEDIA_SDPNEG_ENOMEDIA; pj_bool_t has_active = PJ_FALSE; pjmedia_sdp_session *answer; char media_used[PJMEDIA_MAX_SDP_MEDIA]; @@ -879,10 +885,8 @@ static pj_status_t create_answer( pj_pool_t *pool, media_used[j] == 0) { /* See if it has matching codec. */ - pj_bool_t match; - - match = match_offer(pool, om, im, &am); - if (match) { + status = match_offer(pool, om, im, &am); + if (status == PJ_SUCCESS) { /* Mark media as used. */ media_used[j] = 1; break; @@ -925,8 +929,7 @@ static pj_status_t create_answer( pj_pool_t *pool, *p_answer = answer; - return has_active ? PJ_SUCCESS : PJMEDIA_SDPNEG_ENOMEDIA; - //return PJ_SUCCESS; + return has_active ? PJ_SUCCESS : status; } /* The best bit: SDP negotiation function! */ -- cgit v1.2.3