mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 917619 - Fix setup direction when a=setup is missing r=ehugg

This commit is contained in:
EKR 2013-09-17 17:43:05 -07:00
Родитель 69e9dad50b
Коммит 95b562f5a4
2 изменённых файлов: 75 добавлений и 136 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

132
media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
Просмотреть файл

@ -1891,39 +1891,6 @@ gsmsdp_set_setup_attribute(uint16_t level,
}
}
/*
* gsmsdp_set_connection_attribute
*
* Description:
*
* Adds a connection attribute to the specified SDP.
*
* Parameters:
*
* level - The media level of the SDP where the media attribute exists.
* sdp_p - Pointer to the SDP to set the ice candidate attribute against.
* connection_type - Value for the a=connection line
*/
static void
gsmsdp_set_connection_attribute(uint16_t level,
void *sdp_p, sdp_connection_type_e connection_type) {
uint16_t a_instance = 0;
sdp_result_e result;
result = sdp_add_new_attr(sdp_p, level, 0, SDP_ATTR_CONNECTION,
&a_instance);
if (result != SDP_SUCCESS) {
GSM_ERR_MSG("Failed to add attribute");
return;
}
result = sdp_attr_set_connection_attribute(sdp_p, level, 0,
a_instance, connection_type);
if (result != SDP_SUCCESS) {
GSM_ERR_MSG("Failed to set attribute");
}
}
/*
* gsmsdp_set_dtls_fingerprint_attribute
*
@ -4700,7 +4667,23 @@ gsmsdp_negotiate_rtcp_fb (cc_sdp_t *cc_sdp_p,
* initial_offer - Boolean indicating if the remote SDP came in the first OFFER of this session
* offer - Boolean indicating if the remote SDP came in an OFFER.
* notify_stream_added - Boolean indicating the UI should be notified of streams added
* create_answer - indicates whether the provided offer is supposed to generate
* an answer (versus simply setting the remote description)
*
* In practice, the initial_offer, offer, and create_answer flags only make
* sense in a limited number of configurations:
*
* Phase | i_o | offer | c_a
* ------------------------------+-------+-------+-------
* SetRemote (initial offer) | true | true | false
* SetRemote (reneg. offer) | false | true | false
* SetRemote (answer) | false | false | false
* CreateAnswer (initial) | true | true | true
* CreateAnswer (renegotitaion) | false | true | true
*
* TODO(adam@nostrum.com): These flags make the code very hard to read at
* the calling site. They should be replaced by an enumeration that
* contains only those five valid combinations described above.
*/
cc_causes_t
gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial_offer,
@ -4735,7 +4718,6 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial
sdp_result_e sdp_res;
boolean created_media_stream = FALSE;
int lsm_rc;
sdp_setup_type_e remote_setup_type;
config_get_value(CFGID_SDPMODE, &sdpmode, sizeof(sdpmode));
@ -5032,41 +5014,63 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial
if (!unsupported_line) {
if (sdpmode) {
sdp_setup_type_e remote_setup_type;
int j;
/* Find the remote a=setup value */
sdp_res = sdp_attr_get_setup_attribute(
sdp_p->dest_sdp, i, 0, 1, &remote_setup_type);
/* setup attribute
We are setting our local SDP to be ACTIVE if the value
in the remote SDP is missing, PASSIVE or ACTPASS.
If the remote value is ACTIVE, then we will respond
with PASSIVE.
If the remote value is HOLDCONN we will respond with
HOLDCONN and set the direction to INACTIVE
The DTLS role will then be set when the TransportFlow
is created */
media->setup = SDP_SETUP_ACTIVE;
if (sdp_res == SDP_SUCCESS) {
if (remote_setup_type == SDP_SETUP_ACTIVE) {
media->setup = SDP_SETUP_PASSIVE;
} else if (remote_setup_type == SDP_SETUP_HOLDCONN) {
media->setup = SDP_SETUP_HOLDCONN;
media->direction = SDP_DIRECTION_INACTIVE;
}
/*
* Although a=setup is required for DTLS per RFC 5763,
* there are several implementations (including older
* versions of Firefox) that don't signal direction.
* To work with these cases, we assume that an omitted
* direction in SDP means "PASSIVE" in an offer, and
* "ACTIVE" in an answer.
*/
if (sdp_res != SDP_SUCCESS) {
remote_setup_type =
offer ? SDP_SETUP_PASSIVE : SDP_SETUP_ACTIVE;
}
gsmsdp_set_setup_attribute(media->level, dcb_p->sdp->src_sdp,
media->setup);
/* The DTLS role will be set based on the media->setup
value when the TransportFlow is created */
switch (remote_setup_type) {
case SDP_SETUP_ACTIVE:
media->setup = SDP_SETUP_PASSIVE;
break;
case SDP_SETUP_PASSIVE:
media->setup = SDP_SETUP_ACTIVE;
break;
case SDP_SETUP_ACTPASS:
/*
* This should only happen in an offer. If the
* remote side is ACTPASS, we choose to be ACTIVE;
* this allows us to start setting up the DTLS
* association immediately, saving 1/2 RTT in
* association establishment.
*/
media->setup = SDP_SETUP_ACTIVE;
break;
case SDP_SETUP_HOLDCONN:
media->setup = SDP_SETUP_HOLDCONN;
media->direction = SDP_DIRECTION_INACTIVE;
break;
default:
/*
* If we don't recognize the remote endpoint's setup
* attribute, we fall back to being active if they
* sent an offer, and passive if they sent an answer.
*/
media->setup =
offer ? SDP_SETUP_ACTIVE : SDP_SETUP_PASSIVE;
}
/* TODO(ehugg) we are not yet supporting existing connections
See bug 857115. We currently always respond with
connection:new */
gsmsdp_set_connection_attribute(media->level,
dcb_p->sdp->src_sdp, SDP_CONNECTION_NEW);
if (create_answer) {
gsmsdp_set_setup_attribute(media->level,
dcb_p->sdp->src_sdp,
media->setup);
}
/* Set ICE */
for (j=0; j<media->candidate_ct; j++) {
@ -5585,13 +5589,9 @@ gsmsdp_add_media_line (fsmdef_dcb_t *dcb_p, const cc_media_cap_t *media_cap,
sdp_rtcp_fb_ccm_to_bitmap(SDP_RTCP_FB_CCM_FIR));
}
/* setup and connection attributes */
/* Add a=setup attribute */
gsmsdp_set_setup_attribute(level, dcb_p->sdp->src_sdp, media->setup);
/* This is a new media line so we should send connection:new */
gsmsdp_set_connection_attribute(level, dcb_p->sdp->src_sdp,
SDP_CONNECTION_NEW);
/*
* wait until here to set ICE candidates as SDP is now initialized
*/

79
media/webrtc/signaling/test/signaling_unittests.cpp
Просмотреть файл

@ -2872,57 +2872,8 @@ TEST_F(SignalingTest, AudioCallGarbageSetup)
ASSERT_GE(a2_.GetPacketsReceived(0), 40);
}
// In this test we will change the offer SDP's a=connection value
// from new to garbage. It should ignore the garbage value
// and respond with connection:new
TEST_F(SignalingTest, AudioCallGarbageConnection)
{
sipcc::MediaConstraints constraints;
size_t match;
a1_.CreateOffer(constraints, OFFER_AUDIO, SHOULD_SENDRECV_AUDIO);
// By default the offer should give connection:new
std::string offer(a1_.offer());
match = offer.find("\r\na=connection:new");
ASSERT_NE(match, std::string::npos);
// Now replace the 'new' with a garbage value
offer.replace(match, strlen("\r\na=connection:new"),
"\r\na=connection:G4rb4g3V4lu3");
std::cout << "Modified SDP " << std::endl
<< indent(offer) << std::endl;
a1_.SetLocal(TestObserver::OFFER, offer.c_str(), false);
a2_.SetRemote(TestObserver::OFFER, offer.c_str(), false);
a2_.CreateAnswer(constraints, offer.c_str(), OFFER_AUDIO | ANSWER_AUDIO);
// Now the answer should contain a=connection:new
std::string answer(a2_.answer());
match = answer.find("\r\na=connection:new");
ASSERT_NE(match, std::string::npos);
// This should setup the DTLS with the same roles
// as the regular tests above.
a2_.SetLocal(TestObserver::ANSWER, a2_.answer(), false);
a1_.SetRemote(TestObserver::ANSWER, a2_.answer(), false);
ASSERT_TRUE_WAIT(a1_.IceCompleted() == true, kDefaultTimeout);
ASSERT_TRUE_WAIT(a2_.IceCompleted() == true, kDefaultTimeout);
// Wait for some data to get written
ASSERT_TRUE_WAIT(a1_.GetPacketsSent(0) >= 40 &&
a2_.GetPacketsReceived(0) >= 40, kDefaultTimeout * 2);
a1_.CloseSendStreams();
a2_.CloseReceiveStreams();
ASSERT_GE(a1_.GetPacketsSent(0), 40);
ASSERT_GE(a2_.GetPacketsReceived(0), 40);
}
// In this test we will change the offer SDP to remove the
// a=setup and a=connection lines. Answer should respond with
// a=setup:active and a=connection:new
// a=setup line. Answer should respond with a=setup:active.
TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
{
sipcc::MediaConstraints constraints;
@ -2930,16 +2881,12 @@ TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
a1_.CreateOffer(constraints, OFFER_AUDIO, SHOULD_SENDRECV_AUDIO);
// By default the offer should give setup:actpass and connection:new
// By default the offer should give setup:actpass
std::string offer(a1_.offer());
match = offer.find("\r\na=setup:actpass");
ASSERT_NE(match, std::string::npos);
// Remove the a=setup line
offer.replace(match, strlen("\r\na=setup:actpass"), "");
match = offer.find("\r\na=connection:new");
ASSERT_NE(match, std::string::npos);
// Remove the a=connection line
offer.replace(match, strlen("\r\na=connection:new"), "");
std::cout << "Modified SDP " << std::endl
<< indent(offer) << std::endl;
@ -2947,12 +2894,10 @@ TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
a2_.SetRemote(TestObserver::OFFER, offer.c_str(), false);
a2_.CreateAnswer(constraints, offer.c_str(), OFFER_AUDIO | ANSWER_AUDIO);
// Now the answer should contain a=setup:active and a=connection:new
// Now the answer should contain a=setup:active
std::string answer(a2_.answer());
match = answer.find("\r\na=setup:active");
ASSERT_NE(match, std::string::npos);
match = answer.find("\r\na=connection:new");
ASSERT_NE(match, std::string::npos);
// This should setup the DTLS with the same roles
// as the regular tests above.
@ -2974,8 +2919,8 @@ TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
}
// In this test we will change the answer SDP to remove the
// a=setup and a=connection lines. ICE should still connect
// since active will be assumed.
// a=setup line. ICE should still connect since active will
// be assumed.
TEST_F(SignalingTest, AudioCallAnswerNoSetupOrConnection)
{
sipcc::MediaConstraints constraints;
@ -2983,34 +2928,28 @@ TEST_F(SignalingTest, AudioCallAnswerNoSetupOrConnection)
a1_.CreateOffer(constraints, OFFER_AUDIO, SHOULD_SENDRECV_AUDIO);
// By default the offer should give setup:actpass and connection:new
// By default the offer should give setup:actpass
std::string offer(a1_.offer());
match = offer.find("\r\na=setup:actpass");
ASSERT_NE(match, std::string::npos);
match = offer.find("\r\na=connection:new");
ASSERT_NE(match, std::string::npos);
a1_.SetLocal(TestObserver::OFFER, offer.c_str(), false);
a2_.SetRemote(TestObserver::OFFER, offer.c_str(), false);
a2_.CreateAnswer(constraints, offer.c_str(), OFFER_AUDIO | ANSWER_AUDIO);
// Now the answer should contain a=setup:active and a=connection:new
// Now the answer should contain a=setup:active
std::string answer(a2_.answer());
match = answer.find("\r\na=setup:active");
ASSERT_NE(match, std::string::npos);
// Remove the a=setup line
answer.replace(match, strlen("\r\na=setup:active"), "");
match = answer.find("\r\na=connection:new");
ASSERT_NE(match, std::string::npos);
// Remove the a=connection line
answer.replace(match, strlen("\r\na=connection:new"), "");
std::cout << "Modified SDP " << std::endl
<< indent(answer) << std::endl;
// This should setup the DTLS with the same roles
// as the regular tests above.
a2_.SetLocal(TestObserver::ANSWER, a2_.answer(), false);
a1_.SetRemote(TestObserver::ANSWER, a2_.answer(), false);
a2_.SetLocal(TestObserver::ANSWER, answer, false);
a1_.SetRemote(TestObserver::ANSWER, answer, false);
ASSERT_TRUE_WAIT(a1_.IceCompleted() == true, kDefaultTimeout);
ASSERT_TRUE_WAIT(a2_.IceCompleted() == true, kDefaultTimeout);