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}
This commit is contained in:
parent
1e75df26e3
commit
801c9995c8
@ -621,12 +621,8 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
|
||||
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
|
||||
|
||||
@ -2347,64 +2347,112 @@ TEST_F(MediaSessionDescriptionFactoryTest,
|
||||
}
|
||||
|
||||
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:
|
||||
|
||||
@ -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<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,
|
||||
|
||||
@ -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<RidDescription> 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
|
||||
|
||||
@ -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) {
|
||||
|
||||
@ -2570,6 +2570,7 @@ static std::unique_ptr<C> ParseContentDescription(
|
||||
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)) {
|
||||
|
||||
@ -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));
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user