admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change network_priority from a double to an enum.

Browse Source 
It can only be one of four possible values, so it never made sense
for it to be a double. Other than the fact that its neighbor
bitrate_priority is a double, and they're both defined as the same enum
in the web spec. However, while bitrate_priority being a double
offers more flexibility than the web spec, network_priority being a
double is only confusing.

Bug: webrtc:5658
Change-Id: I0784c116f3260c4b3a8b99a3cd85c8d66017e46f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168840
Reviewed-by: Anders Carlsson <andersc@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30685}
This commit is contained in:
Taylor Brandstetter 2020-02-27 11:59:23 -08:00 committed by Commit Bot
parent 14e5f0b2cb
commit 3f1aee3cbb
9 changed files with 88 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
api/rtp_parameters.cc
View File

@ -19,11 +19,6 @@ namespace webrtc {
const double kDefaultBitratePriority = 1.0;
const double Priority::kVeryLow = 0.5;
const double Priority::kLow = 1.0;
const double Priority::kMedium = 2.0;
const double Priority::kHigh = 4.0;
RtcpFeedback::RtcpFeedback() = default;
RtcpFeedback::RtcpFeedback(RtcpFeedbackType type) : type(type) {}
RtcpFeedback::RtcpFeedback(RtcpFeedbackType type,

15
api/rtp_parameters.h
View File

@ -91,12 +91,11 @@ enum class DegradationPreference {
RTC_EXPORT extern const double kDefaultBitratePriority;
// TODO(deadbeef): Switch to an enum class.
struct RTC_EXPORT Priority {
static const double kVeryLow;
static const double kLow;
static const double kMedium;
static const double kHigh;
enum class Priority {
kVeryLow,
kLow,
kMedium,
kHigh,
};
struct RTC_EXPORT RtcpFeedback {
@ -401,7 +400,9 @@ struct RTC_EXPORT RtpEncodingParameters {
// we follow chromium's translation of the allowed string enum values for
// this field to 1.0, 0.5, et cetera, similar to bitrate_priority above.
// TODO(http://crbug.com/webrtc/8630): Implement this per encoding parameter.
double network_priority = Priority::kLow;
// TODO(http://crbug.com/webrtc/11379): TCP connections should use a single
// DSCP value even if shared by multiple senders; this is not implemented.
Priority network_priority = Priority::kLow;
// If set, this represents the Transport Independent Application Specific
// maximum bandwidth defined in RFC3890. If unset, there is no maximum

31
media/engine/webrtc_video_engine.cc
View File

@ -944,22 +944,25 @@ webrtc::RTCError WebRtcVideoChannel::SetRtpSendParameters(
}
if (!parameters.encodings.empty()) {
const auto& priority = parameters.encodings[0].network_priority;
// Note that these values come from:
// https://tools.ietf.org/html/draft-ietf-tsvwg-rtcweb-qos-16#section-5
// TODO(deadbeef): Change values depending on whether we are sending a
// keyframe or non-keyframe.
rtc::DiffServCodePoint new_dscp = rtc::DSCP_DEFAULT;
if (priority == 0.5 * webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_CS1;
} else if (priority == webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_DEFAULT;
} else if (priority == 2.0 * webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_AF42;
} else if (priority == 4.0 * webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_AF41;
} else {
RTC_LOG(LS_WARNING) << "Received invalid send network priority: "
<< priority;
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_RANGE);
switch (parameters.encodings[0].network_priority) {
case webrtc::Priority::kVeryLow:
new_dscp = rtc::DSCP_CS1;
break;
case webrtc::Priority::kLow:
new_dscp = rtc::DSCP_DEFAULT;
break;
case webrtc::Priority::kMedium:
new_dscp = rtc::DSCP_AF42;
break;
case webrtc::Priority::kHigh:
new_dscp = rtc::DSCP_AF41;
break;
}
SetPreferredDscp(new_dscp);
}

31
media/engine/webrtc_video_engine_unittest.cc
View File

@ -5061,18 +5061,13 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) {
ASSERT_FALSE(parameters.encodings.empty());
// Various priorities map to various dscp values.
parameters.encodings[0].network_priority = 4.0;
parameters.encodings[0].network_priority = webrtc::Priority::kHigh;
ASSERT_TRUE(channel->SetRtpSendParameters(kSsrc, parameters).ok());
EXPECT_EQ(rtc::DSCP_AF41, network_interface->dscp());
parameters.encodings[0].network_priority = 0.5;
parameters.encodings[0].network_priority = webrtc::Priority::kVeryLow;
ASSERT_TRUE(channel->SetRtpSendParameters(kSsrc, parameters).ok());
EXPECT_EQ(rtc::DSCP_CS1, network_interface->dscp());
// A bad priority does not change the dscp value.
parameters.encodings[0].network_priority = 0.0;
ASSERT_FALSE(channel->SetRtpSendParameters(kSsrc, parameters).ok());
EXPECT_EQ(rtc::DSCP_CS1, network_interface->dscp());
// Packets should also self-identify their dscp in PacketOptions.
const uint8_t kData[10] = {0};
EXPECT_TRUE(static_cast<webrtc::Transport*>(channel.get())
@ -6238,28 +6233,6 @@ TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersPrioritySimulcastStreams) {
EXPECT_TRUE(channel_->SetVideoSend(primary_ssrc, nullptr, nullptr));
}
// RTCRtpEncodingParameters.network_priority must be one of a few values
// derived from the default priority, corresponding to very-low, low, medium,
// or high.
TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersInvalidNetworkPriority) {
AddSendStream();
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
EXPECT_EQ(1UL, parameters.encodings.size());
EXPECT_EQ(webrtc::kDefaultBitratePriority,
parameters.encodings[0].network_priority);
double good_values[] = {0.5, 1.0, 2.0, 4.0};
double bad_values[] = {-1.0, 0.0, 0.49, 0.51, 1.1, 3.99, 4.1, 5.0};
for (auto it : good_values) {
parameters.encodings[0].network_priority = it;
EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());
}
for (auto it : bad_values) {
parameters.encodings[0].network_priority = it;
EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());
}
}
TEST_F(WebRtcVideoChannelTest,
GetAndSetRtpSendParametersScaleResolutionDownByVP8) {
VideoSendParameters parameters;

37
media/engine/webrtc_voice_engine.cc
View File

@ -711,8 +711,8 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
config_.rtp.c_name = c_name;
config_.rtp.extmap_allow_mixed = extmap_allow_mixed;
config_.rtp.extensions = extensions;
config_.has_dscp = rtp_parameters_.encodings[0].network_priority !=
webrtc::kDefaultBitratePriority;
config_.has_dscp =
rtp_parameters_.encodings[0].network_priority != webrtc::Priority::kLow;
config_.audio_network_adaptor_config = audio_network_adaptor_config;
config_.encoder_factory = encoder_factory;
config_.codec_pair_id = codec_pair_id;
@ -923,11 +923,11 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
const absl::optional<int> old_rtp_max_bitrate =
rtp_parameters_.encodings[0].max_bitrate_bps;
double old_priority = rtp_parameters_.encodings[0].bitrate_priority;
double old_dscp = rtp_parameters_.encodings[0].network_priority;
webrtc::Priority old_dscp = rtp_parameters_.encodings[0].network_priority;
rtp_parameters_ = parameters;
config_.bitrate_priority = rtp_parameters_.encodings[0].bitrate_priority;
config_.has_dscp = (rtp_parameters_.encodings[0].network_priority !=
webrtc::kDefaultBitratePriority);
webrtc::Priority::kLow);
bool reconfigure_send_stream =
(rtp_parameters_.encodings[0].max_bitrate_bps != old_rtp_max_bitrate) ||
@ -1389,22 +1389,23 @@ webrtc::RTCError WebRtcVoiceMediaChannel::SetRtpSendParameters(
}
if (!parameters.encodings.empty()) {
auto& priority = parameters.encodings[0].network_priority;
// Note that these values come from:
// https://tools.ietf.org/html/draft-ietf-tsvwg-rtcweb-qos-16#section-5
rtc::DiffServCodePoint new_dscp = rtc::DSCP_DEFAULT;
if (priority == 0.5 * webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_CS1;
} else if (priority == 1.0 * webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_DEFAULT;
} else if (priority == 2.0 * webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_EF;
} else if (priority == 4.0 * webrtc::kDefaultBitratePriority) {
new_dscp = rtc::DSCP_EF;
} else {
RTC_LOG(LS_WARNING) << "Received invalid send network priority: "
<< priority;
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_RANGE);
switch (parameters.encodings[0].network_priority) {
case webrtc::Priority::kVeryLow:
new_dscp = rtc::DSCP_CS1;
break;
case webrtc::Priority::kLow:
new_dscp = rtc::DSCP_DEFAULT;
break;
case webrtc::Priority::kMedium:
new_dscp = rtc::DSCP_EF;
break;
case webrtc::Priority::kHigh:
new_dscp = rtc::DSCP_EF;
break;
}
SetPreferredDscp(new_dscp);
}

31
media/engine/webrtc_voice_engine_unittest.cc
View File

@ -1191,28 +1191,6 @@ TEST_F(WebRtcVoiceEngineTestFake, RtpParametersArePerStream) {
EXPECT_EQ(32000, GetCodecBitrate(kSsrcs4[2]));
}
// RTCRtpEncodingParameters.network_priority must be one of a few values
// derived from the default priority, corresponding to very-low, low, medium,
// or high.
TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParametersInvalidNetworkPriority) {
EXPECT_TRUE(SetupSendStream());
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(kSsrcX);
EXPECT_EQ(1UL, parameters.encodings.size());
EXPECT_EQ(webrtc::kDefaultBitratePriority,
parameters.encodings[0].network_priority);
double good_values[] = {0.5, 1.0, 2.0, 4.0};
double bad_values[] = {-1.0, 0.0, 0.49, 0.51, 1.1, 3.99, 4.1, 5.0};
for (auto it : good_values) {
parameters.encodings[0].network_priority = it;
EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, parameters).ok());
}
for (auto it : bad_values) {
parameters.encodings[0].network_priority = it;
EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, parameters).ok());
}
}
// Test that GetRtpSendParameters returns the currently configured codecs.
TEST_F(WebRtcVoiceEngineTestFake, GetRtpSendParametersCodecs) {
EXPECT_TRUE(SetupSendStream());
@ -3051,18 +3029,13 @@ TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) {
ASSERT_FALSE(parameters.encodings.empty());
// Various priorities map to various dscp values.
parameters.encodings[0].network_priority = 4.0;
parameters.encodings[0].network_priority = webrtc::Priority::kHigh;
ASSERT_TRUE(channel->SetRtpSendParameters(kSsrcZ, parameters).ok());
EXPECT_EQ(rtc::DSCP_EF, network_interface.dscp());
parameters.encodings[0].network_priority = 0.5;
parameters.encodings[0].network_priority = webrtc::Priority::kVeryLow;
ASSERT_TRUE(channel->SetRtpSendParameters(kSsrcZ, parameters).ok());
EXPECT_EQ(rtc::DSCP_CS1, network_interface.dscp());
// A bad priority does not change the dscp value.
parameters.encodings[0].network_priority = 0.0;
ASSERT_FALSE(channel->SetRtpSendParameters(kSsrcZ, parameters).ok());
EXPECT_EQ(rtc::DSCP_CS1, network_interface.dscp());
// Packets should also self-identify their dscp in PacketOptions.
const uint8_t kData[10] = {0};
EXPECT_TRUE(channel->SendRtcp(kData, sizeof(kData)));

2
pc/rtp_sender.cc
View File

@ -50,7 +50,7 @@ int GenerateUniqueId() {
bool PerSenderRtpEncodingParameterHasValue(
const RtpEncodingParameters& encoding_params) {
if (encoding_params.bitrate_priority != kDefaultBitratePriority ||
encoding_params.network_priority != kDefaultBitratePriority) {
encoding_params.network_priority != Priority::kLow) {
return true;
}
return false;

10
sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h
View File

@ -14,6 +14,14 @@
NS_ASSUME_NONNULL_BEGIN
/** Corresponds to webrtc::Priority. */
typedef NS_ENUM(NSInteger, RTCPriority) {
RTCPriorityVeryLow,
RTCPriorityLow,
RTCPriorityMedium,
RTCPriorityHigh
};
RTC_OBJC_EXPORT
@interface RTCRtpEncodingParameters : NSObject
@ -52,7 +60,7 @@ RTC_OBJC_EXPORT
@property(nonatomic, readonly, nullable) NSNumber *ssrc;
/** The relative DiffServ Code Point priority. */
@property(nonatomic, assign) double networkPriority;
@property(nonatomic, assign) RTCPriority networkPriority;
- (instancetype)init NS_DESIGNATED_INITIALIZER;

32
sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm
View File

@ -56,7 +56,8 @@
if (nativeParameters.ssrc) {
_ssrc = [NSNumber numberWithUnsignedLong:*nativeParameters.ssrc];
}
_networkPriority = nativeParameters.network_priority;
_networkPriority =
[RTCRtpEncodingParameters priorityFromNativePriority:nativeParameters.network_priority];
}
return self;
}
@ -86,8 +87,35 @@
if (_ssrc != nil) {
parameters.ssrc = absl::optional<uint32_t>(_ssrc.unsignedLongValue);
}
parameters.network_priority = _networkPriority;
parameters.network_priority =
[RTCRtpEncodingParameters nativePriorityFromPriority:_networkPriority];
return parameters;
}
+ (webrtc::Priority)nativePriorityFromPriority:(RTCPriority)networkPriority {
switch (networkPriority) {
case RTCPriorityVeryLow:
return webrtc::Priority::kVeryLow;
case RTCPriorityLow:
return webrtc::Priority::kLow;
case RTCPriorityMedium:
return webrtc::Priority::kMedium;
case RTCPriorityHigh:
return webrtc::Priority::kHigh;
}
}
+ (RTCPriority)priorityFromNativePriority:(webrtc::Priority)nativePriority {
switch (nativePriority) {
case webrtc::Priority::kVeryLow:
return RTCPriorityVeryLow;
case webrtc::Priority::kLow:
return RTCPriorityLow;
case webrtc::Priority::kMedium:
return RTCPriorityMedium;
case webrtc::Priority::kHigh:
return RTCPriorityHigh;
}
}
@end