Adds allocated rate without feedback to new congestion controller.
When bitrate is allocated to streams that does not have packet feedback,
the allocated bitrate should be included in the estimate. This was
previously only implemented for the old congestion controller and not
for the new task queue based version.
To make the behavior more robust, the responsibility for tracking this
is moved to BitrateAllocator where it's handled consistently for
multiple streams without feedback.
Bug: webrtc:9586, webrtc:8243
Change-Id: I8af7fec23e1bdc08cc61cf1b4ff10461c3711fb0
Reviewed-on: https://webrtc-review.googlesource.com/102681
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Christoffer Rodbro <crodbro@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24905}diff --git a/api/transport/network_types.h b/api/transport/network_types.h
index eee1de0..ad20abc 100644
--- a/api/transport/network_types.h
+++ b/api/transport/network_types.h
@@ -36,6 +36,8 @@
absl::optional<DataRate> min_pacing_rate;
absl::optional<DataRate> max_padding_rate;
absl::optional<DataRate> max_total_allocated_bitrate;
+ // The send rate of traffic for which feedback is not received.
+ DataRate unacknowledged_rate_allocation = DataRate::Zero();
};
struct TargetRateConstraints {
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 0f725c4..eb9e8df 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -410,12 +410,6 @@
uint8_t fraction_loss,
int64_t rtt,
int64_t bwe_period_ms) {
- // Audio transport feedback will not be reported in this mode, instead update
- // acknowledged bitrate estimator with the bitrate allocated for audio.
- if (webrtc::field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC")) {
- transport_->SetAllocatedBitrateWithoutFeedback(bitrate_bps);
- }
-
// A send stream may be allocated a bitrate of zero if the allocator decides
// to disable it. For now we ignore this decision and keep sending on min
// bitrate.
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index f5d6796..a767e02 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -75,10 +75,11 @@
class MockLimitObserver : public BitrateAllocator::LimitObserver {
public:
- MOCK_METHOD4(OnAllocationLimitsChanged,
+ MOCK_METHOD5(OnAllocationLimitsChanged,
void(uint32_t min_send_bitrate_bps,
uint32_t max_padding_bitrate_bps,
uint32_t total_bitrate_bps,
+ uint32_t allocated_without_feedback_bps,
bool has_packet_feedback));
};
diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc
index c7049ba..acb19ae 100644
--- a/call/bitrate_allocator.cc
+++ b/call/bitrate_allocator.cc
@@ -60,6 +60,7 @@
total_requested_padding_bitrate_(0),
total_requested_min_bitrate_(0),
total_requested_max_bitrate_(0),
+ allocated_without_feedback_(0),
has_packet_feedback_(false),
bitrate_allocation_strategy_(nullptr),
transmission_max_bitrate_multiplier_(
@@ -191,6 +192,7 @@
uint32_t total_requested_min_bitrate = 0;
uint32_t total_requested_max_bitrate = 0;
bool has_packet_feedback = false;
+ uint32_t allocated_without_feedback = 0;
for (const auto& config : bitrate_observer_configs_) {
uint32_t stream_padding = config.pad_up_bitrate_bps;
if (config.enforce_min_bitrate) {
@@ -203,11 +205,16 @@
total_requested_max_bitrate += config.max_bitrate_bps;
if (config.allocated_bitrate_bps > 0 && config.has_packet_feedback)
has_packet_feedback = true;
+ // TODO(srte): Remove field trial check.
+ if (!config.has_packet_feedback &&
+ field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC"))
+ allocated_without_feedback += config.allocated_bitrate_bps;
}
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_ &&
+ allocated_without_feedback == allocated_without_feedback_ &&
has_packet_feedback == has_packet_feedback_) {
return;
}
@@ -215,6 +222,7 @@
total_requested_min_bitrate_ = total_requested_min_bitrate;
total_requested_padding_bitrate_ = total_requested_padding_bitrate;
total_requested_max_bitrate_ = total_requested_max_bitrate;
+ allocated_without_feedback_ = allocated_without_feedback;
has_packet_feedback_ = has_packet_feedback;
RTC_LOG(LS_INFO) << "UpdateAllocationLimits : total_requested_min_bitrate: "
@@ -225,7 +233,8 @@
<< total_requested_max_bitrate << "bps";
limit_observer_->OnAllocationLimitsChanged(
total_requested_min_bitrate, total_requested_padding_bitrate,
- total_requested_max_bitrate, has_packet_feedback);
+ total_requested_max_bitrate, allocated_without_feedback,
+ has_packet_feedback);
}
void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) {
diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h
index c29ea5e..47df7e0 100644
--- a/call/bitrate_allocator.h
+++ b/call/bitrate_allocator.h
@@ -88,10 +88,12 @@
// bitrate and max padding bitrate is changed.
class LimitObserver {
public:
- virtual void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps,
- uint32_t max_padding_bitrate_bps,
- uint32_t total_bitrate_bps,
- bool has_packet_feedback) = 0;
+ virtual void OnAllocationLimitsChanged(
+ uint32_t min_send_bitrate_bps,
+ uint32_t max_padding_bitrate_bps,
+ uint32_t total_bitrate_bps,
+ uint32_t allocated_without_feedback_bps,
+ bool has_packet_feedback) = 0;
protected:
virtual ~LimitObserver() = default;
@@ -242,6 +244,7 @@
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_);
+ uint32_t allocated_without_feedback_ RTC_GUARDED_BY(&sequenced_checker_);
bool has_packet_feedback_ RTC_GUARDED_BY(&sequenced_checker_);
std::unique_ptr<rtc::BitrateAllocationStrategy> bitrate_allocation_strategy_
RTC_GUARDED_BY(&sequenced_checker_);
diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc
index 7814655..2961fd4 100644
--- a/call/bitrate_allocator_unittest.cc
+++ b/call/bitrate_allocator_unittest.cc
@@ -28,6 +28,7 @@
void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps,
uint32_t max_padding_bitrate_bps,
uint32_t total_bitrate_bps,
+ uint32_t allocated_without_feedback_bps,
bool has_packet_feedback) override {
OnAllocationLimitsChanged(min_send_bitrate_bps, max_padding_bitrate_bps,
total_bitrate_bps);
diff --git a/call/call.cc b/call/call.cc
index 6e9a578..1062baf 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -233,6 +233,7 @@
void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps,
uint32_t max_padding_bitrate_bps,
uint32_t total_bitrate_bps,
+ uint32_t allocated_without_feedback_bps,
bool has_packet_feedback) override;
private:
@@ -1107,10 +1108,14 @@
void Call::OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps,
uint32_t max_padding_bitrate_bps,
uint32_t total_bitrate_bps,
+ uint32_t allocated_without_feedback_bps,
bool has_packet_feedback) {
transport_send_ptr_->SetAllocatedSendBitrateLimits(
min_send_bitrate_bps, max_padding_bitrate_bps, total_bitrate_bps);
transport_send_ptr_->SetPerPacketFeedbackAvailable(has_packet_feedback);
+ transport_send_ptr_->SetAllocatedBitrateWithoutFeedback(
+ allocated_without_feedback_bps);
+
rtc::CritScope lock(&bitrate_crit_);
min_allocated_send_bitrate_bps_ = min_send_bitrate_bps;
configured_max_padding_bitrate_bps_ = max_padding_bitrate_bps;
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index b0dd8d8..92d9729 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -307,6 +307,12 @@
void RtpTransportControllerSend::SetAllocatedBitrateWithoutFeedback(
uint32_t bitrate_bps) {
- send_side_cc_->SetAllocatedBitrateWithoutFeedback(bitrate_bps);
+ // Audio transport feedback will not be reported in this mode, instead update
+ // acknowledged bitrate estimator with the bitrate allocated for audio.
+ if (field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC")) {
+ // TODO(srte): Make sure it's safe to always report this and remove the
+ // field trial check.
+ send_side_cc_->SetAllocatedBitrateWithoutFeedback(bitrate_bps);
+ }
}
} // namespace webrtc
diff --git a/modules/congestion_controller/bbr/bbr_network_controller.cc b/modules/congestion_controller/bbr/bbr_network_controller.cc
index 618e90a..b82ded4 100644
--- a/modules/congestion_controller/bbr/bbr_network_controller.cc
+++ b/modules/congestion_controller/bbr/bbr_network_controller.cc
@@ -323,6 +323,8 @@
}
NetworkControlUpdate BbrNetworkController::OnStreamsConfig(StreamsConfig msg) {
+ // TODO(srte): Handle unacknowledged rate allocation.
+ RTC_DCHECK(msg.unacknowledged_rate_allocation.IsZero());
return NetworkControlUpdate();
}
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
index a984f10..44e119d 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -294,6 +294,9 @@
max_padding_rate_ = *msg.max_padding_rate;
pacing_changed = true;
}
+ acknowledged_bitrate_estimator_->SetAllocatedBitrateWithoutFeedback(
+ msg.unacknowledged_rate_allocation.bps());
+
if (pacing_changed)
update.pacer_config = GetPacingRates(msg.at_time);
return update;
diff --git a/modules/congestion_controller/pcc/pcc_network_controller.cc b/modules/congestion_controller/pcc/pcc_network_controller.cc
index e158844..98cda74 100644
--- a/modules/congestion_controller/pcc/pcc_network_controller.cc
+++ b/modules/congestion_controller/pcc/pcc_network_controller.cc
@@ -368,7 +368,9 @@
return NetworkControlUpdate();
}
-NetworkControlUpdate PccNetworkController::OnStreamsConfig(StreamsConfig) {
+NetworkControlUpdate PccNetworkController::OnStreamsConfig(StreamsConfig msg) {
+ // TODO(srte): Handle unacknowledged rate allocation.
+ RTC_DCHECK(msg.unacknowledged_rate_allocation.IsZero());
return NetworkControlUpdate();
}
diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc
index 70a8d49..266a5dc 100644
--- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc
+++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc
@@ -752,7 +752,13 @@
}
void SendSideCongestionController::SetAllocatedBitrateWithoutFeedback(
- uint32_t bitrate_bps) {}
+ uint32_t bitrate_bps) {
+ task_queue_->PostTask([this, bitrate_bps]() {
+ RTC_DCHECK_RUN_ON(task_queue_);
+ streams_config_.unacknowledged_rate_allocation = DataRate::bps(bitrate_bps);
+ UpdateStreamsConfig();
+ });
+}
void SendSideCongestionController::DisablePeriodicTasks() {
task_queue_->PostTask([this]() {
