admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Delete SDP x-alt-protocol

Browse Source 
Bug: webrtc:9719
Change-Id: I921f72d8e80cc36d62b2aeadfb688a7b884668b6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177423
Reviewed-by: Taylor <deadbeef@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31547}
This commit is contained in:
Niels Möller 2020-06-18 10:10:17 +02:00 committed by Commit Bot
parent 7a82467d0d
commit dc80aafe30
10 changed files with 8 additions and 313 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
pc/jsep_transport.cc
View File

@ -38,16 +38,12 @@ JsepTransportDescription::JsepTransportDescription(
const std::vector<CryptoParams>& cryptos,
const std::vector<int>& encrypted_header_extension_ids,
int rtp_abs_sendtime_extn_id,
const TransportDescription& transport_desc,
absl::optional<std::string> media_alt_protocol,
absl::optional<std::string> data_alt_protocol)
const TransportDescription& transport_desc)
: rtcp_mux_enabled(rtcp_mux_enabled),
cryptos(cryptos),
encrypted_header_extension_ids(encrypted_header_extension_ids),
rtp_abs_sendtime_extn_id(rtp_abs_sendtime_extn_id),
transport_desc(transport_desc),
media_alt_protocol(media_alt_protocol),
data_alt_protocol(data_alt_protocol) {}
transport_desc(transport_desc) {}
JsepTransportDescription::JsepTransportDescription(
const JsepTransportDescription& from)
@ -55,9 +51,7 @@ JsepTransportDescription::JsepTransportDescription(
cryptos(from.cryptos),
encrypted_header_extension_ids(from.encrypted_header_extension_ids),
rtp_abs_sendtime_extn_id(from.rtp_abs_sendtime_extn_id),
transport_desc(from.transport_desc),
media_alt_protocol(from.media_alt_protocol),
data_alt_protocol(from.data_alt_protocol) {}
transport_desc(from.transport_desc) {}
JsepTransportDescription::~JsepTransportDescription() = default;
@ -71,8 +65,6 @@ JsepTransportDescription& JsepTransportDescription::operator=(
encrypted_header_extension_ids = from.encrypted_header_extension_ids;
rtp_abs_sendtime_extn_id = from.rtp_abs_sendtime_extn_id;
transport_desc = from.transport_desc;
media_alt_protocol = from.media_alt_protocol;
data_alt_protocol = from.data_alt_protocol;
return *this;
}

12
pc/jsep_transport.h
View File

@ -53,9 +53,7 @@ struct JsepTransportDescription {
const std::vector<CryptoParams>& cryptos,
const std::vector<int>& encrypted_header_extension_ids,
int rtp_abs_sendtime_extn_id,
const TransportDescription& transport_description,
absl::optional<std::string> media_alt_protocol,
absl::optional<std::string> data_alt_protocol);
const TransportDescription& transport_description);
JsepTransportDescription(const JsepTransportDescription& from);
~JsepTransportDescription();
@ -68,14 +66,6 @@ struct JsepTransportDescription {
// TODO(zhihuang): Add the ICE and DTLS related variables and methods from
// TransportDescription and remove this extra layer of abstraction.
TransportDescription transport_desc;
// Alt-protocols that apply to this JsepTransport. Presence indicates a
// request to use an alternative protocol for media and/or data. The
// alt-protocol is handled by a datagram transport. If one or both of these
// values are present, JsepTransport will attempt to negotiate use of the
// datagram transport for media and/or data.
absl::optional<std::string> media_alt_protocol;
absl::optional<std::string> data_alt_protocol;
};
// Helper class used by JsepTransportController that processes

81
pc/jsep_transport_controller.cc
View File

@ -576,16 +576,9 @@ RTCError JsepTransportController::ApplyDescription_n(
}
std::vector<int> merged_encrypted_extension_ids;
absl::optional<std::string> bundle_media_alt_protocol;
absl::optional<std::string> bundle_data_alt_protocol;
if (bundle_group_) {
merged_encrypted_extension_ids =
MergeEncryptedHeaderExtensionIdsForBundle(description);
error = GetAltProtocolsForBundle(description, &bundle_media_alt_protocol,
&bundle_data_alt_protocol);
if (!error.ok()) {
return error;
}
}
for (const cricket::ContentInfo& content_info : description->contents()) {
@ -604,8 +597,6 @@ RTCError JsepTransportController::ApplyDescription_n(
description->transport_infos().size());
for (size_t i = 0; i < description->contents().size(); ++i) {
const cricket::ContentInfo& content_info = description->contents()[i];
const cricket::MediaContentDescription* media_description =
content_info.media_description();
const cricket::TransportInfo& transport_info =
description->transport_infos()[i];
if (content_info.rejected) {
@ -628,23 +619,10 @@ RTCError JsepTransportController::ApplyDescription_n(
}
std::vector<int> extension_ids;
absl::optional<std::string> media_alt_protocol;
absl::optional<std::string> data_alt_protocol;
if (bundled_mid() && content_info.name == *bundled_mid()) {
extension_ids = merged_encrypted_extension_ids;
media_alt_protocol = bundle_media_alt_protocol;
data_alt_protocol = bundle_data_alt_protocol;
} else {
extension_ids = GetEncryptedHeaderExtensionIds(content_info);
switch (media_description->type()) {
case cricket::MEDIA_TYPE_AUDIO:
case cricket::MEDIA_TYPE_VIDEO:
media_alt_protocol = media_description->alt_protocol();
break;
case cricket::MEDIA_TYPE_DATA:
data_alt_protocol = media_description->alt_protocol();
break;
}
}
int rtp_abs_sendtime_extn_id =
@ -658,8 +636,7 @@ RTCError JsepTransportController::ApplyDescription_n(
cricket::JsepTransportDescription jsep_description =
CreateJsepTransportDescription(content_info, transport_info,
extension_ids, rtp_abs_sendtime_extn_id,
media_alt_protocol, data_alt_protocol);
extension_ids, rtp_abs_sendtime_extn_id);
if (local) {
error =
transport->SetLocalJsepTransportDescription(jsep_description, type);
@ -860,9 +837,7 @@ JsepTransportController::CreateJsepTransportDescription(
const cricket::ContentInfo& content_info,
const cricket::TransportInfo& transport_info,
const std::vector<int>& encrypted_extension_ids,
int rtp_abs_sendtime_extn_id,
absl::optional<std::string> media_alt_protocol,
absl::optional<std::string> data_alt_protocol) {
int rtp_abs_sendtime_extn_id) {
const cricket::MediaContentDescription* content_desc =
content_info.media_description();
RTC_DCHECK(content_desc);
@ -872,8 +847,7 @@ JsepTransportController::CreateJsepTransportDescription(
return cricket::JsepTransportDescription(
rtcp_mux_enabled, content_desc->cryptos(), encrypted_extension_ids,
rtp_abs_sendtime_extn_id, transport_info.description, media_alt_protocol,
data_alt_protocol);
rtp_abs_sendtime_extn_id, transport_info.description);
}
bool JsepTransportController::ShouldUpdateBundleGroup(
@ -939,55 +913,6 @@ JsepTransportController::MergeEncryptedHeaderExtensionIdsForBundle(
return merged_ids;
}
RTCError JsepTransportController::GetAltProtocolsForBundle(
const cricket::SessionDescription* description,
absl::optional<std::string>* media_alt_protocol,
absl::optional<std::string>* data_alt_protocol) {
RTC_DCHECK(description);
RTC_DCHECK(bundle_group_);
RTC_DCHECK(media_alt_protocol);
RTC_DCHECK(data_alt_protocol);
bool found_media = false;
bool found_data = false;
for (const cricket::ContentInfo& content : description->contents()) {
if (bundle_group_->HasContentName(content.name)) {
const cricket::MediaContentDescription* media_description =
content.media_description();
switch (media_description->type()) {
case cricket::MEDIA_TYPE_AUDIO:
case cricket::MEDIA_TYPE_VIDEO:
if (found_media &&
*media_alt_protocol != media_description->alt_protocol()) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The BUNDLE group contains conflicting "
"alt-protocols for media ('" +
media_alt_protocol->value_or("") + "' and '" +
media_description->alt_protocol().value_or("") +
"')");
}
found_media = true;
*media_alt_protocol = media_description->alt_protocol();
break;
case cricket::MEDIA_TYPE_DATA:
if (found_data &&
*data_alt_protocol != media_description->alt_protocol()) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The BUNDLE group contains conflicting "
"alt-protocols for data ('" +
data_alt_protocol->value_or("") + "' and '" +
media_description->alt_protocol().value_or("") +
"')");
}
found_data = true;
*data_alt_protocol = media_description->alt_protocol();
break;
}
}
}
return RTCError::OK();
}
int JsepTransportController::GetRtpAbsSendTimeHeaderExtensionId(
const cricket::ContentInfo& content_info) {
if (!config_.enable_external_auth) {

10
pc/jsep_transport_controller.h
View File

@ -246,9 +246,7 @@ class JsepTransportController : public sigslot::has_slots<> {
const cricket::ContentInfo& content_info,
const cricket::TransportInfo& transport_info,
const std::vector<int>& encrypted_extension_ids,
int rtp_abs_sendtime_extn_id,
absl::optional<std::string> media_alt_protocol,
absl::optional<std::string> data_alt_protocol);
int rtp_abs_sendtime_extn_id);
absl::optional<std::string> bundled_mid() const {
absl::optional<std::string> bundled_mid;
@ -270,12 +268,6 @@ class JsepTransportController : public sigslot::has_slots<> {
std::vector<int> GetEncryptedHeaderExtensionIds(
const cricket::ContentInfo& content_info);
// Extracts the alt-protocol settings that apply to the bundle group.
RTCError GetAltProtocolsForBundle(
const cricket::SessionDescription* description,
absl::optional<std::string>* media_alt_protocol,
absl::optional<std::string>* data_alt_protocol);
int GetRtpAbsSendTimeHeaderExtensionId(
const cricket::ContentInfo& content_info);

5
pc/media_session.cc
View File

@ -645,8 +645,6 @@ static bool CreateContentOffer(
}
}
offer->set_alt_protocol(media_description_options.alt_protocol);
if (secure_policy == SEC_REQUIRED && offer->cryptos().empty()) {
return false;
}
@ -1202,9 +1200,6 @@ static bool CreateMediaContentAnswer(
answer->set_direction(NegotiateRtpTransceiverDirection(
offer->direction(), media_description_options.direction));
if (offer->alt_protocol() == media_description_options.alt_protocol) {
answer->set_alt_protocol(media_description_options.alt_protocol);
}
return true;
}

1
pc/media_session.h
View File

@ -78,7 +78,6 @@ struct MediaDescriptionOptions {
// stream information goes in the local descriptions.
std::vector<SenderOptions> sender_options;
std::vector<webrtc::RtpCodecCapability> codec_preferences;
absl::optional<std::string> alt_protocol;
private:
// Doesn't DCHECK on |type|.

118
pc/media_session_unittest.cc
View File

@ -3624,124 +3624,6 @@ TEST_F(MediaSessionDescriptionFactoryTest,
TestTransportInfo(false, options, true);
}
TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolAddedToOffer) {
MediaSessionOptions options;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options);
AddDataSection(cricket::DCT_RTP, RtpTransceiverDirection::kRecvOnly,
&options);
FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo";
FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar";
FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz";
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(options, nullptr);
EXPECT_EQ(offer->GetContentDescriptionByName("audio")->alt_protocol(), "foo");
EXPECT_EQ(offer->GetContentDescriptionByName("video")->alt_protocol(), "bar");
EXPECT_EQ(offer->GetContentDescriptionByName("data")->alt_protocol(), "baz");
}
TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolAddedToAnswer) {
MediaSessionOptions options;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options);
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly,
&options);
FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo";
FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar";
FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz";
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(options, nullptr);
std::unique_ptr<SessionDescription> answer =
f1_.CreateAnswer(offer.get(), options, nullptr);
EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(),
"foo");
EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(),
"bar");
EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(), "baz");
}
TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolNotInOffer) {
MediaSessionOptions options;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options);
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly,
&options);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(options, nullptr);
FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo";
FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar";
FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz";
std::unique_ptr<SessionDescription> answer =
f1_.CreateAnswer(offer.get(), options, nullptr);
EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(),
absl::nullopt);
EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(),
absl::nullopt);
EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(),
absl::nullopt);
}
TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolDifferentInOffer) {
MediaSessionOptions options;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options);
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly,
&options);
FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "not-foo";
FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "not-bar";
FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "not-baz";
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(options, nullptr);
FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo";
FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar";
FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz";
std::unique_ptr<SessionDescription> answer =
f1_.CreateAnswer(offer.get(), options, nullptr);
EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(),
absl::nullopt);
EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(),
absl::nullopt);
EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(),
absl::nullopt);
}
TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolNotInAnswer) {
MediaSessionOptions options;
AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options);
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly,
&options);
FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo";
FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar";
FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz";
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(options, nullptr);
FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol =
absl::nullopt;
FindFirstMediaDescriptionByMid("video", &options)->alt_protocol =
absl::nullopt;
FindFirstMediaDescriptionByMid("data", &options)->alt_protocol =
absl::nullopt;
std::unique_ptr<SessionDescription> answer =
f1_.CreateAnswer(offer.get(), options, nullptr);
EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(),
absl::nullopt);
EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(),
absl::nullopt);
EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(),
absl::nullopt);
}
// Create an offer with bundle enabled and verify the crypto parameters are
// the common set of the available cryptos.
TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoWithOfferBundle) {

9
pc/session_description.h
View File

@ -253,13 +253,6 @@ class MediaContentDescription {
receive_rids_ = rids;
}
virtual const absl::optional<std::string>& alt_protocol() const {
return alt_protocol_;
}
virtual void set_alt_protocol(const absl::optional<std::string>& protocol) {
alt_protocol_ = protocol;
}
protected:
bool rtcp_mux_ = false;
bool rtcp_reduced_size_ = false;
@ -282,8 +275,6 @@ class MediaContentDescription {
SimulcastDescription simulcast_;
std::vector<RidDescription> receive_rids_;
absl::optional<std::string> alt_protocol_;
private:
// Copy function that returns a raw pointer. Caller will assert ownership.
// Should only be called by the Clone() function. Must be implemented

27
pc/webrtc_sdp.cc
View File

@ -229,9 +229,6 @@ static const char kApplicationSpecificMaximum[] = "AS";
static const char kDefaultSctpmapProtocol[] = "webrtc-datachannel";
// This is a non-standardized setting for plugin transports.
static const char kAltProtocolLine[] = "x-alt-protocol";
// RTP payload type is in the 0-127 range. Use -1 to indicate "all" payload
// types.
const int kWildcardPayloadType = -1;
@ -520,14 +517,6 @@ static void InitAttrLine(const std::string& attribute, rtc::StringBuilder* os) {
InitLine(kLineTypeAttributes, attribute, os);
}
static void AddAltProtocolLine(const std::string& protocol,
std::string* message) {
rtc::StringBuilder os;
InitAttrLine(kAltProtocolLine, &os);
os << kSdpDelimiterColon << protocol;
AddLine(os.str(), message);
}
// Writes a SDP attribute line based on |attribute| and |value| to |message|.
static void AddAttributeLine(const std::string& attribute,
int value,
@ -1520,10 +1509,6 @@ void BuildMediaDescription(const ContentInfo* content_info,
}
}
if (media_desc->alt_protocol()) {
AddAltProtocolLine(*media_desc->alt_protocol(), message);
}
// RFC 3388
// mid-attribute = "a=mid:" identification-tag
// identification-tag = token
@ -2086,12 +2071,6 @@ bool ParseConnectionData(const std::string& line,
return true;
}
bool ParseAltProtocolLine(const std::string& line,
std::string* protocol,
SdpParseError* error) {
return GetValue(line, kAltProtocolLine, protocol, error);
}
bool ParseSessionDescription(const std::string& message,
size_t* pos,
std::string* session_id,
@ -3098,12 +3077,6 @@ bool ParseContent(const std::string& message,
if (!ParseIceOptions(line, &transport->transport_options, error)) {
return false;
}
} else if (HasAttribute(line, kAltProtocolLine)) {
std::string alt_protocol;
if (!ParseAltProtocolLine(line, &alt_protocol, error)) {
return false;
}
media_desc->set_alt_protocol(alt_protocol);
} else if (HasAttribute(line, kAttributeFmtp)) {
if (!ParseFmtpAttributes(line, media_type, media_desc, error)) {
return false;

44
pc/webrtc_sdp_unittest.cc
View File

@ -1528,8 +1528,6 @@ class WebRtcSdpTest : public ::testing::Test {
CompareSimulcastDescription(
c1.media_description()->simulcast_description(),
c2.media_description()->simulcast_description());
EXPECT_EQ(c1.media_description()->alt_protocol(),
c2.media_description()->alt_protocol());
}
// group
@ -1677,14 +1675,6 @@ class WebRtcSdpTest : public ::testing::Test {
desc_.AddTransportInfo(transport_info);
}
void AddAltProtocol(const std::string& content_name,
const std::string& alt_protocol) {
ASSERT_TRUE(desc_.GetTransportInfoByName(content_name) != NULL);
cricket::MediaContentDescription* description =
desc_.GetContentDescriptionByName(content_name);
description->set_alt_protocol(alt_protocol);
}
void AddFingerprint() {
desc_.RemoveTransportInfoByName(kAudioContentName);
desc_.RemoveTransportInfoByName(kVideoContentName);
@ -2225,22 +2215,6 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithIceOptions) {
EXPECT_EQ(sdp_with_ice_options, message);
}
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithAltProtocol) {
AddAltProtocol(kAudioContentName, "foo");
AddAltProtocol(kVideoContentName, "bar");
ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(),
jdesc_.session_version()));
std::string message = webrtc::SdpSerialize(jdesc_);
std::string sdp_with_alt_protocol = kSdpFullString;
InjectAfter(kAttributeIcePwdVoice, "a=x-alt-protocol:foo\r\n",
&sdp_with_alt_protocol);
InjectAfter(kAttributeIcePwdVideo, "a=x-alt-protocol:bar\r\n",
&sdp_with_alt_protocol);
EXPECT_EQ(message, sdp_with_alt_protocol);
}
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithRecvOnlyContent) {
EXPECT_TRUE(TestSerializeDirection(RtpTransceiverDirection::kRecvOnly));
}
@ -2653,24 +2627,6 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithIceOptions) {
EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_ice_options));
}
TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithAltProtocol) {
std::string sdp_with_alt_protocol = kSdpFullString;
InjectAfter(kAttributeIcePwdVoice, "a=x-alt-protocol:foo\r\n",
&sdp_with_alt_protocol);
InjectAfter(kAttributeIcePwdVideo, "a=x-alt-protocol:bar\r\n",
&sdp_with_alt_protocol);
JsepSessionDescription jdesc_with_alt_protocol(kDummyType);
EXPECT_TRUE(SdpDeserialize(sdp_with_alt_protocol, &jdesc_with_alt_protocol));
AddAltProtocol(kAudioContentName, "foo");
AddAltProtocol(kVideoContentName, "bar");
ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(),
jdesc_.session_version()));
EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_alt_protocol));
}
TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithUfragPwd) {
// Remove the original ice-ufrag and ice-pwd
JsepSessionDescription jdesc_with_ufrag_pwd(kDummyType);