zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove SdpSemantics::kDefault

Browse Source 
This adds confusion to the native API and is only needed for
Chromium UMA metrics, so the appropriate metrics have been moved
upstream and kDefault option removed.

Bug: chromium:811683
Change-Id: I666d7f7793765b8d6edcd99416c8b6c957766f00
Reviewed-on: https://webrtc-review.googlesource.com/59261
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22864}
This commit is contained in:
Steve Anton
2018-04-12 17:21:03 -07:00
committed by Commit Bot
parent 5d8f8fa0b6
commit 3acffc3b16
5 changed files with 34 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
api/peerconnectioninterface.h
View File

@ -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.

21
pc/peerconnection.cc
View File

@ -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();
}
}
}

43
pc/peerconnection_integrationtest.cc
View File

@ -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<FakeRTCCertificateGenerator>();
}
@ -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();

6
sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm
View File

@ -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:

10
sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h
View File

@ -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;