From cf5b37cc46156fe8a22cbc5d924ce4526be5694c Mon Sep 17 00:00:00 2001 From: zhihuang Date: Thu, 5 May 2016 11:44:35 -0700 Subject: [PATCH] Accept all the media profiles required by JSEP. JSEP section 5.1.3 states that: Any profile matching the following patterns MUST be accepted: "RTP/[S]AVP[F]" and "(UDP/TCP)/TLS/RTP/SAVP[F]" NOTRY=True BUG=webrtc:5638 Committed: https://crrev.com/b7f425ab68ec58e2a5beaaf5ef79f50f1982c6f9 Cr-Commit-Position: refs/heads/master@{#12338} Review-Url: https://codereview.webrtc.org/1880913002 Cr-Commit-Position: refs/heads/master@{#12637} --- webrtc/pc/mediasession.cc | 52 +++++++++++++++++++----- webrtc/pc/mediasession_unittest.cc | 65 ++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 9 deletions(-) diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index ea0eaa2683..6d8138daac 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -1115,21 +1115,55 @@ static bool CreateMediaContentAnswer( return true; } +static bool IsDtlsRtp(const std::string& protocol) { + // Most-likely values first. + return protocol == "UDP/TLS/RTP/SAVPF" || protocol == "TCP/TLS/RTP/SAVPF" || + protocol == "UDP/TLS/RTP/SAVP" || protocol == "TCP/TLS/RTP/SAVP"; +} + +static bool IsPlainRtp(const std::string& protocol) { + // Most-likely values first. + return protocol == "RTP/SAVPF" || protocol == "RTP/AVPF" || + protocol == "RTP/SAVP" || protocol == "RTP/AVP"; +} + +static bool IsDtlsSctp(const std::string& protocol) { + return protocol == "DTLS/SCTP"; +} + +static bool IsPlainSctp(const std::string& protocol) { + return protocol == "SCTP"; +} + static bool IsMediaProtocolSupported(MediaType type, const std::string& protocol, bool secure_transport) { - // Data channels can have a protocol of SCTP or SCTP/DTLS. - if (type == MEDIA_TYPE_DATA && - ((protocol == kMediaProtocolSctp && !secure_transport)|| - (protocol == kMediaProtocolDtlsSctp && secure_transport))) { + // Since not all applications serialize and deserialize the media protocol, + // we will have to accept |protocol| to be empty. + if (protocol.empty()) { return true; } - // Since not all applications serialize and deserialize the media protocol, - // we will have to accept |protocol| to be empty. - return protocol == kMediaProtocolAvpf || protocol.empty() || - protocol == kMediaProtocolSavpf || - (protocol == kMediaProtocolDtlsSavpf && secure_transport); + if (type == MEDIA_TYPE_DATA) { + // Check for SCTP, but also for RTP for RTP-based data channels. + // TODO(pthatcher): Remove RTP once RTP-based data channels are gone. + if (secure_transport) { + // Most likely scenarios first. + return IsDtlsSctp(protocol) || IsDtlsRtp(protocol) || + IsPlainRtp(protocol); + } else { + return IsPlainSctp(protocol) || IsPlainRtp(protocol); + } + } + + // Allow for non-DTLS RTP protocol even when using DTLS because that's what + // JSEP specifies. + if (secure_transport) { + // Most likely scenarios first. + return IsDtlsRtp(protocol) || IsPlainRtp(protocol); + } else { + return IsPlainRtp(protocol); + } } static void SetMediaProtocol(bool secure_transport, diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc index a6f6658d81..3a1a1c865e 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc @@ -170,6 +170,12 @@ static const char kDataTrack1[] = "data_1"; static const char kDataTrack2[] = "data_2"; static const char kDataTrack3[] = "data_3"; +static const char* kMediaProtocols[] = {"RTP/AVP", "RTP/SAVP", "RTP/AVPF", + "RTP/SAVPF"}; +static const char* kMediaProtocolsDtls[] = { + "TCP/TLS/RTP/SAVPF", "TCP/TLS/RTP/SAVP", "UDP/TLS/RTP/SAVPF", + "UDP/TLS/RTP/SAVP"}; + static bool IsMediaContentOfType(const ContentInfo* content, MediaType media_type) { const MediaContentDescription* mdesc = @@ -2379,3 +2385,62 @@ TEST_F(MediaSessionDescriptionFactoryTest, EXPECT_EQ("video_modified", video_content->name); EXPECT_EQ("data_modified", data_content->name); } + +class MediaProtocolTest : public ::testing::TestWithParam { + public: + MediaProtocolTest() : f1_(&tdf1_), f2_(&tdf2_) { + f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1)); + f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1)); + f1_.set_data_codecs(MAKE_VECTOR(kDataCodecs1)); + f2_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs2)); + f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2)); + f2_.set_data_codecs(MAKE_VECTOR(kDataCodecs2)); + f1_.set_secure(SEC_ENABLED); + f2_.set_secure(SEC_ENABLED); + tdf1_.set_certificate(rtc::RTCCertificate::Create( + rtc::scoped_ptr(new rtc::FakeSSLIdentity("id1")))); + tdf2_.set_certificate(rtc::RTCCertificate::Create( + rtc::scoped_ptr(new rtc::FakeSSLIdentity("id2")))); + tdf1_.set_secure(SEC_ENABLED); + tdf2_.set_secure(SEC_ENABLED); + } + + protected: + MediaSessionDescriptionFactory f1_; + MediaSessionDescriptionFactory f2_; + TransportDescriptionFactory tdf1_; + TransportDescriptionFactory tdf2_; +}; + +TEST_P(MediaProtocolTest, TestAudioVideoAcceptance) { + MediaSessionOptions opts; + opts.recv_video = true; + std::unique_ptr offer(f1_.CreateOffer(opts, nullptr)); + ASSERT_TRUE(offer.get() != nullptr); + // Set the protocol for all the contents. + for (auto content : offer.get()->contents()) { + static_cast(content.description) + ->set_protocol(GetParam()); + } + std::unique_ptr answer( + f2_.CreateAnswer(offer.get(), opts, nullptr)); + const ContentInfo* ac = answer->GetContentByName("audio"); + const ContentInfo* vc = answer->GetContentByName("video"); + ASSERT_TRUE(ac != nullptr); + ASSERT_TRUE(vc != nullptr); + EXPECT_FALSE(ac->rejected); // the offer is accepted + EXPECT_FALSE(vc->rejected); + const AudioContentDescription* acd = + static_cast(ac->description); + const VideoContentDescription* vcd = + static_cast(vc->description); + EXPECT_EQ(GetParam(), acd->protocol()); + EXPECT_EQ(GetParam(), vcd->protocol()); +} + +INSTANTIATE_TEST_CASE_P(MediaProtocolPatternTest, + MediaProtocolTest, + ::testing::ValuesIn(kMediaProtocols)); +INSTANTIATE_TEST_CASE_P(MediaProtocolDtlsPatternTest, + MediaProtocolTest, + ::testing::ValuesIn(kMediaProtocolsDtls));