From 8927b0596cd9d32a813f41bbd24a472bf41b341a Mon Sep 17 00:00:00 2001 From: minyue Date: Mon, 7 Nov 2016 07:51:20 -0800 Subject: [PATCH] Reland of "Change TWCC send interval to reduce overhead on low BW situations." "Change TWCC send interval to reduce overhead on low BW situations." was first committed in https://codereview.webrtc.org/2381833003/ but was reverted in https://codereview.webrtc.org/2468413009/ due to "float-cast-overflow". BUG=webrtc:6442, webrtc:6669 Review-Url: https://codereview.webrtc.org/2482823002 Cr-Commit-Position: refs/heads/master@{#14954} --- .../congestion_controller.cc | 1 + .../remote_estimator_proxy.cc | 35 ++++++++++++--- .../remote_estimator_proxy.h | 6 ++- .../remote_estimator_proxy_unittest.cc | 43 ++++++++++++++++++- 4 files changed, 77 insertions(+), 8 deletions(-) diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index f3ce2e5f97..0dafdf65fd 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -342,6 +342,7 @@ void CongestionController::MaybeTriggerOnNetworkChanged() { if (HasNetworkParametersToReportChanged(bitrate_bps, fraction_loss, rtt)) { observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt); + remote_estimator_proxy_.OnBitrateChanged(bitrate_bps); } } diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index b08b30a8ce..5e7560a779 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -11,6 +11,7 @@ #include "webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h" #include +#include #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" @@ -22,8 +23,10 @@ namespace webrtc { // TODO(sprang): Tune these! -const int RemoteEstimatorProxy::kDefaultProcessIntervalMs = 50; const int RemoteEstimatorProxy::kBackWindowMs = 500; +const int RemoteEstimatorProxy::kMinSendIntervalMs = 50; +const int RemoteEstimatorProxy::kMaxSendIntervalMs = 250; +const int RemoteEstimatorProxy::kDefaultSendIntervalMs = 100; // The maximum allowed value for a timestamp in milliseconds. This is lower // than the numerical limit since we often convert to microseconds. @@ -37,7 +40,8 @@ RemoteEstimatorProxy::RemoteEstimatorProxy(Clock* clock, last_process_time_ms_(-1), media_ssrc_(0), feedback_sequence_(0), - window_start_seq_(-1) {} + window_start_seq_(-1), + send_interval_ms_(kDefaultSendIntervalMs) {} RemoteEstimatorProxy::~RemoteEstimatorProxy() {} @@ -68,11 +72,12 @@ bool RemoteEstimatorProxy::LatestEstimate(std::vector* ssrcs, } int64_t RemoteEstimatorProxy::TimeUntilNextProcess() { - int64_t now = clock_->TimeInMilliseconds(); int64_t time_until_next = 0; - if (last_process_time_ms_ != -1 && - now - last_process_time_ms_ < kDefaultProcessIntervalMs) { - time_until_next = (last_process_time_ms_ + kDefaultProcessIntervalMs - now); + if (last_process_time_ms_ != -1) { + rtc::CritScope cs(&lock_); + int64_t now = clock_->TimeInMilliseconds(); + if (now - last_process_time_ms_ < send_interval_ms_) + time_until_next = (last_process_time_ms_ + send_interval_ms_ - now); } return time_until_next; } @@ -92,6 +97,24 @@ void RemoteEstimatorProxy::Process() { } } +void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { + // TwccReportSize = Ipv4(20B) + UDP(8B) + SRTP(10B) + + // AverageTwccReport(30B) + // TwccReport size at 50ms interval is 24 byte. + // TwccReport size at 250ms interval is 36 byte. + // AverageTwccReport = (TwccReport(50ms) + TwccReport(250ms)) / 2 + constexpr int kTwccReportSize = 20 + 8 + 10 + 30; + constexpr double kMinTwccRate = + kTwccReportSize * 8.0 * 1000.0 / kMaxSendIntervalMs; + constexpr double kMaxTwccRate = + kTwccReportSize * 8.0 * 1000.0 / kMinSendIntervalMs; + + // Let TWCC reports occupy 5% of total bandwidth. + rtc::CritScope cs(&lock_); + send_interval_ms_ = static_cast(0.5 + kTwccReportSize * 8.0 * 1000.0 / + (std::max(std::min(0.05 * bitrate_bps, kMaxTwccRate), kMinTwccRate))); +} + void RemoteEstimatorProxy::OnPacketArrival(uint16_t sequence_number, int64_t arrival_time) { if (arrival_time < 0 || arrival_time > kMaxTimeMs) { diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h index 127886300d..71099bf9ed 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -47,8 +47,11 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { void SetMinBitrate(int min_bitrate_bps) override {} int64_t TimeUntilNextProcess() override; void Process() override; + void OnBitrateChanged(int bitrate); - static const int kDefaultProcessIntervalMs; + static const int kMinSendIntervalMs; + static const int kMaxSendIntervalMs; + static const int kDefaultSendIntervalMs; static const int kBackWindowMs; private: @@ -68,6 +71,7 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { int64_t window_start_seq_ GUARDED_BY(&lock_); // Map unwrapped seq -> time. std::map packet_arrival_times_ GUARDED_BY(&lock_); + int64_t send_interval_ms_ GUARDED_BY(&lock_); }; } // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index 9821568260..dbb3eaa4d7 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -42,7 +42,7 @@ class RemoteEstimatorProxyTest : public ::testing::Test { void Process() { clock_.AdvanceTimeMilliseconds( - RemoteEstimatorProxy::kDefaultProcessIntervalMs); + RemoteEstimatorProxy::kDefaultSendIntervalMs); proxy_.Process(); } @@ -350,4 +350,45 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { Process(); } +TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsZeroBeforeFirstProcess) { + EXPECT_EQ(0, proxy_.TimeUntilNextProcess()); +} + +TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsDefaultOnUnkownBitrate) { + Process(); + EXPECT_EQ(RemoteEstimatorProxy::kDefaultSendIntervalMs, + proxy_.TimeUntilNextProcess()); +} + +TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMinIntervalOn300kbps) { + Process(); + proxy_.OnBitrateChanged(300000); + EXPECT_EQ(RemoteEstimatorProxy::kMinSendIntervalMs, + proxy_.TimeUntilNextProcess()); +} + +TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn0kbps) { + Process(); + // TimeUntilNextProcess should be limited by |kMaxSendIntervalMs| when + // bitrate is small. We choose 0 bps as a special case, which also tests + // erroneous behaviors like division-by-zero. + proxy_.OnBitrateChanged(0); + EXPECT_EQ(RemoteEstimatorProxy::kMaxSendIntervalMs, + proxy_.TimeUntilNextProcess()); +} + +TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn20kbps) { + Process(); + proxy_.OnBitrateChanged(20000); + EXPECT_EQ(RemoteEstimatorProxy::kMaxSendIntervalMs, + proxy_.TimeUntilNextProcess()); +} + +TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) { + Process(); + proxy_.OnBitrateChanged(80000); + // 80kbps * 0.05 = TwccReportSize(68B * 8b/B) * 1000ms / SendInterval(136ms) + EXPECT_EQ(136, proxy_.TimeUntilNextProcess()); +} + } // namespace webrtc