From e75d96b5bd3a952083b60efb860706845ce0dcb7 Mon Sep 17 00:00:00 2001 From: terelius Date: Fri, 30 Jun 2017 08:11:44 -0700 Subject: [PATCH] Revert of Test and fix for huge bwe drop after alr state. (patchset #13 id:320001 of https://codereview.webrtc.org/2931873002/ ) Reason for revert: Resetting the estimate means that we need to start gathering data from scratch again. The combination of 1) DelayBasedEstimator not reacting to overuse unless there is a valid estimate of the acknowledged bitrate, and 2) AcknowledgedBitrateEstimator needing a significant amount of time/data to obtain an provide an estimate causes poor performance in simulations/tests. It is not clear whether this will affect real networks negatively, but I suggest reverting this to be on the safe side. See also https://bugs.chromium.org/p/webrtc/issues/detail?id=7884 Original issue's description: > Test and fix for huge bwe drop after alr state. > > BUG=webrtc:7746 > > Review-Url: https://codereview.webrtc.org/2931873002 > Cr-Commit-Position: refs/heads/master@{#18692} > Committed: https://chromium.googlesource.com/external/webrtc/+/37aa8ba61641962119071646175bfe3bc2bda063 TBR=solenberg@webrtc.org,kwiberg@webrtc.org,minyue@webrtc.org,holmer@chromium.org,philipel@webrtc.org,oprypin@webrtc.org,holmer@google.com,stefan@webrtc.org,tschumim@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:7746 Review-Url: https://codereview.webrtc.org/2964213002 Cr-Commit-Position: refs/heads/master@{#18866} --- resources/voice_engine/audio_dtx16.wav.sha1 | 1 - webrtc/BUILD.gn | 1 - webrtc/audio/BUILD.gn | 39 ---- .../audio/test/audio_bwe_integration_test.cc | 146 --------------- .../audio/test/audio_bwe_integration_test.h | 53 ------ webrtc/modules/congestion_controller/BUILD.gn | 8 +- ...or.cc => acknowledge_bitrate_estimator.cc} | 30 ++- ...ator.h => acknowledge_bitrate_estimator.h} | 20 +- .../acknowledged_bitrate_estimator.cc | 81 -------- .../acknowledged_bitrate_estimator.h | 55 ------ ...acknowledged_bitrate_estimator_unittest.cc | 177 ------------------ .../delay_based_bwe_unittest_helper.cc | 4 +- .../delay_based_bwe_unittest_helper.h | 2 +- .../send_side_congestion_controller.cc | 8 +- .../test/estimators/send_side.cc | 2 +- .../test/estimators/send_side.h | 2 +- 16 files changed, 44 insertions(+), 585 deletions(-) delete mode 100644 resources/voice_engine/audio_dtx16.wav.sha1 delete mode 100644 webrtc/audio/test/audio_bwe_integration_test.cc delete mode 100644 webrtc/audio/test/audio_bwe_integration_test.h rename webrtc/modules/congestion_controller/{bitrate_estimator.cc => acknowledge_bitrate_estimator.cc} (75%) rename webrtc/modules/congestion_controller/{bitrate_estimator.h => acknowledge_bitrate_estimator.h} (66%) delete mode 100644 webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc delete mode 100644 webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h delete mode 100644 webrtc/modules/congestion_controller/acknowledged_bitrate_estimator_unittest.cc diff --git a/resources/voice_engine/audio_dtx16.wav.sha1 b/resources/voice_engine/audio_dtx16.wav.sha1 deleted file mode 100644 index 6a552c2953..0000000000 --- a/resources/voice_engine/audio_dtx16.wav.sha1 +++ /dev/null @@ -1 +0,0 @@ -cafd7151d6b7b4313d0bd2a128a31fc83bca7aa3 \ No newline at end of file diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index 2821556e67..9280d540bc 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -496,7 +496,6 @@ if (rtc_include_tests) { configs += [ ":rtc_unittests_config" ] deps = [ - "audio:audio_perf_tests", "call:call_perf_tests", "modules/audio_coding:audio_coding_perf_tests", "modules/audio_processing:audio_processing_perf_tests", diff --git a/webrtc/audio/BUILD.gn b/webrtc/audio/BUILD.gn index 414a7c65bd..1577316cd4 100644 --- a/webrtc/audio/BUILD.gn +++ b/webrtc/audio/BUILD.gn @@ -113,8 +113,6 @@ if (rtc_include_tests) { "../test:fake_audio_device", "../test:test_common", "../test:test_main", - "//testing/gmock", - "//testing/gtest", "//third_party/gflags", ] if (is_android) { @@ -124,7 +122,6 @@ if (rtc_include_tests) { data = [ "//resources/voice_engine/audio_tiny16.wav", "//resources/voice_engine/audio_tiny48.wav", - "//resources/voice_engine/audio_dtx16.wav", ] if (!build_with_chromium && is_clang) { @@ -133,40 +130,4 @@ if (rtc_include_tests) { } } } - - rtc_source_set("audio_perf_tests") { - testonly = true - - # Skip restricting visibility on mobile platforms since the tests on those - # gets additional generated targets which would require many lines here to - # cover (which would be confusing to read and hard to maintain). - if (!is_android && !is_ios) { - visibility = [ "//webrtc:webrtc_perf_tests" ] - } - sources = [ - "test/audio_bwe_integration_test.cc", - "test/audio_bwe_integration_test.h", - ] - deps = [ - "../base:rtc_base_approved", - "../common_audio", - "../system_wrappers", - "../test:fake_audio_device", - "../test:field_trial", - "../test:test_common", - "../test:test_main", - "//testing/gmock", - "//testing/gtest", - "//third_party/gflags", - ] - - data = [ - "//resources/voice_engine/audio_dtx16.wav", - ] - - if (!build_with_chromium && is_clang) { - # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). - suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] - } - } } diff --git a/webrtc/audio/test/audio_bwe_integration_test.cc b/webrtc/audio/test/audio_bwe_integration_test.cc deleted file mode 100644 index e5c0c2facd..0000000000 --- a/webrtc/audio/test/audio_bwe_integration_test.cc +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Copyright (c) 2017 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/audio/test/audio_bwe_integration_test.h" - -#include "webrtc/base/ptr_util.h" -#include "webrtc/common_audio/wav_file.h" -#include "webrtc/system_wrappers/include/sleep.h" -#include "webrtc/test/field_trial.h" -#include "webrtc/test/gtest.h" -#include "webrtc/test/testsupport/fileutils.h" - -namespace webrtc { -namespace test { - -AudioBweTest::AudioBweTest() : EndToEndTest(CallTest::kDefaultTimeoutMs) {} - -size_t AudioBweTest::GetNumVideoStreams() const { - return 0; -} -size_t AudioBweTest::GetNumAudioStreams() const { - return 1; -} -size_t AudioBweTest::GetNumFlexfecStreams() const { - return 0; -} - -std::unique_ptr -AudioBweTest::CreateCapturer() { - return test::FakeAudioDevice::CreateWavFileReader(AudioInputFile()); -} - -void AudioBweTest::OnFakeAudioDevicesCreated( - test::FakeAudioDevice* send_audio_device, - test::FakeAudioDevice* recv_audio_device) { - send_audio_device_ = send_audio_device; -} - -test::PacketTransport* AudioBweTest::CreateSendTransport(Call* sender_call) { - return new test::PacketTransport( - sender_call, this, test::PacketTransport::kSender, - test::CallTest::payload_type_map_, GetNetworkPipeConfig()); -} - -test::PacketTransport* AudioBweTest::CreateReceiveTransport() { - return new test::PacketTransport( - nullptr, this, test::PacketTransport::kReceiver, - test::CallTest::payload_type_map_, GetNetworkPipeConfig()); -} - -void AudioBweTest::PerformTest() { - send_audio_device_->WaitForRecordingEnd(); - SleepMs(GetNetworkPipeConfig().queue_delay_ms); -} - -class StatsPollTask : public rtc::QueuedTask { - public: - explicit StatsPollTask(Call* sender_call) : sender_call_(sender_call) {} - - private: - bool Run() override { - RTC_CHECK(sender_call_); - Call::Stats call_stats = sender_call_->GetStats(); - EXPECT_GT(call_stats.send_bandwidth_bps, 30000); - rtc::TaskQueue::Current()->PostDelayedTask( - std::unique_ptr(this), 100); - return false; - } - Call* sender_call_; -}; - -class NoBandwidthDropAfterDtx : public AudioBweTest { - public: - NoBandwidthDropAfterDtx() - : sender_call_(nullptr), stats_poller_("stats poller task queue") {} - - void ModifyAudioConfigs( - AudioSendStream::Config* send_config, - std::vector* receive_configs) override { - send_config->send_codec_spec = - rtc::Optional( - {test::CallTest::kAudioSendPayloadType, - {"OPUS", - 48000, - 2, - {{"ptime", "60"}, {"usedtx", "1"}, {"stereo", "1"}}}}); - - send_config->min_bitrate_bps = 6000; - send_config->max_bitrate_bps = 100000; - send_config->rtp.extensions.push_back( - RtpExtension(RtpExtension::kTransportSequenceNumberUri, - kTransportSequenceNumberExtensionId)); - for (AudioReceiveStream::Config& recv_config : *receive_configs) { - recv_config.rtp.transport_cc = true; - recv_config.rtp.extensions = send_config->rtp.extensions; - recv_config.rtp.remote_ssrc = send_config->rtp.ssrc; - } - } - - std::string AudioInputFile() override { - return test::ResourcePath("voice_engine/audio_dtx16", "wav"); - } - - FakeNetworkPipe::Config GetNetworkPipeConfig() override { - FakeNetworkPipe::Config pipe_config; - pipe_config.link_capacity_kbps = 50; - pipe_config.queue_length_packets = 1500; - pipe_config.queue_delay_ms = 300; - return pipe_config; - } - - void OnCallsCreated(Call* sender_call, Call* receiver_call) override { - sender_call_ = sender_call; - } - - void PerformTest() override { - stats_poller_.PostDelayedTask( - std::unique_ptr(new StatsPollTask(sender_call_)), 100); - sender_call_->OnTransportOverheadChanged(webrtc::MediaType::AUDIO, 0); - AudioBweTest::PerformTest(); - } - - private: - Call* sender_call_; - rtc::TaskQueue stats_poller_; -}; - -using AudioBweIntegrationTest = CallTest; - -TEST_F(AudioBweIntegrationTest, DISABLED_NoBandwidthDropAfterDtx) { - webrtc::test::ScopedFieldTrials override_field_trials( - "WebRTC-Audio-SendSideBwe/Enabled/" - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); - NoBandwidthDropAfterDtx test; - RunBaseTest(&test); -} - -} // namespace test -} // namespace webrtc diff --git a/webrtc/audio/test/audio_bwe_integration_test.h b/webrtc/audio/test/audio_bwe_integration_test.h deleted file mode 100644 index 769603aca9..0000000000 --- a/webrtc/audio/test/audio_bwe_integration_test.h +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright (c) 2017 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_AUDIO_TEST_AUDIO_BWE_INTEGRATION_TEST_H_ -#define WEBRTC_AUDIO_TEST_AUDIO_BWE_INTEGRATION_TEST_H_ - -#include -#include - -#include "webrtc/test/call_test.h" -#include "webrtc/test/fake_audio_device.h" - -namespace webrtc { -namespace test { - -class AudioBweTest : public test::EndToEndTest { - public: - AudioBweTest(); - - protected: - virtual std::string AudioInputFile() = 0; - - virtual FakeNetworkPipe::Config GetNetworkPipeConfig() = 0; - - size_t GetNumVideoStreams() const override; - size_t GetNumAudioStreams() const override; - size_t GetNumFlexfecStreams() const override; - - std::unique_ptr CreateCapturer() override; - - void OnFakeAudioDevicesCreated( - test::FakeAudioDevice* send_audio_device, - test::FakeAudioDevice* recv_audio_device) override; - - test::PacketTransport* CreateSendTransport(Call* sender_call) override; - test::PacketTransport* CreateReceiveTransport() override; - - void PerformTest() override; - - private: - test::FakeAudioDevice* send_audio_device_; -}; - -} // namespace test -} // namespace webrtc - -#endif // WEBRTC_AUDIO_TEST_AUDIO_BWE_INTEGRATION_TEST_H_ diff --git a/webrtc/modules/congestion_controller/BUILD.gn b/webrtc/modules/congestion_controller/BUILD.gn index eade5598cb..ef45297031 100644 --- a/webrtc/modules/congestion_controller/BUILD.gn +++ b/webrtc/modules/congestion_controller/BUILD.gn @@ -10,10 +10,8 @@ import("../../webrtc.gni") rtc_static_library("congestion_controller") { sources = [ - "acknowledged_bitrate_estimator.cc", - "acknowledged_bitrate_estimator.h", - "bitrate_estimator.cc", - "bitrate_estimator.h", + "acknowledge_bitrate_estimator.cc", + "acknowledge_bitrate_estimator.h", "congestion_controller.cc", "delay_based_bwe.cc", "delay_based_bwe.h", @@ -75,7 +73,6 @@ if (rtc_include_tests) { visibility = [ "//webrtc/modules:modules_unittests" ] } sources = [ - "acknowledged_bitrate_estimator_unittest.cc", "congestion_controller_unittest.cc", "congestion_controller_unittests_helper.cc", "congestion_controller_unittests_helper.h", @@ -93,7 +90,6 @@ if (rtc_include_tests) { ":mock_congestion_controller", "../../base:rtc_base", "../../base:rtc_base_approved", - "../../base:rtc_base_tests_utils", "../../system_wrappers:system_wrappers", "../../test:field_trial", "../../test:test_support", diff --git a/webrtc/modules/congestion_controller/bitrate_estimator.cc b/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc similarity index 75% rename from webrtc/modules/congestion_controller/bitrate_estimator.cc rename to webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc index 831144e201..3fa3ac9995 100644 --- a/webrtc/modules/congestion_controller/bitrate_estimator.cc +++ b/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc @@ -8,7 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "webrtc/modules/congestion_controller/bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" #include @@ -20,18 +20,32 @@ namespace webrtc { namespace { constexpr int kInitialRateWindowMs = 500; constexpr int kRateWindowMs = 150; + +bool IsInSendTimeHistory(const PacketFeedback& packet) { + return packet.send_time_ms >= 0; +} + } // namespace -BitrateEstimator::BitrateEstimator() +AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator() : sum_(0), current_win_ms_(0), prev_time_ms_(-1), bitrate_estimate_(-1.0f), bitrate_estimate_var_(50.0f) {} -BitrateEstimator::~BitrateEstimator() = default; +void AcknowledgedBitrateEstimator::IncomingPacketFeedbackVector( + const std::vector& packet_feedback_vector) { + RTC_DCHECK(std::is_sorted(packet_feedback_vector.begin(), + packet_feedback_vector.end(), + PacketFeedbackComparator())); + for (const auto& packet : packet_feedback_vector) { + if (IsInSendTimeHistory(packet)) + Update(packet.arrival_time_ms, packet.payload_size); + } +} -void BitrateEstimator::Update(int64_t now_ms, int bytes) { +void AcknowledgedBitrateEstimator::Update(int64_t now_ms, int bytes) { int rate_window_ms = kRateWindowMs; // We use a larger window at the beginning to get a more stable sample that // we can use to initialize the estimate. @@ -64,9 +78,9 @@ void BitrateEstimator::Update(int64_t now_ms, int bytes) { bitrate_estimate_ * 1000); } -float BitrateEstimator::UpdateWindow(int64_t now_ms, - int bytes, - int rate_window_ms) { +float AcknowledgedBitrateEstimator::UpdateWindow(int64_t now_ms, + int bytes, + int rate_window_ms) { // Reset if time moves backwards. if (now_ms < prev_time_ms_) { prev_time_ms_ = -1; @@ -92,7 +106,7 @@ float BitrateEstimator::UpdateWindow(int64_t now_ms, return bitrate_sample; } -rtc::Optional BitrateEstimator::bitrate_bps() const { +rtc::Optional AcknowledgedBitrateEstimator::bitrate_bps() const { if (bitrate_estimate_ < 0.f) return rtc::Optional(); return rtc::Optional(bitrate_estimate_ * 1000); diff --git a/webrtc/modules/congestion_controller/bitrate_estimator.h b/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h similarity index 66% rename from webrtc/modules/congestion_controller/bitrate_estimator.h rename to webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h index 7d562081b2..7a9d669a1c 100644 --- a/webrtc/modules/congestion_controller/bitrate_estimator.h +++ b/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h @@ -8,8 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_BITRATE_ESTIMATOR_H_ -#define WEBRTC_MODULES_CONGESTION_CONTROLLER_BITRATE_ESTIMATOR_H_ +#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ +#define WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ #include @@ -17,21 +17,25 @@ namespace webrtc { +struct PacketFeedback; + // Computes a bayesian estimate of the throughput given acks containing // the arrival time and payload size. Samples which are far from the current // estimate or are based on few packets are given a smaller weight, as they // are considered to be more likely to have been caused by, e.g., delay spikes // unrelated to congestion. -class BitrateEstimator { +class AcknowledgedBitrateEstimator { public: - BitrateEstimator(); - virtual ~BitrateEstimator(); - virtual void Update(int64_t now_ms, int bytes); + AcknowledgedBitrateEstimator(); - virtual rtc::Optional bitrate_bps() const; + void IncomingPacketFeedbackVector( + const std::vector& packet_feedback_vector); + rtc::Optional bitrate_bps() const; private: + void Update(int64_t now_ms, int bytes); float UpdateWindow(int64_t now_ms, int bytes, int rate_window_ms); + int sum_; int64_t current_win_ms_; int64_t prev_time_ms_; @@ -41,4 +45,4 @@ class BitrateEstimator { } // namespace webrtc -#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_BITRATE_ESTIMATOR_H_ +#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ diff --git a/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc b/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc deleted file mode 100644 index bcccb43b32..0000000000 --- a/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright (c) 2017 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/congestion_controller/acknowledged_bitrate_estimator.h" - -#include - -#include "webrtc/base/ptr_util.h" -#include "webrtc/base/timeutils.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" - -namespace webrtc { - -namespace { -bool IsInSendTimeHistory(const PacketFeedback& packet) { - return packet.send_time_ms >= 0; -} -} // namespace - -std::unique_ptr BitrateEstimatorCreator::Create() { - return rtc::MakeUnique(); -} - -AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator() - : AcknowledgedBitrateEstimator(rtc::MakeUnique()) { -} - -AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator( - std::unique_ptr bitrate_estimator_creator) - : was_in_alr_(false), - bitrate_estimator_creator_(std::move(bitrate_estimator_creator)), - bitrate_estimator_(bitrate_estimator_creator_->Create()) {} - -void AcknowledgedBitrateEstimator::IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector, - bool currently_in_alr) { - RTC_DCHECK(std::is_sorted(packet_feedback_vector.begin(), - packet_feedback_vector.end(), - PacketFeedbackComparator())); - MaybeResetBitrateEstimator(currently_in_alr); - for (const auto& packet : packet_feedback_vector) { - if (IsInSendTimeHistory(packet) && !SentBeforeAlrEnded(packet)) - bitrate_estimator_->Update(packet.arrival_time_ms, packet.payload_size); - } -} - -rtc::Optional AcknowledgedBitrateEstimator::bitrate_bps() const { - return bitrate_estimator_->bitrate_bps(); -} - -bool AcknowledgedBitrateEstimator::SentBeforeAlrEnded( - const PacketFeedback& packet) { - if (alr_ended_time_ms_) { - if (*alr_ended_time_ms_ > packet.send_time_ms) { - return true; - } - } - return false; -} - -bool AcknowledgedBitrateEstimator::AlrEnded(bool currently_in_alr) const { - return was_in_alr_ && !currently_in_alr; -} - -void AcknowledgedBitrateEstimator::MaybeResetBitrateEstimator( - bool currently_in_alr) { - if (AlrEnded(currently_in_alr)) { - bitrate_estimator_ = bitrate_estimator_creator_->Create(); - alr_ended_time_ms_ = rtc::Optional(rtc::TimeMillis()); - } - was_in_alr_ = currently_in_alr; -} - -} // namespace webrtc diff --git a/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h b/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h deleted file mode 100644 index afed1dcac0..0000000000 --- a/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (c) 2017 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_CONGESTION_CONTROLLER_ACKNOWLEDGED_BITRATE_ESTIMATOR_H_ -#define WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGED_BITRATE_ESTIMATOR_H_ - -#include -#include - -#include "webrtc/base/optional.h" -#include "webrtc/modules/congestion_controller/bitrate_estimator.h" - -namespace webrtc { - -struct PacketFeedback; - -class BitrateEstimatorCreator { - public: - virtual ~BitrateEstimatorCreator() = default; - virtual std::unique_ptr Create(); -}; - -class AcknowledgedBitrateEstimator { - public: - explicit AcknowledgedBitrateEstimator( - std::unique_ptr bitrate_estimator_creator); - - AcknowledgedBitrateEstimator(); - - void IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector, - bool currently_in_alr); - rtc::Optional bitrate_bps() const; - - private: - bool SentBeforeAlrEnded(const PacketFeedback& packet); - bool AlrEnded(bool currently_in_alr) const; - void MaybeResetBitrateEstimator(bool currently_in_alr); - - bool was_in_alr_; - rtc::Optional alr_ended_time_ms_; - const std::unique_ptr bitrate_estimator_creator_; - std::unique_ptr bitrate_estimator_; -}; - -} // namespace webrtc - -#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGED_BITRATE_ESTIMATOR_H_ diff --git a/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator_unittest.cc b/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator_unittest.cc deleted file mode 100644 index 0c8522d5cf..0000000000 --- a/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator_unittest.cc +++ /dev/null @@ -1,177 +0,0 @@ -/* - * Copyright (c) 2017 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/congestion_controller/acknowledged_bitrate_estimator.h" - -#include - -#include "webrtc/base/fakeclock.h" -#include "webrtc/base/ptr_util.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "webrtc/test/gmock.h" -#include "webrtc/test/gtest.h" - -using testing::_; -using testing::NiceMock; -using testing::InSequence; -using testing::Return; - -namespace webrtc { - -namespace { - -constexpr int64_t first_arrival_time_ms = 10; -constexpr int64_t first_send_time_ms = 10; -constexpr uint16_t sequence_number = 1; -constexpr size_t payload_size = 10; - -class MockBitrateEstimator : public BitrateEstimator { - public: - MOCK_METHOD2(Update, void(int64_t now_ms, int bytes)); - MOCK_CONST_METHOD0(bitrate_bps, rtc::Optional()); -}; - -class MockBitrateEstimatorCreator : public BitrateEstimatorCreator { - public: - MockBitrateEstimatorCreator() - : mock_bitrate_estimator_(nullptr), num_created_bitrate_estimators_(0) {} - std::unique_ptr Create() override { - auto bitrate_estimator = rtc::MakeUnique>(); - mock_bitrate_estimator_ = bitrate_estimator.get(); - num_created_bitrate_estimators_++; - return bitrate_estimator; - } - int num_created_bitrate_estimators() { - return num_created_bitrate_estimators_; - } - - MockBitrateEstimator* get_mock_bitrate_estimator() { - RTC_CHECK(mock_bitrate_estimator_); - return mock_bitrate_estimator_; - } - - private: - MockBitrateEstimator* mock_bitrate_estimator_; - int num_created_bitrate_estimators_; -}; - -struct AcknowledgedBitrateEstimatorTestStates { - std::unique_ptr acknowledged_bitrate_estimator; - MockBitrateEstimatorCreator* mock_bitrate_estimator_creator; -}; - -AcknowledgedBitrateEstimatorTestStates CreateTestStates() { - AcknowledgedBitrateEstimatorTestStates states; - auto mock_bitrate_estimator_creator = - rtc::MakeUnique(); - states.mock_bitrate_estimator_creator = mock_bitrate_estimator_creator.get(); - states.acknowledged_bitrate_estimator = - rtc::MakeUnique( - std::move(mock_bitrate_estimator_creator)); - return states; -} - -std::vector CreateFeedbackVector() { - std::vector packet_feedback_vector; - const PacedPacketInfo pacing_info; - packet_feedback_vector.push_back( - PacketFeedback(first_arrival_time_ms, first_send_time_ms, sequence_number, - payload_size, pacing_info)); - packet_feedback_vector.push_back( - PacketFeedback(first_arrival_time_ms + 10, first_send_time_ms + 10, - sequence_number, payload_size + 10, pacing_info)); - return packet_feedback_vector; -} - -} // anonymous namespace - -TEST(TestAcknowledgedBitrateEstimator, DontAddPacketsWhichAreNotInSendHistory) { - auto states = CreateTestStates(); - std::vector packet_feedback_vector; - packet_feedback_vector.push_back( - PacketFeedback(first_arrival_time_ms, sequence_number)); - EXPECT_CALL( - *states.mock_bitrate_estimator_creator->get_mock_bitrate_estimator(), - Update(_, _)) - .Times(0); - states.acknowledged_bitrate_estimator->IncomingPacketFeedbackVector( - packet_feedback_vector, false); -} - -TEST(TestAcknowledgedBitrateEstimator, UpdateBandwith) { - auto states = CreateTestStates(); - auto packet_feedback_vector = CreateFeedbackVector(); - { - InSequence dummy; - EXPECT_CALL( - *states.mock_bitrate_estimator_creator->get_mock_bitrate_estimator(), - Update(packet_feedback_vector[0].arrival_time_ms, - static_cast(packet_feedback_vector[0].payload_size))) - .Times(1); - EXPECT_CALL( - *states.mock_bitrate_estimator_creator->get_mock_bitrate_estimator(), - Update(packet_feedback_vector[1].arrival_time_ms, - static_cast(packet_feedback_vector[1].payload_size))) - .Times(1); - } - states.acknowledged_bitrate_estimator->IncomingPacketFeedbackVector( - packet_feedback_vector, false); -} - -TEST(TestAcknowledgedBitrateEstimator, - ResetAfterLeafAlrStateAndDontAddOldPackets) { - auto states = CreateTestStates(); - auto packet_feedback_vector = CreateFeedbackVector(); - states.acknowledged_bitrate_estimator->IncomingPacketFeedbackVector( - packet_feedback_vector, true); - - rtc::ScopedFakeClock fake_clock; - - fake_clock.AdvanceTime( - rtc::TimeDelta::FromMilliseconds(first_arrival_time_ms + 1)); - - EXPECT_EQ( - 1, - states.mock_bitrate_estimator_creator->num_created_bitrate_estimators()); - states.acknowledged_bitrate_estimator->IncomingPacketFeedbackVector( - std::vector(), false); - EXPECT_EQ( - 2, - states.mock_bitrate_estimator_creator->num_created_bitrate_estimators()); - - { - InSequence dummy; - EXPECT_CALL( - *states.mock_bitrate_estimator_creator->get_mock_bitrate_estimator(), - Update(packet_feedback_vector[0].arrival_time_ms, - static_cast(packet_feedback_vector[0].payload_size))) - .Times(0); - EXPECT_CALL( - *states.mock_bitrate_estimator_creator->get_mock_bitrate_estimator(), - Update(packet_feedback_vector[1].arrival_time_ms, - static_cast(packet_feedback_vector[1].payload_size))) - .Times(1); - } - states.acknowledged_bitrate_estimator->IncomingPacketFeedbackVector( - packet_feedback_vector, false); -} - -TEST(TestAcknowledgedBitrateEstimator, ReturnBitrate) { - auto states = CreateTestStates(); - rtc::Optional return_value(42); - EXPECT_CALL( - *states.mock_bitrate_estimator_creator->get_mock_bitrate_estimator(), - bitrate_bps()) - .Times(1) - .WillOnce(Return(return_value)); - EXPECT_EQ(return_value, states.acknowledged_bitrate_estimator->bitrate_bps()); -} - -} // namespace webrtc diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc index 288886d450..62c9f881bc 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -184,7 +184,7 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, sequence_number, payload_size, pacing_info); std::vector packets; packets.push_back(packet); - acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets, false); + acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets); DelayBasedBwe::Result result = bitrate_estimator_->IncomingPacketFeedbackVector( packets, acknowledged_bitrate_estimator_->bitrate_bps()); @@ -219,7 +219,7 @@ bool DelayBasedBweTest::GenerateAndProcessFrame(uint32_t ssrc, packet.arrival_time_ms += arrival_time_offset_ms_; } - acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets, false); + acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets); DelayBasedBwe::Result result = bitrate_estimator_->IncomingPacketFeedbackVector( packets, acknowledged_bitrate_estimator_->bitrate_bps()); diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h index b7b7905866..a12bee52b9 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h @@ -18,7 +18,7 @@ #include #include "webrtc/base/constructormagic.h" -#include "webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" #include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/system_wrappers/include/clock.h" diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc index ce6b96b0b4..d5f966aeaa 100644 --- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc +++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc @@ -20,7 +20,7 @@ #include "webrtc/base/rate_limiter.h" #include "webrtc/base/socket.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" -#include "webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" #include "webrtc/modules/congestion_controller/probe_controller.h" #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h" @@ -89,8 +89,7 @@ SendSideCongestionController::SendSideCongestionController( bitrate_controller_( BitrateController::CreateBitrateController(clock_, event_log)), acknowledged_bitrate_estimator_( - rtc::MakeUnique( - rtc::MakeUnique())), + rtc::MakeUnique()), probe_controller_(new ProbeController(pacer_.get(), clock_)), retransmission_rate_limiter_( new RateLimiter(clock, kRetransmitWindowSizeMs)), @@ -279,8 +278,7 @@ void SendSideCongestionController::OnTransportFeedback( transport_feedback_adapter_.GetTransportFeedbackVector()); SortPacketFeedbackVector(&feedback_vector); acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( - feedback_vector, - static_cast(pacer_->GetApplicationLimitedRegionStartTime())); + feedback_vector); DelayBasedBwe::Result result; { rtc::CritScope cs(&bwe_lock_); diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc index a6e857f7fe..905659d0ee 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -78,7 +78,7 @@ void SendSideBweSender::GiveFeedback(const FeedbackPacket& feedback) { std::sort(packet_feedback_vector.begin(), packet_feedback_vector.end(), PacketFeedbackComparator()); acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( - packet_feedback_vector, false); + packet_feedback_vector); DelayBasedBwe::Result result = bwe_->IncomingPacketFeedbackVector( packet_feedback_vector, acknowledged_bitrate_estimator_->bitrate_bps()); if (result.updated) diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h index ed6a7cdee8..d2aa8de930 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h @@ -15,7 +15,7 @@ #include #include "webrtc/logging/rtc_event_log/mock/mock_rtc_event_log.h" -#include "webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h"