Revert "Wiring the RTCRtpEncodingParameters.priority down to the bitrate allocator."
This reverts commit d2b912aed132c751919ed286439fb39bbd714dda.
Reason for revert: broke internal tests
Original change's description:
> Wiring the RTCRtpEncodingParameters.priority down to the bitrate allocator.
>
> I followed the wiring path for the max bitrate.
> Doc:
> https://docs.google.com/a/google.com/document/d/1sGT6y00prOIErFuGD44zWZacDpR6Rkjg_HXA_Z3Vw4Q/edit?usp=sharing
>
> Bug: webrtc:8630
> Change-Id: I6b861816670442656721c20f81d035ee5eb6218c
> Reviewed-on: https://webrtc-review.googlesource.com/30380
> Commit-Queue: Seth Hampson <shampson@webrtc.org>
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Elad Alon <eladalon@webrtc.org>
> Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#21397}
TBR=solenberg@webrtc.org,eladalon@webrtc.org,sprang@webrtc.org,stefan@webrtc.org,pthatcher@webrtc.org,shampson@webrtc.org
Change-Id: If82810072e21818ae452a0fc3f984d44e5dac70c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8630
Reviewed-on: https://webrtc-review.googlesource.com/35540
Reviewed-by: Lu Liu <lliuu@webrtc.org>
Commit-Queue: Lu Liu <lliuu@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21399}diff --git a/api/rtpparameters.h b/api/rtpparameters.h
index e340825..221f5b6 100644
--- a/api/rtpparameters.h
+++ b/api/rtpparameters.h
@@ -73,6 +73,7 @@
};
extern const double kDefaultBitratePriority;
+enum class PriorityType { VERY_LOW, LOW, MEDIUM, HIGH };
struct RtcpFeedback {
RtcpFeedbackType type = RtcpFeedbackType::CCM;
@@ -361,10 +362,9 @@
// codec as long as it's present.
rtc::Optional<DtxStatus> 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<PriorityType> 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 @@
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 2e5654a..ff4ae97 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -243,8 +243,7 @@
!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 @@
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 @@
(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 @@
// 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 9060e64..3771767 100644
--- a/audio/audio_send_stream.h
+++ b/audio/audio_send_stream.h
@@ -106,9 +106,7 @@
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 30e3b5b..55f4e2f 100644
--- a/call/audio_send_stream.h
+++ b/call/audio_send_stream.h
@@ -104,8 +104,6 @@
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<std::string> audio_network_adaptor_config;
diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h
index b1afc8c..568bee6 100644
--- a/call/bitrate_allocator.h
+++ b/call/bitrate_allocator.h
@@ -87,7 +87,10 @@
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 154d14e..c0786a0 100644
--- a/call/bitrate_allocator_unittest.cc
+++ b/call/bitrate_allocator_unittest.cc
@@ -61,7 +61,6 @@
namespace {
constexpr int64_t kDefaultProbingIntervalMs = 3000;
-const double kDefaultBitratePriority = 1.0;
}
class BitrateAllocatorTest : public ::testing::Test {
@@ -83,8 +82,7 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
// 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 @@
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 @@
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<int>(bitrate_observer_1.last_bitrate_bps_));
EXPECT_EQ(200000, static_cast<int>(bitrate_observer_2.last_bitrate_bps_));
@@ -404,8 +389,7 @@
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 @@
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, 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, 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, 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, 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 1b1cce9..2271339 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -525,7 +525,6 @@
":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 ec772f8..4ec4afa 100644
--- a/media/engine/simulcast.cc
+++ b/media/engine/simulcast.cc
@@ -173,7 +173,6 @@
int width,
int height,
int max_bitrate_bps,
- double bitrate_priority,
int max_qp,
int max_framerate,
bool is_screencast) {
@@ -277,9 +276,6 @@
}
}
- // 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 31fdcae..84f8c31 100644
--- a/media/engine/simulcast.h
+++ b/media/engine/simulcast.h
@@ -52,7 +52,6 @@
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 d6c51d1..4c9d90e 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -1787,10 +1787,8 @@
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 @@
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 @@
}
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 @@
(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 @@
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 f54ab6d..7b1eb4a 100644
--- a/media/engine/webrtcvideoengine_unittest.cc
+++ b/media/engine/webrtcvideoengine_unittest.cc
@@ -14,7 +14,6 @@
#include <utility>
#include <vector>
-#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 @@
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<FakeVideoSendStream*> 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<double>(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<uint32_t> 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<FakeVideoSendStream*> 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<double>(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 @@
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 324b4bc..98f0aa4 100644
--- a/media/engine/webrtcvoiceengine.cc
+++ b/media/engine/webrtcvoiceengine.cc
@@ -980,11 +980,6 @@
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 @@
const rtc::Optional<int> 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 5e661c7..4df63b3 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 @@
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<int>(6000);
EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, parameters));
EXPECT_TRUE(GetSendStream(kSsrcX).IsSending());
}
@@ -1184,42 +1181,6 @@
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 6334f72..03bb84b 100644
--- a/pc/rtpsenderreceiver_unittest.cc
+++ b/pc/rtpsenderreceiver_unittest.cc
@@ -12,7 +12,6 @@
#include <string>
#include <utility>
-#include "api/rtpparameters.h"
#include "media/base/fakemediaengine.h"
#include "media/base/rtpdataengine.h"
#include "media/engine/fakewebrtccall.h"
@@ -601,28 +600,6 @@
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 @@
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 84d3916..bd700cf 100644
--- a/test/encoder_settings.cc
+++ b/test/encoder_settings.cc
@@ -55,7 +55,6 @@
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 141a5fb..2392e9f 100644
--- a/video/video_quality_test.cc
+++ b/video/video_quality_test.cc
@@ -133,8 +133,6 @@
std::vector<webrtc::VideoStream> 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 e5a850f..6b406a0 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -257,7 +257,6 @@
RtcEventLog* event_log,
const VideoSendStream::Config* config,
int initial_encoder_max_bitrate,
- double initial_encoder_bitrate_priority,
std::map<uint32_t, RtpState> suspended_ssrcs,
std::map<uint32_t, RtpPayloadState> suspended_payload_states,
VideoEncoderConfig::ContentType content_type);
@@ -352,7 +351,6 @@
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 @@
RtcEventLog* event_log,
const VideoSendStream::Config* config,
int initial_encoder_max_bitrate,
- double initial_encoder_bitrate_priority,
const std::map<uint32_t, RtpState>& suspended_ssrcs,
const std::map<uint32_t, RtpPayloadState>& suspended_payload_states,
VideoEncoderConfig::ContentType content_type)
@@ -409,7 +406,6 @@
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 @@
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 @@
RtcEventLog* const event_log_;
const VideoSendStream::Config* config_;
int initial_encoder_max_bitrate_;
- double initial_encoder_bitrate_priority_;
std::map<uint32_t, RtpState> suspended_ssrcs_;
std::map<uint32_t, RtpPayloadState> suspended_payload_states_;
const VideoEncoderConfig::ContentType content_type_;
@@ -577,8 +572,8 @@
&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 @@
RtcEventLog* event_log,
const VideoSendStream::Config* config,
int initial_encoder_max_bitrate,
- double initial_encoder_bitrate_priority,
std::map<uint32_t, RtpState> suspended_ssrcs,
std::map<uint32_t, RtpPayloadState> suspended_payload_states,
VideoEncoderConfig::ContentType content_type)
@@ -716,7 +710,6 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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);
}
}
