diff --git a/api/transport/network_types.cc b/api/transport/network_types.cc index d0a0c4a05f..db2a30ba40 100644 --- a/api/transport/network_types.cc +++ b/api/transport/network_types.cc @@ -31,15 +31,6 @@ PacketResult::PacketResult() = default; PacketResult::PacketResult(const PacketResult& other) = default; PacketResult::~PacketResult() = default; -bool PacketResult::ReceiveTimeOrder::operator()(const PacketResult& lhs, - const PacketResult& rhs) { - if (lhs.receive_time != rhs.receive_time) - return lhs.receive_time < rhs.receive_time; - if (lhs.sent_packet.send_time != rhs.sent_packet.send_time) - return lhs.sent_packet.send_time < rhs.sent_packet.send_time; - return lhs.sent_packet.sequence_number < rhs.sent_packet.sequence_number; -} - TransportPacketsFeedback::TransportPacketsFeedback() = default; TransportPacketsFeedback::TransportPacketsFeedback( const TransportPacketsFeedback& other) = default; @@ -73,13 +64,23 @@ std::vector TransportPacketsFeedback::PacketsWithFeedback() std::vector TransportPacketsFeedback::SortedByReceiveTime() const { + class PacketResultComparator { + public: + inline bool operator()(const PacketResult& lhs, const PacketResult& rhs) { + if (lhs.receive_time != rhs.receive_time) + return lhs.receive_time < rhs.receive_time; + if (lhs.sent_packet.send_time != rhs.sent_packet.send_time) + return lhs.sent_packet.send_time < rhs.sent_packet.send_time; + return lhs.sent_packet.sequence_number < rhs.sent_packet.sequence_number; + } + }; std::vector res; for (const PacketResult& fb : packet_feedbacks) { if (fb.receive_time.IsFinite()) { res.push_back(fb); } } - std::sort(res.begin(), res.end(), PacketResult::ReceiveTimeOrder()); + std::sort(res.begin(), res.end(), PacketResultComparator()); return res; } diff --git a/api/transport/network_types.h b/api/transport/network_types.h index dcbe3c4edc..16c8211727 100644 --- a/api/transport/network_types.h +++ b/api/transport/network_types.h @@ -131,11 +131,6 @@ struct TransportLossReport { // Packet level feedback struct PacketResult { - class ReceiveTimeOrder { - public: - bool operator()(const PacketResult& lhs, const PacketResult& rhs); - }; - PacketResult(); PacketResult(const PacketResult&); ~PacketResult(); diff --git a/modules/congestion_controller/goog_cc/BUILD.gn b/modules/congestion_controller/goog_cc/BUILD.gn index 46e5bdba05..394844411b 100644 --- a/modules/congestion_controller/goog_cc/BUILD.gn +++ b/modules/congestion_controller/goog_cc/BUILD.gn @@ -52,6 +52,7 @@ rtc_static_library("goog_cc") { "../../../system_wrappers", "../../bitrate_controller", "../../remote_bitrate_estimator", + "../../rtp_rtcp:rtp_rtcp_format", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/types:optional", ] @@ -122,10 +123,8 @@ rtc_source_set("estimators") { deps = [ "../../../api:network_state_predictor_api", - "../../../api/transport:network_control", "../../../api/transport:webrtc_key_value_config", "../../../api/units:data_rate", - "../../../api/units:timestamp", "../../../logging:rtc_event_bwe", "../../../logging:rtc_event_log_api", "../../../rtc_base:checks", @@ -136,6 +135,7 @@ rtc_source_set("estimators") { "../../../rtc_base:safe_minmax", "../../../rtc_base/experiments:field_trial_parser", "../../remote_bitrate_estimator", + "../../rtp_rtcp:rtp_rtcp_format", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/types:optional", ] @@ -152,7 +152,6 @@ rtc_source_set("delay_based_bwe") { ":estimators", "../../../api:network_state_predictor_api", "../../../api/transport:network_control", - "../../../api/transport:network_control", "../../../api/transport:webrtc_key_value_config", "../../../logging:rtc_event_bwe", "../../../logging:rtc_event_log_api", @@ -162,6 +161,7 @@ rtc_source_set("delay_based_bwe") { "../../../system_wrappers:metrics", "../../pacing", "../../remote_bitrate_estimator", + "../../rtp_rtcp:rtp_rtcp_format", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/types:optional", ] @@ -256,6 +256,8 @@ if (rtc_include_tests) { "../../../test:test_support", "../../../test/scenario", "../../pacing", + "../../remote_bitrate_estimator", + "../../rtp_rtcp:rtp_rtcp_format", "//testing/gmock", "//third_party/abseil-cpp/absl/memory", ] diff --git a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc index 9129516a0d..4566984ee7 100644 --- a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc +++ b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc @@ -15,11 +15,18 @@ #include #include "absl/memory/memory.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/checks.h" #include "rtc_base/numerics/safe_conversions.h" namespace webrtc { +namespace { +bool IsInSendTimeHistory(const PacketFeedback& packet) { + return packet.send_time_ms != PacketFeedback::kNoSendTime; +} +} // namespace + AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator( const WebRtcKeyValueConfig* key_value_config) : AcknowledgedBitrateEstimator( @@ -34,36 +41,56 @@ AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator( : in_alr_(false), bitrate_estimator_(std::move(bitrate_estimator)) {} void AcknowledgedBitrateEstimator::IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector) { + const std::vector& packet_feedback_vector) { RTC_DCHECK(std::is_sorted(packet_feedback_vector.begin(), packet_feedback_vector.end(), - PacketResult::ReceiveTimeOrder())); + PacketFeedbackComparator())); for (const auto& packet : packet_feedback_vector) { - if (alr_ended_time_ && packet.sent_packet.send_time > *alr_ended_time_) { - bitrate_estimator_->ExpectFastRateChange(); - alr_ended_time_.reset(); + if (IsInSendTimeHistory(packet)) { + MaybeExpectFastRateChange(packet.send_time_ms); + int acknowledged_estimate = rtc::dchecked_cast(packet.payload_size); + acknowledged_estimate += packet.unacknowledged_data; + bitrate_estimator_->Update(packet.arrival_time_ms, acknowledged_estimate, + in_alr_); } - DataSize acknowledged_estimate = packet.sent_packet.size; - acknowledged_estimate += packet.sent_packet.prior_unacked_data; - bitrate_estimator_->Update(packet.receive_time, acknowledged_estimate, - in_alr_); } } +absl::optional AcknowledgedBitrateEstimator::bitrate_bps() const { + return bitrate_estimator_->bitrate_bps(); +} + +absl::optional AcknowledgedBitrateEstimator::PeekBps() const { + return bitrate_estimator_->PeekBps(); +} + absl::optional AcknowledgedBitrateEstimator::bitrate() const { - return bitrate_estimator_->bitrate(); + if (bitrate_bps()) + return DataRate::bps(*bitrate_bps()); + return absl::nullopt; } absl::optional AcknowledgedBitrateEstimator::PeekRate() const { - return bitrate_estimator_->PeekRate(); + if (PeekBps()) + return DataRate::bps(*PeekBps()); + return absl::nullopt; } -void AcknowledgedBitrateEstimator::SetAlrEndedTime(Timestamp alr_ended_time) { - alr_ended_time_.emplace(alr_ended_time); +void AcknowledgedBitrateEstimator::SetAlrEndedTimeMs( + int64_t alr_ended_time_ms) { + alr_ended_time_ms_.emplace(alr_ended_time_ms); } void AcknowledgedBitrateEstimator::SetAlr(bool in_alr) { in_alr_ = in_alr; } +void AcknowledgedBitrateEstimator::MaybeExpectFastRateChange( + int64_t packet_send_time_ms) { + if (alr_ended_time_ms_ && packet_send_time_ms > *alr_ended_time_ms_) { + bitrate_estimator_->ExpectFastRateChange(); + alr_ended_time_ms_.reset(); + } +} + } // namespace webrtc diff --git a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h index a1730075c0..986d17a24c 100644 --- a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h +++ b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h @@ -15,13 +15,14 @@ #include #include "absl/types/optional.h" -#include "api/transport/network_types.h" #include "api/transport/webrtc_key_value_config.h" #include "api/units/data_rate.h" #include "modules/congestion_controller/goog_cc/bitrate_estimator.h" namespace webrtc { +struct PacketFeedback; + class AcknowledgedBitrateEstimator { public: AcknowledgedBitrateEstimator( @@ -33,14 +34,17 @@ class AcknowledgedBitrateEstimator { ~AcknowledgedBitrateEstimator(); void IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector); + const std::vector& packet_feedback_vector); + absl::optional bitrate_bps() const; + absl::optional PeekBps() const; absl::optional bitrate() const; absl::optional PeekRate() const; void SetAlr(bool in_alr); - void SetAlrEndedTime(Timestamp alr_ended_time); + void SetAlrEndedTimeMs(int64_t alr_ended_time_ms); private: - absl::optional alr_ended_time_; + void MaybeExpectFastRateChange(int64_t packet_arrival_time_ms); + absl::optional alr_ended_time_ms_; bool in_alr_; std::unique_ptr bitrate_estimator_; }; diff --git a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator_unittest.cc b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator_unittest.cc index e16849d951..f406d97c36 100644 --- a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator_unittest.cc +++ b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator_unittest.cc @@ -14,6 +14,7 @@ #include "absl/memory/memory.h" #include "api/transport/field_trial_based_config.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/fake_clock.h" #include "test/gmock.h" #include "test/gtest.h" @@ -35,9 +36,8 @@ constexpr size_t kPayloadSize = 10; class MockBitrateEstimator : public BitrateEstimator { public: using BitrateEstimator::BitrateEstimator; - MOCK_METHOD3(Update, - void(Timestamp at_time, DataSize data_size, bool in_alr)); - MOCK_CONST_METHOD0(bitrate, absl::optional()); + MOCK_METHOD3(Update, void(int64_t now_ms, int bytes, bool in_alr)); + MOCK_CONST_METHOD0(bitrate_bps, absl::optional()); MOCK_METHOD0(ExpectFastRateChange, void()); }; @@ -58,38 +58,43 @@ AcknowledgedBitrateEstimatorTestStates CreateTestStates() { return states; } -std::vector CreateFeedbackVector() { - std::vector packet_feedback_vector(2); - packet_feedback_vector[0].receive_time = Timestamp::ms(kFirstArrivalTimeMs); - packet_feedback_vector[0].sent_packet.send_time = - Timestamp::ms(kFirstSendTimeMs); - packet_feedback_vector[0].sent_packet.sequence_number = kSequenceNumber; - packet_feedback_vector[0].sent_packet.size = DataSize::bytes(kPayloadSize); - packet_feedback_vector[1].receive_time = - Timestamp::ms(kFirstArrivalTimeMs + 10); - packet_feedback_vector[1].sent_packet.send_time = - Timestamp::ms(kFirstSendTimeMs + 10); - packet_feedback_vector[1].sent_packet.sequence_number = kSequenceNumber; - packet_feedback_vector[1].sent_packet.size = - DataSize::bytes(kPayloadSize + 10); +std::vector CreateFeedbackVector() { + std::vector packet_feedback_vector; + const PacedPacketInfo pacing_info; + packet_feedback_vector.push_back( + PacketFeedback(kFirstArrivalTimeMs, kFirstSendTimeMs, kSequenceNumber, + kPayloadSize, pacing_info)); + packet_feedback_vector.push_back( + PacketFeedback(kFirstArrivalTimeMs + 10, kFirstSendTimeMs + 10, + kSequenceNumber, kPayloadSize + 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(kFirstArrivalTimeMs, kSequenceNumber)); + EXPECT_CALL(*states.mock_bitrate_estimator, Update(_, _, _)).Times(0); + states.acknowledged_bitrate_estimator->IncomingPacketFeedbackVector( + packet_feedback_vector); +} + TEST(TestAcknowledgedBitrateEstimator, UpdateBandwidth) { auto states = CreateTestStates(); auto packet_feedback_vector = CreateFeedbackVector(); { InSequence dummy; EXPECT_CALL(*states.mock_bitrate_estimator, - Update(packet_feedback_vector[0].receive_time, - packet_feedback_vector[0].sent_packet.size, + Update(packet_feedback_vector[0].arrival_time_ms, + static_cast(packet_feedback_vector[0].payload_size), /*in_alr*/ false)) .Times(1); EXPECT_CALL(*states.mock_bitrate_estimator, - Update(packet_feedback_vector[1].receive_time, - packet_feedback_vector[1].sent_packet.size, + Update(packet_feedback_vector[1].arrival_time_ms, + static_cast(packet_feedback_vector[1].payload_size), /*in_alr*/ false)) .Times(1); } @@ -103,31 +108,31 @@ TEST(TestAcknowledgedBitrateEstimator, ExpectFastRateChangeWhenLeftAlr) { { InSequence dummy; EXPECT_CALL(*states.mock_bitrate_estimator, - Update(packet_feedback_vector[0].receive_time, - packet_feedback_vector[0].sent_packet.size, + Update(packet_feedback_vector[0].arrival_time_ms, + static_cast(packet_feedback_vector[0].payload_size), /*in_alr*/ false)) .Times(1); EXPECT_CALL(*states.mock_bitrate_estimator, ExpectFastRateChange()) .Times(1); EXPECT_CALL(*states.mock_bitrate_estimator, - Update(packet_feedback_vector[1].receive_time, - packet_feedback_vector[1].sent_packet.size, + Update(packet_feedback_vector[1].arrival_time_ms, + static_cast(packet_feedback_vector[1].payload_size), /*in_alr*/ false)) .Times(1); } - states.acknowledged_bitrate_estimator->SetAlrEndedTime( - Timestamp::ms(kFirstArrivalTimeMs + 1)); + states.acknowledged_bitrate_estimator->SetAlrEndedTimeMs(kFirstArrivalTimeMs + + 1); states.acknowledged_bitrate_estimator->IncomingPacketFeedbackVector( packet_feedback_vector); } TEST(TestAcknowledgedBitrateEstimator, ReturnBitrate) { auto states = CreateTestStates(); - absl::optional return_value = DataRate::kbps(42); - EXPECT_CALL(*states.mock_bitrate_estimator, bitrate()) + absl::optional return_value(42); + EXPECT_CALL(*states.mock_bitrate_estimator, bitrate_bps()) .Times(1) .WillOnce(Return(return_value)); - EXPECT_EQ(return_value, states.acknowledged_bitrate_estimator->bitrate()); + EXPECT_EQ(return_value, states.acknowledged_bitrate_estimator->bitrate_bps()); } } // namespace webrtc*/ diff --git a/modules/congestion_controller/goog_cc/bitrate_estimator.cc b/modules/congestion_controller/goog_cc/bitrate_estimator.cc index 68e924ea4d..a2c9857ccc 100644 --- a/modules/congestion_controller/goog_cc/bitrate_estimator.cc +++ b/modules/congestion_controller/goog_cc/bitrate_estimator.cc @@ -58,14 +58,13 @@ BitrateEstimator::BitrateEstimator(const WebRtcKeyValueConfig* key_value_config) BitrateEstimator::~BitrateEstimator() = default; -void BitrateEstimator::Update(Timestamp at_time, DataSize amount, bool in_alr) { +void BitrateEstimator::Update(int64_t now_ms, int bytes, bool in_alr) { int rate_window_ms = noninitial_window_ms_.Get(); // We use a larger window at the beginning to get a more stable sample that // we can use to initialize the estimate. if (bitrate_estimate_kbps_ < 0.f) rate_window_ms = initial_window_ms_.Get(); - float bitrate_sample_kbps = - UpdateWindow(at_time.ms(), amount.bytes(), rate_window_ms); + float bitrate_sample_kbps = UpdateWindow(now_ms, bytes, rate_window_ms); if (bitrate_sample_kbps < 0.0f) return; if (bitrate_estimate_kbps_ < 0.0f) { @@ -101,7 +100,7 @@ void BitrateEstimator::Update(Timestamp at_time, DataSize amount, bool in_alr) { std::max(bitrate_estimate_kbps_, estimate_floor_.Get().kbps()); bitrate_estimate_var_ = sample_var * pred_bitrate_estimate_var / (sample_var + pred_bitrate_estimate_var); - BWE_TEST_LOGGING_PLOT(1, "acknowledged_bitrate", at_time.ms(), + BWE_TEST_LOGGING_PLOT(1, "acknowledged_bitrate", now_ms, bitrate_estimate_kbps_ * 1000); } @@ -133,15 +132,15 @@ float BitrateEstimator::UpdateWindow(int64_t now_ms, return bitrate_sample; } -absl::optional BitrateEstimator::bitrate() const { +absl::optional BitrateEstimator::bitrate_bps() const { if (bitrate_estimate_kbps_ < 0.f) return absl::nullopt; - return DataRate::kbps(bitrate_estimate_kbps_); + return bitrate_estimate_kbps_ * 1000; } -absl::optional BitrateEstimator::PeekRate() const { +absl::optional BitrateEstimator::PeekBps() const { if (current_window_ms_ > 0) - return DataSize::bytes(sum_) / TimeDelta::ms(current_window_ms_); + return sum_ * 8000 / current_window_ms_; return absl::nullopt; } diff --git a/modules/congestion_controller/goog_cc/bitrate_estimator.h b/modules/congestion_controller/goog_cc/bitrate_estimator.h index c657dbcf8d..0ef429b16e 100644 --- a/modules/congestion_controller/goog_cc/bitrate_estimator.h +++ b/modules/congestion_controller/goog_cc/bitrate_estimator.h @@ -16,7 +16,6 @@ #include "absl/types/optional.h" #include "api/transport/webrtc_key_value_config.h" #include "api/units/data_rate.h" -#include "api/units/timestamp.h" #include "rtc_base/experiments/field_trial_parser.h" namespace webrtc { @@ -30,10 +29,10 @@ class BitrateEstimator { public: explicit BitrateEstimator(const WebRtcKeyValueConfig* key_value_config); virtual ~BitrateEstimator(); - virtual void Update(Timestamp at_time, DataSize amount, bool in_alr); + virtual void Update(int64_t now_ms, int bytes, bool in_alr); - virtual absl::optional bitrate() const; - absl::optional PeekRate() const; + virtual absl::optional bitrate_bps() const; + absl::optional PeekBps() const; virtual void ExpectFastRateChange(); diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc index bd140f67ea..0f60002764 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc @@ -79,14 +79,17 @@ DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config, DelayBasedBwe::~DelayBasedBwe() {} DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( - const TransportPacketsFeedback& msg, + const std::vector& packet_feedback_vector, absl::optional acked_bitrate, absl::optional probe_bitrate, absl::optional network_estimate, - bool in_alr) { + bool in_alr, + Timestamp at_time) { + RTC_DCHECK(std::is_sorted(packet_feedback_vector.begin(), + packet_feedback_vector.end(), + PacketFeedbackComparator())); RTC_DCHECK_RUNS_SERIALIZED(&network_race_); - auto packet_feedback_vector = msg.SortedByReceiveTime(); // TODO(holmer): An empty feedback vector here likely means that // all acks were too late and that the send time history had // timed out. We should reduce the rate when this occurs. @@ -105,8 +108,10 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( bool recovered_from_overuse = false; BandwidthUsage prev_detector_state = delay_detector_->State(); for (const auto& packet_feedback : packet_feedback_vector) { + if (packet_feedback.send_time_ms < 0) + continue; delayed_feedback = false; - IncomingPacketFeedback(packet_feedback, msg.feedback_time); + IncomingPacketFeedback(packet_feedback, at_time); if (prev_detector_state == BandwidthUsage::kBwUnderusing && delay_detector_->State() == BandwidthUsage::kBwNormal) { recovered_from_overuse = true; @@ -123,11 +128,12 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( rate_control_.SetNetworkStateEstimate(network_estimate); return MaybeUpdateEstimate(acked_bitrate, probe_bitrate, std::move(network_estimate), - recovered_from_overuse, in_alr, msg.feedback_time); + recovered_from_overuse, in_alr, at_time); } -void DelayBasedBwe::IncomingPacketFeedback(const PacketResult& packet_feedback, - Timestamp at_time) { +void DelayBasedBwe::IncomingPacketFeedback( + const PacketFeedback& packet_feedback, + Timestamp at_time) { // Reset if the stream has timed out. if (last_seen_packet_.IsInfinite() || at_time - last_seen_packet_ > kStreamTimeOut) { @@ -141,7 +147,7 @@ void DelayBasedBwe::IncomingPacketFeedback(const PacketResult& packet_feedback, uint32_t send_time_24bits = static_cast( - ((static_cast(packet_feedback.sent_packet.send_time.ms()) + ((static_cast(packet_feedback.send_time_ms) << kAbsSendTimeFraction) + 500) / 1000) & @@ -154,13 +160,11 @@ void DelayBasedBwe::IncomingPacketFeedback(const PacketResult& packet_feedback, int64_t t_delta = 0; int size_delta = 0; bool calculated_deltas = inter_arrival_->ComputeDeltas( - timestamp, packet_feedback.receive_time.ms(), at_time.ms(), - packet_feedback.sent_packet.size.bytes(), &ts_delta, &t_delta, - &size_delta); + timestamp, packet_feedback.arrival_time_ms, at_time.ms(), + packet_feedback.payload_size, &ts_delta, &t_delta, &size_delta); double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift); - delay_detector_->Update(t_delta, ts_delta_ms, - packet_feedback.sent_packet.send_time.ms(), - packet_feedback.receive_time.ms(), calculated_deltas); + delay_detector_->Update(t_delta, ts_delta_ms, packet_feedback.send_time_ms, + packet_feedback.arrival_time_ms, calculated_deltas); } DataRate DelayBasedBwe::TriggerOveruse(Timestamp at_time, diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.h b/modules/congestion_controller/goog_cc/delay_based_bwe.h index c24295d356..4841dde8de 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.h +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.h @@ -25,6 +25,7 @@ #include "modules/remote_bitrate_estimator/aimd_rate_control.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "modules/remote_bitrate_estimator/inter_arrival.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" // For PacketFeedback #include "rtc_base/constructor_magic.h" #include "rtc_base/race_checker.h" @@ -50,11 +51,12 @@ class DelayBasedBwe { virtual ~DelayBasedBwe(); Result IncomingPacketFeedbackVector( - const TransportPacketsFeedback& msg, + const std::vector& packet_feedback_vector, absl::optional acked_bitrate, absl::optional probe_bitrate, absl::optional network_estimate, - bool in_alr); + bool in_alr, + Timestamp at_time); void OnRttUpdate(TimeDelta avg_rtt); bool LatestEstimate(std::vector* ssrcs, DataRate* bitrate) const; void SetStartBitrate(DataRate start_bitrate); @@ -67,7 +69,7 @@ class DelayBasedBwe { private: friend class GoogCcStatePrinter; - void IncomingPacketFeedback(const PacketResult& packet_feedback, + void IncomingPacketFeedback(const PacketFeedback& packet_feedback, Timestamp at_time); Result MaybeUpdateEstimate( absl::optional acked_bitrate, diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc index 9d8d226c61..9a602c2391 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc @@ -24,16 +24,40 @@ constexpr int kNumProbesCluster1 = 8; const PacedPacketInfo kPacingInfo0(0, kNumProbesCluster0, 2000); const PacedPacketInfo kPacingInfo1(1, kNumProbesCluster1, 4000); constexpr float kTargetUtilizationFraction = 0.95f; +constexpr Timestamp kDummyTimestamp = Timestamp::Seconds<1000>(); } // namespace +TEST_F(DelayBasedBweTest, NoCrashEmptyFeedback) { + std::vector packet_feedback_vector; + bitrate_estimator_->IncomingPacketFeedbackVector( + packet_feedback_vector, /*acked_bitrate*/ absl::nullopt, + /*probe_bitrate*/ absl::nullopt, /*network_estimate*/ absl::nullopt, + /*in_alr*/ false, kDummyTimestamp); +} + +TEST_F(DelayBasedBweTest, NoCrashOnlyLostFeedback) { + std::vector packet_feedback_vector; + packet_feedback_vector.push_back(PacketFeedback(PacketFeedback::kNotReceived, + PacketFeedback::kNoSendTime, + 0, 1500, PacedPacketInfo())); + packet_feedback_vector.push_back(PacketFeedback(PacketFeedback::kNotReceived, + PacketFeedback::kNoSendTime, + 1, 1500, PacedPacketInfo())); + bitrate_estimator_->IncomingPacketFeedbackVector( + packet_feedback_vector, /*acked_bitrate*/ absl::nullopt, + /*probe_bitrate*/ absl::nullopt, /*network_estimate*/ absl::nullopt, + /*in_alr*/ false, kDummyTimestamp); +} + TEST_F(DelayBasedBweTest, ProbeDetection) { int64_t now_ms = clock_.TimeInMilliseconds(); + uint16_t seq_num = 0; // First burst sent at 8 * 1000 / 10 = 800 kbps. for (int i = 0; i < kNumProbesCluster0; ++i) { clock_.AdvanceTimeMilliseconds(10); now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, now_ms, 1000, kPacingInfo0); + IncomingFeedback(now_ms, now_ms, seq_num++, 1000, kPacingInfo0); } EXPECT_TRUE(bitrate_observer_.updated()); @@ -41,7 +65,7 @@ TEST_F(DelayBasedBweTest, ProbeDetection) { for (int i = 0; i < kNumProbesCluster1; ++i) { clock_.AdvanceTimeMilliseconds(5); now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, now_ms, 1000, kPacingInfo1); + IncomingFeedback(now_ms, now_ms, seq_num++, 1000, kPacingInfo1); } EXPECT_TRUE(bitrate_observer_.updated()); @@ -50,15 +74,16 @@ TEST_F(DelayBasedBweTest, ProbeDetection) { TEST_F(DelayBasedBweTest, ProbeDetectionNonPacedPackets) { int64_t now_ms = clock_.TimeInMilliseconds(); + uint16_t seq_num = 0; // First burst sent at 8 * 1000 / 10 = 800 kbps, but with every other packet // not being paced which could mess things up. for (int i = 0; i < kNumProbesCluster0; ++i) { clock_.AdvanceTimeMilliseconds(5); now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, now_ms, 1000, kPacingInfo0); + IncomingFeedback(now_ms, now_ms, seq_num++, 1000, kPacingInfo0); // Non-paced packet, arriving 5 ms after. clock_.AdvanceTimeMilliseconds(5); - IncomingFeedback(now_ms, now_ms, 100, PacedPacketInfo()); + IncomingFeedback(now_ms, now_ms, seq_num++, 100, PacedPacketInfo()); } EXPECT_TRUE(bitrate_observer_.updated()); @@ -67,6 +92,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionNonPacedPackets) { TEST_F(DelayBasedBweTest, ProbeDetectionFasterArrival) { int64_t now_ms = clock_.TimeInMilliseconds(); + uint16_t seq_num = 0; // First burst sent at 8 * 1000 / 10 = 800 kbps. // Arriving at 8 * 1000 / 5 = 1600 kbps. int64_t send_time_ms = 0; @@ -74,7 +100,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionFasterArrival) { clock_.AdvanceTimeMilliseconds(1); send_time_ms += 10; now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, send_time_ms, 1000, kPacingInfo0); + IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, kPacingInfo0); } EXPECT_FALSE(bitrate_observer_.updated()); @@ -82,6 +108,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionFasterArrival) { TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrival) { int64_t now_ms = clock_.TimeInMilliseconds(); + uint16_t seq_num = 0; // First burst sent at 8 * 1000 / 5 = 1600 kbps. // Arriving at 8 * 1000 / 7 = 1142 kbps. // Since the receive rate is significantly below the send rate, we expect to @@ -91,7 +118,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrival) { clock_.AdvanceTimeMilliseconds(7); send_time_ms += 5; now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, send_time_ms, 1000, kPacingInfo1); + IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, kPacingInfo1); } EXPECT_TRUE(bitrate_observer_.updated()); @@ -101,6 +128,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrival) { TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) { int64_t now_ms = clock_.TimeInMilliseconds(); + uint16_t seq_num = 0; // Burst sent at 8 * 1000 / 1 = 8000 kbps. // Arriving at 8 * 1000 / 2 = 4000 kbps. // Since the receive rate is significantly below the send rate, we expect to @@ -110,7 +138,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) { clock_.AdvanceTimeMilliseconds(2); send_time_ms += 1; now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, send_time_ms, 1000, kPacingInfo1); + IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, kPacingInfo1); } EXPECT_TRUE(bitrate_observer_.updated()); @@ -135,7 +163,7 @@ TEST_F(DelayBasedBweTest, RateIncreaseReordering) { RateIncreaseReorderingTestHelper(730000); } TEST_F(DelayBasedBweTest, RateIncreaseRtpTimestamps) { - RateIncreaseRtpTimestampsTestHelper(622); + RateIncreaseRtpTimestampsTestHelper(627); } TEST_F(DelayBasedBweTest, CapacityDropOneStream) { @@ -194,7 +222,7 @@ TEST_F(DelayBasedBweTest, TestInitialOveruse) { // The purpose of this test is to ensure that we back down even if we don't // have any acknowledged bitrate estimate yet. Hence, if the test works // as expected, we should not have a measured bitrate yet. - EXPECT_FALSE(acknowledged_bitrate_estimator_->bitrate().has_value()); + EXPECT_FALSE(acknowledged_bitrate_estimator_->bitrate_bps().has_value()); if (overuse) { EXPECT_TRUE(bitrate_observer_.updated()); EXPECT_NEAR(bitrate_observer_.latest_bitrate(), kStartBitrate.bps() / 2, @@ -242,7 +270,7 @@ TEST_F(DelayBasedBweTestWithBackoffTimeoutExperiment, TestInitialOveruse) { // The purpose of this test is to ensure that we back down even if we don't // have any acknowledged bitrate estimate yet. Hence, if the test works // as expected, we should not have a measured bitrate yet. - EXPECT_FALSE(acknowledged_bitrate_estimator_->bitrate().has_value()); + EXPECT_FALSE(acknowledged_bitrate_estimator_->bitrate_bps().has_value()); if (overuse) { EXPECT_TRUE(bitrate_observer_.updated()); EXPECT_NEAR(bitrate_observer_.latest_bitrate(), kStartBitrate.bps() / 2, diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc index bc0be2b976..8baaf3d93b 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc @@ -28,13 +28,17 @@ constexpr int kInitialProbingPackets = 5; namespace test { void TestBitrateObserver::OnReceiveBitrateChanged( + const std::vector& ssrcs, uint32_t bitrate) { latest_bitrate_ = bitrate; updated_ = true; } RtpStream::RtpStream(int fps, int bitrate_bps) - : fps_(fps), bitrate_bps_(bitrate_bps), next_rtp_time_(0) { + : fps_(fps), + bitrate_bps_(bitrate_bps), + next_rtp_time_(0), + sequence_number_(0) { RTC_CHECK_GT(fps_, 0); } @@ -42,7 +46,7 @@ RtpStream::RtpStream(int fps, int bitrate_bps) // previous frame, no frame will be generated. The frame is split into // packets. int64_t RtpStream::GenerateFrame(int64_t time_now_us, - std::vector* packets) { + std::vector* packets) { if (time_now_us < next_rtp_time_) { return next_rtp_time_; } @@ -52,10 +56,9 @@ int64_t RtpStream::GenerateFrame(int64_t time_now_us, std::max((bits_per_frame + 4 * kMtu) / (8 * kMtu), 1u); size_t payload_size = (bits_per_frame + 4 * n_packets) / (8 * n_packets); for (size_t i = 0; i < n_packets; ++i) { - PacketResult packet; - packet.sent_packet.send_time = - Timestamp::us(time_now_us + kSendSideOffsetUs); - packet.sent_packet.size = DataSize::bytes(payload_size); + PacketFeedback packet(-1, sequence_number_++); + packet.send_time_ms = (time_now_us + kSendSideOffsetUs) / 1000; + packet.payload_size = payload_size; packets->push_back(packet); } next_rtp_time_ = time_now_us + (1000000 + fps_ / 2) / fps_; @@ -121,7 +124,7 @@ void StreamGenerator::SetBitrateBps(int bitrate_bps) { // TODO(holmer): Break out the channel simulation part from this class to make // it possible to simulate different types of channels. -int64_t StreamGenerator::GenerateFrame(std::vector* packets, +int64_t StreamGenerator::GenerateFrame(std::vector* packets, int64_t time_now_us) { RTC_CHECK(packets != NULL); RTC_CHECK(packets->empty()); @@ -130,15 +133,14 @@ int64_t StreamGenerator::GenerateFrame(std::vector* packets, std::min_element(streams_.begin(), streams_.end(), RtpStream::Compare); (*it)->GenerateFrame(time_now_us, packets); int i = 0; - for (PacketResult& packet : *packets) { + for (PacketFeedback& packet : *packets) { int capacity_bpus = capacity_ / 1000; int64_t required_network_time_us = - (8 * 1000 * packet.sent_packet.size.bytes() + capacity_bpus / 2) / - capacity_bpus; + (8 * 1000 * packet.payload_size + capacity_bpus / 2) / capacity_bpus; prev_arrival_time_us_ = std::max(time_now_us + required_network_time_us, prev_arrival_time_us_ + required_network_time_us); - packet.receive_time = Timestamp::us(prev_arrival_time_us_); + packet.arrival_time_ms = prev_arrival_time_us_ / 1000; ++i; } it = std::min_element(streams_.begin(), streams_.end(), RtpStream::Compare); @@ -185,38 +187,37 @@ const uint32_t DelayBasedBweTest::kDefaultSsrc = 0; void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, int64_t send_time_ms, + uint16_t sequence_number, size_t payload_size) { - IncomingFeedback(arrival_time_ms, send_time_ms, payload_size, + IncomingFeedback(arrival_time_ms, send_time_ms, sequence_number, payload_size, PacedPacketInfo()); } void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, int64_t send_time_ms, + uint16_t sequence_number, size_t payload_size, const PacedPacketInfo& pacing_info) { RTC_CHECK_GE(arrival_time_ms + arrival_time_offset_ms_, 0); - PacketResult packet; - packet.receive_time = - Timestamp::ms(arrival_time_ms + arrival_time_offset_ms_); - packet.sent_packet.send_time = Timestamp::ms(send_time_ms); - packet.sent_packet.size = DataSize::bytes(payload_size); - packet.sent_packet.pacing_info = pacing_info; - if (packet.sent_packet.pacing_info.probe_cluster_id != - PacedPacketInfo::kNotAProbe) + PacketFeedback packet(arrival_time_ms + arrival_time_offset_ms_, send_time_ms, + sequence_number, payload_size, pacing_info); + std::vector packets; + packets.push_back(packet); + if (packet.send_time_ms != PacketFeedback::kNoSendTime && + packet.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(packet); - TransportPacketsFeedback msg; - msg.feedback_time = Timestamp::ms(clock_.TimeInMilliseconds()); - msg.packet_feedbacks.push_back(packet); - acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( - msg.SortedByReceiveTime()); + acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets); DelayBasedBwe::Result result = bitrate_estimator_->IncomingPacketFeedbackVector( - msg, acknowledged_bitrate_estimator_->bitrate(), + packets, acknowledged_bitrate_estimator_->bitrate(), probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate(), - /*network_estimate*/ absl::nullopt, /*in_alr*/ false); + /*network_estimate*/ absl::nullopt, /*in_alr*/ false, + Timestamp::ms(clock_.TimeInMilliseconds())); + const uint32_t kDummySsrc = 0; if (result.updated) { - bitrate_observer_.OnReceiveBitrateChanged(result.target_bitrate.bps()); + bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, + result.target_bitrate.bps()); } } @@ -229,8 +230,7 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, bool DelayBasedBweTest::GenerateAndProcessFrame(uint32_t ssrc, uint32_t bitrate_bps) { stream_generator_->SetBitrateBps(bitrate_bps); - std::vector packets; - + std::vector packets; int64_t next_time_us = stream_generator_->GenerateFrame(&packets, clock_.TimeInMicroseconds()); if (packets.empty()) @@ -238,29 +238,28 @@ bool DelayBasedBweTest::GenerateAndProcessFrame(uint32_t ssrc, bool overuse = false; bitrate_observer_.Reset(); - clock_.AdvanceTimeMicroseconds(packets.back().receive_time.us() - + clock_.AdvanceTimeMicroseconds(1000 * packets.back().arrival_time_ms - clock_.TimeInMicroseconds()); for (auto& packet : packets) { - RTC_CHECK_GE(packet.receive_time.ms() + arrival_time_offset_ms_, 0); - packet.receive_time += TimeDelta::ms(arrival_time_offset_ms_); + RTC_CHECK_GE(packet.arrival_time_ms + arrival_time_offset_ms_, 0); + packet.arrival_time_ms += arrival_time_offset_ms_; - if (packet.sent_packet.pacing_info.probe_cluster_id != - PacedPacketInfo::kNotAProbe) + if (packet.send_time_ms != PacketFeedback::kNoSendTime && + packet.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(packet); } acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets); - TransportPacketsFeedback msg; - msg.packet_feedbacks = packets; - msg.feedback_time = Timestamp::ms(clock_.TimeInMilliseconds()); - DelayBasedBwe::Result result = bitrate_estimator_->IncomingPacketFeedbackVector( - msg, acknowledged_bitrate_estimator_->bitrate(), + packets, acknowledged_bitrate_estimator_->bitrate(), probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate(), - /*network_estimate*/ absl::nullopt, /*in_alr*/ false); + /*network_estimate*/ absl::nullopt, /*in_alr*/ false, + Timestamp::ms(clock_.TimeInMilliseconds())); + const uint32_t kDummySsrc = 0; if (result.updated) { - bitrate_observer_.OnReceiveBitrateChanged(result.target_bitrate.bps()); + bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, + result.target_bitrate.bps()); if (!first_update_ && result.target_bitrate.bps() < bitrate_bps) overuse = true; first_update_ = false; @@ -309,6 +308,7 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( const PacedPacketInfo kPacingInfo(0, 5, 5000); DataRate bitrate = DataRate::Zero(); int64_t send_time_ms = 0; + uint16_t sequence_number = 0; std::vector ssrcs; EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate)); EXPECT_EQ(0u, ssrcs.size()); @@ -330,8 +330,8 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( EXPECT_FALSE(bitrate_observer_.updated()); bitrate_observer_.Reset(); } - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, kMtu, - pacing_info); + IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, + sequence_number++, kMtu, pacing_info); clock_.AdvanceTimeMilliseconds(1000 / kFramerate); send_time_ms += kFrameIntervalMs; } @@ -351,6 +351,7 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( const int kFrameIntervalMs = 1000 / kFramerate; const PacedPacketInfo kPacingInfo(0, 5, 5000); int64_t send_time_ms = 0; + uint16_t sequence_number = 0; // Inserting packets for five seconds to get a valid estimate. for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) { // NOTE!!! If the following line is moved under the if case then this test @@ -365,8 +366,8 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( EXPECT_FALSE(bitrate_observer_.updated()); // No valid estimate. } - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, kMtu, - pacing_info); + IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, + sequence_number++, kMtu, pacing_info); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); send_time_ms += kFrameIntervalMs; } @@ -376,9 +377,12 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( for (int i = 0; i < 10; ++i) { clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs); send_time_ms += 2 * kFrameIntervalMs; - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, 1000); + IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, + sequence_number + 2, 1000); IncomingFeedback(clock_.TimeInMilliseconds(), - send_time_ms - kFrameIntervalMs, 1000); + send_time_ms - kFrameIntervalMs, sequence_number + 1, + 1000); + sequence_number += 2; } EXPECT_TRUE(bitrate_observer_.updated()); EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_.latest_bitrate(), @@ -477,10 +481,12 @@ void DelayBasedBweTest::TestTimestampGroupingTestHelper() { const int kFramerate = 50; // 50 fps to avoid rounding errors. const int kFrameIntervalMs = 1000 / kFramerate; int64_t send_time_ms = 0; + uint16_t sequence_number = 0; // Initial set of frames to increase the bitrate. 6 seconds to have enough // time for the first estimate to be generated and for Process() to be called. for (int i = 0; i <= 6 * kFramerate; ++i) { - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, 1000); + IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, + sequence_number++, 1000); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); send_time_ms += kFrameIntervalMs; @@ -495,7 +501,8 @@ void DelayBasedBweTest::TestTimestampGroupingTestHelper() { for (int j = 0; j < kTimestampGroupLength; ++j) { // Insert |kTimestampGroupLength| frames with just 1 timestamp ticks in // between. Should be treated as part of the same group by the estimator. - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, 100); + IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, + sequence_number++, 100); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs / kTimestampGroupLength); send_time_ms += 1; } @@ -512,9 +519,11 @@ void DelayBasedBweTest::TestWrappingHelper(int silence_time_s) { const int kFramerate = 100; const int kFrameIntervalMs = 1000 / kFramerate; int64_t send_time_ms = 0; + uint16_t sequence_number = 0; for (size_t i = 0; i < 3000; ++i) { - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, 1000); + IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, + sequence_number++, 1000); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); send_time_ms += kFrameIntervalMs; } @@ -526,7 +535,8 @@ void DelayBasedBweTest::TestWrappingHelper(int silence_time_s) { send_time_ms += silence_time_s * 1000; for (size_t i = 0; i < 24; ++i) { - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, 1000); + IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, + sequence_number++, 1000); clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs); send_time_ms += kFrameIntervalMs; } diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h index e6275d61e9..84831d91aa 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h +++ b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h @@ -21,6 +21,8 @@ #include "api/transport/network_types.h" #include "modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h" #include "modules/congestion_controller/goog_cc/delay_based_bwe.h" +#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/constructor_magic.h" #include "system_wrappers/include/clock.h" #include "test/field_trial.h" @@ -29,12 +31,13 @@ namespace webrtc { namespace test { -class TestBitrateObserver { +class TestBitrateObserver : public RemoteBitrateObserver { public: TestBitrateObserver() : updated_(false), latest_bitrate_(0) {} - ~TestBitrateObserver() {} + ~TestBitrateObserver() override {} - void OnReceiveBitrateChanged(uint32_t bitrate); + void OnReceiveBitrateChanged(const std::vector& ssrcs, + uint32_t bitrate) override; void Reset() { updated_ = false; } @@ -57,7 +60,7 @@ class RtpStream { // previous frame, no frame will be generated. The frame is split into // packets. int64_t GenerateFrame(int64_t time_now_us, - std::vector* packets); + std::vector* packets); // The send-side time when the next frame can be generated. int64_t next_rtp_time() const; @@ -73,6 +76,7 @@ class RtpStream { int fps_; int bitrate_bps_; int64_t next_rtp_time_; + uint16_t sequence_number_; RTC_DISALLOW_COPY_AND_ASSIGN(RtpStream); }; @@ -97,7 +101,7 @@ class StreamGenerator { // TODO(holmer): Break out the channel simulation part from this class to make // it possible to simulate different types of channels. - int64_t GenerateFrame(std::vector* packets, + int64_t GenerateFrame(std::vector* packets, int64_t time_now_us); private: @@ -124,9 +128,11 @@ class DelayBasedBweTest : public ::testing::Test { // Helpers to insert a single packet into the delay-based BWE. void IncomingFeedback(int64_t arrival_time_ms, int64_t send_time_ms, + uint16_t sequence_number, size_t payload_size); void IncomingFeedback(int64_t arrival_time_ms, int64_t send_time_ms, + uint16_t sequence_number, size_t payload_size, const PacedPacketInfo& pacing_info); diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc index 8bd3cfb228..95bbf56d7d 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -27,6 +27,7 @@ #include "modules/congestion_controller/goog_cc/probe_controller.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "modules/remote_bitrate_estimator/test/bwe_test_logging.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -42,6 +43,25 @@ constexpr TimeDelta kLossUpdateInterval = TimeDelta::Millis<1000>(); // overshoots from the encoder. const float kDefaultPaceMultiplier = 2.5f; +std::vector ReceivedPacketsFeedbackAsRtp( + const TransportPacketsFeedback report) { + std::vector packet_feedback_vector; + for (auto& fb : report.PacketsWithFeedback()) { + if (fb.receive_time.IsFinite()) { + PacketFeedback pf(fb.receive_time.ms(), 0); + pf.creation_time_ms = report.feedback_time.ms(); + pf.payload_size = fb.sent_packet.size.bytes(); + pf.pacing_info = fb.sent_packet.pacing_info; + pf.send_time_ms = fb.sent_packet.send_time.ms(); + pf.unacknowledged_data = fb.sent_packet.prior_unacked_data.bytes(); + packet_feedback_vector.push_back(pf); + } + } + std::sort(packet_feedback_vector.begin(), packet_feedback_vector.end(), + PacketFeedbackComparator()); + return packet_feedback_vector; +} + int64_t GetBpsOrDefault(const absl::optional& rate, int64_t fallback_bps) { if (rate && rate->IsFinite()) { @@ -483,21 +503,24 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback( lost_packets_since_last_loss_update_ = 0; } } + + std::vector received_feedback_vector = + ReceivedPacketsFeedbackAsRtp(report); + absl::optional alr_start_time = alr_detector_->GetApplicationLimitedRegionStartTime(); if (previously_in_alr_ && !alr_start_time.has_value()) { int64_t now_ms = report.feedback_time.ms(); - acknowledged_bitrate_estimator_->SetAlrEndedTime(report.feedback_time); + acknowledged_bitrate_estimator_->SetAlrEndedTimeMs(now_ms); probe_controller_->SetAlrEndedTimeMs(now_ms); } previously_in_alr_ = alr_start_time.has_value(); acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( - report.SortedByReceiveTime()); + received_feedback_vector); auto acknowledged_bitrate = acknowledged_bitrate_estimator_->bitrate(); - for (const auto& feedback : report.SortedByReceiveTime()) { - if (feedback.sent_packet.pacing_info.probe_cluster_id != - PacedPacketInfo::kNotAProbe) { + for (const auto& feedback : received_feedback_vector) { + if (feedback.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) { probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(feedback); } } @@ -522,8 +545,8 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback( network_estimator_ ? network_estimator_->GetCurrentEstimate() : absl::nullopt; result = delay_based_bwe_->IncomingPacketFeedbackVector( - report, acknowledged_bitrate, probe_bitrate, network_estimate, - alr_start_time.has_value()); + received_feedback_vector, acknowledged_bitrate, probe_bitrate, + network_estimate, alr_start_time.has_value(), report.feedback_time); if (result.updated) { if (result.probe) { diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc index 34650da9e0..5bb3491e0e 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc @@ -708,30 +708,6 @@ TEST_F(GoogCcNetworkControllerTest, NoRttBackoffCollapseWhenVideoStops) { EXPECT_GT(client->send_bandwidth().kbps(), 1000); } -TEST_F(GoogCcNetworkControllerTest, NoCrashOnVeryLateFeedback) { - Scenario s; - auto ret_net = s.CreateMutableSimulationNode(NetworkSimulationConfig()); - auto* route = s.CreateRoutes( - s.CreateClient("send", CallClientConfig()), - {s.CreateSimulationNode(NetworkSimulationConfig())}, - s.CreateClient("return", CallClientConfig()), {ret_net->node()}); - auto* video = s.CreateVideoStream(route->forward(), VideoStreamConfig()); - s.RunFor(TimeDelta::seconds(5)); - // Delay feedback by several minutes. This will cause removal of the send time - // history for the packets as long as kSendTimeHistoryWindow is configured for - // a shorter time span. - ret_net->PauseTransmissionUntil(s.Now() + TimeDelta::seconds(300)); - // Stopping video stream while waiting to save test execution time. - video->send()->Stop(); - s.RunFor(TimeDelta::seconds(299)); - // Starting to cause addition of new packet to history, which cause old - // packets to be removed. - video->send()->Start(); - // Runs until the lost packets are received. We expect that this will run - // without causing any runtime failures. - s.RunFor(TimeDelta::seconds(2)); -} - TEST_F(GoogCcNetworkControllerTest, IsFairToTCP) { Scenario s("googcc_unit/tcp_fairness"); NetworkSimulationConfig net_conf; diff --git a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc index 2160d51a07..380e7d30c2 100644 --- a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc +++ b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc @@ -20,15 +20,14 @@ #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" -namespace webrtc { namespace { // The minumum number of probes we need to receive feedback about in percent // in order to have a valid estimate. -constexpr double kMinReceivedProbesRatio = .80; +constexpr int kMinReceivedProbesPercent = 80; // The minumum number of bytes we need to receive feedback about in percent // in order to have a valid estimate. -constexpr double kMinReceivedBytesRatio = .80; +constexpr int kMinReceivedBytesPercent = 80; // The maximum |receive rate| / |send rate| ratio for a valid estimate. constexpr float kMaxValidRatio = 2.0f; @@ -46,151 +45,148 @@ constexpr float kTargetUtilizationFraction = 0.95f; // The maximum time period over which the cluster history is retained. // This is also the maximum time period beyond which a probing burst is not // expected to last. -constexpr TimeDelta kMaxClusterHistory = TimeDelta::Seconds<1>(); +constexpr int kMaxClusterHistoryMs = 1000; // The maximum time interval between first and the last probe on a cluster // on the sender side as well as the receive side. -constexpr TimeDelta kMaxProbeInterval = TimeDelta::Seconds<1>(); - +constexpr int kMaxProbeIntervalMs = 1000; } // namespace +namespace webrtc { ProbeBitrateEstimator::ProbeBitrateEstimator(RtcEventLog* event_log) : event_log_(event_log) {} ProbeBitrateEstimator::~ProbeBitrateEstimator() = default; -absl::optional ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( - const PacketResult& packet_feedback) { - int cluster_id = packet_feedback.sent_packet.pacing_info.probe_cluster_id; +int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( + const PacketFeedback& packet_feedback) { + int cluster_id = packet_feedback.pacing_info.probe_cluster_id; RTC_DCHECK_NE(cluster_id, PacedPacketInfo::kNotAProbe); - EraseOldClusters(packet_feedback.receive_time - kMaxClusterHistory); + EraseOldClusters(packet_feedback.arrival_time_ms - kMaxClusterHistoryMs); + int payload_size_bits = + rtc::dchecked_cast(packet_feedback.payload_size * 8); AggregatedCluster* cluster = &clusters_[cluster_id]; - if (packet_feedback.sent_packet.send_time < cluster->first_send) { - cluster->first_send = packet_feedback.sent_packet.send_time; + if (packet_feedback.send_time_ms < cluster->first_send_ms) { + cluster->first_send_ms = packet_feedback.send_time_ms; } - if (packet_feedback.sent_packet.send_time > cluster->last_send) { - cluster->last_send = packet_feedback.sent_packet.send_time; - cluster->size_last_send = packet_feedback.sent_packet.size; + if (packet_feedback.send_time_ms > cluster->last_send_ms) { + cluster->last_send_ms = packet_feedback.send_time_ms; + cluster->size_last_send = payload_size_bits; } - if (packet_feedback.receive_time < cluster->first_receive) { - cluster->first_receive = packet_feedback.receive_time; - cluster->size_first_receive = packet_feedback.sent_packet.size; + if (packet_feedback.arrival_time_ms < cluster->first_receive_ms) { + cluster->first_receive_ms = packet_feedback.arrival_time_ms; + cluster->size_first_receive = payload_size_bits; } - if (packet_feedback.receive_time > cluster->last_receive) { - cluster->last_receive = packet_feedback.receive_time; + if (packet_feedback.arrival_time_ms > cluster->last_receive_ms) { + cluster->last_receive_ms = packet_feedback.arrival_time_ms; } - cluster->size_total += packet_feedback.sent_packet.size; + cluster->size_total += payload_size_bits; cluster->num_probes += 1; - RTC_DCHECK_GT( - packet_feedback.sent_packet.pacing_info.probe_cluster_min_probes, 0); - RTC_DCHECK_GT(packet_feedback.sent_packet.pacing_info.probe_cluster_min_bytes, - 0); + RTC_DCHECK_GT(packet_feedback.pacing_info.probe_cluster_min_probes, 0); + RTC_DCHECK_GT(packet_feedback.pacing_info.probe_cluster_min_bytes, 0); - int min_probes = - packet_feedback.sent_packet.pacing_info.probe_cluster_min_probes * - kMinReceivedProbesRatio; - DataSize min_size = - DataSize::bytes( - packet_feedback.sent_packet.pacing_info.probe_cluster_min_bytes) * - kMinReceivedBytesRatio; - if (cluster->num_probes < min_probes || cluster->size_total < min_size) - return absl::nullopt; + int min_probes = packet_feedback.pacing_info.probe_cluster_min_probes * + kMinReceivedProbesPercent / 100; + int min_bytes = packet_feedback.pacing_info.probe_cluster_min_bytes * + kMinReceivedBytesPercent / 100; + if (cluster->num_probes < min_probes || cluster->size_total < min_bytes * 8) + return -1; - TimeDelta send_interval = cluster->last_send - cluster->first_send; - TimeDelta receive_interval = cluster->last_receive - cluster->first_receive; + float send_interval_ms = cluster->last_send_ms - cluster->first_send_ms; + float receive_interval_ms = + cluster->last_receive_ms - cluster->first_receive_ms; - if (send_interval <= TimeDelta::Zero() || send_interval > kMaxProbeInterval || - receive_interval <= TimeDelta::Zero() || - receive_interval > kMaxProbeInterval) { + if (send_interval_ms <= 0 || send_interval_ms > kMaxProbeIntervalMs || + receive_interval_ms <= 0 || receive_interval_ms > kMaxProbeIntervalMs) { RTC_LOG(LS_INFO) << "Probing unsuccessful, invalid send/receive interval" << " [cluster id: " << cluster_id - << "] [send interval: " << ToString(send_interval) << "]" - << " [receive interval: " << ToString(receive_interval) - << "]"; + << "] [send interval: " << send_interval_ms << " ms]" + << " [receive interval: " << receive_interval_ms << " ms]"; if (event_log_) { event_log_->Log(absl::make_unique( cluster_id, ProbeFailureReason::kInvalidSendReceiveInterval)); } - return absl::nullopt; + return -1; } - // Since the |send_interval| does not include the time it takes to actually + // Since the |send_interval_ms| does not include the time it takes to actually // send the last packet the size of the last sent packet should not be // included when calculating the send bitrate. RTC_DCHECK_GT(cluster->size_total, cluster->size_last_send); - DataSize send_size = cluster->size_total - cluster->size_last_send; - DataRate send_rate = send_size / send_interval; + float send_size = cluster->size_total - cluster->size_last_send; + float send_bps = send_size / send_interval_ms * 1000; - // Since the |receive_interval| does not include the time it takes to + // Since the |receive_interval_ms| does not include the time it takes to // actually receive the first packet the size of the first received packet // should not be included when calculating the receive bitrate. RTC_DCHECK_GT(cluster->size_total, cluster->size_first_receive); - DataSize receive_size = cluster->size_total - cluster->size_first_receive; - DataRate receive_rate = receive_size / receive_interval; + float receive_size = cluster->size_total - cluster->size_first_receive; + float receive_bps = receive_size / receive_interval_ms * 1000; - double ratio = receive_rate / send_rate; + float ratio = receive_bps / send_bps; if (ratio > kMaxValidRatio) { RTC_LOG(LS_INFO) << "Probing unsuccessful, receive/send ratio too high" << " [cluster id: " << cluster_id - << "] [send: " << ToString(send_size) << " / " - << ToString(send_interval) << " = " << ToString(send_rate) - << "]" - << " [receive: " << ToString(receive_size) << " / " - << ToString(receive_interval) << " = " - << ToString(receive_rate) << " ]" - << " [ratio: " << ToString(receive_rate) << " / " - << ToString(send_rate) << " = " << ratio + << "] [send: " << send_size << " bytes / " + << send_interval_ms << " ms = " << send_bps / 1000 + << " kb/s]" + << " [receive: " << receive_size << " bytes / " + << receive_interval_ms << " ms = " << receive_bps / 1000 + << " kb/s]" + << " [ratio: " << receive_bps / 1000 << " / " + << send_bps / 1000 << " = " << ratio << " > kMaxValidRatio (" << kMaxValidRatio << ")]"; if (event_log_) { event_log_->Log(absl::make_unique( cluster_id, ProbeFailureReason::kInvalidSendReceiveRatio)); } - return absl::nullopt; + return -1; } RTC_LOG(LS_INFO) << "Probing successful" - << " [cluster id: " << cluster_id - << "] [send: " << ToString(send_size) << " / " - << ToString(send_interval) << " = " << ToString(send_rate) - << " ]" - << " [receive: " << ToString(receive_size) << " / " - << ToString(receive_interval) << " = " - << ToString(receive_rate) << "]"; + << " [cluster id: " << cluster_id << "] [send: " << send_size + << " bytes / " << send_interval_ms + << " ms = " << send_bps / 1000 << " kb/s]" + << " [receive: " << receive_size << " bytes / " + << receive_interval_ms << " ms = " << receive_bps / 1000 + << " kb/s]"; - DataRate res = std::min(send_rate, receive_rate); + float res = std::min(send_bps, receive_bps); // If we're receiving at significantly lower bitrate than we were sending at, // it suggests that we've found the true capacity of the link. In this case, // set the target bitrate slightly lower to not immediately overuse. - if (receive_rate < kMinRatioForUnsaturatedLink * send_rate) { - RTC_DCHECK_GT(send_rate, receive_rate); - res = kTargetUtilizationFraction * receive_rate; + if (receive_bps < kMinRatioForUnsaturatedLink * send_bps) { + RTC_DCHECK_GT(send_bps, receive_bps); + res = kTargetUtilizationFraction * receive_bps; } if (event_log_) { event_log_->Log( - absl::make_unique(cluster_id, res.bps())); + absl::make_unique(cluster_id, res)); } - last_estimate_ = res; - estimated_data_rate_ = res; - return res; + last_estimate_ = DataRate::bps(res); + estimated_bitrate_bps_ = res; + return *estimated_bitrate_bps_; } absl::optional ProbeBitrateEstimator::FetchAndResetLastEstimatedBitrate() { - absl::optional estimated_data_rate = estimated_data_rate_; - estimated_data_rate_.reset(); - return estimated_data_rate; + absl::optional estimated_bitrate_bps = estimated_bitrate_bps_; + estimated_bitrate_bps_.reset(); + if (estimated_bitrate_bps) + return DataRate::bps(*estimated_bitrate_bps); + return absl::nullopt; } absl::optional ProbeBitrateEstimator::last_estimate() const { return last_estimate_; } -void ProbeBitrateEstimator::EraseOldClusters(Timestamp timestamp) { +void ProbeBitrateEstimator::EraseOldClusters(int64_t timestamp_ms) { for (auto it = clusters_.begin(); it != clusters_.end();) { - if (it->second.last_receive < timestamp) { + if (it->second.last_receive_ms < timestamp_ms) { it = clusters_.erase(it); } else { ++it; diff --git a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h index bf9cb22eea..ce4eb99126 100644 --- a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h +++ b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h @@ -15,8 +15,8 @@ #include #include "absl/types/optional.h" -#include "api/transport/network_types.h" #include "api/units/data_rate.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" namespace webrtc { class RtcEventLog; @@ -28,8 +28,7 @@ class ProbeBitrateEstimator { // Should be called for every probe packet we receive feedback about. // Returns the estimated bitrate if the probe completes a valid cluster. - absl::optional HandleProbeAndEstimateBitrate( - const PacketResult& packet_feedback); + int HandleProbeAndEstimateBitrate(const PacketFeedback& packet_feedback); absl::optional FetchAndResetLastEstimatedBitrate(); @@ -38,21 +37,21 @@ class ProbeBitrateEstimator { private: struct AggregatedCluster { int num_probes = 0; - Timestamp first_send = Timestamp::PlusInfinity(); - Timestamp last_send = Timestamp::MinusInfinity(); - Timestamp first_receive = Timestamp::PlusInfinity(); - Timestamp last_receive = Timestamp::MinusInfinity(); - DataSize size_last_send = DataSize::Zero(); - DataSize size_first_receive = DataSize::Zero(); - DataSize size_total = DataSize::Zero(); + int64_t first_send_ms = std::numeric_limits::max(); + int64_t last_send_ms = 0; + int64_t first_receive_ms = std::numeric_limits::max(); + int64_t last_receive_ms = 0; + int size_last_send = 0; + int size_first_receive = 0; + int size_total = 0; }; - // Erases old cluster data that was seen before |timestamp|. - void EraseOldClusters(Timestamp timestamp); + // Erases old cluster data that was seen before |timestamp_ms|. + void EraseOldClusters(int64_t timestamp_ms); std::map clusters_; RtcEventLog* const event_log_; - absl::optional estimated_data_rate_; + absl::optional estimated_bitrate_bps_; absl::optional last_estimate_; }; diff --git a/modules/congestion_controller/goog_cc/probe_bitrate_estimator_unittest.cc b/modules/congestion_controller/goog_cc/probe_bitrate_estimator_unittest.cc index b886add2d0..85c398b1c8 100644 --- a/modules/congestion_controller/goog_cc/probe_bitrate_estimator_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_bitrate_estimator_unittest.cc @@ -18,6 +18,7 @@ namespace webrtc { namespace { +constexpr int kInvalidBitrate = -1; constexpr int kDefaultMinProbes = 5; constexpr int kDefaultMinBytes = 5000; constexpr float kTargetUtilizationFraction = 0.95f; @@ -35,20 +36,15 @@ class TestProbeBitrateEstimator : public ::testing::Test { int64_t arrival_time_ms, int min_probes = kDefaultMinProbes, int min_bytes = kDefaultMinBytes) { - const Timestamp kReferenceTime = Timestamp::seconds(1000); - PacketResult feedback; - feedback.sent_packet.send_time = - kReferenceTime + TimeDelta::ms(send_time_ms); - feedback.sent_packet.size = DataSize::bytes(size_bytes); - feedback.sent_packet.pacing_info = - PacedPacketInfo(probe_cluster_id, min_probes, min_bytes); - feedback.receive_time = kReferenceTime + TimeDelta::ms(arrival_time_ms); - measured_data_rate_ = - probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(feedback); + PacedPacketInfo pacing_info(probe_cluster_id, min_probes, min_bytes); + PacketFeedback packet_feedback(arrival_time_ms, send_time_ms, 0, size_bytes, + pacing_info); + measured_bps_ = + probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(packet_feedback); } protected: - absl::optional measured_data_rate_; + int measured_bps_ = kInvalidBitrate; ProbeBitrateEstimator probe_bitrate_estimator_; }; @@ -58,7 +54,7 @@ TEST_F(TestProbeBitrateEstimator, OneCluster) { AddPacketFeedback(0, 1000, 20, 30); AddPacketFeedback(0, 1000, 30, 40); - EXPECT_NEAR(measured_data_rate_->bps(), 800000, 10); + EXPECT_NEAR(measured_bps_, 800000, 10); } TEST_F(TestProbeBitrateEstimator, OneClusterTooFewProbes) { @@ -66,7 +62,7 @@ TEST_F(TestProbeBitrateEstimator, OneClusterTooFewProbes) { AddPacketFeedback(0, 2000, 10, 20); AddPacketFeedback(0, 2000, 20, 30); - EXPECT_FALSE(measured_data_rate_); + EXPECT_EQ(kInvalidBitrate, measured_bps_); } TEST_F(TestProbeBitrateEstimator, OneClusterTooFewBytes) { @@ -77,7 +73,7 @@ TEST_F(TestProbeBitrateEstimator, OneClusterTooFewBytes) { AddPacketFeedback(0, 800, 30, 40, kDefaultMinProbes, kMinBytes); AddPacketFeedback(0, 800, 40, 50, kDefaultMinProbes, kMinBytes); - EXPECT_FALSE(measured_data_rate_); + EXPECT_EQ(kInvalidBitrate, measured_bps_); } TEST_F(TestProbeBitrateEstimator, SmallCluster) { @@ -88,7 +84,7 @@ TEST_F(TestProbeBitrateEstimator, SmallCluster) { AddPacketFeedback(0, 150, 30, 40, kDefaultMinProbes, kMinBytes); AddPacketFeedback(0, 150, 40, 50, kDefaultMinProbes, kMinBytes); AddPacketFeedback(0, 150, 50, 60, kDefaultMinProbes, kMinBytes); - EXPECT_NEAR(measured_data_rate_->bps(), 120000, 10); + EXPECT_NEAR(measured_bps_, 120000, 10); } TEST_F(TestProbeBitrateEstimator, LargeCluster) { @@ -102,7 +98,7 @@ TEST_F(TestProbeBitrateEstimator, LargeCluster) { ++send_time; ++receive_time; } - EXPECT_NEAR(measured_data_rate_->bps(), 100000000, 10); + EXPECT_NEAR(measured_bps_, 100000000, 10); } TEST_F(TestProbeBitrateEstimator, FastReceive) { @@ -111,7 +107,7 @@ TEST_F(TestProbeBitrateEstimator, FastReceive) { AddPacketFeedback(0, 1000, 20, 35); AddPacketFeedback(0, 1000, 30, 40); - EXPECT_NEAR(measured_data_rate_->bps(), 800000, 10); + EXPECT_NEAR(measured_bps_, 800000, 10); } TEST_F(TestProbeBitrateEstimator, TooFastReceive) { @@ -120,7 +116,7 @@ TEST_F(TestProbeBitrateEstimator, TooFastReceive) { AddPacketFeedback(0, 1000, 20, 25); AddPacketFeedback(0, 1000, 40, 27); - EXPECT_FALSE(measured_data_rate_); + EXPECT_EQ(measured_bps_, kInvalidBitrate); } TEST_F(TestProbeBitrateEstimator, SlowReceive) { @@ -130,8 +126,7 @@ TEST_F(TestProbeBitrateEstimator, SlowReceive) { AddPacketFeedback(0, 1000, 30, 85); // Expected send rate = 800 kbps, expected receive rate = 320 kbps. - EXPECT_NEAR(measured_data_rate_->bps(), kTargetUtilizationFraction * 320000, - 10); + EXPECT_NEAR(measured_bps_, kTargetUtilizationFraction * 320000, 10); } TEST_F(TestProbeBitrateEstimator, BurstReceive) { @@ -140,7 +135,7 @@ TEST_F(TestProbeBitrateEstimator, BurstReceive) { AddPacketFeedback(0, 1000, 20, 50); AddPacketFeedback(0, 1000, 40, 50); - EXPECT_FALSE(measured_data_rate_); + EXPECT_EQ(measured_bps_, kInvalidBitrate); } TEST_F(TestProbeBitrateEstimator, MultipleClusters) { @@ -149,12 +144,11 @@ TEST_F(TestProbeBitrateEstimator, MultipleClusters) { AddPacketFeedback(0, 1000, 20, 30); AddPacketFeedback(0, 1000, 40, 60); // Expected send rate = 600 kbps, expected receive rate = 480 kbps. - EXPECT_NEAR(measured_data_rate_->bps(), kTargetUtilizationFraction * 480000, - 10); + EXPECT_NEAR(measured_bps_, kTargetUtilizationFraction * 480000, 10); AddPacketFeedback(0, 1000, 50, 60); // Expected send rate = 640 kbps, expected receive rate = 640 kbps. - EXPECT_NEAR(measured_data_rate_->bps(), 640000, 10); + EXPECT_NEAR(measured_bps_, 640000, 10); AddPacketFeedback(1, 1000, 60, 70); AddPacketFeedback(1, 1000, 65, 77); @@ -162,8 +156,7 @@ TEST_F(TestProbeBitrateEstimator, MultipleClusters) { AddPacketFeedback(1, 1000, 75, 90); // Expected send rate = 1600 kbps, expected receive rate = 1200 kbps. - EXPECT_NEAR(measured_data_rate_->bps(), kTargetUtilizationFraction * 1200000, - 10); + EXPECT_NEAR(measured_bps_, kTargetUtilizationFraction * 1200000, 10); } TEST_F(TestProbeBitrateEstimator, IgnoreOldClusters) { @@ -177,13 +170,12 @@ TEST_F(TestProbeBitrateEstimator, IgnoreOldClusters) { AddPacketFeedback(1, 1000, 75, 90); // Expected send rate = 1600 kbps, expected receive rate = 1200 kbps. - EXPECT_NEAR(measured_data_rate_->bps(), kTargetUtilizationFraction * 1200000, - 10); + EXPECT_NEAR(measured_bps_, kTargetUtilizationFraction * 1200000, 10); // Coming in 6s later AddPacketFeedback(0, 1000, 40 + 6000, 60 + 6000); - EXPECT_FALSE(measured_data_rate_); + EXPECT_EQ(measured_bps_, kInvalidBitrate); } TEST_F(TestProbeBitrateEstimator, IgnoreSizeLastSendPacket) { @@ -194,7 +186,7 @@ TEST_F(TestProbeBitrateEstimator, IgnoreSizeLastSendPacket) { AddPacketFeedback(0, 1500, 40, 50); // Expected send rate = 800 kbps, expected receive rate = 900 kbps. - EXPECT_NEAR(measured_data_rate_->bps(), 800000, 10); + EXPECT_NEAR(measured_bps_, 800000, 10); } TEST_F(TestProbeBitrateEstimator, IgnoreSizeFirstReceivePacket) { @@ -204,8 +196,7 @@ TEST_F(TestProbeBitrateEstimator, IgnoreSizeFirstReceivePacket) { AddPacketFeedback(0, 1000, 30, 40); // Expected send rate = 933 kbps, expected receive rate = 800 kbps. - EXPECT_NEAR(measured_data_rate_->bps(), kTargetUtilizationFraction * 800000, - 10); + EXPECT_NEAR(measured_bps_, kTargetUtilizationFraction * 800000, 10); } TEST_F(TestProbeBitrateEstimator, NoLastEstimatedBitrateBps) { diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index 3a8ef13a3d..71ce001f1d 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -26,6 +26,8 @@ namespace webrtc { +class Clock; + struct ProbeControllerConfig { explicit ProbeControllerConfig(const WebRtcKeyValueConfig* key_value_config); ProbeControllerConfig(const ProbeControllerConfig&); diff --git a/rtc_tools/event_log_visualizer/analyzer.cc b/rtc_tools/event_log_visualizer/analyzer.cc index 11e33f29e2..428300dbe2 100644 --- a/rtc_tools/event_log_visualizer/analyzer.cc +++ b/rtc_tools/event_log_visualizer/analyzer.cc @@ -71,6 +71,14 @@ namespace { const int kNumMicrosecsPerSec = 1000000; +void SortPacketFeedbackVector(std::vector* vec) { + auto pred = [](const PacketFeedback& packet_feedback) { + return packet_feedback.arrival_time_ms == PacketFeedback::kNotReceived; + }; + vec->erase(std::remove_if(vec->begin(), vec->end(), pred), vec->end()); + std::sort(vec->begin(), vec->end(), PacketFeedbackComparator()); +} + std::string SsrcToString(uint32_t ssrc) { rtc::StringBuilder ss; ss << "SSRC " << ssrc; @@ -1308,16 +1316,16 @@ void EventLogAnalyzer::CreateSendSideBweSimulationGraph(Plot* plot) { absl::optional bitrate_bps; if (feedback_msg) { observer.Update(goog_cc->OnTransportPacketsFeedback(*feedback_msg)); - std::vector feedback = - feedback_msg->SortedByReceiveTime(); + std::vector feedback = + transport_feedback.GetTransportFeedbackVector(); + SortPacketFeedbackVector(&feedback); if (!feedback.empty()) { #if !(BWE_TEST_LOGGING_COMPILE_TIME_ENABLE) acknowledged_bitrate_estimator.IncomingPacketFeedbackVector(feedback); #endif // !(BWE_TEST_LOGGING_COMPILE_TIME_ENABLE) - for (const PacketResult& packet : feedback) - acked_bitrate.Update(packet.sent_packet.size.bytes(), - packet.receive_time.ms()); - bitrate_bps = acked_bitrate.Rate(feedback.back().receive_time.ms()); + for (const PacketFeedback& packet : feedback) + acked_bitrate.Update(packet.payload_size, packet.arrival_time_ms); + bitrate_bps = acked_bitrate.Rate(feedback.back().arrival_time_ms); } } @@ -1325,9 +1333,7 @@ void EventLogAnalyzer::CreateSendSideBweSimulationGraph(Plot* plot) { float y = bitrate_bps.value_or(0) / 1000; acked_time_series.points.emplace_back(x, y); #if !(BWE_TEST_LOGGING_COMPILE_TIME_ENABLE) - y = acknowledged_bitrate_estimator.bitrate() - .value_or(DataRate::Zero()) - .kbps(); + y = acknowledged_bitrate_estimator.bitrate_bps().value_or(0) / 1000; acked_estimate_time_series.points.emplace_back(x, y); #endif // !(BWE_TEST_LOGGING_COMPILE_TIME_ENABLE) ++rtcp_iterator;