From ce8e8677df1c34cc52c2b5f98818455f4cc4bb0c Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Fri, 22 Feb 2019 13:06:44 +0100 Subject: [PATCH] Add support for TransportSequenceNumberV2 in SDP negotiation TransportSequenceNumberV2 is an experimental feature that should not be part of the default offer. However, if we receive an offer with this extension we should respond that we support it. Bug: webrtc:10264 Change-Id: Id2424d421361e5d71f3a608cb8f74b63645c264a Reviewed-on: https://webrtc-review.googlesource.com/c/123783 Commit-Queue: Johannes Kron Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#26817} --- api/rtp_parameters.cc | 3 ++ pc/media_session.cc | 31 ++++++++++++++-- pc/media_session_unittest.cc | 71 ++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc index b51ea7db3c..2b1abe0eff 100644 --- a/api/rtp_parameters.cc +++ b/api/rtp_parameters.cc @@ -167,6 +167,7 @@ constexpr int RtpExtension::kOneByteHeaderExtensionMaxValueSize; bool RtpExtension::IsSupportedForAudio(const std::string& uri) { return uri == webrtc::RtpExtension::kAudioLevelUri || uri == webrtc::RtpExtension::kTransportSequenceNumberUri || + uri == webrtc::RtpExtension::kTransportSequenceNumberV2Uri || uri == webrtc::RtpExtension::kMidUri || uri == webrtc::RtpExtension::kRidUri || uri == webrtc::RtpExtension::kRepairedRidUri; @@ -177,6 +178,7 @@ bool RtpExtension::IsSupportedForVideo(const std::string& uri) { uri == webrtc::RtpExtension::kAbsSendTimeUri || uri == webrtc::RtpExtension::kVideoRotationUri || uri == webrtc::RtpExtension::kTransportSequenceNumberUri || + uri == webrtc::RtpExtension::kTransportSequenceNumberV2Uri || uri == webrtc::RtpExtension::kPlayoutDelayUri || uri == webrtc::RtpExtension::kVideoContentTypeUri || uri == webrtc::RtpExtension::kVideoTimingUri || @@ -202,6 +204,7 @@ bool RtpExtension::IsEncryptionSupported(const std::string& uri) { #endif uri == webrtc::RtpExtension::kVideoRotationUri || uri == webrtc::RtpExtension::kTransportSequenceNumberUri || + uri == webrtc::RtpExtension::kTransportSequenceNumberV2Uri || uri == webrtc::RtpExtension::kPlayoutDelayUri || uri == webrtc::RtpExtension::kVideoContentTypeUri || uri == webrtc::RtpExtension::kMidUri || diff --git a/pc/media_session.cc b/pc/media_session.cc index 19eb769dfc..5e6b97bb43 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -1091,16 +1091,41 @@ static void NegotiateRtpHeaderExtensions( const RtpHeaderExtensions& local_extensions, const RtpHeaderExtensions& offered_extensions, bool enable_encrypted_rtp_header_extensions, - RtpHeaderExtensions* negotiated_extenstions) { + RtpHeaderExtensions* negotiated_extensions) { + // TransportSequenceNumberV2 is not offered by default. The special logic for + // the TransportSequenceNumber extensions works as follows: + // Offer Answer + // V1 V1 if in local_extensions. + // V1 and V2 V2 regardless of local_extensions. + // V2 V2 regardless of local_extensions. + const webrtc::RtpExtension* transport_sequence_number_v2_offer = + webrtc::RtpExtension::FindHeaderExtensionByUri( + offered_extensions, + webrtc::RtpExtension::kTransportSequenceNumberV2Uri); + for (const webrtc::RtpExtension& ours : local_extensions) { webrtc::RtpExtension theirs; if (FindByUriWithEncryptionPreference( offered_extensions, ours, enable_encrypted_rtp_header_extensions, &theirs)) { - // We respond with their RTP header extension id. - negotiated_extenstions->push_back(theirs); + if (transport_sequence_number_v2_offer && + ours.uri == webrtc::RtpExtension::kTransportSequenceNumberUri) { + // Don't respond to + // http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01 + // if we get an offer including + // http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-02 + continue; + } else { + // We respond with their RTP header extension id. + negotiated_extensions->push_back(theirs); + } } } + + if (transport_sequence_number_v2_offer) { + // Respond that we support kTransportSequenceNumberV2Uri. + negotiated_extensions->push_back(*transport_sequence_number_v2_offer); + } } static void StripCNCodecs(AudioCodecs* audio_codecs) { diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index 3f7631eccf..e876c36301 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -213,6 +213,27 @@ static const RtpExtension kVideoRtpExtensionEncryptedAnswer[] = { RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 11, true), }; +static const RtpExtension kRtpExtensionTransportSequenceNumber01[] = { + RtpExtension("http://www.ietf.org/id/" + "draft-holmer-rmcat-transport-wide-cc-extensions-01", + 1), +}; + +static const RtpExtension kRtpExtensionTransportSequenceNumber01And02[] = { + RtpExtension("http://www.ietf.org/id/" + "draft-holmer-rmcat-transport-wide-cc-extensions-01", + 1), + RtpExtension("http://www.ietf.org/id/" + "draft-holmer-rmcat-transport-wide-cc-extensions-02", + 2), +}; + +static const RtpExtension kRtpExtensionTransportSequenceNumber02[] = { + RtpExtension("http://www.ietf.org/id/" + "draft-holmer-rmcat-transport-wide-cc-extensions-02", + 2), +}; + static const uint32_t kSimulcastParamsSsrc[] = {10, 11, 20, 21, 30, 31}; static const uint32_t kSimSsrc[] = {10, 20, 30}; static const uint32_t kFec1Ssrc[] = {10, 11}; @@ -689,6 +710,30 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { EXPECT_EQ(cricket::kMediaProtocolSavpf, vcd->protocol()); } + void TestTransportSequenceNumberNegotiation( + const cricket::RtpHeaderExtensions& local, + const cricket::RtpHeaderExtensions& offered, + const cricket::RtpHeaderExtensions& expectedAnswer) { + MediaSessionOptions opts; + AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts); + f1_.set_audio_rtp_header_extensions(offered); + f1_.set_video_rtp_header_extensions(offered); + f2_.set_audio_rtp_header_extensions(local); + f2_.set_video_rtp_header_extensions(local); + + std::unique_ptr offer = f1_.CreateOffer(opts, NULL); + ASSERT_TRUE(offer.get() != NULL); + std::unique_ptr answer = + f2_.CreateAnswer(offer.get(), opts, NULL); + + EXPECT_EQ( + expectedAnswer, + GetFirstAudioContentDescription(answer.get())->rtp_header_extensions()); + EXPECT_EQ( + expectedAnswer, + GetFirstVideoContentDescription(answer.get())->rtp_header_extensions()); + } + protected: UniqueRandomIdGenerator ssrc_generator1; UniqueRandomIdGenerator ssrc_generator2; @@ -1506,6 +1551,32 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestOfferAnswerWithRtpExtensions) { GetFirstVideoContentDescription(answer.get())->rtp_header_extensions()); } +// Create a audio/video offer and answer and ensure that the +// TransportSequenceNumber RTP header extensions are handled correctly. 02 is +// supported and should take precedence even though not listed among locally +// supported extensions. +TEST_F(MediaSessionDescriptionFactoryTest, + TestOfferAnswerWithTransportSequenceNumberInOffer) { + TestTransportSequenceNumberNegotiation( + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber01), // Local. + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber01), // Offer. + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber01)); // Expected answer. +} +TEST_F(MediaSessionDescriptionFactoryTest, + TestOfferAnswerWithTransportSequenceNumber01And02InOffer) { + TestTransportSequenceNumberNegotiation( + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber01), // Local. + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber01And02), // Offer. + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber02)); // Expected answer. +} +TEST_F(MediaSessionDescriptionFactoryTest, + TestOfferAnswerWithTransportSequenceNumber02InOffer) { + TestTransportSequenceNumberNegotiation( + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber01), // Local. + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber02), // Offer. + MAKE_VECTOR(kRtpExtensionTransportSequenceNumber02)); // Expected answer. +} + TEST_F(MediaSessionDescriptionFactoryTest, TestOfferAnswerWithEncryptedRtpExtensionsBoth) { MediaSessionOptions opts;