Revert "Now calculates RTT in SendSideCongestionController."
This reverts commit 8c319e951a6c59212e23af858a4c51d28b4eedc1.
Reason for revert: Increase in dropped frames and decreased send bandwidth in perf tests.
Original change's description:
> Now calculates RTT in SendSideCongestionController.
>
> Moved calculation of round trip time from transport feedback adapter to send side congestion
> controller. This reduces the role of the transport specific transport feedback adapter and
> gives more power to the congestion controller to decide how the feedback rtt should be
> calculated and used.
>
> Bug: webrtc:8415
> Change-Id: I7878d9fb32c3f4ed11993a6f39e6d9c69fab190a
> Reviewed-on: https://webrtc-review.googlesource.com/27980
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#20973}
TBR=terelius@webrtc.org,stefan@webrtc.org,srte@webrtc.org
Change-Id: I993d00de7171a163a41b486d68b9255fd5c0f5da
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8415
Reviewed-on: https://webrtc-review.googlesource.com/28300
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20984}diff --git a/modules/congestion_controller/include/send_side_congestion_controller.h b/modules/congestion_controller/include/send_side_congestion_controller.h
index ac70d60..b33fad0 100644
--- a/modules/congestion_controller/include/send_side_congestion_controller.h
+++ b/modules/congestion_controller/include/send_side_congestion_controller.h
@@ -11,7 +11,6 @@
#ifndef MODULES_CONGESTION_CONTROLLER_INCLUDE_SEND_SIDE_CONGESTION_CONTROLLER_H_
#define MODULES_CONGESTION_CONTROLLER_INCLUDE_SEND_SIDE_CONGESTION_CONTROLLER_H_
-#include <deque>
#include <memory>
#include <vector>
@@ -137,11 +136,7 @@
const std::unique_ptr<ProbeController> probe_controller_;
const std::unique_ptr<RateLimiter> retransmission_rate_limiter_;
TransportFeedbackAdapter transport_feedback_adapter_;
-
rtc::CriticalSection network_state_lock_;
- std::deque<int64_t> feedback_rtts_ RTC_GUARDED_BY(network_state_lock_);
- rtc::Optional<int64_t> min_feedback_rtt_ms_
- RTC_GUARDED_BY(network_state_lock_);
uint32_t last_reported_bitrate_bps_ RTC_GUARDED_BY(network_state_lock_);
uint8_t last_reported_fraction_loss_ RTC_GUARDED_BY(network_state_lock_);
int64_t last_reported_rtt_ RTC_GUARDED_BY(network_state_lock_);
diff --git a/modules/congestion_controller/send_side_congestion_controller.cc b/modules/congestion_controller/send_side_congestion_controller.cc
index 5bee61f..ccca692 100644
--- a/modules/congestion_controller/send_side_congestion_controller.cc
+++ b/modules/congestion_controller/send_side_congestion_controller.cc
@@ -315,36 +315,11 @@
void SendSideCongestionController::OnTransportFeedback(
const rtcp::TransportFeedback& feedback) {
RTC_DCHECK_RUNS_SERIALIZED(&worker_race_);
- int64_t feedback_time_ms = clock_->TimeInMilliseconds();
-
transport_feedback_adapter_.OnTransportFeedback(feedback);
- std::vector<PacketFeedback> feedback_vector =
- transport_feedback_adapter_.GetTransportFeedbackVector();
+ std::vector<PacketFeedback> feedback_vector = ReceivedPacketFeedbackVector(
+ transport_feedback_adapter_.GetTransportFeedbackVector());
SortPacketFeedbackVector(&feedback_vector);
- int64_t feedback_rtt = -1;
- for (const auto& packet_feedback : feedback_vector) {
- if (packet_feedback.send_time_ms != PacketFeedback::kNoSendTime &&
- packet_feedback.arrival_time_ms != PacketFeedback::kNotReceived) {
- int64_t rtt = feedback_time_ms - packet_feedback.send_time_ms;
- // max() is used to account for feedback being delayed by the
- // receiver.
- feedback_rtt = std::max(rtt, feedback_rtt);
- }
- }
- if (feedback_rtt > -1) {
- rtc::CritScope cs(&network_state_lock_);
- feedback_rtts_.push_back(feedback_rtt);
- const size_t kFeedbackRttWindow = 32;
- if (feedback_rtts_.size() > kFeedbackRttWindow)
- feedback_rtts_.pop_front();
- min_feedback_rtt_ms_.emplace(
- *std::min_element(feedback_rtts_.begin(), feedback_rtts_.end()));
- }
-
- std::vector<PacketFeedback> received_feedback_vector =
- ReceivedPacketFeedbackVector(feedback_vector);
-
bool currently_in_alr =
pacer_->GetApplicationLimitedRegionStartTime().has_value();
if (was_in_alr_ && !currently_in_alr) {
@@ -377,20 +352,22 @@
size_t num_outstanding_bytes) {
RTC_DCHECK(in_cwnd_experiment_);
rtc::CritScope lock(&network_state_lock_);
+ rtc::Optional<int64_t> min_rtt_ms =
+ transport_feedback_adapter_.GetMinFeedbackLoopRtt();
// No valid RTT. Could be because send-side BWE isn't used, in which case
// we don't try to limit the outstanding packets.
- if (!min_feedback_rtt_ms_)
+ if (!min_rtt_ms)
return;
const size_t kMinCwndBytes = 2 * 1500;
size_t max_outstanding_bytes =
- std::max<size_t>((*min_feedback_rtt_ms_ + accepted_queue_ms_) *
+ std::max<size_t>((*min_rtt_ms + accepted_queue_ms_) *
last_reported_bitrate_bps_ / 1000 / 8,
kMinCwndBytes);
RTC_LOG(LS_INFO) << clock_->TimeInMilliseconds()
<< " Outstanding bytes: " << num_outstanding_bytes
<< " pacer queue: " << pacer_->QueueInMs()
<< " max outstanding: " << max_outstanding_bytes;
- RTC_LOG(LS_INFO) << "Feedback rtt: " << *min_feedback_rtt_ms_
+ RTC_LOG(LS_INFO) << "Feedback rtt: " << *min_rtt_ms
<< " Bitrate: " << last_reported_bitrate_bps_;
pause_pacer_ = num_outstanding_bytes > max_outstanding_bytes;
}
diff --git a/modules/congestion_controller/transport_feedback_adapter.cc b/modules/congestion_controller/transport_feedback_adapter.cc
index 89ddce0..0989875 100644
--- a/modules/congestion_controller/transport_feedback_adapter.cc
+++ b/modules/congestion_controller/transport_feedback_adapter.cc
@@ -131,6 +131,7 @@
return packet_feedback_vector;
}
packet_feedback_vector.reserve(feedback.GetPacketStatusCount());
+ int64_t feedback_rtt = -1;
{
rtc::CritScope cs(&lock_);
size_t failed_lookups = 0;
@@ -160,6 +161,12 @@
++failed_lookups;
if (packet_feedback.local_net_id == local_net_id_ &&
packet_feedback.remote_net_id == remote_net_id_) {
+ if (packet_feedback.send_time_ms >= 0) {
+ int64_t rtt = now_ms - packet_feedback.send_time_ms;
+ // max() is used to account for feedback being delayed by the
+ // receiver.
+ feedback_rtt = std::max(rtt, feedback_rtt);
+ }
packet_feedback_vector.push_back(packet_feedback);
}
@@ -171,6 +178,14 @@
<< " packet" << (failed_lookups > 1 ? "s" : "")
<< ". Send time history too small?";
}
+ if (feedback_rtt > -1) {
+ feedback_rtts_.push_back(feedback_rtt);
+ const size_t kFeedbackRttWindow = 32;
+ if (feedback_rtts_.size() > kFeedbackRttWindow)
+ feedback_rtts_.pop_front();
+ min_feedback_rtt_.emplace(
+ *std::min_element(feedback_rtts_.begin(), feedback_rtts_.end()));
+ }
}
return packet_feedback_vector;
}
@@ -191,6 +206,11 @@
return last_packet_feedback_vector_;
}
+rtc::Optional<int64_t> TransportFeedbackAdapter::GetMinFeedbackLoopRtt() const {
+ rtc::CritScope cs(&lock_);
+ return min_feedback_rtt_;
+}
+
size_t TransportFeedbackAdapter::GetOutstandingBytes() const {
rtc::CritScope cs(&lock_);
return send_time_history_.GetOutstandingBytes(local_net_id_, remote_net_id_);
diff --git a/modules/congestion_controller/transport_feedback_adapter.h b/modules/congestion_controller/transport_feedback_adapter.h
index bb3a1f7..008bab8 100644
--- a/modules/congestion_controller/transport_feedback_adapter.h
+++ b/modules/congestion_controller/transport_feedback_adapter.h
@@ -11,6 +11,7 @@
#ifndef MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_
#define MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_
+#include <deque>
#include <vector>
#include "modules/remote_bitrate_estimator/include/send_time_history.h"
@@ -46,6 +47,7 @@
// to the CongestionController interface.
void OnTransportFeedback(const rtcp::TransportFeedback& feedback);
std::vector<PacketFeedback> GetTransportFeedbackVector() const;
+ rtc::Optional<int64_t> GetMinFeedbackLoopRtt() const;
void SetTransportOverhead(int transport_overhead_bytes_per_packet);
@@ -67,6 +69,8 @@
std::vector<PacketFeedback> last_packet_feedback_vector_;
uint16_t local_net_id_ RTC_GUARDED_BY(&lock_);
uint16_t remote_net_id_ RTC_GUARDED_BY(&lock_);
+ std::deque<int64_t> feedback_rtts_ RTC_GUARDED_BY(&lock_);
+ rtc::Optional<int64_t> min_feedback_rtt_ RTC_GUARDED_BY(&lock_);
rtc::CriticalSection observers_lock_;
std::vector<PacketFeedbackObserver*> observers_
