From 5bbf43f9d480f89fa57bbdbff0b028a413202695 Mon Sep 17 00:00:00 2001 From: "elad.alon" Date: Thu, 9 Mar 2017 06:40:08 -0800 Subject: [PATCH] Move delay_based_bwe_ into CongestionController BUG=webrtc:7058 Review-Url: https://codereview.webrtc.org/2725823002 Cr-Commit-Position: refs/heads/master@{#17146} --- webrtc/audio/audio_send_stream.cc | 3 +- webrtc/audio/audio_send_stream_unittest.cc | 6 +- webrtc/modules/congestion_controller/BUILD.gn | 2 + .../congestion_controller.cc | 80 ++++++-- .../congestion_controller_unittest.cc | 187 ++++++++++++++++- .../congestion_controller_unittests_helper.cc | 42 ++++ .../congestion_controller_unittests_helper.h | 23 +++ .../include/congestion_controller.h | 32 ++- .../transport_feedback_adapter.cc | 50 +---- .../transport_feedback_adapter.h | 38 +--- .../transport_feedback_adapter_unittest.cc | 190 ++---------------- webrtc/tools/event_log_visualizer/analyzer.cc | 46 +---- webrtc/video/video_send_stream.cc | 2 +- 13 files changed, 370 insertions(+), 331 deletions(-) create mode 100644 webrtc/modules/congestion_controller/congestion_controller_unittests_helper.cc create mode 100644 webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index 2d0c99b33a..438d1cc78a 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -86,8 +86,7 @@ AudioSendStream::AudioSendStream( } } channel_proxy_->RegisterSenderCongestionControlObjects( - congestion_controller->pacer(), - congestion_controller->GetTransportFeedbackObserver(), packet_router, + congestion_controller->pacer(), congestion_controller, packet_router, bandwidth_observer_.get()); if (!SetupSendCodec()) { LOG(LS_ERROR) << "Failed to set up send codec state."; diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index 1a838d891b..318ea955cf 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -151,15 +151,13 @@ struct ConfigHelper { .Times(1); EXPECT_CALL(*channel_proxy_, RegisterSenderCongestionControlObjects( - congestion_controller_.pacer(), - congestion_controller_.GetTransportFeedbackObserver(), + congestion_controller_.pacer(), &congestion_controller_, packet_router(), Ne(nullptr))) .Times(1); } else { EXPECT_CALL(*channel_proxy_, RegisterSenderCongestionControlObjects( - congestion_controller_.pacer(), - congestion_controller_.GetTransportFeedbackObserver(), + congestion_controller_.pacer(), &congestion_controller_, packet_router(), Eq(nullptr))) .Times(1); } diff --git a/webrtc/modules/congestion_controller/BUILD.gn b/webrtc/modules/congestion_controller/BUILD.gn index 787464aae2..2d3658845a 100644 --- a/webrtc/modules/congestion_controller/BUILD.gn +++ b/webrtc/modules/congestion_controller/BUILD.gn @@ -62,6 +62,8 @@ if (rtc_include_tests) { testonly = true sources = [ "congestion_controller_unittest.cc", + "congestion_controller_unittests_helper.cc", + "congestion_controller_unittests_helper.h", "delay_based_bwe_unittest.cc", "delay_based_bwe_unittest_helper.cc", "delay_based_bwe_unittest_helper.h", diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index bc2f29cf0c..6ed64a299d 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -160,6 +160,7 @@ CongestionController::CongestionController( std::unique_ptr pacer) : clock_(clock), observer_(observer), + event_log_(event_log), packet_router_(packet_router), pacer_(std::move(pacer)), bitrate_controller_( @@ -169,15 +170,16 @@ CongestionController::CongestionController( new RateLimiter(clock, kRetransmitWindowSizeMs)), remote_bitrate_estimator_(remote_bitrate_observer, clock_), remote_estimator_proxy_(clock_, packet_router_), - transport_feedback_adapter_(event_log, clock_, bitrate_controller_.get()), + transport_feedback_adapter_(clock_), min_bitrate_bps_(congestion_controller::GetMinBitrateBps()), max_bitrate_bps_(0), last_reported_bitrate_bps_(0), last_reported_fraction_loss_(0), last_reported_rtt_(0), - network_state_(kNetworkUp) { - transport_feedback_adapter_.InitBwe(); - transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_); + network_state_(kNetworkUp), + delay_based_bwe_(new DelayBasedBwe(event_log_, clock_)) { + delay_based_bwe_->SetMinBitrate(min_bitrate_bps_); + worker_thread_checker_.DetachFromThread(); } CongestionController::~CongestionController() {} @@ -210,9 +212,12 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps, remote_bitrate_estimator_.SetMinBitrate(min_bitrate_bps); min_bitrate_bps_ = min_bitrate_bps; - if (start_bitrate_bps > 0) - transport_feedback_adapter_.SetStartBitrate(start_bitrate_bps); - transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_); + { + rtc::CritScope cs(&bwe_lock_); + if (start_bitrate_bps > 0) + delay_based_bwe_->SetStartBitrate(start_bitrate_bps); + delay_based_bwe_->SetMinBitrate(min_bitrate_bps_); + } MaybeTriggerOnNetworkChanged(); } @@ -231,9 +236,12 @@ void CongestionController::ResetBweAndBitrates(int bitrate_bps, remote_bitrate_estimator_.SetMinBitrate(min_bitrate_bps); transport_feedback_adapter_.ClearSendTimeHistory(); - transport_feedback_adapter_.InitBwe(); - transport_feedback_adapter_.SetStartBitrate(bitrate_bps); - transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps); + { + rtc::CritScope cs(&bwe_lock_); + delay_based_bwe_.reset(new DelayBasedBwe(event_log_, clock_)); + delay_based_bwe_->SetStartBitrate(bitrate_bps); + delay_based_bwe_->SetMinBitrate(min_bitrate_bps); + } probe_controller_->Reset(); probe_controller_->SetBitrates(min_bitrate_bps, bitrate_bps, max_bitrate_bps); @@ -254,11 +262,6 @@ RemoteBitrateEstimator* CongestionController::GetRemoteBitrateEstimator( } } -TransportFeedbackObserver* -CongestionController::GetTransportFeedbackObserver() { - return &transport_feedback_adapter_; -} - RateLimiter* CongestionController::GetRetransmissionRateLimiter() { return retransmission_rate_limiter_.get(); } @@ -286,7 +289,7 @@ void CongestionController::SignalNetworkState(NetworkState state) { pacer_->Pause(); } { - rtc::CritScope cs(&critsect_); + rtc::CritScope cs(&network_state_lock_); network_state_ = state; } probe_controller_->OnNetworkStateChanged(state); @@ -310,7 +313,10 @@ 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); + { + rtc::CritScope cs(&bwe_lock_); + delay_based_bwe_->OnRttUpdate(avg_rtt_ms, max_rtt_ms); + } } int64_t CongestionController::TimeUntilNextProcess() { @@ -325,6 +331,32 @@ void CongestionController::Process() { MaybeTriggerOnNetworkChanged(); } +void CongestionController::AddPacket(uint16_t sequence_number, + size_t length, + const PacedPacketInfo& pacing_info) { + transport_feedback_adapter_.AddPacket(sequence_number, length, pacing_info); +} + +void CongestionController::OnTransportFeedback( + const rtcp::TransportFeedback& feedback) { + RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); + transport_feedback_adapter_.OnTransportFeedback(feedback); + DelayBasedBwe::Result result; + { + rtc::CritScope cs(&bwe_lock_); + result = delay_based_bwe_->IncomingPacketFeedbackVector( + transport_feedback_adapter_.GetTransportFeedbackVector()); + } + if (result.updated) + bitrate_controller_->OnDelayBasedBweResult(result); +} + +std::vector CongestionController::GetTransportFeedbackVector() + const { + RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); + return transport_feedback_adapter_.GetTransportFeedbackVector(); +} + void CongestionController::MaybeTriggerOnNetworkChanged() { // TODO(perkj): |observer_| can be nullptr if the ctor that accepts a // BitrateObserver is used. Remove this check once the ctor is removed. @@ -345,9 +377,13 @@ void CongestionController::MaybeTriggerOnNetworkChanged() { bitrate_bps = IsNetworkDown() || IsSendQueueFull() ? 0 : bitrate_bps; if (HasNetworkParametersToReportChanged(bitrate_bps, fraction_loss, rtt)) { - observer_->OnNetworkChanged( - bitrate_bps, fraction_loss, rtt, - transport_feedback_adapter_.GetProbingIntervalMs()); + int64_t probing_interval_ms; + { + rtc::CritScope cs(&bwe_lock_); + probing_interval_ms = delay_based_bwe_->GetProbingIntervalMs(); + } + observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt, + probing_interval_ms); remote_estimator_proxy_.OnBitrateChanged(bitrate_bps); } } @@ -356,7 +392,7 @@ bool CongestionController::HasNetworkParametersToReportChanged( uint32_t bitrate_bps, uint8_t fraction_loss, int64_t rtt) { - rtc::CritScope cs(&critsect_); + rtc::CritScope cs(&network_state_lock_); bool changed = last_reported_bitrate_bps_ != bitrate_bps || (bitrate_bps > 0 && (last_reported_fraction_loss_ != fraction_loss || @@ -376,7 +412,7 @@ bool CongestionController::IsSendQueueFull() const { } bool CongestionController::IsNetworkDown() const { - rtc::CritScope cs(&critsect_); + rtc::CritScope cs(&network_state_lock_); return network_state_ == kNetworkDown; } diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index 925631b9bb..802f9d1ea4 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -8,13 +8,15 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/logging/rtc_event_log/mock/mock_rtc_event_log.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" -#include "webrtc/modules/congestion_controller/include/congestion_controller.h" +#include "webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h" #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" #include "webrtc/modules/pacing/mock/mock_paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h" #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/system_wrappers/include/clock.h" #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" @@ -27,6 +29,8 @@ using testing::SaveArg; using testing::StrictMock; namespace { +const webrtc::PacedPacketInfo kPacingInfo0(0, 5, 2000); +const webrtc::PacedPacketInfo kPacingInfo1(1, 8, 4000); // Helper to convert some time format to resolution used in absolute send time // header extension, rounded upwards. |t| is the time to convert, in some @@ -43,7 +47,7 @@ namespace test { class CongestionControllerTest : public ::testing::Test { protected: - CongestionControllerTest() : clock_(123456) {} + CongestionControllerTest() : clock_(123456), target_bitrate_observer_(this) {} ~CongestionControllerTest() override {} void SetUp() override { @@ -64,8 +68,74 @@ class CongestionControllerTest : public ::testing::Test { controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps); } + // Custom setup - use an observer that tracks the target bitrate, without + // prescribing on which iterations it must change (like a mock would). + void TargetBitrateTrackingSetup() { + std::unique_ptr pacer(new NiceMock()); + controller_.reset(new CongestionController( + &clock_, &target_bitrate_observer_, &remote_bitrate_observer_, + &event_log_, &packet_router_, std::move(pacer))); + controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps); + } + + void OnSentPacket(const PacketFeedback& packet_feedback) { + controller_->AddPacket(packet_feedback.sequence_number, + packet_feedback.payload_size, + packet_feedback.pacing_info); + controller_->OnSentPacket(rtc::SentPacket(packet_feedback.sequence_number, + packet_feedback.send_time_ms)); + } + + // Allows us to track the target bitrate, without prescribing the exact + // iterations when this would hapen, like a mock would. + class TargetBitrateObserver : public CongestionController::Observer { + public: + explicit TargetBitrateObserver(CongestionControllerTest* owner) + : owner_(owner) {} + ~TargetBitrateObserver() override = default; + void OnNetworkChanged(uint32_t bitrate_bps, + uint8_t fraction_loss, // 0 - 255. + int64_t rtt_ms, + int64_t probing_interval_ms) override { + owner_->target_bitrate_bps_ = rtc::Optional(bitrate_bps); + } + + private: + CongestionControllerTest* owner_; + }; + + void PacketTransmissionAndFeedbackBlock(uint16_t* seq_num, + int64_t runtime_ms, + int64_t delay) { + int64_t delay_buildup = 0; + int64_t start_time_ms = clock_.TimeInMilliseconds(); + while (clock_.TimeInMilliseconds() - start_time_ms < runtime_ms) { + constexpr size_t kPayloadSize = 1000; + PacketFeedback packet(clock_.TimeInMilliseconds() + delay_buildup, + clock_.TimeInMilliseconds(), *seq_num, kPayloadSize, + PacedPacketInfo()); + delay_buildup += delay; // Delay has to increase, or it's just RTT. + OnSentPacket(packet); + // Create expected feedback and send into adapter. + std::unique_ptr feedback( + new rtcp::TransportFeedback()); + feedback->SetBase(packet.sequence_number, packet.arrival_time_ms * 1000); + EXPECT_TRUE(feedback->AddReceivedPacket(packet.sequence_number, + packet.arrival_time_ms * 1000)); + rtc::Buffer raw_packet = feedback->Build(); + feedback = rtcp::TransportFeedback::ParseFrom(raw_packet.data(), + raw_packet.size()); + EXPECT_TRUE(feedback.get() != nullptr); + controller_->OnTransportFeedback(*feedback.get()); + clock_.AdvanceTimeMilliseconds(50); + controller_->Process(); + ++(*seq_num); + } + } + SimulatedClock clock_; StrictMock observer_; + TargetBitrateObserver target_bitrate_observer_; NiceMock* pacer_; NiceMock remote_bitrate_observer_; NiceMock event_log_; @@ -73,6 +143,8 @@ class CongestionControllerTest : public ::testing::Test { PacketRouter packet_router_; std::unique_ptr controller_; const uint32_t kInitialBitrateBps = 60000; + + rtc::Optional target_bitrate_bps_; }; TEST_F(CongestionControllerTest, OnNetworkChanged) { @@ -253,5 +325,116 @@ TEST_F(CongestionControllerTest, ProbeOnBweReset) { 20 * kInitialBitrateBps); } +// Estimated bitrate reduced when the feedbacks arrive with such a long delay, +// that the send-time-history no longer holds the feedbacked packets. +TEST_F(CongestionControllerTest, LongFeedbackDelays) { + TargetBitrateTrackingSetup(); + + const int64_t kFeedbackTimeoutMs = 60001; + const int kMaxConsecutiveFailedLookups = 5; + for (int i = 0; i < kMaxConsecutiveFailedLookups; ++i) { + std::vector packets; + packets.push_back( + PacketFeedback(i * 100, 2 * i * 100, 0, 1500, kPacingInfo0)); + packets.push_back( + PacketFeedback(i * 100 + 10, 2 * i * 100 + 10, 1, 1500, kPacingInfo0)); + packets.push_back( + PacketFeedback(i * 100 + 20, 2 * i * 100 + 20, 2, 1500, kPacingInfo0)); + packets.push_back( + PacketFeedback(i * 100 + 30, 2 * i * 100 + 30, 3, 1500, kPacingInfo1)); + packets.push_back( + PacketFeedback(i * 100 + 40, 2 * i * 100 + 40, 4, 1500, kPacingInfo1)); + + for (const PacketFeedback& packet : packets) + OnSentPacket(packet); + + rtcp::TransportFeedback feedback; + feedback.SetBase(packets[0].sequence_number, + packets[0].arrival_time_ms * 1000); + + for (const PacketFeedback& packet : packets) { + EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number, + packet.arrival_time_ms * 1000)); + } + + feedback.Build(); + + clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs); + PacketFeedback later_packet(kFeedbackTimeoutMs + i * 100 + 40, + kFeedbackTimeoutMs + i * 200 + 40, 5, 1500, + kPacingInfo1); + OnSentPacket(later_packet); + + controller_->OnTransportFeedback(feedback); + + // Check that packets have timed out. + for (PacketFeedback& packet : packets) { + packet.send_time_ms = -1; + packet.payload_size = 0; + packet.pacing_info = PacedPacketInfo(); + } + ComparePacketFeedbackVectors(packets, + controller_->GetTransportFeedbackVector()); + } + + controller_->Process(); + + EXPECT_EQ(kInitialBitrateBps / 2, target_bitrate_bps_); + + // Test with feedback that isn't late enough to time out. + { + std::vector packets; + packets.push_back(PacketFeedback(100, 200, 0, 1500, kPacingInfo0)); + packets.push_back(PacketFeedback(110, 210, 1, 1500, kPacingInfo0)); + packets.push_back(PacketFeedback(120, 220, 2, 1500, kPacingInfo0)); + packets.push_back(PacketFeedback(130, 230, 3, 1500, kPacingInfo1)); + packets.push_back(PacketFeedback(140, 240, 4, 1500, kPacingInfo1)); + + for (const PacketFeedback& packet : packets) + OnSentPacket(packet); + + rtcp::TransportFeedback feedback; + feedback.SetBase(packets[0].sequence_number, + packets[0].arrival_time_ms * 1000); + + for (const PacketFeedback& packet : packets) { + EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number, + packet.arrival_time_ms * 1000)); + } + + feedback.Build(); + + clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs - 1); + PacketFeedback later_packet(kFeedbackTimeoutMs + 140, + kFeedbackTimeoutMs + 240, 5, 1500, + kPacingInfo1); + OnSentPacket(later_packet); + + controller_->OnTransportFeedback(feedback); + ComparePacketFeedbackVectors(packets, + controller_->GetTransportFeedbackVector()); + } +} + +// Bandwidth estimation is updated when feedbacks are received. +// Feedbacks which show an increasing delay cause the estimation to be reduced. +TEST_F(CongestionControllerTest, UpdatesDelayBasedEstimate) { + TargetBitrateTrackingSetup(); + + const int64_t kRunTimeMs = 6000; + uint16_t seq_num = 0; + + // The test must run and insert packets/feedback long enough that the + // BWE computes a valid estimate. This is first done in an environment which + // simulates no bandwidth limitation, and therefore not built-up delay. + PacketTransmissionAndFeedbackBlock(&seq_num, kRunTimeMs, 0); + ASSERT_TRUE(target_bitrate_bps_); + + // Repeat, but this time with a building delay, and make sure that the + // estimation is adjusted downwards. + uint32_t bitrate_before_delay = *target_bitrate_bps_; + PacketTransmissionAndFeedbackBlock(&seq_num, kRunTimeMs, 50); + EXPECT_LT(*target_bitrate_bps_, bitrate_before_delay); +} } // namespace test } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittests_helper.cc b/webrtc/modules/congestion_controller/congestion_controller_unittests_helper.cc new file mode 100644 index 0000000000..f70b360028 --- /dev/null +++ b/webrtc/modules/congestion_controller/congestion_controller_unittests_helper.cc @@ -0,0 +1,42 @@ +/* + * 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/congestion_controller_unittests_helper.h" + +#include "webrtc/base/checks.h" +#include "webrtc/test/gtest.h" + +namespace webrtc { +void ComparePacketFeedbackVectors(const std::vector& truth, + const std::vector& input) { + ASSERT_EQ(truth.size(), input.size()); + size_t len = truth.size(); + // truth contains the input data for the test, and input is what will be + // sent to the bandwidth estimator. truth.arrival_tims_ms is used to + // populate the transport feedback messages. As these times may be changed + // (because of resolution limits in the packets, and because of the time + // base adjustment performed by the TransportFeedbackAdapter at the first + // packet, the truth[x].arrival_time and input[x].arrival_time may not be + // equal. However, the difference must be the same for all x. + int64_t arrival_time_delta = + truth[0].arrival_time_ms - input[0].arrival_time_ms; + for (size_t i = 0; i < len; ++i) { + RTC_CHECK(truth[i].arrival_time_ms != PacketFeedback::kNotReceived); + if (input[i].arrival_time_ms != PacketFeedback::kNotReceived) { + EXPECT_EQ(truth[i].arrival_time_ms, + input[i].arrival_time_ms + arrival_time_delta); + } + EXPECT_EQ(truth[i].send_time_ms, input[i].send_time_ms); + EXPECT_EQ(truth[i].sequence_number, input[i].sequence_number); + EXPECT_EQ(truth[i].payload_size, input[i].payload_size); + EXPECT_EQ(truth[i].pacing_info, input[i].pacing_info); + } +} +} // namespace webrtc diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h b/webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h new file mode 100644 index 0000000000..474e9a7da6 --- /dev/null +++ b/webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h @@ -0,0 +1,23 @@ +/* + * 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_CONGESTION_CONTROLLER_UNITTESTS_HELPER_H_ +#define WEBRTC_MODULES_CONGESTION_CONTROLLER_CONGESTION_CONTROLLER_UNITTESTS_HELPER_H_ + +#include + +#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" + +namespace webrtc { +void ComparePacketFeedbackVectors(const std::vector& truth, + const std::vector& input); +} // namespace webrtc + +#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_CONGESTION_CONTROLLER_UNITTESTS_HELPER_H_ diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 500966b7c1..02f3e3429c 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -16,12 +16,14 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/criticalsection.h" +#include "webrtc/base/thread_checker.h" #include "webrtc/common_types.h" +#include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/congestion_controller/transport_feedback_adapter.h" #include "webrtc/modules/include/module.h" #include "webrtc/modules/include/module_common_types.h" -#include "webrtc/modules/pacing/packet_router.h" #include "webrtc/modules/pacing/paced_sender.h" +#include "webrtc/modules/pacing/packet_router.h" #include "webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h" namespace rtc { @@ -37,9 +39,10 @@ class RateLimiter; class RemoteBitrateEstimator; class RemoteBitrateObserver; class RtcEventLog; -class TransportFeedbackObserver; -class CongestionController : public CallStatsObserver, public Module { +class CongestionController : public CallStatsObserver, + public Module, + public TransportFeedbackObserver { public: // Observer class for bitrate changes announced due to change in bandwidth // estimate or due to that the send pacer is full. Fraction loss and rtt is @@ -91,7 +94,6 @@ class CongestionController : public CallStatsObserver, public Module { // TODO(nisse): Delete this accessor function. The pacer should be // internal to the congestion controller. virtual PacedSender* pacer() { return pacer_.get(); } - virtual TransportFeedbackObserver* GetTransportFeedbackObserver(); RateLimiter* GetRetransmissionRateLimiter(); void EnablePeriodicAlrProbing(bool enable); @@ -117,6 +119,13 @@ class CongestionController : public CallStatsObserver, public Module { int64_t TimeUntilNextProcess() override; void Process() override; + // Implements TransportFeedbackObserver. + void AddPacket(uint16_t sequence_number, + size_t length, + const PacedPacketInfo& pacing_info) override; + void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override; + std::vector GetTransportFeedbackVector() const override; + private: class WrappingBitrateEstimator : public RemoteBitrateEstimator { public: @@ -165,6 +174,7 @@ class CongestionController : public CallStatsObserver, public Module { int64_t rtt); Clock* const clock_; Observer* const observer_; + RtcEventLog* const event_log_; PacketRouter* const packet_router_; const std::unique_ptr pacer_; const std::unique_ptr bitrate_controller_; @@ -175,11 +185,15 @@ class CongestionController : public CallStatsObserver, public Module { TransportFeedbackAdapter transport_feedback_adapter_; int min_bitrate_bps_; int max_bitrate_bps_; - rtc::CriticalSection critsect_; - uint32_t last_reported_bitrate_bps_ GUARDED_BY(critsect_); - uint8_t last_reported_fraction_loss_ GUARDED_BY(critsect_); - int64_t last_reported_rtt_ GUARDED_BY(critsect_); - NetworkState network_state_ GUARDED_BY(critsect_); + rtc::CriticalSection network_state_lock_; + uint32_t last_reported_bitrate_bps_ GUARDED_BY(network_state_lock_); + uint8_t last_reported_fraction_loss_ GUARDED_BY(network_state_lock_); + int64_t last_reported_rtt_ GUARDED_BY(network_state_lock_); + NetworkState network_state_ GUARDED_BY(network_state_lock_); + rtc::CriticalSection bwe_lock_; + std::unique_ptr delay_based_bwe_ GUARDED_BY(bwe_lock_); + + rtc::ThreadChecker worker_thread_checker_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(CongestionController); }; diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc index d158eb97a1..3d1601cff8 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -10,17 +10,11 @@ #include "webrtc/modules/congestion_controller/transport_feedback_adapter.h" -#include -#include - #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/base/mod_ops.h" -#include "webrtc/logging/rtc_event_log/rtc_event_log.h" -#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" -#include "webrtc/modules/utility/include/process_thread.h" #include "webrtc/system_wrappers/include/field_trial.h" namespace webrtc { @@ -31,27 +25,17 @@ const int64_t kBaseTimestampScaleFactor = rtcp::TransportFeedback::kDeltaScaleFactor * (1 << 8); const int64_t kBaseTimestampRangeSizeUs = kBaseTimestampScaleFactor * (1 << 24); -TransportFeedbackAdapter::TransportFeedbackAdapter( - RtcEventLog* event_log, - Clock* clock, - BitrateController* bitrate_controller) +TransportFeedbackAdapter::TransportFeedbackAdapter(Clock* clock) : send_side_bwe_with_overhead_( webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")), transport_overhead_bytes_per_packet_(0), send_time_history_(clock, kSendTimeHistoryWindowMs), - event_log_(event_log), clock_(clock), current_offset_ms_(kNoTimestamp), - last_timestamp_us_(kNoTimestamp), - bitrate_controller_(bitrate_controller) {} + last_timestamp_us_(kNoTimestamp) {} TransportFeedbackAdapter::~TransportFeedbackAdapter() {} -void TransportFeedbackAdapter::InitBwe() { - rtc::CritScope cs(&bwe_lock_); - delay_based_bwe_.reset(new DelayBasedBwe(event_log_, clock_)); -} - void TransportFeedbackAdapter::AddPacket(uint16_t sequence_number, size_t length, const PacedPacketInfo& pacing_info) { @@ -68,27 +52,12 @@ void TransportFeedbackAdapter::OnSentPacket(uint16_t sequence_number, send_time_history_.OnSentPacket(sequence_number, send_time_ms); } -void TransportFeedbackAdapter::SetStartBitrate(int start_bitrate_bps) { - rtc::CritScope cs(&bwe_lock_); - delay_based_bwe_->SetStartBitrate(start_bitrate_bps); -} - -void TransportFeedbackAdapter::SetMinBitrate(int min_bitrate_bps) { - rtc::CritScope cs(&bwe_lock_); - delay_based_bwe_->SetMinBitrate(min_bitrate_bps); -} - void TransportFeedbackAdapter::SetTransportOverhead( int transport_overhead_bytes_per_packet) { rtc::CritScope cs(&lock_); transport_overhead_bytes_per_packet_ = transport_overhead_bytes_per_packet; } -int64_t TransportFeedbackAdapter::GetProbingIntervalMs() const { - rtc::CritScope cs(&bwe_lock_); - return delay_based_bwe_->GetProbingIntervalMs(); -} - void TransportFeedbackAdapter::ClearSendTimeHistory() { rtc::CritScope cs(&lock_); send_time_history_.Clear(); @@ -172,25 +141,10 @@ std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( void TransportFeedbackAdapter::OnTransportFeedback( const rtcp::TransportFeedback& feedback) { last_packet_feedback_vector_ = GetPacketFeedbackVector(feedback); - DelayBasedBwe::Result result; - { - rtc::CritScope cs(&bwe_lock_); - result = delay_based_bwe_->IncomingPacketFeedbackVector( - last_packet_feedback_vector_); - } - if (result.updated) - bitrate_controller_->OnDelayBasedBweResult(result); } std::vector TransportFeedbackAdapter::GetTransportFeedbackVector() const { return last_packet_feedback_vector_; } - -void TransportFeedbackAdapter::OnRttUpdate(int64_t avg_rtt_ms, - int64_t max_rtt_ms) { - rtc::CritScope cs(&bwe_lock_); - delay_based_bwe_->OnRttUpdate(avg_rtt_ms, max_rtt_ms); -} - } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.h b/webrtc/modules/congestion_controller/transport_feedback_adapter.h index 0819ad969c..8a4856c980 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h @@ -11,52 +11,38 @@ #ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_ #define WEBRTC_MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_ -#include #include #include "webrtc/base/criticalsection.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/base/thread_checker.h" -#include "webrtc/modules/congestion_controller/delay_based_bwe.h" -#include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" +#include "webrtc/system_wrappers/include/clock.h" namespace webrtc { -class BitrateController; -class RtcEventLog; -class ProcessThread; +namespace rtcp { +class TransportFeedback; +} // namespace rtcp -class TransportFeedbackAdapter : public TransportFeedbackObserver, - public CallStatsObserver { +class TransportFeedbackAdapter { public: - TransportFeedbackAdapter(RtcEventLog* event_log, - Clock* clock, - BitrateController* bitrate_controller); + explicit TransportFeedbackAdapter(Clock* clock); virtual ~TransportFeedbackAdapter(); - void InitBwe(); - // Implements TransportFeedbackObserver. void AddPacket(uint16_t sequence_number, size_t length, - const PacedPacketInfo& pacing_info) override; + const PacedPacketInfo& pacing_info); void OnSentPacket(uint16_t sequence_number, int64_t send_time_ms); // TODO(holmer): This method should return DelayBasedBwe::Result so that we // can get rid of the dependency on BitrateController. Requires changes // to the CongestionController interface. - void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override; - std::vector GetTransportFeedbackVector() const override; + void OnTransportFeedback(const rtcp::TransportFeedback& feedback); + std::vector GetTransportFeedbackVector() const; - // Implements CallStatsObserver. - void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; - - void SetStartBitrate(int start_bitrate_bps); - void SetMinBitrate(int min_bitrate_bps); void SetTransportOverhead(int transport_overhead_bytes_per_packet); - int64_t GetProbingIntervalMs() const; - void ClearSendTimeHistory(); private: @@ -65,15 +51,11 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver, const bool send_side_bwe_with_overhead_; rtc::CriticalSection lock_; - rtc::CriticalSection bwe_lock_; int transport_overhead_bytes_per_packet_ GUARDED_BY(&lock_); SendTimeHistory send_time_history_ GUARDED_BY(&lock_); - std::unique_ptr delay_based_bwe_ GUARDED_BY(&bwe_lock_); - RtcEventLog* const event_log_; - Clock* const clock_; + const Clock* const clock_; int64_t current_offset_ms_; int64_t last_timestamp_us_; - BitrateController* const bitrate_controller_; std::vector last_packet_feedback_vector_; }; diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc index 42effebcf9..bf4dbcae84 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc @@ -15,6 +15,7 @@ #include "webrtc/base/checks.h" #include "webrtc/base/safe_conversions.h" #include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h" +#include "webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h" #include "webrtc/modules/congestion_controller/transport_feedback_adapter.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" @@ -39,69 +40,23 @@ namespace test { class TransportFeedbackAdapterTest : public ::testing::Test { public: - TransportFeedbackAdapterTest() - : clock_(0), bitrate_controller_(this), target_bitrate_bps_(0) {} + TransportFeedbackAdapterTest() : clock_(0) {} virtual ~TransportFeedbackAdapterTest() {} virtual void SetUp() { - adapter_.reset( - new TransportFeedbackAdapter(nullptr, &clock_, &bitrate_controller_)); - adapter_->InitBwe(); - adapter_->SetStartBitrate(300000); + adapter_.reset(new TransportFeedbackAdapter(&clock_)); } virtual void TearDown() { adapter_.reset(); } protected: - // Proxy class used since TransportFeedbackAdapter will own the instance - // passed at construction. - class MockBitrateControllerAdapter : public MockBitrateController { - public: - explicit MockBitrateControllerAdapter(TransportFeedbackAdapterTest* owner) - : MockBitrateController(), owner_(owner) {} - - ~MockBitrateControllerAdapter() override {} - - void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override { - owner_->target_bitrate_bps_ = result.target_bitrate_bps; - } - - TransportFeedbackAdapterTest* const owner_; - }; - void OnReceivedEstimatedBitrate(uint32_t bitrate) {} void OnReceivedRtcpReceiverReport(const ReportBlockList& report_blocks, int64_t rtt, int64_t now_ms) {} - void ComparePacketVectors(const std::vector& truth, - const std::vector& input) { - ASSERT_EQ(truth.size(), input.size()); - size_t len = truth.size(); - // truth contains the input data for the test, and input is what will be - // sent to the bandwidth estimator. truth.arrival_tims_ms is used to - // populate the transport feedback messages. As these times may be changed - // (because of resolution limits in the packets, and because of the time - // base adjustment performed by the TransportFeedbackAdapter at the first - // packet, the truth[x].arrival_time and input[x].arrival_time may not be - // equal. However, the difference must be the same for all x. - int64_t arrival_time_delta = - truth[0].arrival_time_ms - input[0].arrival_time_ms; - for (size_t i = 0; i < len; ++i) { - RTC_CHECK(truth[i].arrival_time_ms != PacketFeedback::kNotReceived); - if (input[i].arrival_time_ms != PacketFeedback::kNotReceived) { - EXPECT_EQ(truth[i].arrival_time_ms, - input[i].arrival_time_ms + arrival_time_delta); - } - EXPECT_EQ(truth[i].send_time_ms, input[i].send_time_ms); - EXPECT_EQ(truth[i].sequence_number, input[i].sequence_number); - EXPECT_EQ(truth[i].payload_size, input[i].payload_size); - EXPECT_EQ(truth[i].pacing_info, input[i].pacing_info); - } - } - void OnSentPacket(const PacketFeedback& packet_feedback) { adapter_->AddPacket(packet_feedback.sequence_number, packet_feedback.payload_size, @@ -111,10 +66,7 @@ class TransportFeedbackAdapterTest : public ::testing::Test { } SimulatedClock clock_; - MockBitrateControllerAdapter bitrate_controller_; std::unique_ptr adapter_; - - uint32_t target_bitrate_bps_; }; TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) { @@ -140,7 +92,7 @@ TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) { feedback.Build(); adapter_->OnTransportFeedback(feedback); - ComparePacketVectors(packets, adapter_->GetTransportFeedbackVector()); + ComparePacketFeedbackVectors(packets, adapter_->GetTransportFeedbackVector()); } TEST_F(TransportFeedbackAdapterTest, FeedbackVectorReportsUnreceived) { @@ -175,91 +127,8 @@ TEST_F(TransportFeedbackAdapterTest, FeedbackVectorReportsUnreceived) { feedback.Build(); adapter_->OnTransportFeedback(feedback); - ComparePacketVectors(sent_packets, adapter_->GetTransportFeedbackVector()); -} - -TEST_F(TransportFeedbackAdapterTest, LongFeedbackDelays) { - const int64_t kFeedbackTimeoutMs = 60001; - const int kMaxConsecutiveFailedLookups = 5; - for (int i = 0; i < kMaxConsecutiveFailedLookups; ++i) { - std::vector packets; - packets.push_back( - PacketFeedback(i * 100, 2 * i * 100, 0, 1500, kPacingInfo0)); - packets.push_back( - PacketFeedback(i * 100 + 10, 2 * i * 100 + 10, 1, 1500, kPacingInfo0)); - packets.push_back( - PacketFeedback(i * 100 + 20, 2 * i * 100 + 20, 2, 1500, kPacingInfo0)); - packets.push_back( - PacketFeedback(i * 100 + 30, 2 * i * 100 + 30, 3, 1500, kPacingInfo1)); - packets.push_back( - PacketFeedback(i * 100 + 40, 2 * i * 100 + 40, 4, 1500, kPacingInfo1)); - - for (const PacketFeedback& packet : packets) - OnSentPacket(packet); - - rtcp::TransportFeedback feedback; - feedback.SetBase(packets[0].sequence_number, - packets[0].arrival_time_ms * 1000); - - for (const PacketFeedback& packet : packets) { - EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number, - packet.arrival_time_ms * 1000)); - } - - feedback.Build(); - - clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs); - PacketFeedback later_packet(kFeedbackTimeoutMs + i * 100 + 40, - kFeedbackTimeoutMs + i * 200 + 40, 5, 1500, - kPacingInfo1); - OnSentPacket(later_packet); - - adapter_->OnTransportFeedback(feedback); - - // Check that packets have timed out. - for (PacketFeedback& packet : packets) { - packet.send_time_ms = -1; - packet.payload_size = 0; - packet.pacing_info = PacedPacketInfo(); - } - ComparePacketVectors(packets, adapter_->GetTransportFeedbackVector()); - } - - // Target bitrate should have halved due to feedback delays. - EXPECT_EQ(150000u, target_bitrate_bps_); - - // Test with feedback that isn't late enough to time out. - { - std::vector packets; - packets.push_back(PacketFeedback(100, 200, 0, 1500, kPacingInfo0)); - packets.push_back(PacketFeedback(110, 210, 1, 1500, kPacingInfo0)); - packets.push_back(PacketFeedback(120, 220, 2, 1500, kPacingInfo0)); - packets.push_back(PacketFeedback(130, 230, 3, 1500, kPacingInfo1)); - packets.push_back(PacketFeedback(140, 240, 4, 1500, kPacingInfo1)); - - for (const PacketFeedback& packet : packets) - OnSentPacket(packet); - - rtcp::TransportFeedback feedback; - feedback.SetBase(packets[0].sequence_number, - packets[0].arrival_time_ms * 1000); - - for (const PacketFeedback& packet : packets) { - EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number, - packet.arrival_time_ms * 1000)); - } - - feedback.Build(); - - clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs - 1); - PacketFeedback later_packet(kFeedbackTimeoutMs + 140, - kFeedbackTimeoutMs + 240, 5, 1500, - kPacingInfo1); - OnSentPacket(later_packet); - - adapter_->OnTransportFeedback(feedback); - ComparePacketVectors(packets, adapter_->GetTransportFeedbackVector()); - } + ComparePacketFeedbackVectors(sent_packets, + adapter_->GetTransportFeedbackVector()); } TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) { @@ -302,8 +171,8 @@ TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) { } adapter_->OnTransportFeedback(feedback); - ComparePacketVectors(expected_packets, - adapter_->GetTransportFeedbackVector()); + ComparePacketFeedbackVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); } TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { @@ -338,8 +207,8 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { expected_packets.push_back(packets[i]); adapter_->OnTransportFeedback(*feedback.get()); - ComparePacketVectors(expected_packets, - adapter_->GetTransportFeedbackVector()); + ComparePacketFeedbackVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); } } @@ -367,7 +236,7 @@ TEST_F(TransportFeedbackAdapterTest, HandlesArrivalReordering) { // assigned by the order of transmission). Reordering by some other criteria, // eg. arrival time, is up to the observers. adapter_->OnTransportFeedback(feedback); - ComparePacketVectors(packets, adapter_->GetTransportFeedbackVector()); + ComparePacketFeedbackVectors(packets, adapter_->GetTransportFeedbackVector()); } TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { @@ -430,7 +299,8 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { EXPECT_TRUE(feedback.get() != nullptr); adapter_->OnTransportFeedback(*feedback.get()); - ComparePacketVectors(sent_packets, adapter_->GetTransportFeedbackVector()); + ComparePacketFeedbackVectors(sent_packets, + adapter_->GetTransportFeedbackVector()); // Create a new feedback message and add the trailing item. feedback.reset(new rtcp::TransportFeedback()); @@ -447,39 +317,9 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { { std::vector expected_packets; expected_packets.push_back(packet_feedback); - ComparePacketVectors(expected_packets, - adapter_->GetTransportFeedbackVector()); + ComparePacketFeedbackVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); } } - -TEST_F(TransportFeedbackAdapterTest, UpdatesDelayBasedEstimate) { - uint16_t seq_num = 0; - size_t kPayloadSize = 1000; - // The test must run and insert packets/feedback long enough that the - // BWE computes a valid estimate. - const int64_t kRunTimeMs = 6000; - int64_t start_time_ms = clock_.TimeInMilliseconds(); - while (clock_.TimeInMilliseconds() - start_time_ms < kRunTimeMs) { - PacketFeedback packet(clock_.TimeInMilliseconds(), - clock_.TimeInMilliseconds(), seq_num, kPayloadSize, - PacedPacketInfo()); - OnSentPacket(packet); - // Create expected feedback and send into adapter. - std::unique_ptr feedback( - new rtcp::TransportFeedback()); - feedback->SetBase(packet.sequence_number, packet.arrival_time_ms * 1000); - EXPECT_TRUE(feedback->AddReceivedPacket(packet.sequence_number, - packet.arrival_time_ms * 1000)); - rtc::Buffer raw_packet = feedback->Build(); - feedback = rtcp::TransportFeedback::ParseFrom(raw_packet.data(), - raw_packet.size()); - EXPECT_TRUE(feedback.get() != nullptr); - adapter_->OnTransportFeedback(*feedback.get()); - clock_.AdvanceTimeMilliseconds(50); - ++seq_num; - } - EXPECT_GT(target_bitrate_bps_, 0u); -} - } // namespace test } // namespace webrtc diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index 15b3e821b8..e80ca38d72 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -24,7 +24,6 @@ #include "webrtc/call/audio_send_stream.h" #include "webrtc/call/call.h" #include "webrtc/common_types.h" -#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" @@ -1072,11 +1071,9 @@ void EventLogAnalyzer::CreateBweSimulationGraph(Plot* plot) { RTC_DCHECK_EQ(clock.TimeInMicroseconds(), NextRtcpTime()); const LoggedRtcpPacket& rtcp = *rtcp_iterator->second; if (rtcp.type == kRtcpTransportFeedback) { - TransportFeedbackObserver* observer = cc.GetTransportFeedbackObserver(); - observer->OnTransportFeedback(*static_cast( - rtcp.packet.get())); - std::vector feedback = - observer->GetTransportFeedbackVector(); + cc.OnTransportFeedback( + *static_cast(rtcp.packet.get())); + std::vector feedback = cc.GetTransportFeedbackVector(); SortPacketFeedbackVector(&feedback); rtc::Optional bitrate_bps; if (!feedback.empty()) { @@ -1098,9 +1095,8 @@ void EventLogAnalyzer::CreateBweSimulationGraph(Plot* plot) { const LoggedRtpPacket& rtp = *rtp_iterator->second; if (rtp.header.extension.hasTransportSequenceNumber) { RTC_DCHECK(rtp.header.extension.hasTransportSequenceNumber); - cc.GetTransportFeedbackObserver()->AddPacket( - rtp.header.extension.transportSequenceNumber, rtp.total_length, - PacedPacketInfo()); + cc.AddPacket(rtp.header.extension.transportSequenceNumber, + rtp.total_length, PacedPacketInfo()); rtc::SentPacket sent_packet( rtp.header.extension.transportSequenceNumber, rtp.timestamp / 1000); cc.OnSentPacket(sent_packet); @@ -1130,34 +1126,6 @@ void EventLogAnalyzer::CreateBweSimulationGraph(Plot* plot) { plot->SetTitle("Simulated BWE behavior"); } -// TODO(holmer): Remove once TransportFeedbackAdapter no longer needs a -// BitrateController. -class NullBitrateController : public BitrateController { - public: - ~NullBitrateController() override {} - RtcpBandwidthObserver* CreateRtcpBandwidthObserver() override { - return nullptr; - } - void SetStartBitrate(int start_bitrate_bps) override {} - void SetMinMaxBitrate(int min_bitrate_bps, int max_bitrate_bps) override {} - void SetBitrates(int start_bitrate_bps, - int min_bitrate_bps, - int max_bitrate_bps) override {} - void ResetBitrates(int bitrate_bps, - int min_bitrate_bps, - int max_bitrate_bps) override {} - void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override {} - bool AvailableBandwidth(uint32_t* bandwidth) const override { return false; } - void SetReservedBitrate(uint32_t reserved_bitrate_bps) override {} - bool GetNetworkParameters(uint32_t* bitrate, - uint8_t* fraction_loss, - int64_t* rtt) override { - return false; - } - int64_t TimeUntilNextProcess() override { return 0; } - void Process() override {} -}; - void EventLogAnalyzer::CreateNetworkDelayFeedbackGraph(Plot* plot) { std::map outgoing_rtp; std::map incoming_rtcp; @@ -1178,9 +1146,7 @@ void EventLogAnalyzer::CreateNetworkDelayFeedbackGraph(Plot* plot) { } SimulatedClock clock(0); - NullBitrateController null_controller; - TransportFeedbackAdapter feedback_adapter(nullptr, &clock, &null_controller); - feedback_adapter.InitBwe(); + TransportFeedbackAdapter feedback_adapter(&clock); TimeSeries time_series; time_series.label = "Network Delay Change"; diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index bc7338e5ee..9e4dff6238 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -789,7 +789,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( config_->send_transport, &encoder_feedback_, bandwidth_observer_.get(), - congestion_controller_->GetTransportFeedbackObserver(), + congestion_controller_, call_stats_->rtcp_rtt_stats(), congestion_controller_->pacer(), packet_router_,