From 8f66ddbae3250f71b173f9f5c96c1edf3d4b585f Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Mon, 10 Dec 2018 16:08:05 -0800 Subject: [PATCH] Move is_unified_plan flag to a member variable This changes MediaSessionFactory to take the unified plan configuration option as an explicit setter rathen than a MediaSessionOptions flag. This is fine since a PeerConnection will always be in unified plan mode or not, and we know this at construction. Bug: None Change-Id: Ifca45d1d7c9d62b2b41bb879f8665fb39b4cdcd0 Reviewed-on: https://webrtc-review.googlesource.com/c/113824 Reviewed-by: Seth Hampson Cr-Commit-Position: refs/heads/master@{#25960} --- pc/mediasession.cc | 26 ++++++++++++-------------- pc/mediasession.h | 15 +++++++++------ pc/peerconnection.cc | 3 +-- pc/webrtcsessiondescriptionfactory.h | 4 ++++ 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/pc/mediasession.cc b/pc/mediasession.cc index 263f3693db..2b7750b585 100644 --- a/pc/mediasession.cc +++ b/pc/mediasession.cc @@ -1311,8 +1311,8 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer( RtpHeaderExtensions audio_rtp_extensions; RtpHeaderExtensions video_rtp_extensions; - GetRtpHdrExtsToOffer(current_active_contents, session_options.is_unified_plan, - &audio_rtp_extensions, &video_rtp_extensions); + GetRtpHdrExtsToOffer(current_active_contents, &audio_rtp_extensions, + &video_rtp_extensions); auto offer = absl::make_unique(); @@ -1387,7 +1387,7 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer( // The following determines how to signal MSIDs to ensure compatibility with // older endpoints (in particular, older Plan B endpoints). - if (session_options.is_unified_plan) { + if (is_unified_plan_) { // Be conservative and signal using both a=msid and a=ssrc lines. Unified // Plan answerers will look at a=msid and Plan B answerers will look at the // a=ssrc MSID line. @@ -1544,7 +1544,7 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer( // The following determines how to signal MSIDs to ensure compatibility with // older endpoints (in particular, older Plan B endpoints). - if (session_options.is_unified_plan) { + if (is_unified_plan_) { // Unified Plan needs to look at what the offer included to find the most // compatible answer. if (offer->msid_signaling() == 0) { @@ -1734,10 +1734,8 @@ void MediaSessionDescriptionFactory::GetCodecsForAnswer( &used_pltypes); } -// TODO(steveanton): Replace |is_unified_plan| flag with a member variable. void MediaSessionDescriptionFactory::GetRtpHdrExtsToOffer( const std::vector& current_active_contents, - bool is_unified_plan, RtpHeaderExtensions* offer_audio_extensions, RtpHeaderExtensions* offer_video_extensions) const { // All header extensions allocated from the same range to avoid potential @@ -1768,12 +1766,12 @@ void MediaSessionDescriptionFactory::GetRtpHdrExtsToOffer( // Add our default RTP header extensions that are not in the current // description. - MergeRtpHdrExts(audio_rtp_header_extensions(is_unified_plan), - offer_audio_extensions, &all_regular_extensions, - &all_encrypted_extensions, &used_ids); - MergeRtpHdrExts(video_rtp_header_extensions(is_unified_plan), - offer_video_extensions, &all_regular_extensions, - &all_encrypted_extensions, &used_ids); + MergeRtpHdrExts(audio_rtp_header_extensions(), offer_audio_extensions, + &all_regular_extensions, &all_encrypted_extensions, + &used_ids); + MergeRtpHdrExts(video_rtp_header_extensions(), offer_video_extensions, + &all_regular_extensions, &all_encrypted_extensions, + &used_ids); // TODO(jbauch): Support adding encrypted header extensions to existing // sessions. @@ -2147,7 +2145,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( if (!CreateMediaContentAnswer( offer_audio_description, media_description_options, session_options, filtered_codecs, sdes_policy, GetCryptos(current_content), - audio_rtp_header_extensions(session_options.is_unified_plan), + audio_rtp_header_extensions(), enable_encrypted_rtp_header_extensions_, current_streams, bundle_enabled, audio_answer.get())) { return false; // Fails the session setup. @@ -2236,7 +2234,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( if (!CreateMediaContentAnswer( offer_video_description, media_description_options, session_options, filtered_codecs, sdes_policy, GetCryptos(current_content), - video_rtp_header_extensions(session_options.is_unified_plan), + video_rtp_header_extensions(), enable_encrypted_rtp_header_extensions_, current_streams, bundle_enabled, video_answer.get())) { return false; // Failed the sessin setup. diff --git a/pc/mediasession.h b/pc/mediasession.h index 9495bdc293..13ecf702d2 100644 --- a/pc/mediasession.h +++ b/pc/mediasession.h @@ -93,7 +93,6 @@ struct MediaSessionOptions { bool vad_enabled = true; // When disabled, removes all CN codecs from SDP. bool rtcp_mux_enabled = true; bool bundle_enabled = false; - bool is_unified_plan = false; bool offer_extmap_allow_mixed = false; std::string rtcp_cname = kDefaultRtcpCname; webrtc::CryptoOptions crypto_options; @@ -127,10 +126,10 @@ class MediaSessionDescriptionFactory { void set_audio_rtp_header_extensions(const RtpHeaderExtensions& extensions) { audio_rtp_extensions_ = extensions; } - RtpHeaderExtensions audio_rtp_header_extensions(bool unified_plan) const { + RtpHeaderExtensions audio_rtp_header_extensions() const { RtpHeaderExtensions extensions = audio_rtp_extensions_; // If we are Unified Plan, also offer the MID header extension. - if (unified_plan) { + if (is_unified_plan_) { extensions.push_back(webrtc::RtpExtension( webrtc::RtpExtension::kMidUri, webrtc::RtpExtension::kMidDefaultId)); } @@ -141,10 +140,10 @@ class MediaSessionDescriptionFactory { void set_video_rtp_header_extensions(const RtpHeaderExtensions& extensions) { video_rtp_extensions_ = extensions; } - RtpHeaderExtensions video_rtp_header_extensions(bool unified_plan) const { + RtpHeaderExtensions video_rtp_header_extensions() const { RtpHeaderExtensions extensions = video_rtp_extensions_; // If we are Unified Plan, also offer the MID header extension. - if (unified_plan) { + if (is_unified_plan_) { extensions.push_back(webrtc::RtpExtension( webrtc::RtpExtension::kMidUri, webrtc::RtpExtension::kMidDefaultId)); } @@ -159,6 +158,10 @@ class MediaSessionDescriptionFactory { enable_encrypted_rtp_header_extensions_ = enable; } + void set_is_unified_plan(bool is_unified_plan) { + is_unified_plan_ = is_unified_plan; + } + SessionDescription* CreateOffer( const MediaSessionOptions& options, const SessionDescription* current_description) const; @@ -186,7 +189,6 @@ class MediaSessionDescriptionFactory { DataCodecs* data_codecs) const; void GetRtpHdrExtsToOffer( const std::vector& current_active_contents, - bool is_unified_plan, RtpHeaderExtensions* audio_extensions, RtpHeaderExtensions* video_extensions) const; bool AddTransportOffer(const std::string& content_name, @@ -284,6 +286,7 @@ class MediaSessionDescriptionFactory { void ComputeAudioCodecsIntersectionAndUnion(); + bool is_unified_plan_ = false; AudioCodecs audio_send_codecs_; AudioCodecs audio_recv_codecs_; // Intersection of send and recv. diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index c341888200..d7247c4c9f 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1074,6 +1074,7 @@ bool PeerConnection::Initialize( webrtc_session_desc_factory_->set_enable_encrypted_rtp_header_extensions( GetCryptoOptions().srtp.enable_encrypted_rtp_header_extensions); + webrtc_session_desc_factory_->set_is_unified_plan(IsUnifiedPlan()); // Add default audio/video transceivers for Plan B SDP. if (!IsUnifiedPlan()) { @@ -3809,7 +3810,6 @@ void PeerConnection::GetOptionsForOffer( session_options->rtcp_cname = rtcp_cname_; session_options->crypto_options = GetCryptoOptions(); - session_options->is_unified_plan = IsUnifiedPlan(); session_options->pooled_ice_credentials = network_thread()->Invoke>( RTC_FROM_HERE, @@ -4092,7 +4092,6 @@ void PeerConnection::GetOptionsForAnswer( session_options->rtcp_cname = rtcp_cname_; session_options->crypto_options = GetCryptoOptions(); - session_options->is_unified_plan = IsUnifiedPlan(); session_options->pooled_ice_credentials = network_thread()->Invoke>( RTC_FROM_HERE, diff --git a/pc/webrtcsessiondescriptionfactory.h b/pc/webrtcsessiondescriptionfactory.h index 24ed422da1..21859c7697 100644 --- a/pc/webrtcsessiondescriptionfactory.h +++ b/pc/webrtcsessiondescriptionfactory.h @@ -104,6 +104,10 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, session_desc_factory_.set_enable_encrypted_rtp_header_extensions(enable); } + void set_is_unified_plan(bool is_unified_plan) { + session_desc_factory_.set_is_unified_plan(is_unified_plan); + } + sigslot::signal1&> SignalCertificateReady;