From 05ce4ae31f635c16e10b68601e7e07fdf7bbb29b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 8 Jul 2016 19:11:10 +0200 Subject: [PATCH] Reland Issue 2061423003: Refactor NACK bitrate allocation This is a reland of https://codereview.webrtc.org/2061423003/ Which was reverted in https://codereview.webrtc.org/2131913003/ The reason for the revert was that some upstream code used RtpSender::SetTargetBitrate(). I've added that back as a no-op until we it's been brought up to date. TBR=tommi@webrtc.org Review URL: https://codereview.webrtc.org/2131313002 . Cr-Commit-Position: refs/heads/master@{#13418} --- webrtc/BUILD.gn | 1 + webrtc/base/BUILD.gn | 2 + webrtc/base/base.gyp | 2 + webrtc/base/rate_limiter.cc | 65 ++++++ webrtc/base/rate_limiter.h | 56 +++++ webrtc/base/rate_limiter_unittest.cc | 205 ++++++++++++++++++ webrtc/base/rate_statistics.cc | 8 +- webrtc/base/rate_statistics.h | 23 +- webrtc/call/rtc_event_log_unittest.cc | 3 +- webrtc/common_types.h | 7 +- .../congestion_controller.cc | 23 +- .../include/congestion_controller.h | 3 + webrtc/modules/rtp_rtcp/BUILD.gn | 2 - .../rtp_rtcp/include/receive_statistics.h | 4 +- webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 9 +- webrtc/modules/rtp_rtcp/rtp_rtcp.gypi | 2 - webrtc/modules/rtp_rtcp/source/bitrate.cc | 121 ----------- webrtc/modules/rtp_rtcp/source/bitrate.h | 77 ------- .../rtp_rtcp/source/nack_rtx_unittest.cc | 9 +- .../source/receive_statistics_impl.cc | 36 +-- .../rtp_rtcp/source/receive_statistics_impl.h | 14 +- .../rtp_rtcp/source/rtp_receiver_video.h | 1 - .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 10 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 2 - .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 8 +- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 205 ++++-------------- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 62 +----- .../rtp_rtcp/source/rtp_sender_unittest.cc | 73 ++++--- .../rtp_rtcp/source/rtp_sender_video.cc | 26 ++- .../rtp_rtcp/source/rtp_sender_video.h | 14 +- webrtc/video/end_to_end_tests.cc | 49 ++++- webrtc/video/payload_router.cc | 19 -- webrtc/video/payload_router.h | 3 - webrtc/video/payload_router_unittest.cc | 21 -- webrtc/video/rtp_stream_receiver.cc | 3 +- webrtc/video/send_statistics_proxy.cc | 8 +- webrtc/video/send_statistics_proxy.h | 4 +- .../video/send_statistics_proxy_unittest.cc | 35 ++- webrtc/video/video_send_stream.cc | 4 +- webrtc/webrtc_tests.gypi | 1 + 40 files changed, 604 insertions(+), 616 deletions(-) create mode 100644 webrtc/base/rate_limiter.cc create mode 100644 webrtc/base/rate_limiter.h create mode 100644 webrtc/base/rate_limiter_unittest.cc delete mode 100644 webrtc/modules/rtp_rtcp/source/bitrate.cc delete mode 100644 webrtc/modules/rtp_rtcp/source/bitrate.h diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index f151f108e7..bba273db1b 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -442,6 +442,7 @@ if (rtc_include_tests) { "base/proxy_unittest.cc", "base/proxydetect_unittest.cc", "base/random_unittest.cc", + "base/rate_limiter_unittest.cc", "base/rate_statistics_unittest.cc", "base/ratelimiter_unittest.cc", "base/ratetracker_unittest.cc", diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index cd4bc1fe43..c4d5c15e0e 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -141,6 +141,8 @@ static_library("rtc_base_approved") { "race_checker.h", "random.cc", "random.h", + "rate_limiter.cc", + "rate_limiter.h", "rate_statistics.cc", "rate_statistics.h", "ratetracker.cc", diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 93be3bc706..be9218e26e 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -74,6 +74,8 @@ 'random.h', 'rate_statistics.cc', 'rate_statistics.h', + 'rate_limiter.cc', + 'rate_limiter.h', 'ratetracker.cc', 'ratetracker.h', 'refcount.h', diff --git a/webrtc/base/rate_limiter.cc b/webrtc/base/rate_limiter.cc new file mode 100644 index 0000000000..89bdb94e08 --- /dev/null +++ b/webrtc/base/rate_limiter.cc @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/base/rate_limiter.h" +#include "webrtc/system_wrappers/include/clock.h" + +namespace webrtc { + +RateLimiter::RateLimiter(Clock* clock, int64_t max_window_ms) + : clock_(clock), + current_rate_(max_window_ms, RateStatistics::kBpsScale), + window_size_ms_(max_window_ms), + max_rate_bps_(std::numeric_limits::max()) {} + +RateLimiter::~RateLimiter() {} + +// Usage note: This class is intended be usable in a scenario where different +// threads may call each of the the different method. For instance, a network +// thread trying to send data calling TryUseRate(), the bandwidth estimator +// calling SetMaxRate() and a timed maintenance thread periodically updating +// the RTT. +bool RateLimiter::TryUseRate(size_t packet_size_bytes) { + rtc::CritScope cs(&lock_); + int64_t now_ms = clock_->TimeInMilliseconds(); + rtc::Optional current_rate = current_rate_.Rate(now_ms); + if (current_rate) { + // If there is a current rate, check if adding bytes would cause maximum + // bitrate target to be exceeded. If there is NOT a valid current rate, + // allow allocating rate even if target is exceeded. This prevents + // problems + // at very low rates, where for instance retransmissions would never be + // allowed due to too high bitrate caused by a single packet. + + size_t bitrate_addition_bps = + (packet_size_bytes * 8 * 1000) / window_size_ms_; + if (*current_rate + bitrate_addition_bps > max_rate_bps_) + return false; + } + + current_rate_.Update(packet_size_bytes, now_ms); + return true; +} + +void RateLimiter::SetMaxRate(uint32_t max_rate_bps) { + rtc::CritScope cs(&lock_); + max_rate_bps_ = max_rate_bps; +} + +// Set the window size over which to measure the current bitrate. +// For retransmissions, this is typically the RTT. +bool RateLimiter::SetWindowSize(int64_t window_size_ms) { + rtc::CritScope cs(&lock_); + window_size_ms_ = window_size_ms; + return current_rate_.SetWindowSize(window_size_ms, + clock_->TimeInMilliseconds()); +} + +} // namespace webrtc diff --git a/webrtc/base/rate_limiter.h b/webrtc/base/rate_limiter.h new file mode 100644 index 0000000000..5809fc125a --- /dev/null +++ b/webrtc/base/rate_limiter.h @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_BASE_RATE_LIMITER_H_ +#define WEBRTC_BASE_RATE_LIMITER_H_ + +#include + +#include "webrtc/base/constructormagic.h" +#include "webrtc/base/criticalsection.h" +#include "webrtc/base/rate_statistics.h" + +namespace webrtc { + +class Clock; + +// Class used to limit a bitrate, making sure the average does not exceed a +// maximum as measured over a sliding window. This class is thread safe; all +// methods will acquire (the same) lock befeore executing. +class RateLimiter { + public: + RateLimiter(Clock* clock, int64_t max_window_ms); + ~RateLimiter(); + + // Try to use rate to send bytes. Returns true on success and if so updates + // current rate. + bool TryUseRate(size_t packet_size_bytes); + + // Set the maximum bitrate, in bps, that this limiter allows to send. + void SetMaxRate(uint32_t max_rate_bps); + + // Set the window size over which to measure the current bitrate. + // For example, irt retransmissions, this is typically the RTT. + // Returns true on success and false if window_size_ms is out of range. + bool SetWindowSize(int64_t window_size_ms); + + private: + Clock* const clock_; + rtc::CriticalSection lock_; + RateStatistics current_rate_ GUARDED_BY(lock_); + int64_t window_size_ms_ GUARDED_BY(lock_); + uint32_t max_rate_bps_ GUARDED_BY(lock_); + + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RateLimiter); +}; + +} // namespace webrtc + +#endif // WEBRTC_BASE_RATE_LIMITER_H_ diff --git a/webrtc/base/rate_limiter_unittest.cc b/webrtc/base/rate_limiter_unittest.cc new file mode 100644 index 0000000000..d441128c98 --- /dev/null +++ b/webrtc/base/rate_limiter_unittest.cc @@ -0,0 +1,205 @@ +/* + * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include +#include + +#include "testing/gtest/include/gtest/gtest.h" + +#include "webrtc/base/event.h" +#include "webrtc/base/platform_thread.h" +#include "webrtc/base/rate_limiter.h" +#include "webrtc/base/task_queue.h" +#include "webrtc/system_wrappers/include/clock.h" + +namespace webrtc { + +class RateLimitTest : public ::testing::Test { + public: + RateLimitTest() + : clock_(0), rate_limiter(new RateLimiter(&clock_, kWindowSizeMs)) {} + virtual ~RateLimitTest() {} + + void SetUp() override { rate_limiter->SetMaxRate(kMaxRateBps); } + + protected: + static constexpr int64_t kWindowSizeMs = 1000; + static constexpr uint32_t kMaxRateBps = 100000; + // Bytes needed to completely saturate the rate limiter. + static constexpr size_t kRateFillingBytes = + (kMaxRateBps * kWindowSizeMs) / (8 * 1000); + SimulatedClock clock_; + std::unique_ptr rate_limiter; +}; + +TEST_F(RateLimitTest, IncreasingMaxRate) { + // Fill rate, extend window to full size. + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2)); + clock_.AdvanceTimeMilliseconds(kWindowSizeMs - 1); + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2)); + + // All rate consumed. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); + + // Double the available rate and fill that too. + rate_limiter->SetMaxRate(kMaxRateBps * 2); + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes)); + + // All rate consumed again. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); +} + +TEST_F(RateLimitTest, DecreasingMaxRate) { + // Fill rate, extend window to full size. + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2)); + clock_.AdvanceTimeMilliseconds(kWindowSizeMs - 1); + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2)); + + // All rate consumed. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); + + // Halve the available rate and move window so half of the data falls out. + rate_limiter->SetMaxRate(kMaxRateBps / 2); + clock_.AdvanceTimeMilliseconds(1); + + // All rate still consumed. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); +} + +TEST_F(RateLimitTest, ChangingWindowSize) { + // Fill rate, extend window to full size. + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2)); + clock_.AdvanceTimeMilliseconds(kWindowSizeMs - 1); + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2)); + + // All rate consumed. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); + + // Decrease window size so half of the data falls out. + rate_limiter->SetWindowSize(kWindowSizeMs / 2); + // Average rate should still be the same, so rate is still all consumed. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); + + // Increase window size again. Now the rate is only half used (removed data + // points don't come back to life). + rate_limiter->SetWindowSize(kWindowSizeMs); + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2)); + + // All rate consumed again. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); +} + +TEST_F(RateLimitTest, SingleUsageAlwaysOk) { + // Using more bytes than can fit in a window is OK for a single packet. + EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes + 1)); +} + +TEST_F(RateLimitTest, WindowSizeLimits) { + EXPECT_TRUE(rate_limiter->SetWindowSize(1)); + EXPECT_FALSE(rate_limiter->SetWindowSize(0)); + EXPECT_TRUE(rate_limiter->SetWindowSize(kWindowSizeMs)); + EXPECT_FALSE(rate_limiter->SetWindowSize(kWindowSizeMs + 1)); +} + +static const int64_t kMaxTimeoutMs = 30000; + +class ThreadTask { + public: + explicit ThreadTask(RateLimiter* rate_limiter) + : rate_limiter_(rate_limiter), + start_signal_(false, false), + end_signal_(false, false) {} + virtual ~ThreadTask() {} + + void Run() { + start_signal_.Wait(kMaxTimeoutMs); + DoRun(); + end_signal_.Set(); + } + + virtual void DoRun() = 0; + + RateLimiter* const rate_limiter_; + rtc::Event start_signal_; + rtc::Event end_signal_; +}; + +bool RunTask(void* thread_task) { + reinterpret_cast(thread_task)->Run(); + return false; +} + +TEST_F(RateLimitTest, MultiThreadedUsage) { + // Simple sanity test, with different threads calling the various methods. + // Runs a few simple tasks, each on its own thread, but coordinated with + // events so that they run in a serialized order. Intended to catch data + // races when run with tsan et al. + + // Half window size, double rate -> same amount of bytes needed to fill rate. + + class SetWindowSizeTask : public ThreadTask { + public: + explicit SetWindowSizeTask(RateLimiter* rate_limiter) + : ThreadTask(rate_limiter) {} + virtual ~SetWindowSizeTask() {} + + void DoRun() override { + EXPECT_TRUE(rate_limiter_->SetWindowSize(kWindowSizeMs / 2)); + } + } set_window_size_task(rate_limiter.get()); + rtc::PlatformThread thread1(RunTask, &set_window_size_task, "Thread1"); + thread1.Start(); + + class SetMaxRateTask : public ThreadTask { + public: + explicit SetMaxRateTask(RateLimiter* rate_limiter) + : ThreadTask(rate_limiter) {} + virtual ~SetMaxRateTask() {} + + void DoRun() override { rate_limiter_->SetMaxRate(kMaxRateBps * 2); } + } set_max_rate_task(rate_limiter.get()); + rtc::PlatformThread thread2(RunTask, &set_max_rate_task, "Thread2"); + thread2.Start(); + + class UseRateTask : public ThreadTask { + public: + UseRateTask(RateLimiter* rate_limiter, SimulatedClock* clock) + : ThreadTask(rate_limiter), clock_(clock) {} + virtual ~UseRateTask() {} + + void DoRun() override { + EXPECT_TRUE(rate_limiter_->TryUseRate(kRateFillingBytes / 2)); + clock_->AdvanceTimeMilliseconds((kWindowSizeMs / 2) - 1); + EXPECT_TRUE(rate_limiter_->TryUseRate(kRateFillingBytes / 2)); + } + + SimulatedClock* const clock_; + } use_rate_task(rate_limiter.get(), &clock_); + rtc::PlatformThread thread3(RunTask, &use_rate_task, "Thread3"); + thread3.Start(); + + set_window_size_task.start_signal_.Set(); + EXPECT_TRUE(set_window_size_task.end_signal_.Wait(kMaxTimeoutMs)); + + set_max_rate_task.start_signal_.Set(); + EXPECT_TRUE(set_max_rate_task.end_signal_.Wait(kMaxTimeoutMs)); + + use_rate_task.start_signal_.Set(); + EXPECT_TRUE(use_rate_task.end_signal_.Wait(kMaxTimeoutMs)); + + // All rate consumed. + EXPECT_FALSE(rate_limiter->TryUseRate(1)); + + thread1.Stop(); + thread2.Stop(); + thread3.Stop(); +} + +} // namespace webrtc diff --git a/webrtc/base/rate_statistics.cc b/webrtc/base/rate_statistics.cc index 1fd63cc6d2..3122dbb3e6 100644 --- a/webrtc/base/rate_statistics.cc +++ b/webrtc/base/rate_statistics.cc @@ -61,8 +61,10 @@ void RateStatistics::Update(size_t count, int64_t now_ms) { ++num_samples_; } -rtc::Optional RateStatistics::Rate(int64_t now_ms) { - EraseOld(now_ms); +rtc::Optional RateStatistics::Rate(int64_t now_ms) const { + // Yeah, this const_cast ain't pretty, but the alternative is to declare most + // of the members as mutable... + const_cast(this)->EraseOld(now_ms); // If window is a single bucket or there is only one sample in a data set that // has not grown to the full window size, treat this as rate unavailable. @@ -112,7 +114,7 @@ bool RateStatistics::SetWindowSize(int64_t window_size_ms, int64_t now_ms) { return true; } -bool RateStatistics::IsInitialized() { +bool RateStatistics::IsInitialized() const { return oldest_time_ != -max_window_size_ms_; } diff --git a/webrtc/base/rate_statistics.h b/webrtc/base/rate_statistics.h index 3e913cc1bb..8a90a46a84 100644 --- a/webrtc/base/rate_statistics.h +++ b/webrtc/base/rate_statistics.h @@ -20,22 +20,37 @@ namespace webrtc { class RateStatistics { public: + static constexpr float kBpsScale = 8000.0f; + // max_window_size_ms = Maximum window size in ms for the rate estimation. // Initial window size is set to this, but may be changed // to something lower by calling SetWindowSize(). - // scale = coefficient to convert counts/ms to desired units, - // ex: if counts represents bytes, use 8*1000 to go to bits/s + // scale = coefficient to convert counts/ms to desired unit + // ex: kBpsScale (8000) for bits/s if count represents bytes. RateStatistics(int64_t max_window_size_ms, float scale); ~RateStatistics(); + // Reset instance to original state. void Reset(); + + // Update rate with a new data point, moving averaging window as needed. void Update(size_t count, int64_t now_ms); - rtc::Optional Rate(int64_t now_ms); + + // Note that despite this being a const method, it still updates the internal + // state (moves averaging window), but it doesn't make any alterations that + // are observable from the other methods, as long as supplied timestamps are + // from a monotonic clock. Ie, it doesn't matter if this call moves the + // window, since any subsequent call to Update or Rate would still have moved + // the window as much or more. + rtc::Optional Rate(int64_t now_ms) const; + + // Update the size of the averaging window. The maximum allowed value for + // window_size_ms is max_window_size_ms as supplied in the constructor. bool SetWindowSize(int64_t window_size_ms, int64_t now_ms); private: void EraseOld(int64_t now_ms); - bool IsInitialized(); + bool IsInitialized() const; // Counters are kept in buckets (circular buffer), with one bucket // per millisecond. diff --git a/webrtc/call/rtc_event_log_unittest.cc b/webrtc/call/rtc_event_log_unittest.cc index 82f23a97ed..2d583a928e 100644 --- a/webrtc/call/rtc_event_log_unittest.cc +++ b/webrtc/call/rtc_event_log_unittest.cc @@ -122,7 +122,8 @@ size_t GenerateRtpPacket(uint32_t extensions_bitvector, nullptr, // FrameCountObserver* nullptr, // SendSideDelayObserver* nullptr, // RtcEventLog* - nullptr); // SendPacketObserver* + nullptr, // SendPacketObserver* + nullptr); // NackRateLimiter* std::vector csrcs; for (unsigned i = 0; i < csrcs_count; i++) { diff --git a/webrtc/common_types.h b/webrtc/common_types.h index 13d0c3f808..bb29af0beb 100644 --- a/webrtc/common_types.h +++ b/webrtc/common_types.h @@ -252,11 +252,10 @@ class RtcpPacketTypeCounterObserver { // Rate statistics for a stream. struct BitrateStatistics { - BitrateStatistics() : bitrate_bps(0), packet_rate(0), timestamp_ms(0) {} + BitrateStatistics() : bitrate_bps(0), packet_rate(0) {} uint32_t bitrate_bps; // Bitrate in bits per second. uint32_t packet_rate; // Packet rate in packets per second. - uint64_t timestamp_ms; // Ntp timestamp in ms at time of rate estimation. }; // Callback, used to notify an observer whenever new rates have been estimated. @@ -264,8 +263,8 @@ class BitrateStatisticsObserver { public: virtual ~BitrateStatisticsObserver() {} - virtual void Notify(const BitrateStatistics& total_stats, - const BitrateStatistics& retransmit_stats, + virtual void Notify(uint32_t total_bitrate_bps, + uint32_t retransmit_bitrate_bps, uint32_t ssrc) = 0; }; diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index 7582258185..d1ab03af0f 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -17,6 +17,7 @@ #include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" #include "webrtc/base/logging.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/base/socket.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" @@ -32,6 +33,8 @@ namespace webrtc { namespace { static const uint32_t kTimeOffsetSwitchThreshold = 30; +static const int64_t kMinRetransmitWindowSizeMs = 30; +static const int64_t kMaxRetransmitWindowSizeMs = 1000; // Makes sure that the bitrate and the min, max values are in valid range. static void ClampBitrates(int* bitrate_bps, @@ -164,6 +167,8 @@ CongestionController::CongestionController( new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), bitrate_controller_( BitrateController::CreateBitrateController(clock_, event_log)), + retransmission_rate_limiter_( + new RateLimiter(clock, kMaxRetransmitWindowSizeMs)), remote_estimator_proxy_(clock_, packet_router_.get()), transport_feedback_adapter_(bitrate_controller_.get(), clock_), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), @@ -191,6 +196,8 @@ CongestionController::CongestionController( // construction. bitrate_controller_( BitrateController::CreateBitrateController(clock_, event_log)), + retransmission_rate_limiter_( + new RateLimiter(clock, kMaxRetransmitWindowSizeMs)), remote_estimator_proxy_(clock_, packet_router_.get()), transport_feedback_adapter_(bitrate_controller_.get(), clock_), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), @@ -266,6 +273,10 @@ CongestionController::GetTransportFeedbackObserver() { return &transport_feedback_adapter_; } +RateLimiter* CongestionController::GetRetransmissionRateLimiter() { + return retransmission_rate_limiter_.get(); +} + void CongestionController::SetAllocatedSendBitrateLimits( int min_send_bitrate_bps, int max_padding_bitrate_bps) { @@ -299,6 +310,14 @@ void CongestionController::OnSentPacket(const rtc::SentPacket& sent_packet) { void CongestionController::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { remote_bitrate_estimator_->OnRttUpdate(avg_rtt_ms, max_rtt_ms); transport_feedback_adapter_.OnRttUpdate(avg_rtt_ms, max_rtt_ms); + + int64_t nack_window_size_ms = max_rtt_ms; + if (nack_window_size_ms > kMaxRetransmitWindowSizeMs) { + nack_window_size_ms = kMaxRetransmitWindowSizeMs; + } else if (nack_window_size_ms < kMinRetransmitWindowSizeMs) { + nack_window_size_ms = kMinRetransmitWindowSizeMs; + } + retransmission_rate_limiter_->SetWindowSize(nack_window_size_ms); } int64_t CongestionController::TimeUntilNextProcess() { @@ -323,8 +342,10 @@ void CongestionController::MaybeTriggerOnNetworkChanged() { int64_t rtt; bool estimate_changed = bitrate_controller_->GetNetworkParameters( &bitrate_bps, &fraction_loss, &rtt); - if (estimate_changed) + if (estimate_changed) { pacer_->SetEstimatedBitrate(bitrate_bps); + retransmission_rate_limiter_->SetMaxRate(bitrate_bps); + } bitrate_bps = IsNetworkDown() || IsSendQueueFull() ? 0 : bitrate_bps; diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index a48f0008cc..a0531cc27e 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -31,6 +31,7 @@ namespace webrtc { class BitrateController; class Clock; class ProcessThread; +class RateLimiter; class RemoteBitrateEstimator; class RemoteBitrateObserver; class RtcEventLog; @@ -80,6 +81,7 @@ class CongestionController : public CallStatsObserver, public Module { virtual PacedSender* pacer() { return pacer_.get(); } virtual PacketRouter* packet_router() { return packet_router_.get(); } virtual TransportFeedbackObserver* GetTransportFeedbackObserver(); + RateLimiter* GetRetransmissionRateLimiter(); // SetAllocatedSendBitrateLimits sets bitrates limits imposed by send codec // settings. @@ -118,6 +120,7 @@ class CongestionController : public CallStatsObserver, public Module { const std::unique_ptr pacer_; const std::unique_ptr remote_bitrate_estimator_; const std::unique_ptr bitrate_controller_; + const std::unique_ptr retransmission_rate_limiter_; RemoteEstimatorProxy remote_estimator_proxy_; TransportFeedbackAdapter transport_feedback_adapter_; int min_bitrate_bps_; diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn index 77cc175a62..39aa1507e5 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -19,8 +19,6 @@ source_set("rtp_rtcp") { "include/rtp_rtcp.h", "include/rtp_rtcp_defines.h", "mocks/mock_rtp_rtcp.h", - "source/bitrate.cc", - "source/bitrate.h", "source/byte_io.h", "source/dtmf_queue.cc", "source/dtmf_queue.h", diff --git a/webrtc/modules/rtp_rtcp/include/receive_statistics.h b/webrtc/modules/rtp_rtcp/include/receive_statistics.h index cc21e22df5..90a848ad63 100644 --- a/webrtc/modules/rtp_rtcp/include/receive_statistics.h +++ b/webrtc/modules/rtp_rtcp/include/receive_statistics.h @@ -46,7 +46,7 @@ class StreamStatistician { typedef std::map StatisticianMap; -class ReceiveStatistics : public Module { +class ReceiveStatistics { public: virtual ~ReceiveStatistics() {} @@ -89,8 +89,6 @@ class NullReceiveStatistics : public ReceiveStatistics { size_t packet_length) override; StatisticianMap GetActiveStatisticians() const override; StreamStatistician* GetStatistician(uint32_t ssrc) const override; - int64_t TimeUntilNextProcess() override; - void Process() override; void SetMaxReorderingThreshold(int max_reordering_threshold) override; void RegisterRtcpStatisticsCallback( RtcpStatisticsCallback* callback) override; diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index 7c72e5917c..bfd8e65743 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -23,11 +23,12 @@ namespace webrtc { // Forward declarations. +class RateLimiter; class ReceiveStatistics; class RemoteBitrateEstimator; +class RtcEventLog; class RtpReceiver; class Transport; -class RtcEventLog; RTPExtensionType StringToRtpExtensionType(const std::string& extension); @@ -79,6 +80,7 @@ class RtpRtcp : public Module { SendSideDelayObserver* send_side_delay_observer; RtcEventLog* event_log; SendPacketObserver* send_packet_observer; + RateLimiter* retransmission_rate_limiter; RTC_DISALLOW_COPY_AND_ASSIGN(Configuration); }; @@ -615,11 +617,6 @@ class RtpRtcp : public Module { * ***************************************************************************/ - /* - * Set the target send bitrate - */ - virtual void SetTargetSendBitrate(uint32_t bitrate_bps) = 0; - /* * Turn on/off generic FEC */ diff --git a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi index 0c8477f3cb..00829049da 100644 --- a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi +++ b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi @@ -26,8 +26,6 @@ 'include/rtp_receiver.h', 'include/rtp_rtcp.h', 'include/rtp_rtcp_defines.h', - 'source/bitrate.cc', - 'source/bitrate.h', 'source/byte_io.h', 'source/fec_receiver_impl.cc', 'source/fec_receiver_impl.h', diff --git a/webrtc/modules/rtp_rtcp/source/bitrate.cc b/webrtc/modules/rtp_rtcp/source/bitrate.cc deleted file mode 100644 index 49a23592bf..0000000000 --- a/webrtc/modules/rtp_rtcp/source/bitrate.cc +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "webrtc/modules/rtp_rtcp/source/bitrate.h" - -#include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" - -namespace webrtc { - -Bitrate::Bitrate(Clock* clock, Observer* observer) - : clock_(clock), - packet_rate_(0), - bitrate_(0), - bitrate_next_idx_(0), - time_last_rate_update_(0), - bytes_count_(0), - packet_count_(0), - observer_(observer) { - memset(packet_rate_array_, 0, sizeof(packet_rate_array_)); - memset(bitrate_diff_ms_, 0, sizeof(bitrate_diff_ms_)); - memset(bitrate_array_, 0, sizeof(bitrate_array_)); -} - -Bitrate::~Bitrate() {} - -void Bitrate::Update(const size_t bytes) { - rtc::CritScope cs(&crit_); - bytes_count_ += bytes; - packet_count_++; -} - -uint32_t Bitrate::PacketRate() const { - rtc::CritScope cs(&crit_); - return packet_rate_; -} - -uint32_t Bitrate::BitrateLast() const { - rtc::CritScope cs(&crit_); - return bitrate_; -} - -uint32_t Bitrate::BitrateNow() const { - rtc::CritScope cs(&crit_); - int64_t now = clock_->TimeInMilliseconds(); - int64_t diff_ms = now - time_last_rate_update_; - - if (diff_ms > 10000) { // 10 seconds. - // Too high difference, ignore. - return bitrate_; - } - int64_t bits_since_last_rate_update = 8 * bytes_count_ * 1000; - - // We have to consider the time when the measurement was done: - // ((bits/sec * sec) + (bits)) / sec. - int64_t bitrate = (static_cast(bitrate_) * 1000 + - bits_since_last_rate_update) / (1000 + diff_ms); - return static_cast(bitrate); -} - -int64_t Bitrate::time_last_rate_update() const { - rtc::CritScope cs(&crit_); - return time_last_rate_update_; -} - -// Triggered by timer. -void Bitrate::Process() { - BitrateStatistics stats; - { - rtc::CritScope cs(&crit_); - int64_t now = clock_->CurrentNtpInMilliseconds(); - int64_t diff_ms = now - time_last_rate_update_; - - if (diff_ms < 100) { - // Not enough data, wait... - return; - } - if (diff_ms > 10000) { // 10 seconds. - // Too high difference, ignore. - time_last_rate_update_ = now; - bytes_count_ = 0; - packet_count_ = 0; - return; - } - packet_rate_array_[bitrate_next_idx_] = (packet_count_ * 1000) / diff_ms; - bitrate_array_[bitrate_next_idx_] = 8 * ((bytes_count_ * 1000) / diff_ms); - bitrate_diff_ms_[bitrate_next_idx_] = diff_ms; - bitrate_next_idx_++; - if (bitrate_next_idx_ >= 10) { - bitrate_next_idx_ = 0; - } - int64_t sum_diffMS = 0; - int64_t sum_bitrateMS = 0; - int64_t sum_packetrateMS = 0; - for (int i = 0; i < 10; i++) { - sum_diffMS += bitrate_diff_ms_[i]; - sum_bitrateMS += bitrate_array_[i] * bitrate_diff_ms_[i]; - sum_packetrateMS += packet_rate_array_[i] * bitrate_diff_ms_[i]; - } - time_last_rate_update_ = now; - bytes_count_ = 0; - packet_count_ = 0; - packet_rate_ = static_cast(sum_packetrateMS / sum_diffMS); - bitrate_ = static_cast(sum_bitrateMS / sum_diffMS); - - stats.bitrate_bps = bitrate_; - stats.packet_rate = packet_rate_; - stats.timestamp_ms = now; - } - - if (observer_) - observer_->BitrateUpdated(stats); -} - -} // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/bitrate.h b/webrtc/modules/rtp_rtcp/source/bitrate.h deleted file mode 100644 index 7aaaead42d..0000000000 --- a/webrtc/modules/rtp_rtcp/source/bitrate.h +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_BITRATE_H_ -#define WEBRTC_MODULES_RTP_RTCP_SOURCE_BITRATE_H_ - -#include - -#include - -#include "webrtc/base/criticalsection.h" -#include "webrtc/common_types.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h" -#include "webrtc/typedefs.h" - -namespace webrtc { - -class Clock; - -class Bitrate { - public: - class Observer; - Bitrate(Clock* clock, Observer* observer); - virtual ~Bitrate(); - - // Calculates rates. - void Process(); - - // Update with a packet. - void Update(const size_t bytes); - - // Packet rate last second, updated roughly every 100 ms. - uint32_t PacketRate() const; - - // Bitrate last second, updated roughly every 100 ms. - uint32_t BitrateLast() const; - - // Bitrate last second, updated now. - uint32_t BitrateNow() const; - - int64_t time_last_rate_update() const; - - class Observer { - public: - Observer() {} - virtual ~Observer() {} - - virtual void BitrateUpdated(const BitrateStatistics& stats) = 0; - }; - - protected: - Clock* clock_; - - private: - rtc::CriticalSection crit_; - uint32_t packet_rate_; - uint32_t bitrate_; - uint8_t bitrate_next_idx_; - int64_t packet_rate_array_[10]; - int64_t bitrate_array_[10]; - int64_t bitrate_diff_ms_[10]; - int64_t time_last_rate_update_; - size_t bytes_count_; - uint32_t packet_count_; - Observer* const observer_; -}; - -} // namespace webrtc - -#endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_BITRATE_H_ diff --git a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc index f8d9243858..b12c08e642 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -15,6 +15,7 @@ #include #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/receive_statistics.h" #include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" @@ -34,6 +35,7 @@ const int kTestNumberOfRtxPackets = 149; const int kNumFrames = 30; const int kPayloadType = 123; const int kRtxPayloadType = 98; +const int64_t kMaxRttMs = 1000; class VerifyingRtxReceiver : public NullRtpData { public: @@ -168,11 +170,12 @@ class RtpRtcpRtxNackTest : public ::testing::Test { protected: RtpRtcpRtxNackTest() : rtp_payload_registry_(RTPPayloadStrategy::CreateStrategy(false)), - rtp_rtcp_module_(NULL), + rtp_rtcp_module_(nullptr), transport_(kTestSsrc + 1), receiver_(), payload_data_length(sizeof(payload_data)), - fake_clock(123456) {} + fake_clock(123456), + retranmission_rate_limiter_(&fake_clock, kMaxRttMs) {} ~RtpRtcpRtxNackTest() {} void SetUp() override { @@ -182,6 +185,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { receive_statistics_.reset(ReceiveStatistics::Create(&fake_clock)); configuration.receive_statistics = receive_statistics_.get(); configuration.outgoing_transport = &transport_; + configuration.retransmission_rate_limiter = &retranmission_rate_limiter_; rtp_rtcp_module_ = RtpRtcp::CreateRtpRtcp(configuration); rtp_feedback_.reset(new TestRtpFeedback(rtp_rtcp_module_)); @@ -288,6 +292,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { uint8_t payload_data[65000]; size_t payload_data_length; SimulatedClock fake_clock; + RateLimiter retranmission_rate_limiter_; }; TEST_F(RtpRtcpRtxNackTest, LongNackList) { diff --git a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc b/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc index 932be1bb9e..4ec11b6345 100644 --- a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -14,7 +14,7 @@ #include -#include "webrtc/modules/rtp_rtcp/source/bitrate.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "webrtc/modules/rtp_rtcp/source/time_util.h" namespace webrtc { @@ -29,7 +29,8 @@ StreamStatisticianImpl::StreamStatisticianImpl( RtcpStatisticsCallback* rtcp_callback, StreamDataCountersCallback* rtp_callback) : clock_(clock), - incoming_bitrate_(clock, NULL), + incoming_bitrate_(kStatisticsProcessIntervalMs, + RateStatistics::kBpsScale), ssrc_(0), max_reordering_threshold_(kDefaultMaxReorderingThreshold), jitter_q4_(0), @@ -61,7 +62,7 @@ void StreamStatisticianImpl::UpdateCounters(const RTPHeader& header, rtc::CritScope cs(&stream_lock_); bool in_order = InOrderPacketInternal(header.sequenceNumber); ssrc_ = header.ssrc; - incoming_bitrate_.Update(packet_length); + incoming_bitrate_.Update(packet_length, clock_->TimeInMilliseconds()); receive_counters_.transmitted.AddPacket(packet_length, header); if (!in_order && retransmitted) { receive_counters_.retransmitted.AddPacket(packet_length, header); @@ -300,12 +301,7 @@ void StreamStatisticianImpl::GetReceiveStreamDataCounters( uint32_t StreamStatisticianImpl::BitrateReceived() const { rtc::CritScope cs(&stream_lock_); - return incoming_bitrate_.BitrateNow(); -} - -void StreamStatisticianImpl::ProcessBitrate() { - rtc::CritScope cs(&stream_lock_); - incoming_bitrate_.Process(); + return incoming_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0); } void StreamStatisticianImpl::LastReceiveTimeNtp(uint32_t* secs, @@ -376,7 +372,6 @@ ReceiveStatistics* ReceiveStatistics::Create(Clock* clock) { ReceiveStatisticsImpl::ReceiveStatisticsImpl(Clock* clock) : clock_(clock), - last_rate_update_ms_(0), rtcp_stats_callback_(NULL), rtp_stats_callback_(NULL) {} @@ -452,23 +447,6 @@ void ReceiveStatisticsImpl::SetMaxReorderingThreshold( } } -void ReceiveStatisticsImpl::Process() { - rtc::CritScope cs(&receive_statistics_lock_); - for (StatisticianImplMap::iterator it = statisticians_.begin(); - it != statisticians_.end(); ++it) { - it->second->ProcessBitrate(); - } - last_rate_update_ms_ = clock_->TimeInMilliseconds(); -} - -int64_t ReceiveStatisticsImpl::TimeUntilNextProcess() { - rtc::CritScope cs(&receive_statistics_lock_); - int64_t time_since_last_update = clock_->TimeInMilliseconds() - - last_rate_update_ms_; - return std::max( - kStatisticsProcessIntervalMs - time_since_last_update, 0); -} - void ReceiveStatisticsImpl::RegisterRtcpStatisticsCallback( RtcpStatisticsCallback* callback) { rtc::CritScope cs(&receive_statistics_lock_); @@ -525,10 +503,6 @@ StreamStatistician* NullReceiveStatistics::GetStatistician( void NullReceiveStatistics::SetMaxReorderingThreshold( int max_reordering_threshold) {} -int64_t NullReceiveStatistics::TimeUntilNextProcess() { return 0; } - -void NullReceiveStatistics::Process() {} - void NullReceiveStatistics::RegisterRtcpStatisticsCallback( RtcpStatisticsCallback* callback) {} diff --git a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h b/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h index 39679673d0..913f3b5041 100644 --- a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -17,7 +17,7 @@ #include #include "webrtc/base/criticalsection.h" -#include "webrtc/modules/rtp_rtcp/source/bitrate.h" +#include "webrtc/base/rate_statistics.h" #include "webrtc/system_wrappers/include/ntp_time.h" namespace webrtc { @@ -44,7 +44,6 @@ class StreamStatisticianImpl : public StreamStatistician { bool retransmitted); void FecPacketReceived(const RTPHeader& header, size_t packet_length); void SetMaxReorderingThreshold(int max_reordering_threshold); - void ProcessBitrate(); virtual void LastReceiveTimeNtp(uint32_t* secs, uint32_t* frac) const; private: @@ -57,9 +56,9 @@ class StreamStatisticianImpl : public StreamStatistician { void NotifyRtpCallback() LOCKS_EXCLUDED(stream_lock_); void NotifyRtcpCallback() LOCKS_EXCLUDED(stream_lock_); - Clock* clock_; + Clock* const clock_; rtc::CriticalSection stream_lock_; - Bitrate incoming_bitrate_; + RateStatistics incoming_bitrate_; uint32_t ssrc_; int max_reordering_threshold_; // In number of packets or sequence numbers. @@ -108,10 +107,6 @@ class ReceiveStatisticsImpl : public ReceiveStatistics, StreamStatistician* GetStatistician(uint32_t ssrc) const override; void SetMaxReorderingThreshold(int max_reordering_threshold) override; - // Implement Module. - void Process() override; - int64_t TimeUntilNextProcess() override; - void RegisterRtcpStatisticsCallback( RtcpStatisticsCallback* callback) override; @@ -127,9 +122,8 @@ class ReceiveStatisticsImpl : public ReceiveStatistics, typedef std::map StatisticianImplMap; - Clock* clock_; + Clock* const clock_; rtc::CriticalSection receive_statistics_lock_; - int64_t last_rate_update_ms_; StatisticianImplMap statisticians_; RtcpStatisticsCallback* rtcp_stats_callback_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h index 486eced364..a8aaf5da18 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h @@ -13,7 +13,6 @@ #include "webrtc/base/onetimeevent.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "webrtc/modules/rtp_rtcp/source/bitrate.h" #include "webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" #include "webrtc/typedefs.h" diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index be8ab34a27..dbd919d056 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -63,7 +63,8 @@ RtpRtcp::Configuration::Configuration() send_frame_count_observer(nullptr), send_side_delay_observer(nullptr), event_log(nullptr), - send_packet_observer(nullptr) {} + send_packet_observer(nullptr), + retransmission_rate_limiter(nullptr) {} RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) { if (configuration.clock) { @@ -89,7 +90,8 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) configuration.send_frame_count_observer, configuration.send_side_delay_observer, configuration.event_log, - configuration.send_packet_observer), + configuration.send_packet_observer, + configuration.retransmission_rate_limiter), rtcp_sender_(configuration.audio, configuration.clock, configuration.receive_statistics, @@ -820,10 +822,6 @@ int32_t ModuleRtpRtcpImpl::SendREDPayloadType(int8_t* payload_type) const { return rtp_sender_.RED(payload_type); } -void ModuleRtpRtcpImpl::SetTargetSendBitrate(uint32_t bitrate_bps) { - rtp_sender_.SetTargetBitrate(bitrate_bps); -} - int32_t ModuleRtpRtcpImpl::SetKeyFrameRequestMethod( const KeyFrameRequestMethod method) { key_frame_req_method_ = method; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 369cdca0b2..ff3f01a21d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -285,8 +285,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp { // Send a request for a keyframe. int32_t RequestKeyFrame() override; - void SetTargetSendBitrate(uint32_t bitrate_bps) override; - void SetGenericFECStatus(bool enable, uint8_t payload_type_red, uint8_t payload_type_fec) override; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 98269cfb84..1e2cc61fca 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -15,6 +15,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -37,6 +38,7 @@ const int64_t kOneWayNetworkDelayMs = 100; const uint8_t kBaseLayerTid = 0; const uint8_t kHigherLayerTid = 1; const uint16_t kSequenceNumber = 100; +const int64_t kMaxRttMs = 1000; class RtcpRttStatsTestImpl : public RtcpRttStats { public: @@ -99,7 +101,9 @@ class SendTransport : public Transport, class RtpRtcpModule : public RtcpPacketTypeCounterObserver { public: explicit RtpRtcpModule(SimulatedClock* clock) - : receive_statistics_(ReceiveStatistics::Create(clock)) { + : receive_statistics_(ReceiveStatistics::Create(clock)), + remote_ssrc_(0), + retransmission_rate_limiter_(clock, kMaxRttMs) { RtpRtcp::Configuration config; config.audio = false; config.clock = clock; @@ -107,6 +111,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver { config.receive_statistics = receive_statistics_.get(); config.rtcp_packet_type_counter_observer = this; config.rtt_stats = &rtt_stats_; + config.retransmission_rate_limiter = &retransmission_rate_limiter_; impl_.reset(new ModuleRtpRtcpImpl(config)); impl_->SetRTCPStatus(RtcpMode::kCompound); @@ -121,6 +126,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver { RtcpRttStatsTestImpl rtt_stats_; std::unique_ptr impl_; uint32_t remote_ssrc_; + RateLimiter retransmission_rate_limiter_; void SetRemoteSsrc(uint32_t ssrc) { remote_ssrc_ = ssrc; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 4ee2524abc..f62fcc3dfb 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -16,6 +16,7 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/base/trace_event.h" #include "webrtc/base/timeutils.h" #include "webrtc/call.h" @@ -33,6 +34,7 @@ namespace webrtc { static const size_t kMaxPaddingLength = 224; static const int kSendSideDelayWindowMs = 1000; static const uint32_t kAbsSendTimeFraction = 18; +static const int kBitrateStatisticsWindowMs = 1000; namespace { @@ -63,47 +65,6 @@ uint32_t ConvertMsTo24Bits(int64_t time_ms) { } } // namespace -RTPSender::BitrateAggregator::BitrateAggregator( - BitrateStatisticsObserver* bitrate_callback) - : callback_(bitrate_callback), - total_bitrate_observer_(*this), - retransmit_bitrate_observer_(*this), - ssrc_(0) {} - -void RTPSender::BitrateAggregator::OnStatsUpdated() const { - if (callback_) { - callback_->Notify(total_bitrate_observer_.statistics(), - retransmit_bitrate_observer_.statistics(), ssrc_); - } -} - -Bitrate::Observer* RTPSender::BitrateAggregator::total_bitrate_observer() { - return &total_bitrate_observer_; -} -Bitrate::Observer* RTPSender::BitrateAggregator::retransmit_bitrate_observer() { - return &retransmit_bitrate_observer_; -} - -void RTPSender::BitrateAggregator::set_ssrc(uint32_t ssrc) { - ssrc_ = ssrc; -} - -RTPSender::BitrateAggregator::BitrateObserver::BitrateObserver( - const BitrateAggregator& aggregator) - : aggregator_(aggregator) {} - -// Implements Bitrate::Observer. -void RTPSender::BitrateAggregator::BitrateObserver::BitrateUpdated( - const BitrateStatistics& stats) { - statistics_ = stats; - aggregator_.OnStatsUpdated(); -} - -const BitrateStatistics& -RTPSender::BitrateAggregator::BitrateObserver::statistics() const { - return statistics_; -} - RTPSender::RTPSender( bool audio, Clock* clock, @@ -115,13 +76,12 @@ RTPSender::RTPSender( FrameCountObserver* frame_count_observer, SendSideDelayObserver* send_side_delay_observer, RtcEventLog* event_log, - SendPacketObserver* send_packet_observer) + SendPacketObserver* send_packet_observer, + RateLimiter* retransmission_rate_limiter) : clock_(clock), // TODO(holmer): Remove this conversion? clock_delta_ms_(clock_->TimeInMilliseconds() - rtc::TimeMillis()), random_(clock_->TimeInMicroseconds()), - bitrates_(bitrate_callback), - total_bitrate_sent_(clock, bitrates_.total_bitrate_observer()), audio_configured_(audio), audio_(audio ? new RTPSenderAudio(clock, this) : nullptr), video_(audio ? nullptr : new RTPSenderVideo(clock, this)), @@ -140,18 +100,18 @@ RTPSender::RTPSender( rotation_(kVideoRotation_0), video_rotation_active_(false), transport_sequence_number_(0), - // NACK. - nack_byte_count_times_(), - nack_byte_count_(), - nack_bitrate_(clock, bitrates_.retransmit_bitrate_observer()), playout_delay_active_(false), packet_history_(clock), // Statistics - rtp_stats_callback_(NULL), + rtp_stats_callback_(nullptr), + total_bitrate_sent_(kBitrateStatisticsWindowMs, + RateStatistics::kBpsScale), + nack_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale), frame_count_observer_(frame_count_observer), send_side_delay_observer_(send_side_delay_observer), event_log_(event_log), send_packet_observer_(send_packet_observer), + bitrate_callback_(bitrate_callback), // RTP variables start_timestamp_forced_(false), start_timestamp_(0), @@ -166,9 +126,7 @@ RTPSender::RTPSender( last_packet_marker_bit_(false), csrcs_(), rtx_(kRtxOff), - target_bitrate_(0) { - memset(nack_byte_count_times_, 0, sizeof(nack_byte_count_times_)); - memset(nack_byte_count_, 0, sizeof(nack_byte_count_)); + retransmission_rate_limiter_(retransmission_rate_limiter) { // We need to seed the random generator for BuildPaddingPacket() below. // TODO(holmer,tommi): Note that TimeInMilliseconds might return 0 on Mac // early on in the process. @@ -178,7 +136,6 @@ RTPSender::RTPSender( ssrc_rtx_ = ssrc_db_->CreateSSRC(); RTC_DCHECK(ssrc_rtx_ != 0); - bitrates_.set_ssrc(ssrc_); // Random start, 16 bits. Can't be 0. sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber); sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); @@ -209,17 +166,14 @@ RTPSender::~RTPSender() { } void RTPSender::SetTargetBitrate(uint32_t bitrate) { - rtc::CritScope cs(&target_bitrate_critsect_); - target_bitrate_ = bitrate; -} - -uint32_t RTPSender::GetTargetBitrate() { - rtc::CritScope cs(&target_bitrate_critsect_); - return target_bitrate_; + // TODO(sprang): Remove this when dependencies have been updated. } uint16_t RTPSender::ActualSendBitrateKbit() const { - return (uint16_t)(total_bitrate_sent_.BitrateNow() / 1000); + rtc::CritScope cs(&statistics_crit_); + return static_cast( + total_bitrate_sent_.Rate(clock_->TimeInMilliseconds()).value_or(0) / + 1000); } uint32_t RTPSender::VideoBitrateSent() const { @@ -237,7 +191,8 @@ uint32_t RTPSender::FecOverheadRate() const { } uint32_t RTPSender::NackOverheadRate() const { - return nack_bitrate_.BitrateLast(); + rtc::CritScope cs(&statistics_crit_); + return nack_bitrate_sent_.Rate(clock_->TimeInMilliseconds()).value_or(0); } int32_t RTPSender::SetTransmissionTimeOffset(int32_t transmission_time_offset) { @@ -754,6 +709,12 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id, int64_t min_resend_time) { return 0; } + // Check if we're overusing retransmission bitrate. + // TODO(sprang): Add histograms for nack success or failure reasons. + RTC_DCHECK(retransmission_rate_limiter_); + if (!retransmission_rate_limiter_->TryUseRate(length)) + return -1; + if (paced_sender_) { RtpUtility::RtpHeaderParser rtp_parser(data_buffer, length); RTPHeader header; @@ -824,44 +785,14 @@ void RTPSender::OnReceivedNACK(const std::list& nack_sequence_numbers, TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"), "RTPSender::OnReceivedNACK", "num_seqnum", nack_sequence_numbers.size(), "avg_rtt", avg_rtt); - const int64_t now = clock_->TimeInMilliseconds(); - uint32_t bytes_re_sent = 0; - uint32_t target_bitrate = GetTargetBitrate(); - - // Enough bandwidth to send NACK? - if (!ProcessNACKBitRate(now)) { - LOG(LS_INFO) << "NACK bitrate reached. Skip sending NACK response. Target " - << target_bitrate; - return; - } - - for (std::list::const_iterator it = nack_sequence_numbers.begin(); - it != nack_sequence_numbers.end(); ++it) { - const int32_t bytes_sent = ReSendPacket(*it, 5 + avg_rtt); - if (bytes_sent > 0) { - bytes_re_sent += bytes_sent; - } else if (bytes_sent == 0) { - // The packet has previously been resent. - // Try resending next packet in the list. - continue; - } else { + for (uint16_t seq_no : nack_sequence_numbers) { + const int32_t bytes_sent = ReSendPacket(seq_no, 5 + avg_rtt); + if (bytes_sent < 0) { // Failed to send one Sequence number. Give up the rest in this nack. - LOG(LS_WARNING) << "Failed resending RTP packet " << *it + LOG(LS_WARNING) << "Failed resending RTP packet " << seq_no << ", Discard rest of packets"; break; } - // Delay bandwidth estimate (RTT * BW). - if (target_bitrate != 0 && avg_rtt) { - // kbits/s * ms = bits => bits/8 = bytes - size_t target_bytes = - (static_cast(target_bitrate / 1000) * avg_rtt) >> 3; - if (bytes_re_sent > target_bytes) { - break; // Ignore the rest of the packets in the list. - } - } - } - if (bytes_re_sent > 0) { - UpdateNACKBitRate(bytes_re_sent, now); } } @@ -870,51 +801,6 @@ void RTPSender::OnReceivedRtcpReportBlocks( playout_delay_oracle_.OnReceivedRtcpReportBlocks(report_blocks); } -bool RTPSender::ProcessNACKBitRate(uint32_t now) { - uint32_t num = 0; - size_t byte_count = 0; - const uint32_t kAvgIntervalMs = 1000; - uint32_t target_bitrate = GetTargetBitrate(); - - rtc::CritScope lock(&send_critsect_); - - if (target_bitrate == 0) { - return true; - } - for (num = 0; num < NACK_BYTECOUNT_SIZE; ++num) { - if ((now - nack_byte_count_times_[num]) > kAvgIntervalMs) { - // Don't use data older than 1sec. - break; - } else { - byte_count += nack_byte_count_[num]; - } - } - uint32_t time_interval = kAvgIntervalMs; - if (num == NACK_BYTECOUNT_SIZE) { - // More than NACK_BYTECOUNT_SIZE nack messages has been received - // during the last msg_interval. - if (nack_byte_count_times_[num - 1] <= now) { - time_interval = now - nack_byte_count_times_[num - 1]; - } - } - return (byte_count * 8) < (target_bitrate / 1000 * time_interval); -} - -void RTPSender::UpdateNACKBitRate(uint32_t bytes, int64_t now) { - rtc::CritScope lock(&send_critsect_); - if (bytes == 0) - return; - nack_bitrate_.Update(bytes); - // Save bitrate statistics. - // Shift all but first time. - for (int i = NACK_BYTECOUNT_SIZE - 2; i >= 0; i--) { - nack_byte_count_[i + 1] = nack_byte_count_[i]; - nack_byte_count_times_[i + 1] = nack_byte_count_times_[i]; - } - nack_byte_count_[0] = bytes; - nack_byte_count_times_[0] = now; -} - // Called from pacer when we can send the packet. bool RTPSender::TimeToSendPacket(uint16_t sequence_number, int64_t capture_time_ms, @@ -1009,6 +895,7 @@ void RTPSender::UpdateRtpStats(const uint8_t* buffer, StreamDataCounters* counters; // Get ssrc before taking statistics_crit_ to avoid possible deadlock. uint32_t ssrc = is_rtx ? RtxSsrc() : SSRC(); + int64_t now_ms = clock_->TimeInMilliseconds(); rtc::CritScope lock(&statistics_crit_); if (is_rtx) { @@ -1017,22 +904,23 @@ void RTPSender::UpdateRtpStats(const uint8_t* buffer, counters = &rtp_stats_; } - total_bitrate_sent_.Update(packet_length); + total_bitrate_sent_.Update(packet_length, now_ms); - if (counters->first_packet_time_ms == -1) { + if (counters->first_packet_time_ms == -1) counters->first_packet_time_ms = clock_->TimeInMilliseconds(); - } - if (IsFecPacket(buffer, header)) { + + if (IsFecPacket(buffer, header)) counters->fec.AddPacket(packet_length, header); - } + if (is_retransmit) { counters->retransmitted.AddPacket(packet_length, header); + nack_bitrate_sent_.Update(packet_length, now_ms); } + counters->transmitted.AddPacket(packet_length, header); - if (rtp_stats_callback_) { + if (rtp_stats_callback_) rtp_stats_callback_->DataCountersUpdated(*counters, ssrc); - } } bool RTPSender::IsFecPacket(const uint8_t* buffer, @@ -1180,13 +1068,18 @@ void RTPSender::UpdateOnSendPacket(int packet_id, } void RTPSender::ProcessBitrate() { - rtc::CritScope lock(&send_critsect_); - total_bitrate_sent_.Process(); - nack_bitrate_.Process(); - if (audio_configured_) { + if (!bitrate_callback_) return; + int64_t now_ms = clock_->TimeInMilliseconds(); + uint32_t ssrc; + { + rtc::CritScope lock(&send_critsect_); + ssrc = ssrc_; } - video_->ProcessBitrate(); + + rtc::CritScope lock(&statistics_crit_); + bitrate_callback_->Notify(total_bitrate_sent_.Rate(now_ms).value_or(0), + nack_bitrate_sent_.Rate(now_ms).value_or(0), ssrc); } size_t RTPSender::RtpHeaderLength() const { @@ -1746,7 +1639,6 @@ void RTPSender::SetSendingStatus(bool enabled) { ssrc_db_->ReturnSSRC(ssrc_); ssrc_ = ssrc_db_->CreateSSRC(); RTC_DCHECK(ssrc_ != 0); - bitrates_.set_ssrc(ssrc_); } // Don't initialize seq number if SSRC passed externally. if (!sequence_number_forced_ && !ssrc_forced_) { @@ -1797,7 +1689,6 @@ uint32_t RTPSender::GenerateNewSSRC() { } ssrc_ = ssrc_db_->CreateSSRC(); RTC_DCHECK(ssrc_ != 0); - bitrates_.set_ssrc(ssrc_); return ssrc_; } @@ -1812,7 +1703,6 @@ void RTPSender::SetSSRC(uint32_t ssrc) { ssrc_db_->ReturnSSRC(ssrc_); ssrc_db_->RegisterSSRC(ssrc); ssrc_ = ssrc; - bitrates_.set_ssrc(ssrc_); if (!sequence_number_forced_) { sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); } @@ -1961,7 +1851,8 @@ StreamDataCountersCallback* RTPSender::GetRtpStatisticsCallback() const { } uint32_t RTPSender::BitrateSent() const { - return total_bitrate_sent_.BitrateLast(); + rtc::CritScope cs(&statistics_crit_); + return total_bitrate_sent_.Rate(clock_->TimeInMilliseconds()).value_or(0); } void RTPSender::SetRtpState(const RtpState& rtp_state) { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index ffbcb817e7..03e7425d46 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -20,10 +20,10 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/criticalsection.h" #include "webrtc/base/random.h" +#include "webrtc/base/rate_statistics.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "webrtc/modules/rtp_rtcp/source/bitrate.h" #include "webrtc/modules/rtp_rtcp/source/playout_delay_oracle.h" #include "webrtc/modules/rtp_rtcp/source/rtp_header_extension.h" #include "webrtc/modules/rtp_rtcp/source/rtp_packet_history.h" @@ -34,6 +34,7 @@ namespace webrtc { +class RateLimiter; class RTPSenderAudio; class RTPSenderVideo; class RtcEventLog; @@ -93,7 +94,8 @@ class RTPSender : public RTPSenderInterface { FrameCountObserver* frame_count_observer, SendSideDelayObserver* send_side_delay_observer, RtcEventLog* event_log, - SendPacketObserver* send_packet_observer); + SendPacketObserver* send_packet_observer, + RateLimiter* nack_rate_limiter); virtual ~RTPSender(); @@ -106,8 +108,6 @@ class RTPSender : public RTPSenderInterface { uint32_t NackOverheadRate() const; void SetTargetBitrate(uint32_t bitrate); - uint32_t GetTargetBitrate(); - // Includes size of RTP and FEC headers. size_t MaxDataPayloadLength() const override; @@ -227,8 +227,6 @@ class RTPSender : public RTPSenderInterface { int32_t ReSendPacket(uint16_t packet_id, int64_t min_resend_time = 0); - bool ProcessNACKBitRate(uint32_t now); - // Feedback to decide when to stop sending playout delay. void OnReceivedRtcpReportBlocks(const ReportBlockList& report_blocks); @@ -340,8 +338,6 @@ class RTPSender : public RTPSenderInterface { uint16_t sequence_number, const std::vector& csrcs) const; - void UpdateNACKBitRate(uint32_t bytes, int64_t now); - bool PrepareAndSendPacket(uint8_t* buffer, size_t length, int64_t capture_time_ms, @@ -406,45 +402,10 @@ class RTPSender : public RTPSenderInterface { bool is_retransmit); bool IsFecPacket(const uint8_t* buffer, const RTPHeader& header) const; - class BitrateAggregator { - public: - explicit BitrateAggregator(BitrateStatisticsObserver* bitrate_callback); - - void OnStatsUpdated() const; - - Bitrate::Observer* total_bitrate_observer(); - Bitrate::Observer* retransmit_bitrate_observer(); - void set_ssrc(uint32_t ssrc); - - private: - // We assume that these observers are called on the same thread, which is - // true for RtpSender as they are called on the Process thread. - class BitrateObserver : public Bitrate::Observer { - public: - explicit BitrateObserver(const BitrateAggregator& aggregator); - - // Implements Bitrate::Observer. - void BitrateUpdated(const BitrateStatistics& stats) override; - const BitrateStatistics& statistics() const; - - private: - BitrateStatistics statistics_; - const BitrateAggregator& aggregator_; - }; - - BitrateStatisticsObserver* const callback_; - BitrateObserver total_bitrate_observer_; - BitrateObserver retransmit_bitrate_observer_; - uint32_t ssrc_; - }; - Clock* const clock_; const int64_t clock_delta_ms_; Random random_ GUARDED_BY(send_critsect_); - BitrateAggregator bitrates_; - Bitrate total_bitrate_sent_; - const bool audio_configured_; const std::unique_ptr audio_; const std::unique_ptr video_; @@ -470,11 +431,6 @@ class RTPSender : public RTPSenderInterface { bool video_rotation_active_; uint16_t transport_sequence_number_; - // NACK - uint32_t nack_byte_count_times_[NACK_BYTECOUNT_SIZE]; - size_t nack_byte_count_[NACK_BYTECOUNT_SIZE]; - Bitrate nack_bitrate_; - // Tracks the current request for playout delay limits from application // and decides whether the current RTP frame should include the playout // delay extension on header. @@ -490,10 +446,13 @@ class RTPSender : public RTPSenderInterface { StreamDataCounters rtp_stats_ GUARDED_BY(statistics_crit_); StreamDataCounters rtx_rtp_stats_ GUARDED_BY(statistics_crit_); StreamDataCountersCallback* rtp_stats_callback_ GUARDED_BY(statistics_crit_); + RateStatistics total_bitrate_sent_ GUARDED_BY(statistics_crit_); + RateStatistics nack_bitrate_sent_ GUARDED_BY(statistics_crit_); FrameCountObserver* const frame_count_observer_; SendSideDelayObserver* const send_side_delay_observer_; RtcEventLog* const event_log_; SendPacketObserver* const send_packet_observer_; + BitrateStatisticsObserver* const bitrate_callback_; // RTP variables bool start_timestamp_forced_ GUARDED_BY(send_critsect_); @@ -516,12 +475,7 @@ class RTPSender : public RTPSenderInterface { // Mapping rtx_payload_type_map_[associated] = rtx. std::map rtx_payload_type_map_ GUARDED_BY(send_critsect_); - // Note: Don't access this variable directly, always go through - // SetTargetBitrateKbps or GetTargetBitrateKbps. Also remember - // that by the time the function returns there is no guarantee - // that the target bitrate is still valid. - rtc::CriticalSection target_bitrate_critsect_; - uint32_t target_bitrate_ GUARDED_BY(target_bitrate_critsect_); + RateLimiter* const retransmission_rate_limiter_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RTPSender); }; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index f0b6411af2..99cef009e7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -15,6 +15,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/buffer.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/rtp_rtcp/include/rtp_cvo.h" #include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" @@ -133,11 +134,11 @@ class RtpSenderTest : public ::testing::Test { : fake_clock_(kStartTime), mock_rtc_event_log_(), mock_paced_sender_(), + retransmission_rate_limiter_(&fake_clock_, 1000), rtp_sender_(), payload_(kPayload), transport_(), - kMarkerBit(true) { - } + kMarkerBit(true) {} void SetUp() override { SetUpRtpSender(true); } @@ -145,7 +146,8 @@ class RtpSenderTest : public ::testing::Test { rtp_sender_.reset(new RTPSender( false, &fake_clock_, &transport_, pacer ? &mock_paced_sender_ : nullptr, &seq_num_allocator_, nullptr, nullptr, nullptr, nullptr, - &mock_rtc_event_log_, &send_packet_observer_)); + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_)); rtp_sender_->SetSequenceNumber(kSeqNum); } @@ -154,6 +156,7 @@ class RtpSenderTest : public ::testing::Test { MockRtpPacketSender mock_paced_sender_; MockTransportSequenceNumberAllocator seq_num_allocator_; MockSendPacketObserver send_packet_observer_; + RateLimiter retransmission_rate_limiter_; std::unique_ptr rtp_sender_; int payload_; LoopbackTransportTest transport_; @@ -743,7 +746,6 @@ TEST_F(RtpSenderTest, TrafficSmoothingWithExtensions) { EXPECT_EQ( 0, rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteSendTime, kAbsoluteSendTimeExtensionId)); - rtp_sender_->SetTargetBitrate(300000); int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); int rtp_length_int = rtp_sender_->BuildRTPheader( packet_, kPayload, kMarkerBit, kTimestamp, capture_time_ms); @@ -797,7 +799,6 @@ TEST_F(RtpSenderTest, TrafficSmoothingRetransmits) { EXPECT_EQ( 0, rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteSendTime, kAbsoluteSendTimeExtensionId)); - rtp_sender_->SetTargetBitrate(300000); int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); int rtp_length_int = rtp_sender_->BuildRTPheader( packet_, kPayload, kMarkerBit, kTimestamp, capture_time_ms); @@ -879,7 +880,6 @@ TEST_F(RtpSenderTest, SendPadding) { kAbsoluteSendTimeExtensionId); webrtc::RTPHeader rtp_header; - rtp_sender_->SetTargetBitrate(300000); int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); int rtp_length_int = rtp_sender_->BuildRTPheader( packet_, kPayload, kMarkerBit, timestamp, capture_time_ms); @@ -1011,7 +1011,7 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { rtp_sender_.reset(new RTPSender( false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr /* TransportSequenceNumberAllocator */, nullptr, nullptr, nullptr, - nullptr, nullptr, &send_packet_observer_)); + nullptr, nullptr, &send_packet_observer_, nullptr)); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetStorePacketsStatus(true, 10); @@ -1029,7 +1029,7 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { MockTransport transport; rtp_sender_.reset(new RTPSender( false, &fake_clock_, &transport, &mock_paced_sender_, nullptr, nullptr, - nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr)); + nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, nullptr)); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); @@ -1054,7 +1054,6 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { kTransmissionTimeOffsetExtensionId); rtp_parser->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteSendTime, kAbsoluteSendTimeExtensionId); - rtp_sender_->SetTargetBitrate(300000); const size_t kNumPayloadSizes = 10; const size_t kPayloadSizes[kNumPayloadSizes] = {500, 550, 600, 650, 700, 750, 800, 850, 900, 950}; @@ -1176,7 +1175,7 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) { rtp_sender_.reset(new RTPSender( false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr, nullptr, - nullptr, &callback, nullptr, nullptr, nullptr)); + nullptr, &callback, nullptr, nullptr, nullptr, nullptr)); char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC"; const uint8_t payload_type = 127; @@ -1213,30 +1212,39 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) { TEST_F(RtpSenderTest, BitrateCallbacks) { class TestCallback : public BitrateStatisticsObserver { public: - TestCallback() : BitrateStatisticsObserver(), num_calls_(0), ssrc_(0) {} + TestCallback() + : BitrateStatisticsObserver(), + num_calls_(0), + ssrc_(0), + total_bitrate_(0), + retransmit_bitrate_(0) {} virtual ~TestCallback() {} - void Notify(const BitrateStatistics& total_stats, - const BitrateStatistics& retransmit_stats, + void Notify(uint32_t total_bitrate, + uint32_t retransmit_bitrate, uint32_t ssrc) override { ++num_calls_; ssrc_ = ssrc; - total_stats_ = total_stats; - retransmit_stats_ = retransmit_stats; + total_bitrate_ = total_bitrate; + retransmit_bitrate_ = retransmit_bitrate; } uint32_t num_calls_; uint32_t ssrc_; - BitrateStatistics total_stats_; - BitrateStatistics retransmit_stats_; + uint32_t total_bitrate_; + uint32_t retransmit_bitrate_; } callback; rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr, nullptr, nullptr, &callback, nullptr, nullptr, - nullptr, nullptr)); + nullptr, nullptr, nullptr)); - // Simulate kNumPackets sent with kPacketInterval ms intervals. - const uint32_t kNumPackets = 15; + // Simulate kNumPackets sent with kPacketInterval ms intervals, with the + // number of packets selected so that we fill (but don't overflow) the one + // second averaging window. + const uint32_t kWindowSizeMs = 1000; const uint32_t kPacketInterval = 20; + const uint32_t kNumPackets = + (kWindowSizeMs - kPacketInterval) / kPacketInterval; // Overhead = 12 bytes RTP header + 1 byte generic header. const uint32_t kPacketOverhead = 13; @@ -1250,7 +1258,6 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { // Initial process call so we get a new time window. rtp_sender_->ProcessBitrate(); - uint64_t start_time = fake_clock_.CurrentNtpInMilliseconds(); // Send a few frames. for (uint32_t i = 0; i < kNumPackets; ++i) { @@ -1262,17 +1269,18 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { rtp_sender_->ProcessBitrate(); - const uint32_t expected_packet_rate = 1000 / kPacketInterval; - // We get one call for every stats updated, thus two calls since both the // stream stats and the retransmit stats are updated once. EXPECT_EQ(2u, callback.num_calls_); EXPECT_EQ(ssrc, callback.ssrc_); - EXPECT_EQ(start_time + (kNumPackets * kPacketInterval), - callback.total_stats_.timestamp_ms); - EXPECT_EQ(expected_packet_rate, callback.total_stats_.packet_rate); - EXPECT_EQ((kPacketOverhead + sizeof(payload)) * 8 * expected_packet_rate, - callback.total_stats_.bitrate_bps); + const uint32_t kTotalPacketSize = kPacketOverhead + sizeof(payload); + // Bitrate measured over delta between last and first timestamp, plus one. + const uint32_t kExpectedWindowMs = kNumPackets * kPacketInterval + 1; + const uint32_t kExpectedBitsAccumulated = kTotalPacketSize * kNumPackets * 8; + const uint32_t kExpectedRateBps = + (kExpectedBitsAccumulated * 1000 + (kExpectedWindowMs / 2)) / + kExpectedWindowMs; + EXPECT_EQ(kExpectedRateBps, callback.total_bitrate_); rtp_sender_.reset(); } @@ -1285,7 +1293,7 @@ class RtpSenderAudioTest : public RtpSenderTest { payload_ = kAudioPayload; rtp_sender_.reset(new RTPSender(true, &fake_clock_, &transport_, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, - nullptr, nullptr)); + nullptr, nullptr, nullptr)); rtp_sender_->SetSequenceNumber(kSeqNum); } }; @@ -1553,9 +1561,9 @@ TEST_F(RtpSenderTestWithoutPacer, RespectsNackBitrateLimit) { const int32_t kPacketSize = 1400; const int32_t kNumPackets = 30; + retransmission_rate_limiter_.SetMaxRate(kPacketSize * kNumPackets * 8); + rtp_sender_->SetStorePacketsStatus(true, kNumPackets); - // Set bitrate (in kbps) to fit kNumPackets รก kPacketSize bytes in one second. - rtp_sender_->SetTargetBitrate(kNumPackets * kPacketSize * 8); const uint16_t kStartSequenceNumber = rtp_sender_->SequenceNumber(); std::list sequence_numbers; for (int32_t i = 0; i < kNumPackets; ++i) { @@ -1573,6 +1581,9 @@ TEST_F(RtpSenderTestWithoutPacer, RespectsNackBitrateLimit) { rtp_sender_->OnReceivedNACK(sequence_numbers, 0); EXPECT_EQ(kNumPackets * 2, transport_.packets_sent_); + // Must be at least 5ms in between retransmission attempts. + fake_clock_.AdvanceTimeMilliseconds(5); + // Resending should not work, bandwidth exceeded. rtp_sender_->OnReceivedNACK(sequence_numbers, 0); EXPECT_EQ(kNumPackets * 2, transport_.packets_sent_); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index 3affb318aa..378ef130d1 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -31,6 +31,7 @@ enum { REDForFECHeaderLength = 1 }; RTPSenderVideo::RTPSenderVideo(Clock* clock, RTPSenderInterface* rtpSender) : _rtpSender(*rtpSender), + clock_(clock), _videoType(kRtpVideoGeneric), _retransmissionSettings(kRetransmitBaseLayer), // Generic FEC @@ -41,8 +42,8 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, RTPSenderInterface* rtpSender) delta_fec_params_(), key_fec_params_(), producer_fec_(&fec_), - _fecOverheadRate(clock, NULL), - _videoBitrate(clock, NULL) { + fec_bitrate_(1000, RateStatistics::kBpsScale), + video_bitrate_(1000, RateStatistics::kBpsScale) { memset(&delta_fec_params_, 0, sizeof(delta_fec_params_)); memset(&key_fec_params_, 0, sizeof(key_fec_params_)); delta_fec_params_.max_fec_frames = key_fec_params_.max_fec_frames = 1; @@ -95,7 +96,9 @@ void RTPSenderVideo::SendVideoPacket(uint8_t* data_buffer, if (_rtpSender.SendToNetwork(data_buffer, payload_length, rtp_header_length, capture_time_ms, storage, RtpPacketSender::kLowPriority) == 0) { - _videoBitrate.Update(payload_length + rtp_header_length); + rtc::CritScope cs(&stats_crit_); + video_bitrate_.Update(payload_length + rtp_header_length, + clock_->TimeInMilliseconds()); TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"), "Video::PacketNormal", "timestamp", capture_timestamp, "seqnum", seq_num); @@ -141,7 +144,8 @@ void RTPSenderVideo::SendVideoPacketAsRed(uint8_t* data_buffer, red_packet->data(), red_packet->length() - rtp_header_length, rtp_header_length, capture_time_ms, media_packet_storage, RtpPacketSender::kLowPriority) == 0) { - _videoBitrate.Update(red_packet->length()); + rtc::CritScope cs(&stats_crit_); + video_bitrate_.Update(red_packet->length(), clock_->TimeInMilliseconds()); TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"), "Video::PacketRed", "timestamp", capture_timestamp, "seqnum", media_seq_num); @@ -153,7 +157,8 @@ void RTPSenderVideo::SendVideoPacketAsRed(uint8_t* data_buffer, fec_packet->data(), fec_packet->length() - rtp_header_length, rtp_header_length, capture_time_ms, fec_storage, RtpPacketSender::kLowPriority) == 0) { - _fecOverheadRate.Update(fec_packet->length()); + rtc::CritScope cs(&stats_crit_); + fec_bitrate_.Update(fec_packet->length(), clock_->TimeInMilliseconds()); TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"), "Video::PacketFec", "timestamp", capture_timestamp, "seqnum", next_fec_sequence_number); @@ -337,17 +342,14 @@ int32_t RTPSenderVideo::SendVideo(const RtpVideoCodecTypes videoType, return 0; } -void RTPSenderVideo::ProcessBitrate() { - _videoBitrate.Process(); - _fecOverheadRate.Process(); -} - uint32_t RTPSenderVideo::VideoBitrateSent() const { - return _videoBitrate.BitrateLast(); + rtc::CritScope cs(&stats_crit_); + return video_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0); } uint32_t RTPSenderVideo::FecOverheadRate() const { - return _fecOverheadRate.BitrateLast(); + rtc::CritScope cs(&stats_crit_); + return fec_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0); } int RTPSenderVideo::SelectiveRetransmissions() const { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h index 8307b83864..7ce889b83c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h @@ -15,10 +15,10 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/onetimeevent.h" +#include "webrtc/base/rate_statistics.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "webrtc/modules/rtp_rtcp/source/bitrate.h" #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" #include "webrtc/modules/rtp_rtcp/source/producer_fec.h" #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h" @@ -68,8 +68,6 @@ class RTPSenderVideo { void SetFecParameters(const FecProtectionParams* delta_params, const FecProtectionParams* key_params); - void ProcessBitrate(); - uint32_t VideoBitrateSent() const; uint32_t FecOverheadRate() const; @@ -95,9 +93,10 @@ class RTPSenderVideo { bool protect); RTPSenderInterface& _rtpSender; + Clock* const clock_; // Should never be held when calling out of this class. - const rtc::CriticalSection crit_; + rtc::CriticalSection crit_; RtpVideoCodecTypes _videoType; int32_t _retransmissionSettings GUARDED_BY(crit_); @@ -111,11 +110,12 @@ class RTPSenderVideo { FecProtectionParams key_fec_params_ GUARDED_BY(crit_); ProducerFec producer_fec_ GUARDED_BY(crit_); + rtc::CriticalSection stats_crit_; // Bitrate used for FEC payload, RED headers, RTP headers for FEC packets // and any padding overhead. - Bitrate _fecOverheadRate; - // Bitrate used for video payload and RTP headers - Bitrate _videoBitrate; + RateStatistics fec_bitrate_ GUARDED_BY(stats_crit_); + // Bitrate used for video payload and RTP headers. + RateStatistics video_bitrate_ GUARDED_BY(stats_crit_); OneTimeEvent first_frame_sent_; }; } // namespace webrtc diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 8e3105f741..21a6654a83 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -26,6 +26,7 @@ #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" #include "webrtc/modules/video_coding/codecs/h264/include/h264.h" #include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h" #include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h" @@ -1527,7 +1528,8 @@ class TransportFeedbackTester : public test::EndToEndTest { : EndToEndTest(::webrtc::EndToEndTest::kDefaultTimeoutMs), feedback_enabled_(feedback_enabled), num_video_streams_(num_video_streams), - num_audio_streams_(num_audio_streams) { + num_audio_streams_(num_audio_streams), + receiver_call_(nullptr) { // Only one stream of each supported for now. EXPECT_LE(num_video_streams, 1u); EXPECT_LE(num_audio_streams, 1u); @@ -2539,6 +2541,16 @@ TEST_F(EndToEndTest, GetStats) { private: Action OnSendRtp(const uint8_t* packet, size_t length) override { + // Drop every 25th packet => 4% loss. + static const int kPacketLossFrac = 25; + RTPHeader header; + RtpUtility::RtpHeaderParser parser(packet, length); + if (parser.Parse(&header) && + expected_send_ssrcs_.find(header.ssrc) != + expected_send_ssrcs_.end() && + header.sequenceNumber % kPacketLossFrac == 0) { + return DROP_PACKET; + } check_stats_event_.Set(); return SEND_PACKET; } @@ -2639,8 +2651,8 @@ TEST_F(EndToEndTest, GetStats) { for (std::map::const_iterator it = stats.substreams.begin(); it != stats.substreams.end(); ++it) { - EXPECT_TRUE(expected_send_ssrcs_.find(it->first) != - expected_send_ssrcs_.end()); + if (expected_send_ssrcs_.find(it->first) == expected_send_ssrcs_.end()) + continue; // Probably RTX. send_stats_filled_[CompoundKey("CapturedFrameRate", it->first)] |= stats.input_frame_rate != 0; @@ -2658,10 +2670,14 @@ TEST_F(EndToEndTest, GetStats) { stream_stats.rtp_stats.retransmitted.packets != 0 || stream_stats.rtp_stats.transmitted.packets != 0; - send_stats_filled_[CompoundKey("BitrateStatisticsObserver", + send_stats_filled_[CompoundKey("BitrateStatisticsObserver.Total", it->first)] |= stream_stats.total_bitrate_bps != 0; + send_stats_filled_[CompoundKey("BitrateStatisticsObserver.Retransmit", + it->first)] |= + stream_stats.retransmit_bitrate_bps != 0; + send_stats_filled_[CompoundKey("FrameCountObserver", it->first)] |= stream_stats.frame_counts.delta_frames != 0 || stream_stats.frame_counts.key_frames != 0; @@ -2692,10 +2708,8 @@ TEST_F(EndToEndTest, GetStats) { } bool AllStatsFilled(const std::map& stats_map) { - for (std::map::const_iterator it = stats_map.begin(); - it != stats_map.end(); - ++it) { - if (!it->second) + for (const auto& stat : stats_map) { + if (!stat.second) return false; } return true; @@ -2718,9 +2732,18 @@ TEST_F(EndToEndTest, GetStats) { VideoSendStream::Config* send_config, std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { + // Set low rates to avoid waiting for rampup. + for (size_t i = 0; i < encoder_config->streams.size(); ++i) { + encoder_config->streams[i].min_bitrate_bps = 10000; + encoder_config->streams[i].target_bitrate_bps = 15000; + encoder_config->streams[i].max_bitrate_bps = 20000; + } send_config->pre_encode_callback = this; // Used to inject delay. expected_cname_ = send_config->rtp.c_name = "SomeCName"; + send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs; + send_config->rtp.rtx.payload_type = kSendRtxPayloadType; + const std::vector& ssrcs = send_config->rtp.ssrcs; for (size_t i = 0; i < ssrcs.size(); ++i) { expected_send_ssrcs_.insert(ssrcs[i]); @@ -2728,7 +2751,17 @@ TEST_F(EndToEndTest, GetStats) { (*receive_configs)[i].rtp.remote_ssrc); (*receive_configs)[i].render_delay_ms = kExpectedRenderDelayMs; (*receive_configs)[i].renderer = &receive_stream_renderer_; + (*receive_configs)[i].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; + + (*receive_configs)[i].rtp.rtx[kFakeVideoSendPayloadType].ssrc = + kSendRtxSsrcs[i]; + (*receive_configs)[i].rtp.rtx[kFakeVideoSendPayloadType].payload_type = + kSendRtxPayloadType; } + + for (size_t i = 0; i < kNumSsrcs; ++i) + send_config->rtp.rtx.ssrcs.push_back(kSendRtxSsrcs[i]); + // Use a delayed encoder to make sure we see CpuOveruseMetrics stats that // are non-zero. send_config->encoder_settings.encoder = &encoder_; diff --git a/webrtc/video/payload_router.cc b/webrtc/video/payload_router.cc index 21439022c1..798325a260 100644 --- a/webrtc/video/payload_router.cc +++ b/webrtc/video/payload_router.cc @@ -167,25 +167,6 @@ int32_t PayloadRouter::Encoded(const EncodedImage& encoded_image, encoded_image._length, fragmentation, &rtp_video_header); } -void PayloadRouter::SetTargetSendBitrate(uint32_t bitrate_bps) { - rtc::CritScope lock(&crit_); - RTC_DCHECK_LE(streams_.size(), rtp_modules_.size()); - - // TODO(sprang): Rebase https://codereview.webrtc.org/1913073002/ on top of - // this. - int bitrate_remainder = bitrate_bps; - for (size_t i = 0; i < streams_.size() && bitrate_remainder > 0; ++i) { - int stream_bitrate = 0; - if (streams_[i].max_bitrate_bps > bitrate_remainder) { - stream_bitrate = bitrate_remainder; - } else { - stream_bitrate = streams_[i].max_bitrate_bps; - } - bitrate_remainder -= stream_bitrate; - rtp_modules_[i]->SetTargetSendBitrate(stream_bitrate); - } -} - size_t PayloadRouter::MaxPayloadLength() const { size_t min_payload_length = DefaultMaxPayloadLength(); rtc::CritScope lock(&crit_); diff --git a/webrtc/video/payload_router.h b/webrtc/video/payload_router.h index ce65bae6f8..9c66bd0d15 100644 --- a/webrtc/video/payload_router.h +++ b/webrtc/video/payload_router.h @@ -50,9 +50,6 @@ class PayloadRouter : public EncodedImageCallback { const CodecSpecificInfo* codec_specific_info, const RTPFragmentationHeader* fragmentation) override; - // Configures current target bitrate. - void SetTargetSendBitrate(uint32_t bitrate_bps); - // Returns the maximum allowed data payload length, given the configured MTU // and RTP headers. size_t MaxPayloadLength() const; diff --git a/webrtc/video/payload_router_unittest.cc b/webrtc/video/payload_router_unittest.cc index 5b6612124c..62dba29c05 100644 --- a/webrtc/video/payload_router_unittest.cc +++ b/webrtc/video/payload_router_unittest.cc @@ -186,25 +186,4 @@ TEST(PayloadRouterTest, MaxPayloadLength) { .WillOnce(Return(kTestMinPayloadLength)); EXPECT_EQ(kTestMinPayloadLength, payload_router.MaxPayloadLength()); } - -TEST(PayloadRouterTest, SetTargetSendBitrates) { - NiceMock rtp_1; - NiceMock rtp_2; - std::vector modules; - modules.push_back(&rtp_1); - modules.push_back(&rtp_2); - PayloadRouter payload_router(modules, 42); - std::vector streams(2); - streams[0].max_bitrate_bps = 10000; - streams[1].max_bitrate_bps = 100000; - payload_router.SetSendStreams(streams); - - const uint32_t bitrate_1 = 10000; - const uint32_t bitrate_2 = 76543; - EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1)) - .Times(1); - EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2)) - .Times(1); - payload_router.SetTargetSendBitrate(bitrate_1 + bitrate_2); -} } // namespace webrtc diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index 90e449c332..4caf55aa8b 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -58,6 +58,7 @@ std::unique_ptr CreateRtpRtcpModule( configuration.send_packet_observer = nullptr; configuration.bandwidth_callback = nullptr; configuration.transport_feedback_callback = nullptr; + configuration.retransmission_rate_limiter = nullptr; std::unique_ptr rtp_rtcp(RtpRtcp::CreateRtpRtcp(configuration)); rtp_rtcp->SetSendingStatus(false); @@ -185,12 +186,10 @@ RtpStreamReceiver::RtpStreamReceiver( // Stats callback for CNAME changes. rtp_rtcp_->RegisterRtcpStatisticsCallback(receive_stats_proxy); - process_thread_->RegisterModule(rtp_receive_statistics_.get()); process_thread_->RegisterModule(rtp_rtcp_.get()); } RtpStreamReceiver::~RtpStreamReceiver() { - process_thread_->DeRegisterModule(rtp_receive_statistics_.get()); process_thread_->DeRegisterModule(rtp_rtcp_.get()); packet_router_->RemoveRtpModule(rtp_rtcp_.get()); diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc index 6815eb3d71..8852de18bf 100644 --- a/webrtc/video/send_statistics_proxy.cc +++ b/webrtc/video/send_statistics_proxy.cc @@ -564,16 +564,16 @@ void SendStatisticsProxy::DataCountersUpdated( uma_container_->first_rtp_stats_time_ms_ = clock_->TimeInMilliseconds(); } -void SendStatisticsProxy::Notify(const BitrateStatistics& total_stats, - const BitrateStatistics& retransmit_stats, +void SendStatisticsProxy::Notify(uint32_t total_bitrate_bps, + uint32_t retransmit_bitrate_bps, uint32_t ssrc) { rtc::CritScope lock(&crit_); VideoSendStream::StreamStats* stats = GetStatsEntry(ssrc); if (!stats) return; - stats->total_bitrate_bps = total_stats.bitrate_bps; - stats->retransmit_bitrate_bps = retransmit_stats.bitrate_bps; + stats->total_bitrate_bps = total_bitrate_bps; + stats->retransmit_bitrate_bps = retransmit_bitrate_bps; } void SendStatisticsProxy::FrameCountUpdated(const FrameCounts& frame_counts, diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h index fa8b3ec5bb..2c9225f51e 100644 --- a/webrtc/video/send_statistics_proxy.h +++ b/webrtc/video/send_statistics_proxy.h @@ -85,8 +85,8 @@ class SendStatisticsProxy : public CpuOveruseMetricsObserver, uint32_t ssrc) override; // From BitrateStatisticsObserver. - void Notify(const BitrateStatistics& total_stats, - const BitrateStatistics& retransmit_stats, + void Notify(uint32_t total_bitrate_bps, + uint32_t retransmit_bitrate_bps, uint32_t ssrc) override; // From FrameCountObserver. diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc index 2bf038d2c4..ad51f2abe6 100644 --- a/webrtc/video/send_statistics_proxy_unittest.cc +++ b/webrtc/video/send_statistics_proxy_unittest.cc @@ -226,24 +226,24 @@ TEST_F(SendStatisticsProxyTest, DataCounters) { TEST_F(SendStatisticsProxyTest, Bitrate) { BitrateStatisticsObserver* observer = statistics_proxy_.get(); for (const auto& ssrc : config_.rtp.ssrcs) { - BitrateStatistics total; - BitrateStatistics retransmit; + uint32_t total; + uint32_t retransmit; // Use ssrc as bitrate_bps to get a unique value for each stream. - total.bitrate_bps = ssrc; - retransmit.bitrate_bps = ssrc + 1; + total = ssrc; + retransmit = ssrc + 1; observer->Notify(total, retransmit, ssrc); - expected_.substreams[ssrc].total_bitrate_bps = total.bitrate_bps; - expected_.substreams[ssrc].retransmit_bitrate_bps = retransmit.bitrate_bps; + expected_.substreams[ssrc].total_bitrate_bps = total; + expected_.substreams[ssrc].retransmit_bitrate_bps = retransmit; } for (const auto& ssrc : config_.rtp.rtx.ssrcs) { - BitrateStatistics total; - BitrateStatistics retransmit; + uint32_t total; + uint32_t retransmit; // Use ssrc as bitrate_bps to get a unique value for each stream. - total.bitrate_bps = ssrc; - retransmit.bitrate_bps = ssrc + 1; + total = ssrc; + retransmit = ssrc + 1; observer->Notify(total, retransmit, ssrc); - expected_.substreams[ssrc].total_bitrate_bps = total.bitrate_bps; - expected_.substreams[ssrc].retransmit_bitrate_bps = retransmit.bitrate_bps; + expected_.substreams[ssrc].total_bitrate_bps = total; + expected_.substreams[ssrc].retransmit_bitrate_bps = retransmit; } VideoSendStream::Stats stats = statistics_proxy_->GetStats(); @@ -397,8 +397,8 @@ TEST_F(SendStatisticsProxyTest, NoSubstreams) { rtcp_callback->StatisticsUpdated(rtcp_stats, excluded_ssrc); // From BitrateStatisticsObserver. - BitrateStatistics total; - BitrateStatistics retransmit; + uint32_t total = 0; + uint32_t retransmit = 0; BitrateStatisticsObserver* bitrate_observer = statistics_proxy_.get(); bitrate_observer->Notify(total, retransmit, excluded_ssrc); @@ -484,8 +484,7 @@ TEST_F(SendStatisticsProxyTest, ClearsResolutionFromInactiveSsrcs) { } TEST_F(SendStatisticsProxyTest, ClearsBitratesFromInactiveSsrcs) { - BitrateStatistics bitrate; - bitrate.bitrate_bps = 42; + uint32_t bitrate = 42; BitrateStatisticsObserver* observer = statistics_proxy_.get(); observer->Notify(bitrate, bitrate, config_.rtp.ssrcs[0]); observer->Notify(bitrate, bitrate, config_.rtp.ssrcs[1]); @@ -493,9 +492,9 @@ TEST_F(SendStatisticsProxyTest, ClearsBitratesFromInactiveSsrcs) { statistics_proxy_->OnInactiveSsrc(config_.rtp.ssrcs[1]); VideoSendStream::Stats stats = statistics_proxy_->GetStats(); - EXPECT_EQ(static_cast(bitrate.bitrate_bps), + EXPECT_EQ(static_cast(bitrate), stats.substreams[config_.rtp.ssrcs[0]].total_bitrate_bps); - EXPECT_EQ(static_cast(bitrate.bitrate_bps), + EXPECT_EQ(static_cast(bitrate), stats.substreams[config_.rtp.ssrcs[0]].retransmit_bitrate_bps); EXPECT_EQ(0, stats.substreams[config_.rtp.ssrcs[1]].total_bitrate_bps); EXPECT_EQ(0, stats.substreams[config_.rtp.ssrcs[1]].retransmit_bitrate_bps); diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 3e95cb0295..a85cf314a2 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -52,6 +52,7 @@ std::vector CreateRtpRtcpModules( SendStatisticsProxy* stats_proxy, SendDelayStats* send_delay_stats, RtcEventLog* event_log, + RateLimiter* retransmission_rate_limiter, size_t num_modules) { RTC_DCHECK_GT(num_modules, 0u); RtpRtcp::Configuration configuration; @@ -73,6 +74,7 @@ std::vector CreateRtpRtcpModules( configuration.send_side_delay_observer = stats_proxy; configuration.send_packet_observer = send_delay_stats; configuration.event_log = event_log; + configuration.retransmission_rate_limiter = retransmission_rate_limiter; std::vector modules; for (size_t i = 0; i < num_modules; ++i) { @@ -428,6 +430,7 @@ VideoSendStream::VideoSendStream( &stats_proxy_, send_delay_stats, event_log, + congestion_controller_->GetRetransmissionRateLimiter(), config_.rtp.ssrcs.size())), payload_router_(rtp_rtcp_modules_, config.encoder_settings.payload_type), input_(&encoder_wakeup_event_, @@ -885,7 +888,6 @@ void VideoSendStream::SignalNetworkState(NetworkState state) { uint32_t VideoSendStream::OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, int64_t rtt) { - payload_router_.SetTargetSendBitrate(bitrate_bps); // Get the encoder target rate. It is the estimated network rate - // protection overhead. uint32_t encoder_target_rate_bps = diff --git a/webrtc/webrtc_tests.gypi b/webrtc/webrtc_tests.gypi index 0047f6930c..243a441efd 100644 --- a/webrtc/webrtc_tests.gypi +++ b/webrtc/webrtc_tests.gypi @@ -63,6 +63,7 @@ 'base/proxy_unittest.cc', 'base/proxydetect_unittest.cc', 'base/random_unittest.cc', + 'base/rate_limiter_unittest.cc', 'base/rate_statistics_unittest.cc', 'base/ratelimiter_unittest.cc', 'base/ratetracker_unittest.cc',