diff --git a/call/rtp_bitrate_configurator.cc b/call/rtp_bitrate_configurator.cc index 99ccfc98f7..b90c1bff5d 100644 --- a/call/rtp_bitrate_configurator.cc +++ b/call/rtp_bitrate_configurator.cc @@ -67,8 +67,12 @@ 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; } - base_bitrate_config_ = bitrate_config; + 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; return UpdateConstraints(new_start); } diff --git a/call/rtp_bitrate_configurator_unittest.cc b/call/rtp_bitrate_configurator_unittest.cc index 6449a1a0f5..7bbdd7c22a 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); - EXPECT_TRUE(result.has_value()); + ASSERT_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,6 +231,23 @@ 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.