diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 92d965b328..d7f4e19f4b 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -621,12 +621,8 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { absl::optional 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 d8cb1591a9..940d746e5f 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -2347,64 +2347,112 @@ TEST_F(MediaSessionDescriptionFactoryTest, } TEST_F(MediaSessionDescriptionFactoryTest, - CreateAnswerSupportsMixedOneAndTwoByteHeaderExtensions) { + OfferAndAnswerDoesNotHaveMixedByteSessionAttribute) { MediaSessionOptions opts; - std::unique_ptr offer = f1_.CreateOffer(opts, NULL); - // Offer without request of mixed one- and two-byte header extensions. + std::unique_ptr offer = + f1_.CreateOffer(opts, /*current_description=*/nullptr); offer->set_extmap_allow_mixed(false); - ASSERT_TRUE(offer.get() != NULL); - std::unique_ptr 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 answer( + f2_.CreateAnswer(offer.get(), opts, /*current_description=*/nullptr)); + + EXPECT_FALSE(answer->extmap_allow_mixed()); +} + +TEST_F(MediaSessionDescriptionFactoryTest, + OfferAndAnswerHaveMixedByteSessionAttribute) { + MediaSessionOptions opts; + std::unique_ptr offer = + f1_.CreateOffer(opts, /*current_description=*/nullptr); offer->set_extmap_allow_mixed(true); - ASSERT_TRUE(offer.get() != NULL); + std::unique_ptr 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 offer = f1_.CreateOffer(opts, NULL); - MediaContentDescription* video_offer = - offer->GetContentDescriptionByName("video"); - ASSERT_TRUE(video_offer); + std::unique_ptr 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 answer( + f2_.CreateAnswer(offer.get(), opts, /*current_description=*/nullptr)); - ASSERT_TRUE(offer.get() != NULL); - std::unique_ptr 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 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 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 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 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 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 abedf48688..b7be44ddb5 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -3900,17 +3900,17 @@ TEST_P(PeerConnectionInterfaceTest, TEST_P(PeerConnectionInterfaceTest, ExtmapAllowMixedIsConfigurable) { RTCConfiguration config; - // Default behavior is false. + // Default behavior is true. CreatePeerConnection(config); std::unique_ptr 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 52a3a1fe04..9ec371efa0 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -272,10 +272,7 @@ class MediaContentDescription { 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 receive_rids_; @@ -633,12 +630,7 @@ class SessionDescription { // 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 75e0974ecd..c990cf6d5f 100644 --- a/pc/session_description_unittest.cc +++ b/pc/session_description_unittest.cc @@ -17,7 +17,8 @@ namespace cricket { 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 edd8db6a96..8ec5a7bc9c 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -2570,6 +2570,7 @@ static std::unique_ptr ParseContentDescription( std::vector>* candidates, webrtc::SdpParseError* error) { auto media_desc = std::make_unique(); + 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 cf5384725b..12ab730df1 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -153,6 +153,7 @@ static const char kSdpFullString[] = "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 @@ static const char kSdpString[] = "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 @@ static const char kBundleOnlySdpFullString[] = "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 @@ static const char kPlanBSdpFullString[] = "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 @@ static const char kUnifiedPlanSdpFullString[] = "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 @@ static const char kUnifiedPlanSdpFullStringWithSpecialMsid[] = "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, DeserializeSessionDescriptionWithoutMsid) { 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, DeserializeSessionDescriptionWithExtmapAllowMixed) { 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));