Media engine and channel support for per-channel dscp values, specified by RtpParameter
- Similar to network priority
- Still requires MediaConfig.enable_dscp = true (i.e. googDscp == true to peerconnection)
- Needs followups 1) Specify value in chrome renderer js idl 2) disable audio bwe when value differs from video 3)remove googDscp guard
Bug: webrtc:5008
Change-Id: Ibdcbb1183f0ca2ae85e3bced6d0aedbccae3ced4
Reviewed-on: https://webrtc-review.googlesource.com/c/93560
Commit-Queue: Tim Haloun <thaloun@chromium.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25280}diff --git a/api/rtpparameters.h b/api/rtpparameters.h
index 678ac02..377fb78 100644
--- a/api/rtpparameters.h
+++ b/api/rtpparameters.h
@@ -404,6 +404,14 @@
// bitrate priority.
double bitrate_priority = kDefaultBitratePriority;
+ // The relative DiffServ Code Point priority for this encoding, allowing
+ // packets to be marked relatively higher or lower without affecting
+ // bandwidth allocations. See https://w3c.github.io/webrtc-dscp-exp/ . NB
+ // 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 = kDefaultBitratePriority;
+
// Indicates the preferred duration of media represented by a packet in
// milliseconds for this encoding. If set, this will take precedence over the
// ptime set in the RtpCodecParameters. This could happen if SDP negotiation
@@ -471,7 +479,8 @@
bool operator==(const RtpEncodingParameters& o) const {
return ssrc == o.ssrc && codec_payload_type == o.codec_payload_type &&
fec == o.fec && rtx == o.rtx && dtx == o.dtx &&
- bitrate_priority == o.bitrate_priority && ptime == o.ptime &&
+ bitrate_priority == o.bitrate_priority &&
+ network_priority == o.network_priority && ptime == o.ptime &&
max_bitrate_bps == o.max_bitrate_bps &&
min_bitrate_bps == o.min_bitrate_bps &&
max_framerate == o.max_framerate &&
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 39cb4e8..c2b0d1a 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -298,6 +298,7 @@
FindExtensionIds(config_.rtp.extensions).transport_sequence_number != 0 &&
!webrtc::field_trial::IsEnabled("WebRTC-Audio-ForceNoTWCC");
if (config_.min_bitrate_bps != -1 && config_.max_bitrate_bps != -1 &&
+ !config_.has_dscp &&
(has_transport_sequence_number ||
!webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe") ||
webrtc::field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC"))) {
@@ -713,13 +714,16 @@
bool has_transport_sequence_number = new_transport_seq_num_id != 0;
if (new_config.min_bitrate_bps != -1 && new_config.max_bitrate_bps != -1 &&
+ !new_config.has_dscp &&
(has_transport_sequence_number ||
!webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) {
+ stream->transport_->packet_sender()->SetAccountForAudioPackets(true);
stream->ConfigureBitrateObserver(
new_config.min_bitrate_bps, new_config.max_bitrate_bps,
new_config.bitrate_priority, has_transport_sequence_number);
stream->rtp_rtcp_module_->SetAsPartOfAllocation(true);
} else {
+ stream->transport_->packet_sender()->SetAccountForAudioPackets(false);
stream->RemoveBitrateObserver();
stream->rtp_rtcp_module_->SetAsPartOfAllocation(false);
}
diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h
index a659dd6..61d2f5c 100644
--- a/call/audio_send_stream.h
+++ b/call/audio_send_stream.h
@@ -100,6 +100,7 @@
int max_bitrate_bps = -1;
double bitrate_priority = 1.0;
+ bool has_dscp = false;
// Defines whether to turn on audio network adaptor, and defines its config
// string.
diff --git a/media/base/mediachannel.cc b/media/base/mediachannel.cc
index f1471b6..a5dadd2 100644
--- a/media/base/mediachannel.cc
+++ b/media/base/mediachannel.cc
@@ -28,7 +28,7 @@
rtc::CritScope cs(&network_interface_crit_);
network_interface_ = iface;
media_transport_ = media_transport;
- SetDscp(enable_dscp_ ? PreferredDscp() : rtc::DSCP_DEFAULT);
+ UpdateDscp();
}
int MediaChannel::GetRtpSendTimeExtnId() const {
diff --git a/media/base/mediachannel.h b/media/base/mediachannel.h
index 9948d96e..56edab7 100644
--- a/media/base/mediachannel.h
+++ b/media/base/mediachannel.h
@@ -267,9 +267,10 @@
bool DscpEnabled() const { return enable_dscp_; }
- private:
// This method sets DSCP |value| on both RTP and RTCP channels.
- int SetDscp(rtc::DiffServCodePoint value) {
+ int UpdateDscp() {
+ rtc::DiffServCodePoint value =
+ enable_dscp_ ? PreferredDscp() : rtc::DSCP_DEFAULT;
int ret;
ret = SetOption(NetworkInterface::ST_RTP, rtc::Socket::OPT_DSCP, value);
if (ret == 0) {
@@ -278,6 +279,7 @@
return ret;
}
+ private:
bool DoSendPacket(rtc::CopyOnWriteBuffer* packet,
bool rtcp,
const rtc::PacketOptions& options) {
diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc
index 37a7f34..e99886e 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -522,6 +522,7 @@
video_config_(config.video),
encoder_factory_(encoder_factory),
decoder_factory_(decoder_factory),
+ preferred_dscp_(rtc::DSCP_DEFAULT),
default_send_options_(options),
last_stats_log_ms_(-1),
discard_unknown_ssrc_packets_(webrtc::field_trial::IsEnabled(
@@ -659,7 +660,7 @@
}
rtc::DiffServCodePoint WebRtcVideoChannel::PreferredDscp() const {
- return rtc::DSCP_AF41;
+ return preferred_dscp_;
}
bool WebRtcVideoChannel::SetSendParameters(const VideoSendParameters& params) {
@@ -779,6 +780,29 @@
return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR);
}
+ if (!parameters.encodings.empty()) {
+ const auto& priority = parameters.encodings[0].network_priority;
+ 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);
+ }
+
+ if (new_dscp != preferred_dscp_) {
+ preferred_dscp_ = new_dscp;
+ MediaChannel::UpdateDscp();
+ }
+ }
+
return it->second->SetRtpParameters(parameters);
}
@@ -1530,6 +1554,7 @@
if (DscpEnabled()) {
rtc_options.dscp = PreferredDscp();
}
+
return MediaChannel::SendRtcp(&packet, rtc_options);
}
diff --git a/media/engine/webrtcvideoengine.h b/media/engine/webrtcvideoengine.h
index fe5f93b..f327bad 100644
--- a/media/engine/webrtcvideoengine.h
+++ b/media/engine/webrtcvideoengine.h
@@ -499,6 +499,7 @@
// TODO(deadbeef): Don't duplicate information between
// send_params/recv_params, rtp_extensions, options, etc.
VideoSendParameters send_params_;
+ rtc::DiffServCodePoint preferred_dscp_;
VideoOptions default_send_options_;
VideoRecvParameters recv_params_;
int64_t last_stats_log_ms_;
diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc
index 1f946cb..ceac3f7 100644
--- a/media/engine/webrtcvideoengine_unittest.cc
+++ b/media/engine/webrtcvideoengine_unittest.cc
@@ -4596,6 +4596,7 @@
new cricket::FakeNetworkInterface);
MediaConfig config;
std::unique_ptr<cricket::WebRtcVideoChannel> channel;
+ webrtc::RtpParameters parameters;
channel.reset(static_cast<cricket::WebRtcVideoChannel*>(engine_.CreateChannel(
call_.get(), config, VideoOptions(), webrtc::CryptoOptions())));
@@ -4603,17 +4604,37 @@
// Default value when DSCP is disabled should be DSCP_DEFAULT.
EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp());
+ // Default value when DSCP is enabled is also DSCP_DEFAULT, until it is set
+ // through rtp parameters.
config.enable_dscp = true;
channel.reset(static_cast<cricket::WebRtcVideoChannel*>(engine_.CreateChannel(
call_.get(), config, VideoOptions(), webrtc::CryptoOptions())));
channel->SetInterface(network_interface.get(), /*media_transport=*/nullptr);
+ EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp());
+
+ // Create a send stream to configure
+ EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc)));
+ parameters = channel->GetRtpSendParameters(kSsrc);
+ ASSERT_FALSE(parameters.encodings.empty());
+
+ // Various priorities map to various dscp values.
+ parameters.encodings[0].network_priority = 4.0;
+ ASSERT_TRUE(channel->SetRtpSendParameters(kSsrc, parameters).ok());
EXPECT_EQ(rtc::DSCP_AF41, network_interface->dscp());
+ parameters.encodings[0].network_priority = 0.5;
+ 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())
->SendRtcp(kData, sizeof(kData)));
- EXPECT_EQ(rtc::DSCP_AF41, network_interface->options().dscp);
+ EXPECT_EQ(rtc::DSCP_CS1, network_interface->options().dscp);
// Verify that setting the option to false resets the
// DiffServCodePoint.
@@ -5679,6 +5700,28 @@
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, GetAndSetRtpSendParametersMaxFramerate) {
const size_t kNumSimulcastStreams = 3;
SetUpSimulcast(true, false);
diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc
index aa0e140..baad232 100644
--- a/media/engine/webrtcvoiceengine.cc
+++ b/media/engine/webrtcvoiceengine.cc
@@ -57,11 +57,6 @@
const int kOpusMinBitrateBps = 6000;
const int kOpusBitrateFbBps = 32000;
-// Default audio dscp value.
-// See http://tools.ietf.org/html/rfc2474 for details.
-// See also http://tools.ietf.org/html/draft-jennings-rtcweb-qos-00
-const rtc::DiffServCodePoint kAudioDscpValue = rtc::DSCP_EF;
-
const int kMinTelephoneEventCode = 0; // RFC4733 (Section 2.3.1)
const int kMaxTelephoneEventCode = 255;
@@ -730,6 +725,8 @@
config_.rtp.mid = mid;
config_.rtp.c_name = c_name;
config_.rtp.extensions = extensions;
+ config_.has_dscp = rtp_parameters_.encodings[0].network_priority !=
+ webrtc::kDefaultBitratePriority;
config_.audio_network_adaptor_config = audio_network_adaptor_config;
config_.encoder_factory = encoder_factory;
config_.codec_pair_id = codec_pair_id;
@@ -926,12 +923,16 @@
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;
rtp_parameters_ = parameters;
config_.bitrate_priority = rtp_parameters_.encodings[0].bitrate_priority;
+ config_.has_dscp = (rtp_parameters_.encodings[0].network_priority !=
+ webrtc::kDefaultBitratePriority);
bool reconfigure_send_stream =
(rtp_parameters_.encodings[0].max_bitrate_bps != old_rtp_max_bitrate) ||
- (rtp_parameters_.encodings[0].bitrate_priority != old_priority);
+ (rtp_parameters_.encodings[0].bitrate_priority != old_priority) ||
+ (rtp_parameters_.encodings[0].network_priority != old_dscp);
if (rtp_parameters_.encodings[0].max_bitrate_bps != old_rtp_max_bitrate) {
// Update the bitrate range.
if (send_rate) {
@@ -1269,7 +1270,7 @@
}
rtc::DiffServCodePoint WebRtcVoiceMediaChannel::PreferredDscp() const {
- return kAudioDscpValue;
+ return preferred_dscp_;
}
bool WebRtcVoiceMediaChannel::SetSendParameters(
@@ -1375,6 +1376,29 @@
return webrtc::RTCError(webrtc::RTCErrorType::UNSUPPORTED_PARAMETER);
}
+ if (!parameters.encodings.empty()) {
+ auto& priority = parameters.encodings[0].network_priority;
+ 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);
+ }
+
+ if (new_dscp != preferred_dscp_) {
+ preferred_dscp_ = new_dscp;
+ MediaChannel::UpdateDscp();
+ }
+ }
+
// TODO(minyue): The following legacy actions go into
// |WebRtcAudioSendStream::SetRtpParameters()| which is called at the end,
// though there are two difference:
diff --git a/media/engine/webrtcvoiceengine.h b/media/engine/webrtcvoiceengine.h
index 9c013cd..be2fe0b 100644
--- a/media/engine/webrtcvoiceengine.h
+++ b/media/engine/webrtcvoiceengine.h
@@ -220,6 +220,7 @@
if (DscpEnabled()) {
rtc_options.dscp = PreferredDscp();
}
+ rtc_options.dscp = PreferredDscp();
rtc_options.info_signaled_after_sent.included_in_feedback =
options.included_in_feedback;
rtc_options.info_signaled_after_sent.included_in_allocation =
@@ -233,6 +234,7 @@
if (DscpEnabled()) {
rtc_options.dscp = PreferredDscp();
}
+
return VoiceMediaChannel::SendRtcp(&packet, rtc_options);
}
@@ -264,6 +266,7 @@
std::vector<AudioCodec> recv_codecs_;
int max_send_bitrate_bps_ = 0;
+ rtc::DiffServCodePoint preferred_dscp_ = rtc::DSCP_DEFAULT;
AudioOptions options_;
absl::optional<int> dtmf_payload_type_;
int dtmf_payload_freq_ = -1;
diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc
index 36c0923..095d8b2 100644
--- a/media/engine/webrtcvoiceengine_unittest.cc
+++ b/media/engine/webrtcvoiceengine_unittest.cc
@@ -1138,6 +1138,28 @@
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());
@@ -3020,6 +3042,7 @@
cricket::FakeNetworkInterface network_interface;
cricket::MediaConfig config;
std::unique_ptr<cricket::WebRtcVoiceMediaChannel> channel;
+ webrtc::RtpParameters parameters;
webrtc::AudioProcessing::Config apm_config;
EXPECT_CALL(*apm_, GetConfig())
@@ -3040,12 +3063,31 @@
static_cast<cricket::WebRtcVoiceMediaChannel*>(engine_->CreateChannel(
&call_, config, cricket::AudioOptions(), webrtc::CryptoOptions())));
channel->SetInterface(&network_interface, /*media_transport=*/nullptr);
+ EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp());
+
+ // Create a send stream to configure
+ EXPECT_TRUE(
+ channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrcZ)));
+ parameters = channel->GetRtpSendParameters(kSsrcZ);
+ ASSERT_FALSE(parameters.encodings.empty());
+
+ // Various priorities map to various dscp values.
+ parameters.encodings[0].network_priority = 4.0;
+ ASSERT_TRUE(channel->SetRtpSendParameters(kSsrcZ, parameters).ok());
EXPECT_EQ(rtc::DSCP_EF, network_interface.dscp());
+ parameters.encodings[0].network_priority = 0.5;
+ 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)));
- EXPECT_EQ(rtc::DSCP_EF, network_interface.options().dscp);
+ EXPECT_EQ(rtc::DSCP_CS1, network_interface.options().dscp);
// Verify that setting the option to false resets the
// DiffServCodePoint.
diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc
index 7493542..dadd13d 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -58,7 +58,8 @@
// layer.
bool PerSenderRtpEncodingParameterHasValue(
const RtpEncodingParameters& encoding_params) {
- if (encoding_params.bitrate_priority != kDefaultBitratePriority) {
+ if (encoding_params.bitrate_priority != kDefaultBitratePriority ||
+ encoding_params.network_priority != kDefaultBitratePriority) {
return true;
}
return false;
