diff --git a/api/rtpparameters.h b/api/rtpparameters.h index e340825860..221f5b6da1 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -73,6 +73,7 @@ enum class DegradationPreference { }; extern const double kDefaultBitratePriority; +enum class PriorityType { VERY_LOW, LOW, MEDIUM, HIGH }; struct RtcpFeedback { RtcpFeedbackType type = RtcpFeedbackType::CCM; @@ -361,10 +362,9 @@ struct RtpEncodingParameters { // codec as long as it's present. rtc::Optional dtx; - // The relative bitrate priority of this encoding. Currently this is - // implemented on the sender level (using the first RtpEncodingParameters - // of the rtp parameters). - double bitrate_priority = kDefaultBitratePriority; + // The relative priority of this encoding. + // TODO(deadbeef): Not implemented. + rtc::Optional priority; // If set, this represents the Transport Independent Application Specific // maximum bandwidth defined in RFC3890. If unset, there is no maximum @@ -408,8 +408,7 @@ 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 && - max_bitrate_bps == o.max_bitrate_bps && + priority == o.priority && max_bitrate_bps == o.max_bitrate_bps && max_framerate == o.max_framerate && scale_resolution_down_by == o.scale_resolution_down_by && scale_framerate_down_by == o.scale_framerate_down_by && diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 2e5654a43f..ff4ae97385 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -243,8 +243,7 @@ void AudioSendStream::Start() { !webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) { // Audio BWE is enabled. transport_->packet_sender()->SetAccountForAudioPackets(true); - ConfigureBitrateObserver(config_.min_bitrate_bps, config_.max_bitrate_bps, - config_.bitrate_priority); + ConfigureBitrateObserver(config_.min_bitrate_bps, config_.max_bitrate_bps); } channel_proxy_->StartSend(); sending_ = true; @@ -632,7 +631,6 @@ void AudioSendStream::ReconfigureBitrateObserver( FindExtensionIds(new_config.rtp.extensions).transport_sequence_number; if (stream->config_.min_bitrate_bps == new_config.min_bitrate_bps && stream->config_.max_bitrate_bps == new_config.max_bitrate_bps && - stream->config_.bitrate_priority == new_config.bitrate_priority && (FindExtensionIds(stream->config_.rtp.extensions) .transport_sequence_number == new_transport_seq_num_id || !webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) { @@ -643,16 +641,14 @@ void AudioSendStream::ReconfigureBitrateObserver( (new_transport_seq_num_id != 0 || !webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) { stream->ConfigureBitrateObserver(new_config.min_bitrate_bps, - new_config.max_bitrate_bps, - new_config.bitrate_priority); + new_config.max_bitrate_bps); } else { stream->RemoveBitrateObserver(); } } void AudioSendStream::ConfigureBitrateObserver(int min_bitrate_bps, - int max_bitrate_bps, - double bitrate_priority) { + int max_bitrate_bps) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); RTC_DCHECK_GE(max_bitrate_bps, min_bitrate_bps); rtc::Event thread_sync_event(false /* manual_reset */, false); @@ -661,10 +657,8 @@ void AudioSendStream::ConfigureBitrateObserver(int min_bitrate_bps, // sure the bitrate limits in config_ are up-to-date. config_.min_bitrate_bps = min_bitrate_bps; config_.max_bitrate_bps = max_bitrate_bps; - config_.bitrate_priority = bitrate_priority; - // This either updates the current observer or adds a new observer. bitrate_allocator_->AddObserver(this, min_bitrate_bps, max_bitrate_bps, 0, - true, config_.track_id, bitrate_priority); + true, config_.track_id); thread_sync_event.Set(); }); thread_sync_event.Wait(rtc::Event::kForever); diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index 9060e64584..37717674e2 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -106,9 +106,7 @@ class AudioSendStream final : public webrtc::AudioSendStream, static void ReconfigureBitrateObserver(AudioSendStream* stream, const Config& new_config); - void ConfigureBitrateObserver(int min_bitrate_bps, - int max_bitrate_bps, - double bitrate_priority); + void ConfigureBitrateObserver(int min_bitrate_bps, int max_bitrate_bps); void RemoveBitrateObserver(); void RegisterCngPayloadType(int payload_type, int clockrate_hz); diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h index 30e3b5b3f7..55f4e2fee5 100644 --- a/call/audio_send_stream.h +++ b/call/audio_send_stream.h @@ -104,8 +104,6 @@ class AudioSendStream { int min_bitrate_bps = -1; int max_bitrate_bps = -1; - double bitrate_priority = 1.0; - // Defines whether to turn on audio network adaptor, and defines its config // string. rtc::Optional audio_network_adaptor_config; diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h index b1afc8c424..568bee6e7f 100644 --- a/call/bitrate_allocator.h +++ b/call/bitrate_allocator.h @@ -87,7 +87,10 @@ class BitrateAllocator { uint32_t pad_up_bitrate_bps, bool enforce_min_bitrate, std::string track_id, - double bitrate_priority); + // TODO(shampson): Take out default value and wire the + // bitrate_priority up to the AudioSendStream::Config and + // VideoSendStream::Config. + double bitrate_priority = 1.0); // Removes a previously added observer, but will not trigger a new bitrate // allocation. diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc index 154d14e3d7..c0786a073d 100644 --- a/call/bitrate_allocator_unittest.cc +++ b/call/bitrate_allocator_unittest.cc @@ -61,7 +61,6 @@ class TestBitrateObserver : public BitrateAllocatorObserver { namespace { constexpr int64_t kDefaultProbingIntervalMs = 3000; -const double kDefaultBitratePriority = 1.0; } class BitrateAllocatorTest : public ::testing::Test { @@ -83,8 +82,7 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps, kPadUpToBitrateBps)); allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000, - kPadUpToBitrateBps, true, "", - kDefaultBitratePriority); + kPadUpToBitrateBps, true, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer)); allocator_->OnNetworkChanged(200000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer)); @@ -98,11 +96,11 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps, 0)); allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 4000000, 0, - true, "", kDefaultBitratePriority); + true, ""); EXPECT_EQ(4000000, allocator_->GetStartBitrate(&bitrate_observer)); allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000, 0, - true, "", kDefaultBitratePriority); + true, ""); EXPECT_EQ(3000000, allocator_->GetStartBitrate(&bitrate_observer)); EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); allocator_->OnNetworkChanged(1500000, 0, 0, kDefaultProbingIntervalMs); @@ -113,13 +111,11 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(100000, 0)); - allocator_->AddObserver(&bitrate_observer_1, 100000, 300000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_1, 100000, 300000, 0, true, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(100000 + 200000, 0)); - allocator_->AddObserver(&bitrate_observer_2, 200000, 300000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_2, 200000, 300000, 0, true, ""); EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer_2)); // Test too low start bitrate, hence lower than sum of min. Min bitrates @@ -163,8 +159,7 @@ TEST_F(BitrateAllocatorTest, RemoveObserverTriggersLimitObserver) { EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps, kPadUpToBitrateBps)); allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000, - kPadUpToBitrateBps, true, "", - kDefaultBitratePriority); + kPadUpToBitrateBps, true, ""); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); allocator_->RemoveObserver(&bitrate_observer); } @@ -188,8 +183,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { // Expect OnAllocationLimitsChanged with |min_send_bitrate_bps| = 0 since // AddObserver is called with |enforce_min_bitrate| = false. EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 120000)); - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); // High BWE. @@ -209,17 +203,14 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; // Set up the observers with min bitrates at 100000, 200000, and 300000. - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); - allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false, ""); EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer_2)); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); - allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, false, ""); EXPECT_EQ(0, allocator_->GetStartBitrate(&bitrate_observer_3)); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); @@ -273,8 +264,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserverWithPacketLoss) { // Expect OnAllocationLimitsChanged with |min_send_bitrate_bps| = 0 since // AddObserver is called with |enforce_min_bitrate| = false. EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 168000)); - allocator_->AddObserver(&bitrate_observer, 100000, 400000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer, 100000, 400000, 0, false, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer)); // High BWE. @@ -320,11 +310,9 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, TwoBitrateObserverWithPacketLoss) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); - allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false, ""); EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer_2)); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); @@ -373,17 +361,14 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowBweEnforceMin) { TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, true, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); - allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, true, ""); EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer_2)); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); - allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, true, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_3)); EXPECT_EQ(100000, static_cast(bitrate_observer_1.last_bitrate_bps_)); EXPECT_EQ(200000, static_cast(bitrate_observer_2.last_bitrate_bps_)); @@ -404,8 +389,7 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { TestBitrateObserver bitrate_observer_1; EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(50000, 0)); - allocator_->AddObserver(&bitrate_observer_1, 50000, 400000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_1, 50000, 400000, 0, true, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); // Set network down, ie, no available bitrate. @@ -416,8 +400,7 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { TestBitrateObserver bitrate_observer_2; // Adding an observer while the network is down should not affect the limits. EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(50000 + 50000, 0)); - allocator_->AddObserver(&bitrate_observer_2, 50000, 400000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&bitrate_observer_2, 50000, 400000, 0, true, ""); // Expect the start_bitrate to be set as if the network was still up but that // the new observer have been notified that the network is down. @@ -433,13 +416,11 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { TEST_F(BitrateAllocatorTest, MixedEnforecedConfigs) { TestBitrateObserver enforced_observer; - allocator_->AddObserver(&enforced_observer, 6000, 30000, 0, true, "", - kDefaultBitratePriority); + allocator_->AddObserver(&enforced_observer, 6000, 30000, 0, true, ""); EXPECT_EQ(60000, allocator_->GetStartBitrate(&enforced_observer)); TestBitrateObserver not_enforced_observer; - allocator_->AddObserver(¬_enforced_observer, 30000, 2500000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(¬_enforced_observer, 30000, 2500000, 0, false, ""); EXPECT_EQ(270000, allocator_->GetStartBitrate(¬_enforced_observer)); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); @@ -477,8 +458,7 @@ TEST_F(BitrateAllocatorTest, MixedEnforecedConfigs) { TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { TestBitrateObserver observer; - allocator_->AddObserver(&observer, 30000, 300000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&observer, 30000, 300000, 0, false, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); allocator_->OnNetworkChanged(30000, 0, 50, kDefaultProbingIntervalMs); @@ -504,8 +484,7 @@ TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { TEST_F(BitrateAllocatorTest, AvoidTogglePercent) { TestBitrateObserver observer; - allocator_->AddObserver(&observer, 300000, 600000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&observer, 300000, 600000, 0, false, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); allocator_->OnNetworkChanged(300000, 0, 50, kDefaultProbingIntervalMs); @@ -531,8 +510,7 @@ TEST_F(BitrateAllocatorTest, AvoidTogglePercent) { TEST_F(BitrateAllocatorTest, PassProbingInterval) { TestBitrateObserver observer; - allocator_->AddObserver(&observer, 300000, 600000, 0, false, "", - kDefaultBitratePriority); + allocator_->AddObserver(&observer, 300000, 600000, 0, false, ""); EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); allocator_->OnNetworkChanged(300000, 0, 50, 5000); diff --git a/media/BUILD.gn b/media/BUILD.gn index 1b1cce941b..227133913f 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -525,7 +525,6 @@ if (rtc_include_tests) { ":rtc_media", ":rtc_media_base", ":rtc_media_tests_utils", - "../api:libjingle_peerconnection_api", "../api:mock_video_codec_factory", "../api:video_frame_api", "../api/audio_codecs:builtin_audio_decoder_factory", diff --git a/media/engine/simulcast.cc b/media/engine/simulcast.cc index ec772f848f..4ec4afa71f 100644 --- a/media/engine/simulcast.cc +++ b/media/engine/simulcast.cc @@ -173,7 +173,6 @@ std::vector GetSimulcastConfig(size_t max_streams, int width, int height, int max_bitrate_bps, - double bitrate_priority, int max_qp, int max_framerate, bool is_screencast) { @@ -277,9 +276,6 @@ std::vector GetSimulcastConfig(size_t max_streams, } } - // The bitrate priority currently implemented on a per-sender level, so we - // just set it for the first video stream. - streams[0].bitrate_priority = bitrate_priority; return streams; } diff --git a/media/engine/simulcast.h b/media/engine/simulcast.h index 31fdcae80b..84f8c31a89 100644 --- a/media/engine/simulcast.h +++ b/media/engine/simulcast.h @@ -52,7 +52,6 @@ std::vector GetSimulcastConfig(size_t max_streams, int width, int height, int max_bitrate_bps, - double bitrate_priority, int max_qp, int max_framerate, bool is_screencast = false); diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index d6c51d1f32..4c9d90e42b 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1787,10 +1787,8 @@ bool WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( return false; } - bool reconfigure_encoder = (new_parameters.encodings[0].max_bitrate_bps != - rtp_parameters_.encodings[0].max_bitrate_bps) || - (new_parameters.encodings[0].bitrate_priority != - rtp_parameters_.encodings[0].bitrate_priority); + bool reconfigure_encoder = new_parameters.encodings[0].max_bitrate_bps != + rtp_parameters_.encodings[0].max_bitrate_bps; rtp_parameters_ = new_parameters; // Codecs are currently handled at the WebRtcVideoChannel level. rtp_parameters_.codecs.clear(); @@ -1820,11 +1818,6 @@ bool WebRtcVideoChannel::WebRtcVideoSendStream::ValidateRtpParameters( RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters with modified SSRC"; return false; } - if (rtp_parameters.encodings[0].bitrate_priority <= 0) { - RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters bitrate_priority to " - "an invalid number. bitrate_priority must be > 0."; - return false; - } return true; } @@ -1883,13 +1876,6 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig( } encoder_config.max_bitrate_bps = stream_max_bitrate; - // The encoder config's default bitrate priority is set to 1.0, - // unless it is set through the sender's encoding parameters. - // The bitrate priority, which is used in the bitrate allocation, is done - // on a per sender basis, so we use the first encoding's value. - encoder_config.bitrate_priority = - rtp_parameters_.encodings[0].bitrate_priority; - int max_qp = kDefaultQpMax; codec.GetParam(kCodecParamMaxQuantization, &max_qp); encoder_config.video_stream_factory = @@ -2594,8 +2580,7 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( (CodecNamesEq(codec_name_, kVp8CodecName) && is_screencast_ && conference_mode_)) { return GetSimulcastConfig(encoder_config.number_of_streams, width, height, - encoder_config.max_bitrate_bps, - encoder_config.bitrate_priority, max_qp_, + encoder_config.max_bitrate_bps, max_qp_, max_framerate_, is_screencast_); } @@ -2612,7 +2597,6 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( stream.min_bitrate_bps = GetMinVideoBitrateBps(); stream.target_bitrate_bps = stream.max_bitrate_bps = max_bitrate_bps; stream.max_qp = max_qp_; - stream.bitrate_priority = encoder_config.bitrate_priority; if (CodecNamesEq(codec_name_, kVp9CodecName) && !is_screencast_) { stream.temporal_layer_thresholds_bps.resize(GetDefaultVp9TemporalLayers() - diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index f54ab6d62e..7b1eb4a583 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -14,7 +14,6 @@ #include #include -#include "api/rtpparameters.h" #include "api/test/mock_video_decoder_factory.h" #include "api/test/mock_video_encoder_factory.h" #include "api/video_codecs/sdp_video_format.h" @@ -4360,122 +4359,6 @@ TEST_F(WebRtcVideoChannelTest, CannotSetSsrcInRtpSendParameters) { EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); } -// Tests that when RTCRtpEncodingParameters.bitrate_priority gets set to -// a value <= 0, setting the parameters returns false. -TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersInvalidBitratePriority) { - AddSendStream(); - webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); - EXPECT_EQ(1UL, parameters.encodings.size()); - EXPECT_EQ(webrtc::kDefaultBitratePriority, - parameters.encodings[0].bitrate_priority); - - parameters.encodings[0].bitrate_priority = 0; - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); - parameters.encodings[0].bitrate_priority = -2; - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); -} - -// Tests when the the RTCRtpEncodingParameters.bitrate_priority gets set -// properly on the VideoChannel and propogates down to the video encoder. -TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersPriorityOneStream) { - AddSendStream(); - webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); - EXPECT_EQ(1UL, parameters.encodings.size()); - EXPECT_EQ(webrtc::kDefaultBitratePriority, - parameters.encodings[0].bitrate_priority); - - // Change the value and set it on the VideoChannel. - double new_bitrate_priority = 2.0; - parameters.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); - - // Verify that the encoding parameters bitrate_priority is set for the - // VideoChannel. - parameters = channel_->GetRtpSendParameters(last_ssrc_); - EXPECT_EQ(1UL, parameters.encodings.size()); - EXPECT_EQ(new_bitrate_priority, parameters.encodings[0].bitrate_priority); - - // Verify that the new value propagated down to the encoder. - std::vector video_send_streams = - fake_call_->GetVideoSendStreams(); - EXPECT_EQ(1UL, video_send_streams.size()); - FakeVideoSendStream* video_send_stream = video_send_streams.front(); - // Check that the WebRtcVideoSendStream updated the VideoEncoderConfig - // appropriately. - EXPECT_EQ(new_bitrate_priority, - video_send_stream->GetEncoderConfig().bitrate_priority); - // Check that the vector of VideoStreams also was propagated correctly. Note - // that this is testing the behavior of the FakeVideoSendStream, which mimics - // the calls to CreateEncoderStreams to get the VideoStreams. - EXPECT_EQ(rtc::Optional(new_bitrate_priority), - video_send_stream->GetVideoStreams()[0].bitrate_priority); -} - -// Tests that the RTCRtpEncodingParameters.bitrate_priority is set for the -// VideoChannel and the value propogates to the video encoder with all simulcast -// streams. -TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersPrioritySimulcastStreams) { - // Create the stream params with multiple ssrcs for simulcast. - const int kNumSimulcastStreams = 3; - std::vector ssrcs = MAKE_VECTOR(kSsrcs3); - StreamParams stream_params = CreateSimStreamParams("cname", ssrcs); - AddSendStream(stream_params); - uint32_t primary_ssrc = stream_params.first_ssrc(); - - // Using the FakeVideoCapturer, we manually send a full size frame. This - // creates multiple VideoStreams for all simulcast layers when reconfiguring, - // and allows us to test this behavior. - cricket::FakeVideoCapturer capturer; - VideoOptions options; - EXPECT_TRUE(channel_->SetVideoSend(primary_ssrc, true, &options, &capturer)); - EXPECT_EQ(cricket::CS_RUNNING, - capturer.Start(cricket::VideoFormat( - 1920, 1080, cricket::VideoFormat::FpsToInterval(30), - cricket::FOURCC_I420))); - channel_->SetSend(true); - EXPECT_TRUE(capturer.CaptureFrame()); - // Get and set the rtp encoding parameters. - webrtc::RtpParameters parameters = - channel_->GetRtpSendParameters(primary_ssrc); - EXPECT_EQ(1UL, parameters.encodings.size()); - EXPECT_EQ(webrtc::kDefaultBitratePriority, - parameters.encodings[0].bitrate_priority); - // Change the value and set it on the VideoChannel. - double new_bitrate_priority = 2.0; - parameters.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(channel_->SetRtpSendParameters(primary_ssrc, parameters)); - - // Verify that the encoding parameters priority is set on the VideoChannel. - parameters = channel_->GetRtpSendParameters(primary_ssrc); - EXPECT_EQ(1UL, parameters.encodings.size()); - EXPECT_EQ(new_bitrate_priority, parameters.encodings[0].bitrate_priority); - - // Verify that the new value propagated down to the encoder. - std::vector video_send_streams = - fake_call_->GetVideoSendStreams(); - EXPECT_EQ(1UL, video_send_streams.size()); - FakeVideoSendStream* video_send_stream = video_send_streams.front(); - // Check that the WebRtcVideoSendStream updated the VideoEncoderConfig - // appropriately. - EXPECT_EQ(kNumSimulcastStreams, - video_send_stream->GetEncoderConfig().number_of_streams); - EXPECT_EQ(new_bitrate_priority, - video_send_stream->GetEncoderConfig().bitrate_priority); - // Check that the vector of VideoStreams also propagated correctly. The - // FakeVideoSendStream calls CreateEncoderStreams, and we are testing that - // these are created appropriately for the simulcast case. - EXPECT_EQ(kNumSimulcastStreams, video_send_stream->GetVideoStreams().size()); - EXPECT_EQ(rtc::Optional(new_bitrate_priority), - video_send_stream->GetVideoStreams()[0].bitrate_priority); - // Since we are only setting bitrate priority per-sender, the other - // VideoStreams should have a bitrate priority of 0. - EXPECT_EQ(rtc::nullopt, - video_send_stream->GetVideoStreams()[1].bitrate_priority); - EXPECT_EQ(rtc::nullopt, - video_send_stream->GetVideoStreams()[2].bitrate_priority); - EXPECT_TRUE(channel_->SetVideoSend(primary_ssrc, true, nullptr, nullptr)); -} - // Test that a stream will not be sending if its encoding is made inactive // through SetRtpSendParameters. // TODO(deadbeef): Update this test when we start supporting setting parameters @@ -4825,8 +4708,7 @@ class WebRtcVideoChannelSimulcastTest : public testing::Test { if (conference_mode) { expected_streams = GetSimulcastConfig( num_configured_streams, capture_width, capture_height, 0, - webrtc::kDefaultBitratePriority, kDefaultQpMax, - kDefaultVideoMaxFramerate, screenshare); + kDefaultQpMax, kDefaultVideoMaxFramerate, screenshare); if (screenshare) { for (const webrtc::VideoStream& stream : expected_streams) { // Never scale screen content. diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc index 324b4bc40c..98f0aa49ca 100644 --- a/media/engine/webrtcvoiceengine.cc +++ b/media/engine/webrtcvoiceengine.cc @@ -980,11 +980,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters with modified SSRC"; return false; } - if (rtp_parameters.encodings[0].bitrate_priority <= 0) { - RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters bitrate_priority to " - "an invalid number."; - return false; - } return true; } @@ -1005,25 +1000,20 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream const rtc::Optional old_rtp_max_bitrate = rtp_parameters_.encodings[0].max_bitrate_bps; - double old_priority = rtp_parameters_.encodings[0].bitrate_priority; - rtp_parameters_ = parameters; - config_.bitrate_priority = rtp_parameters_.encodings[0].bitrate_priority; - 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_ = parameters; + if (rtp_parameters_.encodings[0].max_bitrate_bps != old_rtp_max_bitrate) { - // Update the bitrate range. + // Reconfigure AudioSendStream with new bit rate. if (send_rate) { config_.send_codec_spec->target_bitrate_bps = send_rate; } UpdateAllowedBitrateRange(); - } - if (reconfigure_send_stream) { ReconfigureAudioSendStream(); + } else { + // parameters.encodings[0].active could have changed. + UpdateSendState(); } - // parameters.encodings[0].active could have changed. - UpdateSendState(); return true; } diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index 5e661c7b9e..4df63b398c 100644 --- a/media/engine/webrtcvoiceengine_unittest.cc +++ b/media/engine/webrtcvoiceengine_unittest.cc @@ -13,7 +13,6 @@ #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/audio_codecs/builtin_audio_encoder_factory.h" -#include "api/rtpparameters.h" #include "call/call.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "media/base/fakemediaengine.h" @@ -1086,9 +1085,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRtpParametersEncodingsActive) { EXPECT_FALSE(GetSendStream(kSsrcX).IsSending()); // Now change it back to active and verify we resume sending. - // This should occur even when other parameters are updated. parameters.encodings[0].active = true; - parameters.encodings[0].max_bitrate_bps = rtc::Optional(6000); EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, parameters)); EXPECT_TRUE(GetSendStream(kSsrcX).IsSending()); } @@ -1184,42 +1181,6 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParameterUpdatesMaxBitrate) { EXPECT_EQ(max_bitrate, kMaxBitrateBps); } -// Tests that when RTCRtpEncodingParameters.bitrate_priority gets set to -// a value <= 0, setting the parameters returns false. -TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParameterInvalidBitratePriority) { - EXPECT_TRUE(SetupSendStream()); - webrtc::RtpParameters rtp_parameters = channel_->GetRtpSendParameters(kSsrcX); - EXPECT_EQ(1UL, rtp_parameters.encodings.size()); - EXPECT_EQ(webrtc::kDefaultBitratePriority, - rtp_parameters.encodings[0].bitrate_priority); - - rtp_parameters.encodings[0].bitrate_priority = 0; - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); - rtp_parameters.encodings[0].bitrate_priority = -1.0; - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); -} - -// Test that the bitrate_priority in the send stream config gets updated when -// SetRtpSendParameters is set for the VoiceMediaChannel. -TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParameterUpdatesBitratePriority) { - EXPECT_TRUE(SetupSendStream()); - webrtc::RtpParameters rtp_parameters = channel_->GetRtpSendParameters(kSsrcX); - - EXPECT_EQ(1UL, rtp_parameters.encodings.size()); - EXPECT_EQ(webrtc::kDefaultBitratePriority, - rtp_parameters.encodings[0].bitrate_priority); - double new_bitrate_priority = 2.0; - rtp_parameters.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); - - // The priority should get set for both the audio channel's rtp parameters - // and the audio send stream's audio config. - EXPECT_EQ( - new_bitrate_priority, - channel_->GetRtpSendParameters(kSsrcX).encodings[0].bitrate_priority); - EXPECT_EQ(new_bitrate_priority, GetSendStreamConfig(kSsrcX).bitrate_priority); -} - // Test that GetRtpReceiveParameters returns the currently configured codecs. TEST_F(WebRtcVoiceEngineTestFake, GetRtpReceiveParametersCodecs) { EXPECT_TRUE(SetupRecvStream()); diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index 6334f723e7..03bb84b656 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -12,7 +12,6 @@ #include #include -#include "api/rtpparameters.h" #include "media/base/fakemediaengine.h" #include "media/base/rtpdataengine.h" #include "media/engine/fakewebrtccall.h" @@ -601,28 +600,6 @@ TEST_F(RtpSenderReceiverTest, SetAudioMaxSendBitrate) { DestroyAudioRtpSender(); } -TEST_F(RtpSenderReceiverTest, SetAudioBitratePriority) { - CreateAudioRtpSender(); - - webrtc::RtpParameters params = audio_rtp_sender_->GetParameters(); - EXPECT_EQ(1, params.encodings.size()); - EXPECT_EQ(webrtc::kDefaultBitratePriority, - params.encodings[0].bitrate_priority); - double new_bitrate_priority = 2.0; - params.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(audio_rtp_sender_->SetParameters(params)); - - params = audio_rtp_sender_->GetParameters(); - EXPECT_EQ(1, params.encodings.size()); - EXPECT_EQ(new_bitrate_priority, params.encodings[0].bitrate_priority); - - params = voice_media_channel_->GetRtpSendParameters(kAudioSsrc); - EXPECT_EQ(1, params.encodings.size()); - EXPECT_EQ(new_bitrate_priority, params.encodings[0].bitrate_priority); - - DestroyAudioRtpSender(); -} - TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParameters) { CreateVideoRtpSender(); @@ -659,28 +636,6 @@ TEST_F(RtpSenderReceiverTest, SetVideoMaxSendBitrate) { DestroyVideoRtpSender(); } -TEST_F(RtpSenderReceiverTest, SetVideoBitratePriority) { - CreateVideoRtpSender(); - - webrtc::RtpParameters params = video_rtp_sender_->GetParameters(); - EXPECT_EQ(1, params.encodings.size()); - EXPECT_EQ(webrtc::kDefaultBitratePriority, - params.encodings[0].bitrate_priority); - double new_bitrate_priority = 2.0; - params.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(video_rtp_sender_->SetParameters(params)); - - params = video_rtp_sender_->GetParameters(); - EXPECT_EQ(1, params.encodings.size()); - EXPECT_EQ(new_bitrate_priority, params.encodings[0].bitrate_priority); - - params = video_media_channel_->GetRtpSendParameters(kVideoSsrc); - EXPECT_EQ(1, params.encodings.size()); - EXPECT_EQ(new_bitrate_priority, params.encodings[0].bitrate_priority); - - DestroyVideoRtpSender(); -} - TEST_F(RtpSenderReceiverTest, AudioReceiverCanSetParameters) { CreateAudioRtpReceiver(); diff --git a/test/encoder_settings.cc b/test/encoder_settings.cc index 84d391669e..bd700cf6d2 100644 --- a/test/encoder_settings.cc +++ b/test/encoder_settings.cc @@ -55,7 +55,6 @@ std::vector CreateVideoStreams( stream_settings[encoder_config.number_of_streams - 1].max_bitrate_bps += bitrate_left_bps; - stream_settings[0].bitrate_priority = encoder_config.bitrate_priority; return stream_settings; } diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 141a5fb0d1..2392e9f34b 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -133,8 +133,6 @@ class VideoStreamFactory std::vector streams = streams_; streams[streams_.size() - 1].height = height; streams[streams_.size() - 1].width = width; - - streams[0].bitrate_priority = encoder_config.bitrate_priority; return streams; } diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index e5a850f556..6b406a0577 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -257,7 +257,6 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, RtcEventLog* event_log, const VideoSendStream::Config* config, int initial_encoder_max_bitrate, - double initial_encoder_bitrate_priority, std::map suspended_ssrcs, std::map suspended_payload_states, VideoEncoderConfig::ContentType content_type); @@ -352,7 +351,6 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, int encoder_min_bitrate_bps_; uint32_t encoder_max_bitrate_bps_; uint32_t encoder_target_rate_bps_; - double encoder_bitrate_priority_; VideoStreamEncoder* const video_stream_encoder_; EncoderRtcpFeedback encoder_feedback_; @@ -394,7 +392,6 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { RtcEventLog* event_log, const VideoSendStream::Config* config, int initial_encoder_max_bitrate, - double initial_encoder_bitrate_priority, const std::map& suspended_ssrcs, const std::map& suspended_payload_states, VideoEncoderConfig::ContentType content_type) @@ -409,7 +406,6 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { event_log_(event_log), config_(config), initial_encoder_max_bitrate_(initial_encoder_max_bitrate), - initial_encoder_bitrate_priority_(initial_encoder_bitrate_priority), suspended_ssrcs_(suspended_ssrcs), suspended_payload_states_(suspended_payload_states), content_type_(content_type) {} @@ -422,8 +418,8 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { stats_proxy_, rtc::TaskQueue::Current(), call_stats_, transport_, bitrate_allocator_, send_delay_stats_, video_stream_encoder_, event_log_, config_, initial_encoder_max_bitrate_, - initial_encoder_bitrate_priority_, std::move(suspended_ssrcs_), - std::move(suspended_payload_states_), content_type_)); + std::move(suspended_ssrcs_), std::move(suspended_payload_states_), + content_type_)); return true; } @@ -438,7 +434,6 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { RtcEventLog* const event_log_; const VideoSendStream::Config* config_; int initial_encoder_max_bitrate_; - double initial_encoder_bitrate_priority_; std::map suspended_ssrcs_; std::map suspended_payload_states_; const VideoEncoderConfig::ContentType content_type_; @@ -577,8 +572,8 @@ VideoSendStream::VideoSendStream( &send_stream_, &thread_sync_event_, &stats_proxy_, video_stream_encoder_.get(), module_process_thread, call_stats, transport, bitrate_allocator, send_delay_stats, event_log, &config_, - encoder_config.max_bitrate_bps, encoder_config.bitrate_priority, - suspended_ssrcs, suspended_payload_states, encoder_config.content_type))); + encoder_config.max_bitrate_bps, suspended_ssrcs, suspended_payload_states, + encoder_config.content_type))); // Wait for ConstructionTask to complete so that |send_stream_| can be used. // |module_process_thread| must be registered and deregistered on the thread @@ -696,7 +691,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( RtcEventLog* event_log, const VideoSendStream::Config* config, int initial_encoder_max_bitrate, - double initial_encoder_bitrate_priority, std::map suspended_ssrcs, std::map suspended_payload_states, VideoEncoderConfig::ContentType content_type) @@ -716,7 +710,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( encoder_min_bitrate_bps_(0), encoder_max_bitrate_bps_(initial_encoder_max_bitrate), encoder_target_rate_bps_(0), - encoder_bitrate_priority_(initial_encoder_bitrate_priority), video_stream_encoder_(video_stream_encoder), encoder_feedback_(Clock::GetRealTimeClock(), config_->rtp.ssrcs, @@ -753,7 +746,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( RTC_DCHECK(call_stats_); RTC_DCHECK(transport_); RTC_DCHECK(transport_->send_side_cc()); - RTC_DCHECK_GT(encoder_max_bitrate_bps_, 0); RTC_CHECK(field_trial::FindFullName( AlrDetector::kStrictPacingAndProbingExperimentName) .empty() || @@ -888,7 +880,7 @@ void VideoSendStreamImpl::Start() { bitrate_allocator_->AddObserver( this, encoder_min_bitrate_bps_, encoder_max_bitrate_bps_, max_padding_bitrate_, !config_->suspend_below_min_bitrate, - config_->track_id, encoder_bitrate_priority_); + config_->track_id); // Start monitoring encoder activity. { @@ -942,7 +934,7 @@ void VideoSendStreamImpl::SignalEncoderActive() { bitrate_allocator_->AddObserver( this, encoder_min_bitrate_bps_, encoder_max_bitrate_bps_, max_padding_bitrate_, !config_->suspend_below_min_bitrate, - config_->track_id, encoder_bitrate_priority_); + config_->track_id); } void VideoSendStreamImpl::OnEncoderConfigurationChanged( @@ -962,16 +954,8 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( encoder_min_bitrate_bps_ = std::max(streams[0].min_bitrate_bps, GetEncoderMinBitrateBps()); encoder_max_bitrate_bps_ = 0; - double stream_bitrate_priority_sum = 0; - for (const auto& stream : streams) { + for (const auto& stream : streams) encoder_max_bitrate_bps_ += stream.max_bitrate_bps; - if (stream.bitrate_priority) { - RTC_DCHECK_GT(*stream.bitrate_priority, 0); - stream_bitrate_priority_sum += *stream.bitrate_priority; - } - } - RTC_DCHECK_GT(stream_bitrate_priority_sum, 0); - encoder_bitrate_priority_ = stream_bitrate_priority_sum; max_padding_bitrate_ = CalculateMaxPadBitrateBps( streams, min_transmit_bitrate_bps, config_->suspend_below_min_bitrate); @@ -992,7 +976,7 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( bitrate_allocator_->AddObserver( this, encoder_min_bitrate_bps_, encoder_max_bitrate_bps_, max_padding_bitrate_, !config_->suspend_below_min_bitrate, - config_->track_id, encoder_bitrate_priority_); + config_->track_id); } }