Remove use of RtpHeaderExtension and clean up Currently there are two structs that are identical and track extension details: webrtc::RtpExtension cricket::RtpHeaderExtension The use of the structs is mixed in the code to track the extensions being supported. This results in duplicate definition of the URI constants and there is code to convert between the two structs. Clean up to use a single RtpHeader throughout the codebase. The actual location of RtpHeader may change in future (perhaps to be located in api/). Additionally, this CL renames some of the constants to clarify Uri and Id use. BUG= webrtc:5895 Review-Url: https://codereview.webrtc.org/1984983002 Cr-Commit-Position: refs/heads/master@{#12924}
diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index 97c1d66..cac0ea2 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc
@@ -43,6 +43,20 @@ rtc::PacketOptions options; }; +#if defined(ENABLE_EXTERNAL_AUTH) +// Returns the named header extension if found among all extensions, +// nullptr otherwise. +const webrtc::RtpExtension* FindHeaderExtension( + const std::vector<webrtc::RtpExtension>& extensions, + const std::string& uri) { + for (const auto& extension : extensions) { + if (extension.uri == uri) + return &extension; + } + return nullptr; +} +#endif + } // namespace enum { @@ -1390,13 +1404,13 @@ } void BaseChannel::MaybeCacheRtpAbsSendTimeHeaderExtension_w( - const std::vector<RtpHeaderExtension>& extensions) { + const std::vector<webrtc::RtpExtension>& extensions) { // Absolute Send Time extension id is used only with external auth, // so do not bother searching for it and making asyncronious call to set // something that is not used. #if defined(ENABLE_EXTERNAL_AUTH) - const RtpHeaderExtension* send_time_extension = - FindHeaderExtension(extensions, kRtpAbsoluteSenderTimeHeaderExtension); + const webrtc::RtpExtension* send_time_extension = + FindHeaderExtension(extensions, webrtc::RtpExtension::kAbsSendTimeUri); int rtp_abs_sendtime_extn_id = send_time_extension ? send_time_extension->id : -1; invoker_.AsyncInvoke<void>(
diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index d9f5fd6..3305d0e 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h
@@ -282,7 +282,7 @@ // Helper method to get RTP Absoulute SendTime extension header id if // present in remote supported extensions list. void MaybeCacheRtpAbsSendTimeHeaderExtension_w( - const std::vector<RtpHeaderExtension>& extensions); + const std::vector<webrtc::RtpExtension>& extensions); bool CheckSrtpConfig_n(const std::vector<CryptoParams>& cryptos, bool* dtls,
diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 0fa20d8..358f92d 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc
@@ -374,11 +374,10 @@ // Helper class used for finding duplicate RTP Header extension ids among // audio and video extensions. -class UsedRtpHeaderExtensionIds : public UsedIds<RtpHeaderExtension> { +class UsedRtpHeaderExtensionIds : public UsedIds<webrtc::RtpExtension> { public: UsedRtpHeaderExtensionIds() - : UsedIds<RtpHeaderExtension>(kLocalIdMin, kLocalIdMax) { - } + : UsedIds<webrtc::RtpExtension>(kLocalIdMin, kLocalIdMax) {} private: // Min and Max local identifier for one-byte header extensions, per RFC5285. @@ -890,10 +889,9 @@ } } - static bool FindByUri(const RtpHeaderExtensions& extensions, - const RtpHeaderExtension& ext_to_match, - RtpHeaderExtension* found_extension) { + const webrtc::RtpExtension& ext_to_match, + webrtc::RtpExtension* found_extension) { for (RtpHeaderExtensions::const_iterator it = extensions.begin(); it != extensions.end(); ++it) { // We assume that all URIs are given in a canonical format. @@ -915,7 +913,7 @@ RtpHeaderExtensions* all_extensions, UsedRtpHeaderExtensionIds* used_ids) { for (auto& extension : *offered_extensions) { - RtpHeaderExtension existing; + webrtc::RtpExtension existing; if (FindByUri(*all_extensions, extension, &existing)) { extension.id = existing.id; } else { @@ -934,7 +932,7 @@ UsedRtpHeaderExtensionIds* used_ids) { for (auto reference_extension : reference_extensions) { if (!FindByUri(*offered_extensions, reference_extension, NULL)) { - RtpHeaderExtension existing; + webrtc::RtpExtension existing; if (FindByUri(*all_extensions, reference_extension, &existing)) { offered_extensions->push_back(existing); } else { @@ -953,7 +951,7 @@ RtpHeaderExtensions::const_iterator ours; for (ours = local_extensions.begin(); ours != local_extensions.end(); ++ours) { - RtpHeaderExtension theirs; + webrtc::RtpExtension theirs; if (FindByUri(offered_extensions, *ours, &theirs)) { // We respond with their RTP header extension id. negotiated_extenstions->push_back(theirs);
diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h index 22291c4..0a04bcf 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h
@@ -35,7 +35,7 @@ typedef std::vector<VideoCodec> VideoCodecs; typedef std::vector<DataCodec> DataCodecs; typedef std::vector<CryptoParams> CryptoParamsVec; -typedef std::vector<RtpHeaderExtension> RtpHeaderExtensions; +typedef std::vector<webrtc::RtpExtension> RtpHeaderExtensions; enum MediaType { MEDIA_TYPE_AUDIO, @@ -205,7 +205,7 @@ rtp_header_extensions_ = extensions; rtp_header_extensions_set_ = true; } - void AddRtpHeaderExtension(const RtpHeaderExtension& ext) { + void AddRtpHeaderExtension(const webrtc::RtpExtension& ext) { rtp_header_extensions_.push_back(ext); rtp_header_extensions_set_ = true; } @@ -284,7 +284,7 @@ std::string protocol_; std::vector<CryptoParams> cryptos_; CryptoType crypto_required_ = CT_NONE; - std::vector<RtpHeaderExtension> rtp_header_extensions_; + std::vector<webrtc::RtpExtension> rtp_header_extensions_; bool rtp_header_extensions_set_ = false; bool multistream_ = false; StreamParamsVec streams_;
diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc index b14bd80..753db83 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc
@@ -67,12 +67,12 @@ using cricket::MEDIA_TYPE_AUDIO; using cricket::MEDIA_TYPE_VIDEO; using cricket::MEDIA_TYPE_DATA; -using cricket::RtpHeaderExtension; using cricket::SEC_DISABLED; using cricket::SEC_ENABLED; using cricket::SEC_REQUIRED; using rtc::CS_AES_CM_128_HMAC_SHA1_32; using rtc::CS_AES_CM_128_HMAC_SHA1_80; +using webrtc::RtpExtension; static const AudioCodec kAudioCodecs1[] = { AudioCodec(103, "ISAC", 16000, -1, 1), @@ -113,44 +113,44 @@ static const DataCodec kDataCodecsAnswer[] = {DataCodec(98, "binary-data"), DataCodec(99, "utf8-text")}; -static const RtpHeaderExtension kAudioRtpExtension1[] = { - RtpHeaderExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8), - RtpHeaderExtension("http://google.com/testing/audio_something", 10), +static const RtpExtension kAudioRtpExtension1[] = { + RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8), + RtpExtension("http://google.com/testing/audio_something", 10), }; -static const RtpHeaderExtension kAudioRtpExtension2[] = { - RtpHeaderExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 2), - RtpHeaderExtension("http://google.com/testing/audio_something_else", 8), - RtpHeaderExtension("http://google.com/testing/both_audio_and_video", 7), +static const RtpExtension kAudioRtpExtension2[] = { + RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 2), + RtpExtension("http://google.com/testing/audio_something_else", 8), + RtpExtension("http://google.com/testing/both_audio_and_video", 7), }; -static const RtpHeaderExtension kAudioRtpExtension3[] = { - RtpHeaderExtension("http://google.com/testing/audio_something", 2), - RtpHeaderExtension("http://google.com/testing/both_audio_and_video", 3), +static const RtpExtension kAudioRtpExtension3[] = { + RtpExtension("http://google.com/testing/audio_something", 2), + RtpExtension("http://google.com/testing/both_audio_and_video", 3), }; -static const RtpHeaderExtension kAudioRtpExtensionAnswer[] = { - RtpHeaderExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8), +static const RtpExtension kAudioRtpExtensionAnswer[] = { + RtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8), }; -static const RtpHeaderExtension kVideoRtpExtension1[] = { - RtpHeaderExtension("urn:ietf:params:rtp-hdrext:toffset", 14), - RtpHeaderExtension("http://google.com/testing/video_something", 13), +static const RtpExtension kVideoRtpExtension1[] = { + RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14), + RtpExtension("http://google.com/testing/video_something", 13), }; -static const RtpHeaderExtension kVideoRtpExtension2[] = { - RtpHeaderExtension("urn:ietf:params:rtp-hdrext:toffset", 2), - RtpHeaderExtension("http://google.com/testing/video_something_else", 14), - RtpHeaderExtension("http://google.com/testing/both_audio_and_video", 7), +static const RtpExtension kVideoRtpExtension2[] = { + RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 2), + RtpExtension("http://google.com/testing/video_something_else", 14), + RtpExtension("http://google.com/testing/both_audio_and_video", 7), }; -static const RtpHeaderExtension kVideoRtpExtension3[] = { - RtpHeaderExtension("http://google.com/testing/video_something", 4), - RtpHeaderExtension("http://google.com/testing/both_audio_and_video", 5), +static const RtpExtension kVideoRtpExtension3[] = { + RtpExtension("http://google.com/testing/video_something", 4), + RtpExtension("http://google.com/testing/both_audio_and_video", 5), }; -static const RtpHeaderExtension kVideoRtpExtensionAnswer[] = { - RtpHeaderExtension("urn:ietf:params:rtp-hdrext:toffset", 14), +static const RtpExtension kVideoRtpExtensionAnswer[] = { + RtpExtension("urn:ietf:params:rtp-hdrext:toffset", 14), }; static const uint32_t kSimulcastParamsSsrc[] = {10, 11, 20, 21, 30, 31}; @@ -1904,18 +1904,16 @@ // |f2_| offer. // Since the default local extension id |f2_| uses has already been used by // |f1_| for another extensions, it is changed to 13. - const RtpHeaderExtension kUpdatedAudioRtpExtensions[] = { - kAudioRtpExtensionAnswer[0], - RtpHeaderExtension(kAudioRtpExtension2[1].uri, 13), - kAudioRtpExtension2[2], + const RtpExtension kUpdatedAudioRtpExtensions[] = { + kAudioRtpExtensionAnswer[0], RtpExtension(kAudioRtpExtension2[1].uri, 13), + kAudioRtpExtension2[2], }; // Since the default local extension id |f2_| uses has already been used by // |f1_| for another extensions, is is changed to 12. - const RtpHeaderExtension kUpdatedVideoRtpExtensions[] = { - kVideoRtpExtensionAnswer[0], - RtpHeaderExtension(kVideoRtpExtension2[1].uri, 12), - kVideoRtpExtension2[2], + const RtpExtension kUpdatedVideoRtpExtensions[] = { + kVideoRtpExtensionAnswer[0], RtpExtension(kVideoRtpExtension2[1].uri, 12), + kVideoRtpExtension2[2], }; const AudioContentDescription* updated_acd = @@ -1932,8 +1930,7 @@ // Verify that if the same RTP extension URI is used for audio and video, the // same ID is used. Also verify that the ID isn't changed when creating an // updated offer (this was previously a bug). -TEST_F(MediaSessionDescriptionFactoryTest, - RtpHeaderExtensionIdReused) { +TEST_F(MediaSessionDescriptionFactoryTest, RtpExtensionIdReused) { MediaSessionOptions opts; opts.recv_audio = true; opts.recv_video = true; @@ -1945,9 +1942,8 @@ // Since the audio extensions used ID 3 for "both_audio_and_video", so should // the video extensions. - const RtpHeaderExtension kExpectedVideoRtpExtension[] = { - kVideoRtpExtension3[0], - kAudioRtpExtension3[1], + const RtpExtension kExpectedVideoRtpExtension[] = { + kVideoRtpExtension3[0], kAudioRtpExtension3[1], }; EXPECT_EQ(MAKE_VECTOR(kAudioRtpExtension3),