Use the same draft version in SDP data channel answers as used in the offer.

This change adds a flag, use_sctpmap, to DataContentDescription. The deserialization code sets the flag based on the format of the m= line.
There were already unit tests using SDP in the new format, so I just updated them to check use_sctpmap was set as expected.

The change to mediasession copies use_sctpmap from the offered DataContentDescription to the answer.
I haven't figured out how to test this change yet, but wanted to get feedback before continuing.

BUG=chromium:686212

Review-Url: https://codereview.webrtc.org/2690943011
Cr-Commit-Position: refs/heads/master@{#16686}
This commit is contained in:
zstein 2017-02-17 19:48:38 -08:00 committed by Commit bot
parent 2c546da529
commit 4b2e0829ca
5 changed files with 183 additions and 77 deletions

View File

@ -1996,6 +1996,12 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
return false; // Fails the session setup.
}
// Respond with sctpmap if the offer uses sctpmap.
const DataContentDescription* offer_data_description =
static_cast<const DataContentDescription*>(data_content->description);
bool offer_uses_sctpmap = offer_data_description->use_sctpmap();
data_answer->set_use_sctpmap(offer_uses_sctpmap);
bool rejected = !options.has_data() || data_content->rejected ||
!IsMediaProtocolSupported(MEDIA_TYPE_DATA,
data_answer->protocol(),

View File

@ -401,10 +401,18 @@ class VideoContentDescription : public MediaContentDescriptionImpl<VideoCodec> {
class DataContentDescription : public MediaContentDescriptionImpl<DataCodec> {
public:
DataContentDescription() {}
virtual ContentDescription* Copy() const {
return new DataContentDescription(*this);
}
virtual MediaType type() const { return MEDIA_TYPE_DATA; }
bool use_sctpmap() const { return use_sctpmap_; }
void set_use_sctpmap(bool enable) { use_sctpmap_ = enable; }
private:
bool use_sctpmap_ = true;
};
// Creates media session descriptions according to the supplied codecs and
@ -456,9 +464,9 @@ class MediaSessionDescriptionFactory {
const MediaSessionOptions& options,
const SessionDescription* current_description) const;
SessionDescription* CreateAnswer(
const SessionDescription* offer,
const MediaSessionOptions& options,
const SessionDescription* current_description) const;
const SessionDescription* offer,
const MediaSessionOptions& options,
const SessionDescription* current_description) const;
private:
const AudioCodecs& GetAudioCodecsForOffer(

View File

@ -912,27 +912,27 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswer) {
std::unique_ptr<SessionDescription> answer(
f2_.CreateAnswer(offer.get(), opts, NULL));
const ContentInfo* ac = answer->GetContentByName("audio");
const ContentInfo* vc = answer->GetContentByName("data");
const ContentInfo* dc = answer->GetContentByName("data");
ASSERT_TRUE(ac != NULL);
ASSERT_TRUE(vc != NULL);
ASSERT_TRUE(dc != NULL);
EXPECT_EQ(std::string(NS_JINGLE_RTP), ac->type);
EXPECT_EQ(std::string(NS_JINGLE_RTP), vc->type);
EXPECT_EQ(std::string(NS_JINGLE_RTP), dc->type);
const AudioContentDescription* acd =
static_cast<const AudioContentDescription*>(ac->description);
const DataContentDescription* vcd =
static_cast<const DataContentDescription*>(vc->description);
const DataContentDescription* dcd =
static_cast<const DataContentDescription*>(dc->description);
EXPECT_EQ(MEDIA_TYPE_AUDIO, acd->type());
EXPECT_EQ(MAKE_VECTOR(kAudioCodecsAnswer), acd->codecs());
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw
EXPECT_NE(0U, acd->first_ssrc()); // a random nonzero ssrc
EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux
ASSERT_CRYPTO(acd, 1U, CS_AES_CM_128_HMAC_SHA1_32);
EXPECT_EQ(MEDIA_TYPE_DATA, vcd->type());
EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), vcd->codecs());
EXPECT_NE(0U, vcd->first_ssrc()); // a random nonzero ssrc
EXPECT_TRUE(vcd->rtcp_mux()); // negotiated rtcp-mux
ASSERT_CRYPTO(vcd, 1U, CS_AES_CM_128_HMAC_SHA1_80);
EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), vcd->protocol());
EXPECT_EQ(MEDIA_TYPE_DATA, dcd->type());
EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), dcd->codecs());
EXPECT_NE(0U, dcd->first_ssrc()); // a random nonzero ssrc
EXPECT_TRUE(dcd->rtcp_mux()); // negotiated rtcp-mux
ASSERT_CRYPTO(dcd, 1U, CS_AES_CM_128_HMAC_SHA1_80);
EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), dcd->protocol());
}
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerGcm) {
@ -946,27 +946,70 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerGcm) {
std::unique_ptr<SessionDescription> answer(
f2_.CreateAnswer(offer.get(), opts, NULL));
const ContentInfo* ac = answer->GetContentByName("audio");
const ContentInfo* vc = answer->GetContentByName("data");
const ContentInfo* dc = answer->GetContentByName("data");
ASSERT_TRUE(ac != NULL);
ASSERT_TRUE(vc != NULL);
ASSERT_TRUE(dc != NULL);
EXPECT_EQ(std::string(NS_JINGLE_RTP), ac->type);
EXPECT_EQ(std::string(NS_JINGLE_RTP), vc->type);
EXPECT_EQ(std::string(NS_JINGLE_RTP), dc->type);
const AudioContentDescription* acd =
static_cast<const AudioContentDescription*>(ac->description);
const DataContentDescription* vcd =
static_cast<const DataContentDescription*>(vc->description);
const DataContentDescription* dcd =
static_cast<const DataContentDescription*>(dc->description);
EXPECT_EQ(MEDIA_TYPE_AUDIO, acd->type());
EXPECT_EQ(MAKE_VECTOR(kAudioCodecsAnswer), acd->codecs());
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw
EXPECT_NE(0U, acd->first_ssrc()); // a random nonzero ssrc
EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux
ASSERT_CRYPTO(acd, 1U, CS_AEAD_AES_256_GCM);
EXPECT_EQ(MEDIA_TYPE_DATA, vcd->type());
EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), vcd->codecs());
EXPECT_NE(0U, vcd->first_ssrc()); // a random nonzero ssrc
EXPECT_TRUE(vcd->rtcp_mux()); // negotiated rtcp-mux
ASSERT_CRYPTO(vcd, 1U, CS_AEAD_AES_256_GCM);
EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), vcd->protocol());
EXPECT_EQ(MEDIA_TYPE_DATA, dcd->type());
EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), dcd->codecs());
EXPECT_NE(0U, dcd->first_ssrc()); // a random nonzero ssrc
EXPECT_TRUE(dcd->rtcp_mux()); // negotiated rtcp-mux
ASSERT_CRYPTO(dcd, 1U, CS_AEAD_AES_256_GCM);
EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), dcd->protocol());
}
// The use_sctpmap flag should be set in a DataContentDescription by default.
// The answer's use_sctpmap flag should match the offer's.
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerUsesSctpmap) {
MediaSessionOptions opts;
opts.data_channel_type = cricket::DCT_SCTP;
std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, NULL));
ASSERT_TRUE(offer.get() != NULL);
ContentInfo* dc_offer = offer->GetContentByName("data");
ASSERT_TRUE(dc_offer != NULL);
DataContentDescription* dcd_offer =
static_cast<DataContentDescription*>(dc_offer->description);
EXPECT_TRUE(dcd_offer->use_sctpmap());
std::unique_ptr<SessionDescription> answer(
f2_.CreateAnswer(offer.get(), opts, NULL));
const ContentInfo* dc_answer = answer->GetContentByName("data");
ASSERT_TRUE(dc_answer != NULL);
const DataContentDescription* dcd_answer =
static_cast<const DataContentDescription*>(dc_answer->description);
EXPECT_TRUE(dcd_answer->use_sctpmap());
}
// The answer's use_sctpmap flag should match the offer's.
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerWithoutSctpmap) {
MediaSessionOptions opts;
opts.data_channel_type = cricket::DCT_SCTP;
std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, NULL));
ASSERT_TRUE(offer.get() != NULL);
ContentInfo* dc_offer = offer->GetContentByName("data");
ASSERT_TRUE(dc_offer != NULL);
DataContentDescription* dcd_offer =
static_cast<DataContentDescription*>(dc_offer->description);
dcd_offer->set_use_sctpmap(false);
std::unique_ptr<SessionDescription> answer(
f2_.CreateAnswer(offer.get(), opts, NULL));
const ContentInfo* dc_answer = answer->GetContentByName("data");
ASSERT_TRUE(dc_answer != NULL);
const DataContentDescription* dcd_answer =
static_cast<const DataContentDescription*>(dc_answer->description);
EXPECT_FALSE(dcd_answer->use_sctpmap());
}
// Verifies that the order of the media contents in the offer is preserved in

View File

@ -234,7 +234,9 @@ static void BuildMediaDescription(const ContentInfo* content_info,
const std::vector<Candidate>& candidates,
bool unified_plan_sdp,
std::string* message);
static void BuildSctpContentAttributes(std::string* message, int sctp_port);
static void BuildSctpContentAttributes(std::string* message,
int sctp_port,
bool use_sctpmap);
static void BuildRtpContentAttributes(const MediaContentDescription* media_desc,
const MediaType media_type,
bool unified_plan_sdp,
@ -1275,16 +1277,21 @@ void BuildMediaDescription(const ContentInfo* content_info,
if (IsDtlsSctp(media_desc->protocol())) {
fmt.append(" ");
for (std::vector<cricket::DataCodec>::const_iterator it =
data_desc->codecs().begin();
it != data_desc->codecs().end(); ++it) {
if (cricket::CodecNamesEq(it->name, cricket::kGoogleSctpDataCodecName)
&& it->GetParam(cricket::kCodecParamPort, &sctp_port)) {
break;
if (data_desc->use_sctpmap()) {
for (std::vector<cricket::DataCodec>::const_iterator it =
data_desc->codecs().begin();
it != data_desc->codecs().end(); ++it) {
if (cricket::CodecNamesEq(it->name,
cricket::kGoogleSctpDataCodecName) &&
it->GetParam(cricket::kCodecParamPort, &sctp_port)) {
break;
}
}
}
fmt.append(rtc::ToString<int>(sctp_port));
fmt.append(rtc::ToString<int>(sctp_port));
} else {
fmt.append(kDefaultSctpmapProtocol);
}
} else {
for (std::vector<cricket::DataCodec>::const_iterator it =
data_desc->codecs().begin();
@ -1407,23 +1414,34 @@ void BuildMediaDescription(const ContentInfo* content_info,
AddLine(os.str(), message);
if (IsDtlsSctp(media_desc->protocol())) {
BuildSctpContentAttributes(message, sctp_port);
const DataContentDescription* data_desc =
static_cast<const DataContentDescription*>(media_desc);
bool use_sctpmap = data_desc->use_sctpmap();
BuildSctpContentAttributes(message, sctp_port, use_sctpmap);
} else if (IsRtp(media_desc->protocol())) {
BuildRtpContentAttributes(media_desc, media_type, unified_plan_sdp,
message);
}
}
void BuildSctpContentAttributes(std::string* message, int sctp_port) {
// draft-ietf-mmusic-sctp-sdp-04
// a=sctpmap:sctpmap-number protocol [streams]
// TODO(lally): switch this over to mmusic-sctp-sdp-12 (or later), with
// 'a=sctp-port:'
void BuildSctpContentAttributes(std::string* message,
int sctp_port,
bool use_sctpmap) {
std::ostringstream os;
InitAttrLine(kAttributeSctpmap, &os);
os << kSdpDelimiterColon << sctp_port << kSdpDelimiterSpace
<< kDefaultSctpmapProtocol << kSdpDelimiterSpace
<< cricket::kMaxSctpStreams;
if (use_sctpmap) {
// draft-ietf-mmusic-sctp-sdp-04
// a=sctpmap:sctpmap-number protocol [streams]
InitAttrLine(kAttributeSctpmap, &os);
os << kSdpDelimiterColon << sctp_port << kSdpDelimiterSpace
<< kDefaultSctpmapProtocol << kSdpDelimiterSpace
<< cricket::kMaxSctpStreams;
} else {
// draft-ietf-mmusic-sctp-sdp-23
// a=sctp-port:<port>
InitAttrLine(kAttributeSctpPort, &os);
os << kSdpDelimiterColon << sctp_port;
// TODO(zstein): emit max-message-size here
}
AddLine(os.str(), message);
}
@ -2291,6 +2309,7 @@ bool ParseMediaDescription(const std::string& message,
std::vector<std::string> fields;
rtc::split(line.substr(kLinePrefixLength),
kSdpDelimiterSpace, &fields);
const size_t expected_min_fields = 4;
if (fields.size() < expected_min_fields) {
return ParseFailedExpectMinFieldNum(line, expected_min_fields, error);
@ -2351,10 +2370,15 @@ bool ParseMediaDescription(const std::string& message,
candidates, error);
content.reset(data_desc);
int p;
if (data_desc && IsDtlsSctp(protocol) && rtc::FromString(fields[3], &p)) {
if (!AddSctpDataCodec(data_desc, p))
return false;
if (data_desc && IsDtlsSctp(protocol)) {
int p;
if (rtc::FromString(fields[3], &p)) {
if (!AddSctpDataCodec(data_desc, p)) {
return false;
}
} else if (fields[3] == kDefaultSctpmapProtocol) {
data_desc->set_use_sctpmap(false);
}
}
} else {
LOG(LS_WARNING) << "Unsupported media type: " << line;

View File

@ -65,6 +65,9 @@ typedef std::vector<AudioCodec> AudioCodecs;
typedef std::vector<Candidate> Candidates;
static const uint32_t kDefaultSctpPort = 5000;
static const char kDefaultSctpPortStr[] = "5000";
static const uint16_t kUnusualSctpPort = 9556;
static const char kUnusualSctpPortStr[] = "9556";
static const char kSessionTime[] = "t=0 0\r\n";
static const uint32_t kCandidatePriority = 2130706432U; // pref = 1.0
static const char kAttributeIceUfragVoice[] = "a=ice-ufrag:ufrag_voice\r\n";
@ -1210,6 +1213,11 @@ class WebRtcSdpTest : public testing::Test {
}
}
void CompareDataContentDescription(const DataContentDescription* dcd1,
const DataContentDescription* dcd2) {
EXPECT_EQ(dcd1->use_sctpmap(), dcd2->use_sctpmap());
CompareMediaContentDescription<DataContentDescription>(dcd1, dcd2);
}
void CompareSessionDescription(const SessionDescription& desc1,
const SessionDescription& desc2) {
@ -1251,7 +1259,7 @@ class WebRtcSdpTest : public testing::Test {
static_cast<const DataContentDescription*>(c1.description);
const DataContentDescription* dcd2 =
static_cast<const DataContentDescription*>(c2.description);
CompareMediaContentDescription<DataContentDescription>(dcd1, dcd2);
CompareDataContentDescription(dcd1, dcd2);
}
}
@ -1486,9 +1494,10 @@ class WebRtcSdpTest : public testing::Test {
return true;
}
void AddSctpDataChannel() {
void AddSctpDataChannel(bool use_sctpmap) {
std::unique_ptr<DataContentDescription> data(new DataContentDescription());
data_desc_ = data.get();
data_desc_->set_use_sctpmap(use_sctpmap);
data_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp);
DataCodec codec(cricket::kGoogleSctpDataCodecPlType,
cricket::kGoogleSctpDataCodecName);
@ -2058,7 +2067,8 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithRtpDataChannel) {
}
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithSctpDataChannel) {
AddSctpDataChannel();
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jsep_desc(kDummyString);
ASSERT_TRUE(jsep_desc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
@ -2070,7 +2080,8 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithSctpDataChannel) {
}
TEST_F(WebRtcSdpTest, SerializeWithSctpDataChannelAndNewPort) {
AddSctpDataChannel();
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jsep_desc(kDummyString);
ASSERT_TRUE(jsep_desc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
@ -2570,7 +2581,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithRtpDataChannels) {
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannels) {
AddSctpDataChannel();
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyString);
ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
@ -2596,7 +2608,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannels) {
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpPort) {
AddSctpDataChannel();
bool use_sctpmap = false;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyString);
ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
@ -2622,7 +2635,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpPort) {
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) {
AddSctpDataChannel();
bool use_sctpmap = false;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyString);
ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
@ -2650,7 +2664,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) {
// Test to check the behaviour if sctp-port is specified
// on the m= line and in a=sctp-port.
TEST_F(WebRtcSdpTest, DeserializeSdpWithMultiSctpPort) {
AddSctpDataChannel();
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyString);
ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
@ -2676,20 +2691,10 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithCorruptedSctpDataChannels) {
// No crash is a pass.
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndNewPort) {
AddSctpDataChannel();
const uint16_t kUnusualSctpPort = 9556;
char default_portstr[16];
char unusual_portstr[16];
rtc::sprintfn(default_portstr, sizeof(default_portstr), "%d",
kDefaultSctpPort);
rtc::sprintfn(unusual_portstr, sizeof(unusual_portstr), "%d",
kUnusualSctpPort);
// First setup the expected JsepSessionDescription.
JsepSessionDescription jdesc(kDummyString);
void MutateJsepSctpPort(JsepSessionDescription& jdesc,
const SessionDescription& desc) {
// take our pre-built session description and change the SCTP port.
cricket::SessionDescription* mutant = desc_.Copy();
cricket::SessionDescription* mutant = desc.Copy();
DataContentDescription* dcdesc = static_cast<DataContentDescription*>(
mutant->GetContentDescriptionByName(kDataContentName));
std::vector<cricket::DataCodec> codecs(dcdesc->codecs());
@ -2701,27 +2706,46 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndNewPort) {
// note: mutant's owned by jdesc now.
ASSERT_TRUE(jdesc.Initialize(mutant, kSessionId, kSessionVersion));
mutant = NULL;
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndUnusualPort) {
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
// First setup the expected JsepSessionDescription.
JsepSessionDescription jdesc(kDummyString);
MutateJsepSctpPort(jdesc, desc_);
// Then get the deserialized JsepSessionDescription.
std::string sdp_with_data = kSdpString;
sdp_with_data.append(kSdpSctpDataChannelString);
rtc::replace_substrs(default_portstr, strlen(default_portstr),
unusual_portstr, strlen(unusual_portstr),
&sdp_with_data);
rtc::replace_substrs(kDefaultSctpPortStr, strlen(kDefaultSctpPortStr),
kUnusualSctpPortStr, strlen(kUnusualSctpPortStr),
&sdp_with_data);
JsepSessionDescription jdesc_output(kDummyString);
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
}
TEST_F(WebRtcSdpTest,
DeserializeSdpWithSctpDataChannelAndUnusualPortInAttribute) {
bool use_sctpmap = false;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyString);
MutateJsepSctpPort(jdesc, desc_);
// We need to test the deserialized JsepSessionDescription from
// kSdpSctpDataChannelStringWithSctpPort for
// draft-ietf-mmusic-sctp-sdp-07
// a=sctp-port
sdp_with_data = kSdpString;
std::string sdp_with_data = kSdpString;
sdp_with_data.append(kSdpSctpDataChannelStringWithSctpPort);
rtc::replace_substrs(default_portstr, strlen(default_portstr),
unusual_portstr, strlen(unusual_portstr),
&sdp_with_data);
rtc::replace_substrs(kDefaultSctpPortStr, strlen(kDefaultSctpPortStr),
kUnusualSctpPortStr, strlen(kUnusualSctpPortStr),
&sdp_with_data);
JsepSessionDescription jdesc_output(kDummyString);
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
@ -2744,7 +2768,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithRtpDataChannelsAndBandwidth) {
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsAndBandwidth) {
AddSctpDataChannel();
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyString);
DataContentDescription* dcd = static_cast<DataContentDescription*>(
GetFirstDataContent(&desc_)->description);