Signal extmap-allow-mixed by default on session level
The extmap-allow-mixed SDP attribute signals that one- and two-byte RTP
header extensions can be mixed. In practice, this also means that WebRTC
will support two-byte RTP header extensions when this is signaled by
both peers.
Bug: webrtc:9985
Change-Id: I80a3f97bab162c7d9a5acf2cae07b977641c039d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/197943
Commit-Queue: Emil Lundmark <lndmrk@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33036}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index 92d965b..d7f4e19f 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -621,12 +621,8 @@
absl::optional<CryptoOptions> crypto_options;
// Configure if we should include the SDP attribute extmap-allow-mixed in
- // our offer. Although we currently do support this, it's not included in
- // our offer by default due to a previous bug that caused the SDP parser to
- // abort parsing if this attribute was present. This is fixed in Chrome 71.
- // TODO(webrtc:9985): Change default to true once sufficient time has
- // passed.
- bool offer_extmap_allow_mixed = false;
+ // our offer on session level.
+ bool offer_extmap_allow_mixed = true;
// TURN logging identifier.
// This identifier is added to a TURN allocation
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc
index d8cb159..940d746 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -2347,64 +2347,112 @@
}
TEST_F(MediaSessionDescriptionFactoryTest,
- CreateAnswerSupportsMixedOneAndTwoByteHeaderExtensions) {
+ OfferAndAnswerDoesNotHaveMixedByteSessionAttribute) {
MediaSessionOptions opts;
- std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
- // Offer without request of mixed one- and two-byte header extensions.
+ std::unique_ptr<SessionDescription> offer =
+ f1_.CreateOffer(opts, /*current_description=*/nullptr);
offer->set_extmap_allow_mixed(false);
- ASSERT_TRUE(offer.get() != NULL);
- std::unique_ptr<SessionDescription> answer_no_support(
- f2_.CreateAnswer(offer.get(), opts, NULL));
- EXPECT_FALSE(answer_no_support->extmap_allow_mixed());
- // Offer with request of mixed one- and two-byte header extensions.
+ std::unique_ptr<SessionDescription> answer(
+ f2_.CreateAnswer(offer.get(), opts, /*current_description=*/nullptr));
+
+ EXPECT_FALSE(answer->extmap_allow_mixed());
+}
+
+TEST_F(MediaSessionDescriptionFactoryTest,
+ OfferAndAnswerHaveMixedByteSessionAttribute) {
+ MediaSessionOptions opts;
+ std::unique_ptr<SessionDescription> offer =
+ f1_.CreateOffer(opts, /*current_description=*/nullptr);
offer->set_extmap_allow_mixed(true);
- ASSERT_TRUE(offer.get() != NULL);
+
std::unique_ptr<SessionDescription> answer_support(
- f2_.CreateAnswer(offer.get(), opts, NULL));
+ f2_.CreateAnswer(offer.get(), opts, /*current_description=*/nullptr));
+
EXPECT_TRUE(answer_support->extmap_allow_mixed());
}
TEST_F(MediaSessionDescriptionFactoryTest,
- CreateAnswerSupportsMixedOneAndTwoByteHeaderExtensionsOnMediaLevel) {
+ OfferAndAnswerDoesNotHaveMixedByteMediaAttributes) {
MediaSessionOptions opts;
AddAudioVideoSections(RtpTransceiverDirection::kSendRecv, &opts);
- std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
- MediaContentDescription* video_offer =
- offer->GetContentDescriptionByName("video");
- ASSERT_TRUE(video_offer);
+ std::unique_ptr<SessionDescription> offer =
+ f1_.CreateOffer(opts, /*current_description=*/nullptr);
+ offer->set_extmap_allow_mixed(false);
MediaContentDescription* audio_offer =
offer->GetContentDescriptionByName("audio");
- ASSERT_TRUE(audio_offer);
+ MediaContentDescription* video_offer =
+ offer->GetContentDescriptionByName("video");
+ ASSERT_EQ(MediaContentDescription::kNo,
+ audio_offer->extmap_allow_mixed_enum());
+ ASSERT_EQ(MediaContentDescription::kNo,
+ video_offer->extmap_allow_mixed_enum());
- // Explicit disable of mixed one-two byte header support in offer.
- video_offer->set_extmap_allow_mixed_enum(MediaContentDescription::kNo);
- audio_offer->set_extmap_allow_mixed_enum(MediaContentDescription::kNo);
+ std::unique_ptr<SessionDescription> answer(
+ f2_.CreateAnswer(offer.get(), opts, /*current_description=*/nullptr));
- ASSERT_TRUE(offer.get() != NULL);
- std::unique_ptr<SessionDescription> answer_no_support(
- f2_.CreateAnswer(offer.get(), opts, NULL));
- MediaContentDescription* video_answer =
- answer_no_support->GetContentDescriptionByName("video");
MediaContentDescription* audio_answer =
- answer_no_support->GetContentDescriptionByName("audio");
- EXPECT_EQ(MediaContentDescription::kNo,
- video_answer->extmap_allow_mixed_enum());
+ answer->GetContentDescriptionByName("audio");
+ MediaContentDescription* video_answer =
+ answer->GetContentDescriptionByName("video");
EXPECT_EQ(MediaContentDescription::kNo,
audio_answer->extmap_allow_mixed_enum());
+ EXPECT_EQ(MediaContentDescription::kNo,
+ video_answer->extmap_allow_mixed_enum());
+}
- // Enable mixed one-two byte header support in offer.
- video_offer->set_extmap_allow_mixed_enum(MediaContentDescription::kMedia);
+TEST_F(MediaSessionDescriptionFactoryTest,
+ OfferAndAnswerHaveSameMixedByteMediaAttributes) {
+ MediaSessionOptions opts;
+ AddAudioVideoSections(RtpTransceiverDirection::kSendRecv, &opts);
+ std::unique_ptr<SessionDescription> offer =
+ f1_.CreateOffer(opts, /*current_description=*/nullptr);
+ offer->set_extmap_allow_mixed(false);
+ MediaContentDescription* audio_offer =
+ offer->GetContentDescriptionByName("audio");
audio_offer->set_extmap_allow_mixed_enum(MediaContentDescription::kMedia);
- ASSERT_TRUE(offer.get() != NULL);
- std::unique_ptr<SessionDescription> answer_support(
- f2_.CreateAnswer(offer.get(), opts, NULL));
- video_answer = answer_support->GetContentDescriptionByName("video");
- audio_answer = answer_support->GetContentDescriptionByName("audio");
- EXPECT_EQ(MediaContentDescription::kMedia,
- video_answer->extmap_allow_mixed_enum());
+ MediaContentDescription* video_offer =
+ offer->GetContentDescriptionByName("video");
+ video_offer->set_extmap_allow_mixed_enum(MediaContentDescription::kMedia);
+
+ std::unique_ptr<SessionDescription> answer(
+ f2_.CreateAnswer(offer.get(), opts, /*current_description=*/nullptr));
+
+ MediaContentDescription* audio_answer =
+ answer->GetContentDescriptionByName("audio");
+ MediaContentDescription* video_answer =
+ answer->GetContentDescriptionByName("video");
EXPECT_EQ(MediaContentDescription::kMedia,
audio_answer->extmap_allow_mixed_enum());
+ EXPECT_EQ(MediaContentDescription::kMedia,
+ video_answer->extmap_allow_mixed_enum());
+}
+
+TEST_F(MediaSessionDescriptionFactoryTest,
+ OfferAndAnswerHaveDifferentMixedByteMediaAttributes) {
+ MediaSessionOptions opts;
+ AddAudioVideoSections(RtpTransceiverDirection::kSendRecv, &opts);
+ std::unique_ptr<SessionDescription> offer =
+ f1_.CreateOffer(opts, /*current_description=*/nullptr);
+ offer->set_extmap_allow_mixed(false);
+ MediaContentDescription* audio_offer =
+ offer->GetContentDescriptionByName("audio");
+ audio_offer->set_extmap_allow_mixed_enum(MediaContentDescription::kNo);
+ MediaContentDescription* video_offer =
+ offer->GetContentDescriptionByName("video");
+ video_offer->set_extmap_allow_mixed_enum(MediaContentDescription::kMedia);
+
+ std::unique_ptr<SessionDescription> answer(
+ f2_.CreateAnswer(offer.get(), opts, /*current_description=*/nullptr));
+
+ MediaContentDescription* audio_answer =
+ answer->GetContentDescriptionByName("audio");
+ MediaContentDescription* video_answer =
+ answer->GetContentDescriptionByName("video");
+ EXPECT_EQ(MediaContentDescription::kNo,
+ audio_answer->extmap_allow_mixed_enum());
+ EXPECT_EQ(MediaContentDescription::kMedia,
+ video_answer->extmap_allow_mixed_enum());
}
// Create an audio and video offer with:
diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc
index abedf48..b7be44d 100644
--- a/pc/peer_connection_interface_unittest.cc
+++ b/pc/peer_connection_interface_unittest.cc
@@ -3900,17 +3900,17 @@
TEST_P(PeerConnectionInterfaceTest, ExtmapAllowMixedIsConfigurable) {
RTCConfiguration config;
- // Default behavior is false.
+ // Default behavior is true.
CreatePeerConnection(config);
std::unique_ptr<SessionDescriptionInterface> offer;
ASSERT_TRUE(DoCreateOffer(&offer, nullptr));
- EXPECT_FALSE(offer->description()->extmap_allow_mixed());
- // Possible to set to true.
- config.offer_extmap_allow_mixed = true;
+ EXPECT_TRUE(offer->description()->extmap_allow_mixed());
+ // Possible to set to false.
+ config.offer_extmap_allow_mixed = false;
CreatePeerConnection(config);
offer.release();
ASSERT_TRUE(DoCreateOffer(&offer, nullptr));
- EXPECT_TRUE(offer->description()->extmap_allow_mixed());
+ EXPECT_FALSE(offer->description()->extmap_allow_mixed());
}
INSTANTIATE_TEST_SUITE_P(PeerConnectionInterfaceTest,
diff --git a/pc/session_description.h b/pc/session_description.h
index 52a3a1f..9ec371e 100644
--- a/pc/session_description.h
+++ b/pc/session_description.h
@@ -272,10 +272,7 @@
webrtc::RtpTransceiverDirection direction_ =
webrtc::RtpTransceiverDirection::kSendRecv;
rtc::SocketAddress connection_address_;
- // Mixed one- and two-byte header not included in offer on media level or
- // session level, but we will respond that we support it. The plan is to add
- // it to our offer on session level. See todo in SessionDescription.
- ExtmapAllowMixed extmap_allow_mixed_enum_ = kNo;
+ ExtmapAllowMixed extmap_allow_mixed_enum_ = kMedia;
SimulcastDescription simulcast_;
std::vector<RidDescription> receive_rids_;
@@ -633,12 +630,7 @@
// Default to what Plan B would do.
// TODO(bugs.webrtc.org/8530): Change default to kMsidSignalingMediaSection.
int msid_signaling_ = kMsidSignalingSsrcAttribute;
- // TODO(webrtc:9985): Activate mixed one- and two-byte header extension in
- // offer at session level. It's currently not included in offer by default
- // because clients prior to https://bugs.webrtc.org/9712 cannot parse this
- // correctly. If it's included in offer to us we will respond that we support
- // it.
- bool extmap_allow_mixed_ = false;
+ bool extmap_allow_mixed_ = true;
};
// Indicates whether a session description was sent by the local client or
diff --git a/pc/session_description_unittest.cc b/pc/session_description_unittest.cc
index 75e0974..c990cf6 100644
--- a/pc/session_description_unittest.cc
+++ b/pc/session_description_unittest.cc
@@ -17,7 +17,8 @@
TEST(MediaContentDescriptionTest, ExtmapAllowMixedDefaultValue) {
VideoContentDescription video_desc;
- EXPECT_EQ(MediaContentDescription::kNo, video_desc.extmap_allow_mixed_enum());
+ EXPECT_EQ(MediaContentDescription::kMedia,
+ video_desc.extmap_allow_mixed_enum());
}
TEST(MediaContentDescriptionTest, SetExtmapAllowMixed) {
diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index edd8db6..8ec5a7b 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -2570,6 +2570,7 @@
std::vector<std::unique_ptr<JsepIceCandidate>>* candidates,
webrtc::SdpParseError* error) {
auto media_desc = std::make_unique<C>();
+ media_desc->set_extmap_allow_mixed_enum(MediaContentDescription::kNo);
if (!ParseContent(message, media_type, mline_index, protocol, payload_types,
pos, content_name, bundle_only, msid_signaling,
media_desc.get(), transport, candidates, error)) {
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index cf53847..12ab730 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -153,6 +153,7 @@
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
+ "a=extmap-allow-mixed\r\n"
"a=msid-semantic: WMS local_stream_1\r\n"
"m=audio 2345 RTP/SAVPF 111 103 104\r\n"
"c=IN IP4 74.125.127.126\r\n"
@@ -223,6 +224,7 @@
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
+ "a=extmap-allow-mixed\r\n"
"a=msid-semantic: WMS local_stream_1\r\n"
"m=audio 9 RTP/SAVPF 111 103 104\r\n"
"c=IN IP4 0.0.0.0\r\n"
@@ -373,6 +375,7 @@
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
+ "a=extmap-allow-mixed\r\n"
"a=group:BUNDLE audio_content_name video_content_name\r\n"
"a=msid-semantic: WMS local_stream_1\r\n"
"m=audio 2345 RTP/SAVPF 111 103 104\r\n"
@@ -433,6 +436,7 @@
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
+ "a=extmap-allow-mixed\r\n"
"a=msid-semantic: WMS local_stream_1 local_stream_2\r\n"
"m=audio 2345 RTP/SAVPF 111 103 104\r\n"
"c=IN IP4 74.125.127.126\r\n"
@@ -516,6 +520,7 @@
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
+ "a=extmap-allow-mixed\r\n"
"a=msid-semantic: WMS local_stream_1\r\n"
// Audio track 1, stream 1 (with candidates).
"m=audio 2345 RTP/SAVPF 111 103 104\r\n"
@@ -628,6 +633,7 @@
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
+ "a=extmap-allow-mixed\r\n"
"a=msid-semantic: WMS local_stream_1\r\n"
// Audio track 1, with 1 stream id.
"m=audio 2345 RTP/SAVPF 111 103 104\r\n"
@@ -2752,10 +2758,9 @@
TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithExtmapAllowMixed) {
jdesc_.description()->set_extmap_allow_mixed(true);
std::string sdp_with_extmap_allow_mixed = kSdpFullString;
- InjectAfter("t=0 0\r\n", kExtmapAllowMixed, &sdp_with_extmap_allow_mixed);
// Deserialize
JsepSessionDescription jdesc_deserialized(kDummyType);
- EXPECT_TRUE(SdpDeserialize(sdp_with_extmap_allow_mixed, &jdesc_deserialized));
+ ASSERT_TRUE(SdpDeserialize(sdp_with_extmap_allow_mixed, &jdesc_deserialized));
// Verify
EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_deserialized));
}
@@ -2763,9 +2768,10 @@
TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutExtmapAllowMixed) {
jdesc_.description()->set_extmap_allow_mixed(false);
std::string sdp_without_extmap_allow_mixed = kSdpFullString;
+ Replace(kExtmapAllowMixed, "", &sdp_without_extmap_allow_mixed);
// Deserialize
JsepSessionDescription jdesc_deserialized(kDummyType);
- EXPECT_TRUE(
+ ASSERT_TRUE(
SdpDeserialize(sdp_without_extmap_allow_mixed, &jdesc_deserialized));
// Verify
EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_deserialized));
