From 448f4d50dc75531db755ed6829f33be426429f95 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 4 Apr 2018 14:52:07 +0200 Subject: [PATCH] Checking if total max bitrate has changed in BitrateAllocator. This ensures that the callback will be called if total max bit rate changes even if min bitrate or padding bitrate has not changed. Bug: None Change-Id: I616e95b1f9f5a30733f1d0acb86e18c93001d3db Reviewed-on: https://webrtc-review.googlesource.com/63642 Commit-Queue: Sebastian Jansson Reviewed-by: Stefan Holmer Reviewed-by: Philip Eliasson Cr-Commit-Position: refs/heads/master@{#22734} --- call/bitrate_allocator.cc | 13 +++++++++---- call/bitrate_allocator.h | 1 + call/bitrate_allocator_unittest.cc | 5 +++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc index 04676897d6..753fd7ef0c 100644 --- a/call/bitrate_allocator.cc +++ b/call/bitrate_allocator.cc @@ -59,6 +59,7 @@ BitrateAllocator::BitrateAllocator(LimitObserver* limit_observer) last_bwe_log_time_(0), total_requested_padding_bitrate_(0), total_requested_min_bitrate_(0), + total_requested_max_bitrate_(0), has_packet_feedback_(false), bitrate_allocation_strategy_(nullptr), transmission_max_bitrate_multiplier_( @@ -194,7 +195,7 @@ void BitrateAllocator::UpdateAllocationLimits() { RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_); uint32_t total_requested_padding_bitrate = 0; uint32_t total_requested_min_bitrate = 0; - uint32_t total_requested_bitrate = 0; + uint32_t total_requested_max_bitrate = 0; bool has_packet_feedback = false; for (const auto& config : bitrate_observer_configs_) { uint32_t stream_padding = config.pad_up_bitrate_bps; @@ -205,28 +206,32 @@ void BitrateAllocator::UpdateAllocationLimits() { std::max(config.MinBitrateWithHysteresis(), stream_padding); } total_requested_padding_bitrate += stream_padding; - total_requested_bitrate += config.max_bitrate_bps; + total_requested_max_bitrate += config.max_bitrate_bps; if (config.allocated_bitrate_bps > 0 && config.has_packet_feedback) has_packet_feedback = true; } if (total_requested_padding_bitrate == total_requested_padding_bitrate_ && total_requested_min_bitrate == total_requested_min_bitrate_ && + total_requested_max_bitrate == total_requested_max_bitrate_ && has_packet_feedback == has_packet_feedback_) { return; } total_requested_min_bitrate_ = total_requested_min_bitrate; total_requested_padding_bitrate_ = total_requested_padding_bitrate; + total_requested_max_bitrate_ = total_requested_max_bitrate; has_packet_feedback_ = has_packet_feedback; RTC_LOG(LS_INFO) << "UpdateAllocationLimits : total_requested_min_bitrate: " << total_requested_min_bitrate << "bps, total_requested_padding_bitrate: " - << total_requested_padding_bitrate << "bps"; + << total_requested_padding_bitrate + << "bps, total_requested_max_bitrate: " + << total_requested_max_bitrate << "bps"; limit_observer_->OnAllocationLimitsChanged( total_requested_min_bitrate, total_requested_padding_bitrate, - total_requested_bitrate, has_packet_feedback); + total_requested_max_bitrate, has_packet_feedback); } void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h index ca25e446ba..a4fdaf7c36 100644 --- a/call/bitrate_allocator.h +++ b/call/bitrate_allocator.h @@ -220,6 +220,7 @@ class BitrateAllocator { int64_t last_bwe_log_time_ RTC_GUARDED_BY(&sequenced_checker_); uint32_t total_requested_padding_bitrate_ RTC_GUARDED_BY(&sequenced_checker_); uint32_t total_requested_min_bitrate_ RTC_GUARDED_BY(&sequenced_checker_); + uint32_t total_requested_max_bitrate_ RTC_GUARDED_BY(&sequenced_checker_); bool has_packet_feedback_ RTC_GUARDED_BY(&sequenced_checker_); std::unique_ptr bitrate_allocation_strategy_ RTC_GUARDED_BY(&sequenced_checker_); diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc index ca35d1499d..5b07a81e78 100644 --- a/call/bitrate_allocator_unittest.cc +++ b/call/bitrate_allocator_unittest.cc @@ -117,6 +117,8 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { OnAllocationLimitsChanged(kMinSendBitrateBps, 0, _)); allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 4000000, 0, true, "", kDefaultBitratePriority); + EXPECT_CALL(limit_observer_, + OnAllocationLimitsChanged(kMinSendBitrateBps, 0, _)); EXPECT_EQ(4000000, allocator_->GetStartBitrate(&bitrate_observer)); allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, kMaxBitrateBps, @@ -221,6 +223,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { TestBitrateObserver bitrate_observer_1; // Expect OnAllocationLimitsChanged with |min_send_bitrate_bps| = 0 since // AddObserver is called with |enforce_min_bitrate| = false. + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0, _)); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 120000, _)); allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false, "", kDefaultBitratePriority); @@ -306,6 +309,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserverWithPacketLoss) { TestBitrateObserver bitrate_observer; // Expect OnAllocationLimitsChanged with |min_send_bitrate_bps| = 0 since // AddObserver is called with |enforce_min_bitrate| = false. + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0, _)); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 168000, _)); allocator_->AddObserver(&bitrate_observer, 100000, 400000, 0, false, "", kDefaultBitratePriority); @@ -347,6 +351,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserverWithPacketLoss) { allocator_->OnNetworkChanged(139000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(139000u, bitrate_observer.last_bitrate_bps_); + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0, _)); allocator_->RemoveObserver(&bitrate_observer); }