Version 2 "Refactoring DataContentDescription class"

(substantial changes since version 1)

This CL splits the cricket::DataContentDescription class into
two classes: cricket::RtpDataContentDescription (used for RTP data)
and cricket::SctpDataContentDescription (used for SCTP only).

SctpDataContentDescription no longer inherits from
MediaContentDescriptionImpl, and no longer contains "codecs".

Due to usage of internal interfaces by consumers, shimming the old
DataContentDescription API is needed.

A new cricket::DataContentDescription class is defined, which is
a shim over RtpDataContentDescription and SctpDataContentDescription.
It exposes as little functionality as possible, but supports the
concerned consumer's usage

Design document:
https://docs.google.com/document/d/1H5LfQxJA2ikMWTQ8FZ3_GAmaXM7knfVQWiSz6ph8VQ0/edit#

Version 1 reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132700

Bug: webrtc:10358
Change-Id: Icf95fb7308244d6f2ebfdb403aaffc544e358580
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133900
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27853}
This commit is contained in:
Harald Alvestrand
2019-05-04 11:37:04 +02:00
committed by Commit Bot
parent 2390a139de
commit 14b2758726
21 changed files with 1350 additions and 492 deletions

View File

@ -56,7 +56,6 @@ using cricket::ContentGroup;
using cricket::ContentInfo;
using cricket::CryptoParams;
using cricket::DataCodec;
using cricket::DataContentDescription;
using cricket::ICE_CANDIDATE_COMPONENT_RTCP;
using cricket::ICE_CANDIDATE_COMPONENT_RTP;
using cricket::kFecSsrcGroupSemantics;
@ -65,6 +64,8 @@ using cricket::MediaProtocolType;
using cricket::RELAY_PORT_TYPE;
using cricket::RidDescription;
using cricket::RidDirection;
using cricket::RtpDataContentDescription;
using cricket::SctpDataContentDescription;
using cricket::SessionDescription;
using cricket::SimulcastDescription;
using cricket::SimulcastLayer;
@ -275,6 +276,7 @@ static const char kSdpRtpDataChannelString[] =
"a=ssrc:10 mslabel:data_channel\r\n"
"a=ssrc:10 label:data_channeld0\r\n";
// draft-ietf-mmusic-sctp-sdp-03
static const char kSdpSctpDataChannelString[] =
"m=application 9 DTLS/SCTP 5000\r\n"
"c=IN IP4 0.0.0.0\r\n"
@ -1443,10 +1445,17 @@ class WebRtcSdpTest : public ::testing::Test {
simulcast2.receive_layers().size());
}
void CompareDataContentDescription(const DataContentDescription* dcd1,
const DataContentDescription* dcd2) {
void CompareRtpDataContentDescription(const RtpDataContentDescription* dcd1,
const RtpDataContentDescription* dcd2) {
CompareMediaContentDescription<RtpDataContentDescription>(dcd1, dcd2);
}
void CompareSctpDataContentDescription(
const SctpDataContentDescription* dcd1,
const SctpDataContentDescription* dcd2) {
EXPECT_EQ(dcd1->use_sctpmap(), dcd2->use_sctpmap());
CompareMediaContentDescription<DataContentDescription>(dcd1, dcd2);
EXPECT_EQ(dcd1->port(), dcd2->port());
EXPECT_EQ(dcd1->max_message_size(), dcd2->max_message_size());
}
void CompareSessionDescription(const SessionDescription& desc1,
@ -1484,10 +1493,21 @@ class WebRtcSdpTest : public ::testing::Test {
}
ASSERT_EQ(IsDataContent(&c1), IsDataContent(&c2));
if (IsDataContent(&c1)) {
const DataContentDescription* dcd1 = c1.media_description()->as_data();
const DataContentDescription* dcd2 = c2.media_description()->as_data();
CompareDataContentDescription(dcd1, dcd2);
if (c1.media_description()->as_sctp()) {
ASSERT_TRUE(c2.media_description()->as_sctp());
const SctpDataContentDescription* scd1 =
c1.media_description()->as_sctp();
const SctpDataContentDescription* scd2 =
c2.media_description()->as_sctp();
CompareSctpDataContentDescription(scd1, scd2);
} else {
if (IsDataContent(&c1)) {
const RtpDataContentDescription* dcd1 =
c1.media_description()->as_rtp_data();
const RtpDataContentDescription* dcd2 =
c2.media_description()->as_rtp_data();
CompareRtpDataContentDescription(dcd1, dcd2);
}
}
CompareSimulcastDescription(
@ -1760,14 +1780,12 @@ class WebRtcSdpTest : public ::testing::Test {
}
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);
codec.SetParam(cricket::kCodecParamPort, kDefaultSctpPort);
data_desc_->AddCodec(codec);
std::unique_ptr<SctpDataContentDescription> data(
new SctpDataContentDescription());
sctp_desc_ = data.get();
sctp_desc_->set_use_sctpmap(use_sctpmap);
sctp_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp);
sctp_desc_->set_port(kDefaultSctpPort);
desc_.AddContent(kDataContentName, MediaProtocolType::kSctp,
data.release());
desc_.AddTransportInfo(TransportInfo(
@ -1775,7 +1793,8 @@ class WebRtcSdpTest : public ::testing::Test {
}
void AddRtpDataChannel() {
std::unique_ptr<DataContentDescription> data(new DataContentDescription());
std::unique_ptr<RtpDataContentDescription> data(
new RtpDataContentDescription());
data_desc_ = data.get();
data_desc_->AddCodec(DataCodec(101, "google-data"));
@ -2043,7 +2062,8 @@ class WebRtcSdpTest : public ::testing::Test {
SessionDescription desc_;
AudioContentDescription* audio_desc_;
VideoContentDescription* video_desc_;
DataContentDescription* data_desc_;
RtpDataContentDescription* data_desc_;
SctpDataContentDescription* sctp_desc_;
Candidates candidates_;
std::unique_ptr<IceCandidateInterface> jcandidate_;
JsepSessionDescription jdesc_;
@ -2215,21 +2235,26 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithSctpDataChannel) {
EXPECT_EQ(message, expected_sdp);
}
void MutateJsepSctpPort(JsepSessionDescription* jdesc,
const SessionDescription& desc,
int port) {
// Take our pre-built session description and change the SCTP port.
cricket::SessionDescription* mutant = desc.Copy();
SctpDataContentDescription* dcdesc =
mutant->GetContentDescriptionByName(kDataContentName)->as_sctp();
dcdesc->set_port(port);
// Note: mutant's owned by jdesc now.
ASSERT_TRUE(jdesc->Initialize(mutant, kSessionId, kSessionVersion));
}
TEST_F(WebRtcSdpTest, SerializeWithSctpDataChannelAndNewPort) {
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jsep_desc(kDummyType);
MakeDescriptionWithoutCandidates(&jsep_desc);
DataContentDescription* dcdesc =
jsep_desc.description()
->GetContentDescriptionByName(kDataContentName)
->as_data();
const int kNewPort = 1234;
cricket::DataCodec codec(cricket::kGoogleSctpDataCodecPlType,
cricket::kGoogleSctpDataCodecName);
codec.SetParam(cricket::kCodecParamPort, kNewPort);
dcdesc->AddOrReplaceCodec(codec);
MutateJsepSctpPort(&jsep_desc, desc_, kNewPort);
std::string message = webrtc::SdpSerialize(jsep_desc);
@ -2868,14 +2893,12 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) {
// Helper function to set the max-message-size parameter in the
// SCTP data codec.
void MutateJsepSctpMaxMessageSize(const SessionDescription& desc,
const std::string& new_value,
int new_value,
JsepSessionDescription* jdesc) {
cricket::SessionDescription* mutant = desc.Copy();
DataContentDescription* dcdesc =
mutant->GetContentDescriptionByName(kDataContentName)->as_data();
std::vector<cricket::DataCodec> codecs(dcdesc->codecs());
codecs[0].SetParam(cricket::kCodecParamMaxMessageSize, new_value);
dcdesc->set_codecs(codecs);
SctpDataContentDescription* dcdesc =
mutant->GetContentDescriptionByName(kDataContentName)->as_sctp();
dcdesc->set_max_message_size(new_value);
jdesc->Initialize(mutant, kSessionId, kSessionVersion);
}
@ -2887,7 +2910,7 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithMaxMessageSize) {
sdp_with_data.append(kSdpSctpDataChannelStringWithSctpColonPort);
sdp_with_data.append("a=max-message-size:12345\r\n");
MutateJsepSctpMaxMessageSize(desc_, "12345", &jdesc);
MutateJsepSctpMaxMessageSize(desc_, 12345, &jdesc);
JsepSessionDescription jdesc_output(kDummyType);
// Verify with DTLS/SCTP.
@ -2937,29 +2960,13 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithCorruptedSctpDataChannels) {
// No crash is a pass.
}
void MutateJsepSctpPort(JsepSessionDescription* jdesc,
const SessionDescription& desc) {
// take our pre-built session description and change the SCTP port.
std::unique_ptr<cricket::SessionDescription> mutant = desc.Clone();
DataContentDescription* dcdesc =
mutant->GetContentDescriptionByName(kDataContentName)->as_data();
std::vector<cricket::DataCodec> codecs(dcdesc->codecs());
EXPECT_EQ(1U, codecs.size());
EXPECT_EQ(cricket::kGoogleSctpDataCodecPlType, codecs[0].id);
codecs[0].SetParam(cricket::kCodecParamPort, kUnusualSctpPort);
dcdesc->set_codecs(codecs);
ASSERT_TRUE(
jdesc->Initialize(std::move(mutant), kSessionId, kSessionVersion));
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndUnusualPort) {
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
// First setup the expected JsepSessionDescription.
JsepSessionDescription jdesc(kDummyType);
MutateJsepSctpPort(&jdesc, desc_);
MutateJsepSctpPort(&jdesc, desc_, kUnusualSctpPort);
// Then get the deserialized JsepSessionDescription.
std::string sdp_with_data = kSdpString;
@ -2979,7 +2986,7 @@ TEST_F(WebRtcSdpTest,
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyType);
MutateJsepSctpPort(&jdesc, desc_);
MutateJsepSctpPort(&jdesc, desc_, kUnusualSctpPort);
// We need to test the deserialized JsepSessionDescription from
// kSdpSctpDataChannelStringWithSctpPort for
@ -3015,7 +3022,7 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsAndBandwidth) {
bool use_sctpmap = true;
AddSctpDataChannel(use_sctpmap);
JsepSessionDescription jdesc(kDummyType);
DataContentDescription* dcd = GetFirstDataContentDescription(&desc_);
SctpDataContentDescription* dcd = GetFirstSctpDataContentDescription(&desc_);
dcd->set_bandwidth(100 * 1000);
ASSERT_TRUE(jdesc.Initialize(desc_.Clone(), kSessionId, kSessionVersion));