diff options
author | Benny Prijono <bennylp@teluu.com> | 2008-02-21 10:08:27 +0000 |
---|---|---|
committer | Benny Prijono <bennylp@teluu.com> | 2008-02-21 10:08:27 +0000 |
commit | f7718e1cd91ef1748a998c40fa807d60bc283f22 (patch) | |
tree | 8722726c8029b7c8ac5903048952ed2d8756ecce | |
parent | d277c485a04b43f04fa40dce59ae11b7cbf2fae4 (diff) |
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
-rw-r--r-- | pjmedia/include/pjmedia/sdp.h | 24 | ||||
-rw-r--r-- | pjmedia/include/pjmedia/transport.h | 21 | ||||
-rw-r--r-- | pjmedia/src/pjmedia/sdp.c | 33 | ||||
-rw-r--r-- | pjmedia/src/pjmedia/sdp_neg.c | 12 | ||||
-rw-r--r-- | pjmedia/src/pjmedia/session.c | 12 | ||||
-rw-r--r-- | pjmedia/src/pjmedia/transport_ice.c | 41 | ||||
-rw-r--r-- | pjmedia/src/pjmedia/transport_srtp.c | 34 | ||||
-rw-r--r-- | pjmedia/src/pjmedia/transport_udp.c | 54 | ||||
-rw-r--r-- | pjsip/src/pjsua-lib/pjsua_call.c | 2 | ||||
-rw-r--r-- | pjsip/src/pjsua-lib/pjsua_media.c | 2 |
10 files changed, 197 insertions, 38 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; } diff --git a/pjsip/src/pjsua-lib/pjsua_call.c b/pjsip/src/pjsua-lib/pjsua_call.c index 3d67fc91..6cf63f00 100644 --- a/pjsip/src/pjsua-lib/pjsua_call.c +++ b/pjsip/src/pjsua-lib/pjsua_call.c @@ -2484,7 +2484,7 @@ static void pjsua_call_on_media_update(pjsip_inv_session *inv, if (inv->state != PJSIP_INV_STATE_NULL && inv->state != PJSIP_INV_STATE_CONFIRMED) { - //call_disconnect(inv, PJSIP_SC_UNSUPPORTED_MEDIA_TYPE); + call_disconnect(inv, PJSIP_SC_UNSUPPORTED_MEDIA_TYPE); } PJSUA_UNLOCK(); diff --git a/pjsip/src/pjsua-lib/pjsua_media.c b/pjsip/src/pjsua-lib/pjsua_media.c index 4bd41f61..4a64b8f2 100644 --- a/pjsip/src/pjsua-lib/pjsua_media.c +++ b/pjsip/src/pjsua-lib/pjsua_media.c @@ -859,7 +859,7 @@ pj_status_t pjsua_media_channel_create_sdp(pjsua_call_id call_id, } /* Give the SDP to media transport */ - status = pjmedia_transport_media_create(call->med_tp, pool, + status = pjmedia_transport_media_create(call->med_tp, pool, 0, sdp, rem_sdp, MEDIA_IDX); if (status != PJ_SUCCESS) { if (sip_status_code) *sip_status_code = PJSIP_SC_NOT_ACCEPTABLE; |