From 0854eb6aa24887abd31f9d2be2e4a859d00b4a76 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Wed, 10 Oct 2018 22:33:20 +0200 Subject: [PATCH] Respond to SDP request extmap-allow-mixed. The SDP attribute extmap-allow-mixed shows that the client supports mixing of one- and two-byte header extensions within the same stream. This is supported on the receive side since CL "Parse two-byte header extensions", commit 07ba2b9445525da3eabf7c188d631abf32279d98. Bug: webrtc:7990 Change-Id: I8419da48673f513fcca21a8722614f4601a075fc Reviewed-on: https://webrtc-review.googlesource.com/c/103420 Commit-Queue: Johannes Kron Reviewed-by: Seth Hampson Cr-Commit-Position: refs/heads/master@{#25098} --- pc/mediasession.cc | 6 ++ pc/mediasession_unittest.cc | 66 +++++++++++++++++++++ pc/sessiondescription.h | 42 ++++++++++++++ pc/webrtcsdp.cc | 30 +++++++++- pc/webrtcsdp_unittest.cc | 113 ++++++++++++++++++++++++++++++++++++ 5 files changed, 255 insertions(+), 2 deletions(-) diff --git a/pc/mediasession.cc b/pc/mediasession.cc index 9ae46be27c..2e4accc708 100644 --- a/pc/mediasession.cc +++ b/pc/mediasession.cc @@ -1069,6 +1069,9 @@ static bool CreateMediaContentAnswer( NegotiateCodecs(local_codecs, offer->codecs(), &negotiated_codecs); answer->AddCodecs(negotiated_codecs); answer->set_protocol(offer->protocol()); + + answer->set_mixed_one_two_byte_header_extensions_supported( + offer->mixed_one_two_byte_header_extensions_supported()); RtpHeaderExtensions negotiated_rtp_extensions; NegotiateRtpHeaderExtensions( local_rtp_extenstions, offer->rtp_header_extensions(), @@ -1396,6 +1399,9 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer( // Transport info shared by the bundle group. std::unique_ptr bundle_transport; + answer->set_mixed_one_two_byte_header_extensions_supported( + offer->mixed_one_two_byte_header_extensions_supported()); + // Get list of all possible codecs that respects existing payload type // mappings and uses a single payload type space. // diff --git a/pc/mediasession_unittest.cc b/pc/mediasession_unittest.cc index db325ed2ef..7162a3713c 100644 --- a/pc/mediasession_unittest.cc +++ b/pc/mediasession_unittest.cc @@ -1581,6 +1581,72 @@ TEST_F(MediaSessionDescriptionFactoryTest, EXPECT_TRUE(dc->rejected); } +TEST_F(MediaSessionDescriptionFactoryTest, + CreateAnswerSupportsMixedOneAndTwoByteHeaderExtensions) { + MediaSessionOptions opts; + std::unique_ptr offer(f1_.CreateOffer(opts, NULL)); + // Offer without request of mixed one- and two-byte header extensions. + offer->set_mixed_one_two_byte_header_extensions_supported(false); + ASSERT_TRUE(offer.get() != NULL); + std::unique_ptr answer_no_support( + f2_.CreateAnswer(offer.get(), opts, NULL)); + EXPECT_FALSE( + answer_no_support->mixed_one_two_byte_header_extensions_supported()); + + // Offer with request of mixed one- and two-byte header extensions. + offer->set_mixed_one_two_byte_header_extensions_supported(true); + ASSERT_TRUE(offer.get() != NULL); + std::unique_ptr answer_support( + f2_.CreateAnswer(offer.get(), opts, NULL)); + EXPECT_TRUE(answer_support->mixed_one_two_byte_header_extensions_supported()); +} + +TEST_F(MediaSessionDescriptionFactoryTest, + CreateAnswerSupportsMixedOneAndTwoByteHeaderExtensionsOnMediaLevel) { + MediaSessionOptions opts; + AddAudioVideoSections(RtpTransceiverDirection::kSendRecv, &opts); + std::unique_ptr offer(f1_.CreateOffer(opts, NULL)); + MediaContentDescription* video_offer = + offer->GetContentDescriptionByName("video"); + ASSERT_TRUE(video_offer); + MediaContentDescription* audio_offer = + offer->GetContentDescriptionByName("audio"); + ASSERT_TRUE(audio_offer); + + // Explicit disable of mixed one-two byte header support in offer. + video_offer->set_mixed_one_two_byte_header_extensions_supported( + MediaContentDescription::kNo); + audio_offer->set_mixed_one_two_byte_header_extensions_supported( + MediaContentDescription::kNo); + + 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->mixed_one_two_byte_header_extensions_supported()); + EXPECT_EQ(MediaContentDescription::kNo, + audio_answer->mixed_one_two_byte_header_extensions_supported()); + + // Enable mixed one-two byte header support in offer. + video_offer->set_mixed_one_two_byte_header_extensions_supported( + MediaContentDescription::kMedia); + audio_offer->set_mixed_one_two_byte_header_extensions_supported( + 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->mixed_one_two_byte_header_extensions_supported()); + EXPECT_EQ(MediaContentDescription::kMedia, + audio_answer->mixed_one_two_byte_header_extensions_supported()); +} + // Create an audio and video offer with: // - one video track // - two audio tracks diff --git a/pc/sessiondescription.h b/pc/sessiondescription.h index 645ebc54f8..4fdbf3ba3e 100644 --- a/pc/sessiondescription.h +++ b/pc/sessiondescription.h @@ -183,6 +183,22 @@ class MediaContentDescription { return connection_address_; } + // Determines if it's allowed to mix one- and two-byte rtp header extensions + // within the same rtp stream. + enum ExtmapAllowMixed { kNo, kSession, kMedia }; + void set_mixed_one_two_byte_header_extensions_supported( + ExtmapAllowMixed supported) { + if (supported == kMedia && + mixed_one_two_byte_header_extensions_supported_ == kSession) { + // Do not downgrade from session level to media level. + return; + } + mixed_one_two_byte_header_extensions_supported_ = supported; + } + ExtmapAllowMixed mixed_one_two_byte_header_extensions_supported() const { + return mixed_one_two_byte_header_extensions_supported_; + } + protected: bool rtcp_mux_ = false; bool rtcp_reduced_size_ = false; @@ -196,6 +212,10 @@ 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 mixed_one_two_byte_header_extensions_supported_ = kNo; }; // TODO(bugs.webrtc.org/8620): Remove this alias once downstream projects have @@ -455,6 +475,23 @@ class SessionDescription { } int msid_signaling() const { return msid_signaling_; } + // Determines if it's allowed to mix one- and two-byte rtp header extensions + // within the same rtp stream. + void set_mixed_one_two_byte_header_extensions_supported(bool supported) { + mixed_one_two_byte_header_extensions_supported_ = supported; + MediaContentDescription::ExtmapAllowMixed extmap_allow_mixed = + supported ? MediaContentDescription::kSession + : MediaContentDescription::kNo; + for (auto& content : contents_) { + content.media_description() + ->set_mixed_one_two_byte_header_extensions_supported( + extmap_allow_mixed); + } + } + bool mixed_one_two_byte_header_extensions_supported() const { + return mixed_one_two_byte_header_extensions_supported_; + } + private: SessionDescription(const SessionDescription&); @@ -465,6 +502,11 @@ class SessionDescription { // Default to what Plan B would do. // TODO(bugs.webrtc.org/8530): Change default to kMsidSignalingMediaSection. int msid_signaling_ = kMsidSignalingSsrcAttribute; + // TODO(kron): 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 mixed_one_two_byte_header_extensions_supported_ = false; }; // Indicates whether a session description was sent by the local client or diff --git a/pc/webrtcsdp.cc b/pc/webrtcsdp.cc index 665b4d411a..67d4bbb408 100644 --- a/pc/webrtcsdp.cc +++ b/pc/webrtcsdp.cc @@ -119,6 +119,7 @@ static const char kAttributeRtcpMux[] = "rtcp-mux"; static const char kAttributeRtcpReducedSize[] = "rtcp-rsize"; static const char kAttributeSsrc[] = "ssrc"; static const char kSsrcAttributeCname[] = "cname"; +static const char kAttributeExtmapAllowMixed[] = "extmap-allow-mixed"; static const char kAttributeExtmap[] = "extmap"; // draft-alvestrand-mmusic-msid-01 // a=msid-semantic: WMS @@ -852,6 +853,12 @@ std::string SdpSerialize(const JsepSessionDescription& jdesc) { AddLine(group_line, &message); } + // Mixed one- and two-byte header extension. + if (desc->mixed_one_two_byte_header_extensions_supported()) { + InitAttrLine(kAttributeExtmapAllowMixed, &os); + AddLine(os.str(), &message); + } + // MediaStream semantics InitAttrLine(kAttributeMsidSemantics, &os); os << kSdpDelimiterColon << " " << kMediaStreamSemantic; @@ -1482,7 +1489,17 @@ void BuildRtpContentAttributes(const MediaContentDescription* media_desc, int msid_signaling, std::string* message) { rtc::StringBuilder os; - // RFC 5285 + // RFC 8285 + // a=extmap-allow-mixed + // The attribute MUST be either on session level or media level. We support + // responding on both levels, however, we don't respond on media level if it's + // set on session level. + if (media_desc->mixed_one_two_byte_header_extensions_supported() == + MediaContentDescription::kMedia) { + InitAttrLine(kAttributeExtmapAllowMixed, &os); + AddLine(os.str(), message); + } + // RFC 8285 // a=extmap:["/"] // The definitions MUST be either all session level or all media level. This // implementation uses all media level. @@ -1996,7 +2013,7 @@ bool ParseSessionDescription(const std::string& message, std::string line; desc->set_msid_supported(false); - + desc->set_mixed_one_two_byte_header_extensions_supported(false); // RFC 4566 // v= (protocol version) if (!GetLineWithType(message, pos, &line, kLineTypeVersion)) { @@ -2133,6 +2150,8 @@ bool ParseSessionDescription(const std::string& message, } desc->set_msid_supported( CaseInsensitiveFind(semantics, kMediaStreamSemantic)); + } else if (HasAttribute(line, kAttributeExtmapAllowMixed)) { + desc->set_mixed_one_two_byte_header_extensions_supported(true); } else if (HasAttribute(line, kAttributeExtmap)) { RtpExtension extmap; if (!ParseExtmap(line, &extmap, error)) { @@ -2502,6 +2521,10 @@ bool ParseMediaDescription(const std::string& message, } if (IsRtp(protocol)) { + if (desc->mixed_one_two_byte_header_extensions_supported()) { + content->set_mixed_one_two_byte_header_extensions_supported( + MediaContentDescription::kSession); + } // Set the extmap. if (!session_extmaps.empty() && !content->rtp_header_extensions().empty()) { @@ -2902,6 +2925,9 @@ bool ParseContent(const std::string& message, media_desc->set_direction(RtpTransceiverDirection::kInactive); } else if (HasAttribute(line, kAttributeSendRecv)) { media_desc->set_direction(RtpTransceiverDirection::kSendRecv); + } else if (HasAttribute(line, kAttributeExtmapAllowMixed)) { + media_desc->set_mixed_one_two_byte_header_extensions_supported( + MediaContentDescription::kMedia); } else if (HasAttribute(line, kAttributeExtmap)) { RtpExtension extmap; if (!ParseExtmap(line, &extmap, error)) { diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc index 9c39c41751..6a95685b0a 100644 --- a/pc/webrtcsdp_unittest.cc +++ b/pc/webrtcsdp_unittest.cc @@ -90,6 +90,7 @@ static const char kAttributeCryptoVideo[] = static const char kFingerprint[] = "a=fingerprint:sha-1 " "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"; +static const char kExtmapAllowMixed[] = "a=extmap-allow-mixed\r\n"; static const int kExtmapId = 1; static const char kExtmapUri[] = "http://example.com/082005/ext.htm#ttime"; static const char kExtmap[] = @@ -1283,6 +1284,10 @@ class WebRtcSdpTest : public testing::Test { // streams EXPECT_EQ(cd1->streams(), cd2->streams()); + // extmap-allow-mixed + EXPECT_EQ(cd1->mixed_one_two_byte_header_extensions_supported(), + cd2->mixed_one_two_byte_header_extensions_supported()); + // extmap ASSERT_EQ(cd1->rtp_header_extensions().size(), cd2->rtp_header_extensions().size()); @@ -1399,6 +1404,8 @@ class WebRtcSdpTest : public testing::Test { // global attributes EXPECT_EQ(desc1.msid_supported(), desc2.msid_supported()); + EXPECT_EQ(desc1.mixed_one_two_byte_header_extensions_supported(), + desc2.mixed_one_two_byte_header_extensions_supported()); } bool CompareSessionDescription(const JsepSessionDescription& desc1, @@ -2095,6 +2102,26 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithDataChannelAndBandwidth) { EXPECT_EQ(expected_sdp, message); } +TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithExtmapAllowMixed) { + jdesc_.description()->set_mixed_one_two_byte_header_extensions_supported( + true); + TestSerialize(jdesc_); +} + +TEST_F(WebRtcSdpTest, SerializeMediaContentDescriptionWithExtmapAllowMixed) { + cricket::MediaContentDescription* video_desc = + jdesc_.description()->GetContentDescriptionByName(kVideoContentName); + ASSERT_TRUE(video_desc); + cricket::MediaContentDescription* audio_desc = + jdesc_.description()->GetContentDescriptionByName(kAudioContentName); + ASSERT_TRUE(audio_desc); + video_desc->set_mixed_one_two_byte_header_extensions_supported( + cricket::MediaContentDescription::kMedia); + audio_desc->set_mixed_one_two_byte_header_extensions_supported( + cricket::MediaContentDescription::kMedia); + TestSerialize(jdesc_); +} + TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithExtmap) { bool encrypted = false; AddExtmap(encrypted); @@ -2432,6 +2459,92 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutMsid) { EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc)); } +TEST_F(WebRtcSdpTest, SessionLevelMixedHeaderExtensionsAlsoSetsMediaSetting) { + cricket::MediaContentDescription* video_desc = + jdesc_.description()->GetContentDescriptionByName(kVideoContentName); + ASSERT_TRUE(video_desc); + cricket::MediaContentDescription* audio_desc = + jdesc_.description()->GetContentDescriptionByName(kAudioContentName); + ASSERT_TRUE(audio_desc); + + // Setting true on session level propagates to media level. + jdesc_.description()->set_mixed_one_two_byte_header_extensions_supported( + true); + EXPECT_EQ(cricket::MediaContentDescription::kSession, + video_desc->mixed_one_two_byte_header_extensions_supported()); + EXPECT_EQ(cricket::MediaContentDescription::kSession, + audio_desc->mixed_one_two_byte_header_extensions_supported()); + + // Don't downgrade from session level to media level + video_desc->set_mixed_one_two_byte_header_extensions_supported( + cricket::MediaContentDescription::kMedia); + EXPECT_EQ(cricket::MediaContentDescription::kSession, + video_desc->mixed_one_two_byte_header_extensions_supported()); + + // Setting false on session level propagates to media level. + jdesc_.description()->set_mixed_one_two_byte_header_extensions_supported( + false); + EXPECT_EQ(cricket::MediaContentDescription::kNo, + video_desc->mixed_one_two_byte_header_extensions_supported()); + EXPECT_EQ(cricket::MediaContentDescription::kNo, + audio_desc->mixed_one_two_byte_header_extensions_supported()); + + // Now possible to set at media level. + video_desc->set_mixed_one_two_byte_header_extensions_supported( + cricket::MediaContentDescription::kMedia); + EXPECT_EQ(cricket::MediaContentDescription::kMedia, + video_desc->mixed_one_two_byte_header_extensions_supported()); +} + +TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithExtmapAllowMixed) { + jdesc_.description()->set_mixed_one_two_byte_header_extensions_supported( + 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)); + // Verify + EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_deserialized)); +} + +TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutExtmapAllowMixed) { + jdesc_.description()->set_mixed_one_two_byte_header_extensions_supported( + false); + std::string sdp_without_extmap_allow_mixed = kSdpFullString; + // Deserialize + JsepSessionDescription jdesc_deserialized(kDummyType); + EXPECT_TRUE( + SdpDeserialize(sdp_without_extmap_allow_mixed, &jdesc_deserialized)); + // Verify + EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_deserialized)); +} + +TEST_F(WebRtcSdpTest, DeserializeMediaContentDescriptionWithExtmapAllowMixed) { + cricket::MediaContentDescription* video_desc = + jdesc_.description()->GetContentDescriptionByName(kVideoContentName); + ASSERT_TRUE(video_desc); + cricket::MediaContentDescription* audio_desc = + jdesc_.description()->GetContentDescriptionByName(kAudioContentName); + ASSERT_TRUE(audio_desc); + video_desc->set_mixed_one_two_byte_header_extensions_supported( + cricket::MediaContentDescription::kMedia); + audio_desc->set_mixed_one_two_byte_header_extensions_supported( + cricket::MediaContentDescription::kMedia); + + std::string sdp_with_extmap_allow_mixed = kSdpFullString; + InjectAfter("a=mid:audio_content_name\r\n", kExtmapAllowMixed, + &sdp_with_extmap_allow_mixed); + InjectAfter("a=mid:video_content_name\r\n", kExtmapAllowMixed, + &sdp_with_extmap_allow_mixed); + + // Deserialize + JsepSessionDescription jdesc_deserialized(kDummyType); + EXPECT_TRUE(SdpDeserialize(sdp_with_extmap_allow_mixed, &jdesc_deserialized)); + // Verify + EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_deserialized)); +} + TEST_F(WebRtcSdpTest, DeserializeCandidate) { JsepIceCandidate jcandidate(kDummyMid, kDummyIndex);