From b408bb7b95b9c41cef3ac5626d43d7c003b4019e Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Fri, 10 Jan 2020 15:34:49 +0000 Subject: [PATCH] Revert "In RtpBitrateConfigurator ignore new parameters when set to default values." This reverts commit bcbdeedd432198c3d48effb2162af6344d885b14. Reason for revert: Speculative revert after a perf regression. Original change's description: > In RtpBitrateConfigurator ignore new parameters when set to default values. > > Bug: webrtc:11263 > Change-Id: Ia7539c7c142b059d0295849b916439bb647f112d > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/162207 > Reviewed-by: Sebastian Jansson > Commit-Queue: Danil Chapovalov > Cr-Commit-Position: refs/heads/master@{#30191} TBR=danilchap@webrtc.org,srte@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:11263 Change-Id: I17804655465b27523c462d2aba44519c820b8e04 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165687 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#30213} --- call/rtp_bitrate_configurator.cc | 6 +----- call/rtp_bitrate_configurator_unittest.cc | 19 +------------------ 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/call/rtp_bitrate_configurator.cc b/call/rtp_bitrate_configurator.cc index b90c1bff5d..99ccfc98f7 100644 --- a/call/rtp_bitrate_configurator.cc +++ b/call/rtp_bitrate_configurator.cc @@ -67,12 +67,8 @@ RtpBitrateConfigurator::UpdateWithSdpParameters( bitrate_config.start_bitrate_bps != base_bitrate_config_.start_bitrate_bps) { new_start.emplace(bitrate_config.start_bitrate_bps); - base_bitrate_config_.start_bitrate_bps = bitrate_config.start_bitrate_bps; } - if (bitrate_config.min_bitrate_bps > 0) - base_bitrate_config_.min_bitrate_bps = bitrate_config.min_bitrate_bps; - if (bitrate_config.max_bitrate_bps > 0) - base_bitrate_config_.max_bitrate_bps = bitrate_config.max_bitrate_bps; + base_bitrate_config_ = bitrate_config; return UpdateConstraints(new_start); } diff --git a/call/rtp_bitrate_configurator_unittest.cc b/call/rtp_bitrate_configurator_unittest.cc index 7bbdd7c22a..6449a1a0f5 100644 --- a/call/rtp_bitrate_configurator_unittest.cc +++ b/call/rtp_bitrate_configurator_unittest.cc @@ -27,7 +27,7 @@ class RtpBitrateConfiguratorTest : public ::testing::Test { absl::optional max_bitrate_bps) { absl::optional result = configurator_->UpdateWithSdpParameters(bitrate_config); - ASSERT_TRUE(result.has_value()); + EXPECT_TRUE(result.has_value()); if (start_bitrate_bps.has_value()) EXPECT_EQ(result->start_bitrate_bps, start_bitrate_bps); if (min_bitrate_bps.has_value()) @@ -231,23 +231,6 @@ TEST_F(RtpBitrateConfiguratorTest, NewConfigWithNoChangesDoesNotCallNewConfig) { EXPECT_FALSE(configurator_->UpdateWithSdpParameters(config2).has_value()); } -TEST_F(RtpBitrateConfiguratorTest, - NewConfigWithUnsetMinAndMaxDoesNotCallNewConfig) { - BitrateConstraints config1; - config1.min_bitrate_bps = 100'000; - config1.start_bitrate_bps = 1'000; - config1.max_bitrate_bps = 1'000'000; - - BitrateConstraints config2; - config2.min_bitrate_bps = 0; - config2.start_bitrate_bps = -1; - config2.max_bitrate_bps = -1; - - configurator_->UpdateWithSdpParameters(config1); - // The second call should return nothing because it doesn't change any values. - EXPECT_EQ(configurator_->UpdateWithSdpParameters(config2), absl::nullopt); -} - // If config changes the max, but not the effective max, // new config shouldn't be returned, to avoid unnecessary encoder // reconfigurations.