From 37aa8ba61641962119071646175bfe3bc2bda063 Mon Sep 17 00:00:00 2001 From: tschumim Date: Tue, 20 Jun 2017 23:42:30 -0700 Subject: [PATCH] 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} --- 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 +- .../acknowledged_bitrate_estimator.cc | 81 ++++++++ .../acknowledged_bitrate_estimator.h | 55 ++++++ ...acknowledged_bitrate_estimator_unittest.cc | 177 ++++++++++++++++++ ...rate_estimator.cc => bitrate_estimator.cc} | 30 +-- ...itrate_estimator.h => bitrate_estimator.h} | 20 +- .../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, 585 insertions(+), 44 deletions(-) create mode 100644 resources/voice_engine/audio_dtx16.wav.sha1 create mode 100644 webrtc/audio/test/audio_bwe_integration_test.cc create mode 100644 webrtc/audio/test/audio_bwe_integration_test.h create mode 100644 webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc create mode 100644 webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h create mode 100644 webrtc/modules/congestion_controller/acknowledged_bitrate_estimator_unittest.cc rename webrtc/modules/congestion_controller/{acknowledge_bitrate_estimator.cc => bitrate_estimator.cc} (75%) rename webrtc/modules/congestion_controller/{acknowledge_bitrate_estimator.h => bitrate_estimator.h} (66%) diff --git a/resources/voice_engine/audio_dtx16.wav.sha1 b/resources/voice_engine/audio_dtx16.wav.sha1 new file mode 100644 index 0000000000..6a552c2953 --- /dev/null +++ b/resources/voice_engine/audio_dtx16.wav.sha1 @@ -0,0 +1 @@ +cafd7151d6b7b4313d0bd2a128a31fc83bca7aa3 \ No newline at end of file diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index 053512e99b..53e34277c8 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -488,6 +488,7 @@ 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 1757e4e0dd..48a6706f2d 100644 --- a/webrtc/audio/BUILD.gn +++ b/webrtc/audio/BUILD.gn @@ -112,6 +112,8 @@ if (rtc_include_tests) { "../test:fake_audio_device", "../test:test_common", "../test:test_main", + "//testing/gmock", + "//testing/gtest", "//third_party/gflags", ] if (is_android) { @@ -121,6 +123,7 @@ 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) { @@ -129,4 +132,40 @@ 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 new file mode 100644 index 0000000000..bb5d916543 --- /dev/null +++ b/webrtc/audio/test/audio_bwe_integration_test.cc @@ -0,0 +1,146 @@ +/* + * 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, 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 new file mode 100644 index 0000000000..769603aca9 --- /dev/null +++ b/webrtc/audio/test/audio_bwe_integration_test.h @@ -0,0 +1,53 @@ +/* + * 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 ef45297031..eade5598cb 100644 --- a/webrtc/modules/congestion_controller/BUILD.gn +++ b/webrtc/modules/congestion_controller/BUILD.gn @@ -10,8 +10,10 @@ import("../../webrtc.gni") rtc_static_library("congestion_controller") { sources = [ - "acknowledge_bitrate_estimator.cc", - "acknowledge_bitrate_estimator.h", + "acknowledged_bitrate_estimator.cc", + "acknowledged_bitrate_estimator.h", + "bitrate_estimator.cc", + "bitrate_estimator.h", "congestion_controller.cc", "delay_based_bwe.cc", "delay_based_bwe.h", @@ -73,6 +75,7 @@ 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", @@ -90,6 +93,7 @@ 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/acknowledged_bitrate_estimator.cc b/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc new file mode 100644 index 0000000000..bcccb43b32 --- /dev/null +++ b/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc @@ -0,0 +1,81 @@ +/* + * 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 new file mode 100644 index 0000000000..afed1dcac0 --- /dev/null +++ b/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h @@ -0,0 +1,55 @@ +/* + * 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 new file mode 100644 index 0000000000..0c8522d5cf --- /dev/null +++ b/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator_unittest.cc @@ -0,0 +1,177 @@ +/* + * 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/acknowledge_bitrate_estimator.cc b/webrtc/modules/congestion_controller/bitrate_estimator.cc similarity index 75% rename from webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc rename to webrtc/modules/congestion_controller/bitrate_estimator.cc index 3fa3ac9995..831144e201 100644 --- a/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc +++ b/webrtc/modules/congestion_controller/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/acknowledge_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/bitrate_estimator.h" #include @@ -20,32 +20,18 @@ namespace webrtc { namespace { constexpr int kInitialRateWindowMs = 500; constexpr int kRateWindowMs = 150; - -bool IsInSendTimeHistory(const PacketFeedback& packet) { - return packet.send_time_ms >= 0; -} - } // namespace -AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator() +BitrateEstimator::BitrateEstimator() : sum_(0), current_win_ms_(0), prev_time_ms_(-1), bitrate_estimate_(-1.0f), bitrate_estimate_var_(50.0f) {} -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); - } -} +BitrateEstimator::~BitrateEstimator() = default; -void AcknowledgedBitrateEstimator::Update(int64_t now_ms, int bytes) { +void BitrateEstimator::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. @@ -78,9 +64,9 @@ void AcknowledgedBitrateEstimator::Update(int64_t now_ms, int bytes) { bitrate_estimate_ * 1000); } -float AcknowledgedBitrateEstimator::UpdateWindow(int64_t now_ms, - int bytes, - int rate_window_ms) { +float BitrateEstimator::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; @@ -106,7 +92,7 @@ float AcknowledgedBitrateEstimator::UpdateWindow(int64_t now_ms, return bitrate_sample; } -rtc::Optional AcknowledgedBitrateEstimator::bitrate_bps() const { +rtc::Optional BitrateEstimator::bitrate_bps() const { if (bitrate_estimate_ < 0.f) return rtc::Optional(); return rtc::Optional(bitrate_estimate_ * 1000); diff --git a/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h b/webrtc/modules/congestion_controller/bitrate_estimator.h similarity index 66% rename from webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h rename to webrtc/modules/congestion_controller/bitrate_estimator.h index 7a9d669a1c..7d562081b2 100644 --- a/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h +++ b/webrtc/modules/congestion_controller/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_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ -#define WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ +#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_BITRATE_ESTIMATOR_H_ +#define WEBRTC_MODULES_CONGESTION_CONTROLLER_BITRATE_ESTIMATOR_H_ #include @@ -17,25 +17,21 @@ 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 AcknowledgedBitrateEstimator { +class BitrateEstimator { public: - AcknowledgedBitrateEstimator(); + BitrateEstimator(); + virtual ~BitrateEstimator(); + virtual void Update(int64_t now_ms, int bytes); - void IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector); - rtc::Optional bitrate_bps() const; + virtual 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_; @@ -45,4 +41,4 @@ class AcknowledgedBitrateEstimator { } // namespace webrtc -#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ +#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_BITRATE_ESTIMATOR_H_ 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 62c9f881bc..288886d450 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); + acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets, false); 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); + acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets, false); 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 a12bee52b9..b7b7905866 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/acknowledge_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/acknowledged_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 d5f966aeaa..ce6b96b0b4 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/acknowledge_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h" #include "webrtc/modules/congestion_controller/probe_controller.h" #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h" @@ -89,7 +89,8 @@ 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)), @@ -278,7 +279,8 @@ void SendSideCongestionController::OnTransportFeedback( transport_feedback_adapter_.GetTransportFeedbackVector()); SortPacketFeedbackVector(&feedback_vector); acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( - feedback_vector); + feedback_vector, + static_cast(pacer_->GetApplicationLimitedRegionStartTime())); 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 905659d0ee..a6e857f7fe 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); + packet_feedback_vector, false); 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 d2aa8de930..ed6a7cdee8 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/acknowledge_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h"