diff --git a/api/rtpparameters.h b/api/rtpparameters.h index 678ac02c4b..377fb78954 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -404,6 +404,14 @@ struct RtpEncodingParameters { // 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 @@ struct RtpEncodingParameters { 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 39cb4e8017..c2b0d1a878 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -298,6 +298,7 @@ void AudioSendStream::Start() { 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 @@ void AudioSendStream::ReconfigureBitrateObserver( 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 a659dd68cb..61d2f5c48d 100644 --- a/call/audio_send_stream.h +++ b/call/audio_send_stream.h @@ -100,6 +100,7 @@ class AudioSendStream { 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 f1471b63e7..a5dadd240e 100644 --- a/media/base/mediachannel.cc +++ b/media/base/mediachannel.cc @@ -28,7 +28,7 @@ void MediaChannel::SetInterface( 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 9948d96e5b..56edab72f2 100644 --- a/media/base/mediachannel.h +++ b/media/base/mediachannel.h @@ -267,9 +267,10 @@ class MediaChannel : public sigslot::has_slots<> { 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 @@ class MediaChannel : public sigslot::has_slots<> { 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 37a7f34bbe..e99886e2d7 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -522,6 +522,7 @@ WebRtcVideoChannel::WebRtcVideoChannel( 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 @@ bool WebRtcVideoChannel::GetChangedSendParameters( } rtc::DiffServCodePoint WebRtcVideoChannel::PreferredDscp() const { - return rtc::DSCP_AF41; + return preferred_dscp_; } bool WebRtcVideoChannel::SetSendParameters(const VideoSendParameters& params) { @@ -779,6 +780,29 @@ webrtc::RTCError WebRtcVideoChannel::SetRtpSendParameters( 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 @@ bool WebRtcVideoChannel::SendRtcp(const uint8_t* data, size_t len) { 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 fe5f93b38e..f327bad388 100644 --- a/media/engine/webrtcvideoengine.h +++ b/media/engine/webrtcvideoengine.h @@ -499,6 +499,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport { // 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 1f946cb64a..ceac3f7f7a 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -4596,6 +4596,7 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { new cricket::FakeNetworkInterface); MediaConfig config; std::unique_ptr channel; + webrtc::RtpParameters parameters; channel.reset(static_cast(engine_.CreateChannel( call_.get(), config, VideoOptions(), webrtc::CryptoOptions()))); @@ -4603,17 +4604,37 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { // 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(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(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 @@ 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, GetAndSetRtpSendParametersMaxFramerate) { const size_t kNumSimulcastStreams = 3; SetUpSimulcast(true, false); diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc index aa0e140799..baad232a87 100644 --- a/media/engine/webrtcvoiceengine.cc +++ b/media/engine/webrtcvoiceengine.cc @@ -57,11 +57,6 @@ constexpr int kNackRtpHistoryMs = 5000; 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 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream 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 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream const absl::optional 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 @@ WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() { } rtc::DiffServCodePoint WebRtcVoiceMediaChannel::PreferredDscp() const { - return kAudioDscpValue; + return preferred_dscp_; } bool WebRtcVoiceMediaChannel::SetSendParameters( @@ -1375,6 +1376,29 @@ webrtc::RTCError WebRtcVoiceMediaChannel::SetRtpSendParameters( 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 9c013cd405..be2fe0bfac 100644 --- a/media/engine/webrtcvoiceengine.h +++ b/media/engine/webrtcvoiceengine.h @@ -220,6 +220,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, 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 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, if (DscpEnabled()) { rtc_options.dscp = PreferredDscp(); } + return VoiceMediaChannel::SendRtcp(&packet, rtc_options); } @@ -264,6 +266,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, std::vector recv_codecs_; int max_send_bitrate_bps_ = 0; + rtc::DiffServCodePoint preferred_dscp_ = rtc::DSCP_DEFAULT; AudioOptions options_; absl::optional dtmf_payload_type_; int dtmf_payload_freq_ = -1; diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index 36c0923af1..095d8b240b 100644 --- a/media/engine/webrtcvoiceengine_unittest.cc +++ b/media/engine/webrtcvoiceengine_unittest.cc @@ -1138,6 +1138,28 @@ 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()); @@ -3020,6 +3042,7 @@ TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { cricket::FakeNetworkInterface network_interface; cricket::MediaConfig config; std::unique_ptr channel; + webrtc::RtpParameters parameters; webrtc::AudioProcessing::Config apm_config; EXPECT_CALL(*apm_, GetConfig()) @@ -3040,12 +3063,31 @@ TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { static_cast(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 7493542d6c..dadd13d91b 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -58,7 +58,8 @@ bool UnimplementedRtpEncodingParameterHasValue( // 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;