diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index 6d60b4c36f..fa74b6d325 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -154,9 +154,7 @@ class StatsObserver : public rtc::RefCountInterface { virtual ~StatsObserver() {} }; -// For now, kDefault is interpreted as kPlanB. -// TODO(bugs.webrtc.org/8530): Switch default to kUnifiedPlan. -enum class SdpSemantics { kDefault, kPlanB, kUnifiedPlan }; +enum class SdpSemantics { kPlanB, kUnifiedPlan }; class PeerConnectionInterface : public rtc::RefCountInterface { public: @@ -559,15 +557,12 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // will also cause PeerConnection to ignore all but the first a=ssrc lines // that form a Plan B stream. // - // For users who only send at most one audio and one video track, this - // choice does not matter and should be left as kDefault. - // // For users who wish to send multiple audio/video streams and need to stay - // interoperable with legacy WebRTC implementations, specify kPlanB. + // interoperable with legacy WebRTC implementations or use legacy APIs, + // specify kPlanB. // - // For users who wish to send multiple audio/video streams and/or wish to - // use the new RtpTransceiver API, specify kUnifiedPlan. - SdpSemantics sdp_semantics = SdpSemantics::kDefault; + // For all other users, specify kUnifiedPlan. + SdpSemantics sdp_semantics = SdpSemantics::kPlanB; // // Don't forget to update operator== if adding something. diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index cce90216c1..2721063999 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -3003,27 +3003,6 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { kEnumCounterAddressFamily, kPeerConnection_IPv4, kPeerConnectionAddressFamilyCounter_Max); } - // Send information about the requested SDP semantics. - switch (configuration_.sdp_semantics) { - case SdpSemantics::kDefault: - uma_observer_->IncrementEnumCounter(kEnumCounterSdpSemanticRequested, - kSdpSemanticRequestDefault, - kSdpSemanticRequestMax); - - break; - case SdpSemantics::kPlanB: - uma_observer_->IncrementEnumCounter(kEnumCounterSdpSemanticRequested, - kSdpSemanticRequestPlanB, - kSdpSemanticRequestMax); - break; - case SdpSemantics::kUnifiedPlan: - uma_observer_->IncrementEnumCounter(kEnumCounterSdpSemanticRequested, - kSdpSemanticRequestUnifiedPlan, - kSdpSemanticRequestMax); - break; - default: - RTC_NOTREACHED(); - } } } diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index 12492982ca..7379c711cf 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -1146,9 +1146,7 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { if (config) { modified_config = *config; } - if (modified_config.sdp_semantics == SdpSemantics::kDefault) { - modified_config.sdp_semantics = sdp_semantics_; - } + modified_config.sdp_semantics = sdp_semantics_; if (!cert_generator) { cert_generator = rtc::MakeUnique(); } @@ -1168,6 +1166,25 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { PeerConnectionInterface::RTCConfiguration()); } + bool CreatePeerConnectionWrappersWithSdpSemantics( + SdpSemantics caller_semantics, + SdpSemantics callee_semantics) { + // Can't specify the sdp_semantics in the passed-in configuration since it + // will be overwritten by CreatePeerConnectionWrapper with whatever is + // stored in sdp_semantics_. So get around this by modifying the instance + // variable before calling CreatePeerConnectionWrapper for the caller and + // callee PeerConnections. + SdpSemantics original_semantics = sdp_semantics_; + sdp_semantics_ = caller_semantics; + caller_ = CreatePeerConnectionWrapper("Caller", nullptr, nullptr, nullptr, + nullptr); + sdp_semantics_ = callee_semantics; + callee_ = CreatePeerConnectionWrapper("Callee", nullptr, nullptr, nullptr, + nullptr); + sdp_semantics_ = original_semantics; + return caller_ && callee_; + } + bool CreatePeerConnectionWrappersWithConstraints( MediaConstraintsInterface* caller_constraints, MediaConstraintsInterface* callee_constraints) { @@ -1415,7 +1432,7 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { } protected: - const SdpSemantics sdp_semantics_; + SdpSemantics sdp_semantics_; private: // |ss_| is used by |network_thread_| so it must be destroyed later. @@ -4187,16 +4204,13 @@ class PeerConnectionIntegrationInteropTest // because we specify not to use the test semantics when creating // PeerConnectionWrappers. PeerConnectionIntegrationInteropTest() - : PeerConnectionIntegrationBaseTest(SdpSemantics::kDefault), + : PeerConnectionIntegrationBaseTest(SdpSemantics::kPlanB), caller_semantics_(std::get<0>(GetParam())), callee_semantics_(std::get<1>(GetParam())) {} bool CreatePeerConnectionWrappersWithSemantics() { - RTCConfiguration caller_config; - caller_config.sdp_semantics = caller_semantics_; - RTCConfiguration callee_config; - callee_config.sdp_semantics = callee_semantics_; - return CreatePeerConnectionWrappersWithConfig(caller_config, callee_config); + return CreatePeerConnectionWrappersWithSdpSemantics(caller_semantics_, + callee_semantics_); } const SdpSemantics caller_semantics_; @@ -4304,13 +4318,8 @@ INSTANTIATE_TEST_CASE_P( // Test that if the Unified Plan side offers two video tracks then the Plan B // side will only see the first one and ignore the second. TEST_F(PeerConnectionIntegrationTestPlanB, TwoVideoUnifiedPlanToNoMediaPlanB) { - RTCConfiguration caller_config; - caller_config.sdp_semantics = SdpSemantics::kUnifiedPlan; - RTCConfiguration callee_config; - callee_config.sdp_semantics = SdpSemantics::kPlanB; - ASSERT_TRUE( - CreatePeerConnectionWrappersWithConfig(caller_config, callee_config)); - + ASSERT_TRUE(CreatePeerConnectionWrappersWithSdpSemantics( + SdpSemantics::kUnifiedPlan, SdpSemantics::kPlanB)); ConnectFakeSignaling(); auto first_sender = caller()->AddVideoTrack(); caller()->AddVideoTrack(); diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm index ceaac86ace..e318a9f977 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm @@ -404,8 +404,6 @@ + (webrtc::SdpSemantics)nativeSdpSemanticsForSdpSemantics:(RTCSdpSemantics)sdpSemantics { switch (sdpSemantics) { - case RTCSdpSemanticsDefault: - return webrtc::SdpSemantics::kDefault; case RTCSdpSemanticsPlanB: return webrtc::SdpSemantics::kPlanB; case RTCSdpSemanticsUnifiedPlan: @@ -415,8 +413,6 @@ + (RTCSdpSemantics)sdpSemanticsForNativeSdpSemantics:(webrtc::SdpSemantics)sdpSemantics { switch (sdpSemantics) { - case webrtc::SdpSemantics::kDefault: - return RTCSdpSemanticsDefault; case webrtc::SdpSemantics::kPlanB: return RTCSdpSemanticsPlanB; case webrtc::SdpSemantics::kUnifiedPlan: @@ -426,8 +422,6 @@ + (NSString *)stringForSdpSemantics:(RTCSdpSemantics)sdpSemantics { switch (sdpSemantics) { - case RTCSdpSemanticsDefault: - return @"DEFAULT"; case RTCSdpSemanticsPlanB: return @"PLAN_B"; case RTCSdpSemanticsUnifiedPlan: diff --git a/sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h b/sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h index 97cc5a4946..dd8f364cbd 100644 --- a/sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h +++ b/sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h @@ -65,7 +65,6 @@ typedef NS_ENUM(NSInteger, RTCEncryptionKeyType) { /** Represents the chosen SDP semantics for the RTCPeerConnection. */ typedef NS_ENUM(NSInteger, RTCSdpSemantics) { - RTCSdpSemanticsDefault, RTCSdpSemanticsPlanB, RTCSdpSemanticsUnifiedPlan, }; @@ -155,14 +154,11 @@ RTC_EXPORT * will also cause RTCPeerConnection to ignore all but the first a=ssrc lines * that form a Plan B stream. * - * For users who only send at most one audio and one video track, this - * choice does not matter and should be left as Default. - * * For users who wish to send multiple audio/video streams and need to stay - * interoperable with legacy WebRTC implementations, specify PlanB. + * interoperable with legacy WebRTC implementations or use legacy APIs, + * specify PlanB. * - * For users who wish to send multiple audio/video streams and/or wish to - * use the new RTCRtpTransceiver API, specify UnifiedPlan. + * For all other users, specify UnifiedPlan. */ @property(nonatomic, assign) RTCSdpSemantics sdpSemantics;