diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc index 7a1ee117bd..4e127dce0f 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc @@ -88,7 +88,6 @@ DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log) delay_detector_(), last_seen_packet_(Timestamp::MinusInfinity()), uma_recorded_(false), - probe_bitrate_estimator_(event_log), trendline_window_size_( webrtc::field_trial::IsEnabled(kBweWindowSizeInPacketsExperiment) ? ReadTrendlineFilterWindowSize() @@ -111,6 +110,7 @@ DelayBasedBwe::~DelayBasedBwe() {} DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( const std::vector& packet_feedback_vector, absl::optional acked_bitrate, + absl::optional probe_bitrate, Timestamp at_time) { RTC_DCHECK(std::is_sorted(packet_feedback_vector.begin(), packet_feedback_vector.end(), @@ -155,7 +155,8 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( } else { consecutive_delayed_feedbacks_ = 0; - return MaybeUpdateEstimate(acked_bitrate, recovered_from_overuse, at_time); + return MaybeUpdateEstimate(acked_bitrate, probe_bitrate, + recovered_from_overuse, at_time); } return Result(); } @@ -221,20 +222,15 @@ void DelayBasedBwe::IncomingPacketFeedback( delay_detector_->Update(t_delta, ts_delta_ms, packet_feedback.arrival_time_ms); } - if (packet_feedback.pacing_info.probe_cluster_id != - PacedPacketInfo::kNotAProbe) { - probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(packet_feedback); - } } DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate( absl::optional acked_bitrate, + absl::optional probe_bitrate, bool recovered_from_overuse, Timestamp at_time) { Result result; - absl::optional probe_bitrate = - probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrate(); // Currently overusing the bandwidth. if (delay_detector_->State() == BandwidthUsage::kBwOverusing) { if (acked_bitrate && diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.h b/modules/congestion_controller/goog_cc/delay_based_bwe.h index f7c1964573..616c5b0896 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.h +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.h @@ -47,6 +47,7 @@ class DelayBasedBwe { Result IncomingPacketFeedbackVector( const std::vector& packet_feedback_vector, absl::optional acked_bitrate, + absl::optional probe_bitrate, Timestamp at_time); Result OnDelayedFeedback(Timestamp receive_time); void OnRttUpdate(TimeDelta avg_rtt); @@ -61,6 +62,7 @@ class DelayBasedBwe { Timestamp at_time); Result OnLongFeedbackDelay(Timestamp arrival_time); Result MaybeUpdateEstimate(absl::optional acked_bitrate, + absl::optional probe_bitrate, bool request_probe, Timestamp at_time); // Updates the current remote rate estimate and returns true if a valid @@ -76,7 +78,6 @@ class DelayBasedBwe { Timestamp last_seen_packet_; bool uma_recorded_; AimdRateControl rate_control_; - ProbeBitrateEstimator probe_bitrate_estimator_; size_t trendline_window_size_; double trendline_smoothing_coeff_; double trendline_threshold_gain_; 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 0b2f799864..7de11b1684 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc @@ -31,7 +31,7 @@ constexpr Timestamp kDummyTimestamp = Timestamp::Seconds<1000>(); TEST_F(DelayBasedBweTest, NoCrashEmptyFeedback) { std::vector packet_feedback_vector; bitrate_estimator_->IncomingPacketFeedbackVector( - packet_feedback_vector, absl::nullopt, kDummyTimestamp); + packet_feedback_vector, absl::nullopt, absl::nullopt, kDummyTimestamp); } TEST_F(DelayBasedBweTest, NoCrashOnlyLostFeedback) { @@ -43,7 +43,7 @@ TEST_F(DelayBasedBweTest, NoCrashOnlyLostFeedback) { PacketFeedback::kNoSendTime, 1, 1500, PacedPacketInfo())); bitrate_estimator_->IncomingPacketFeedbackVector( - packet_feedback_vector, absl::nullopt, kDummyTimestamp); + packet_feedback_vector, absl::nullopt, absl::nullopt, kDummyTimestamp); } TEST_F(DelayBasedBweTest, ProbeDetection) { 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 583ce25e85..47288e5fbc 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 @@ -154,6 +154,7 @@ DelayBasedBweTest::DelayBasedBweTest() clock_(100000000), acknowledged_bitrate_estimator_( absl::make_unique()), + probe_bitrate_estimator_(new ProbeBitrateEstimator(nullptr)), bitrate_estimator_(new DelayBasedBwe(nullptr)), stream_generator_(new test::StreamGenerator(1e6, // Capacity. clock_.TimeInMicroseconds())), @@ -166,6 +167,7 @@ DelayBasedBweTest::DelayBasedBweTest(const std::string& field_trial_string) clock_(100000000), acknowledged_bitrate_estimator_( absl::make_unique()), + probe_bitrate_estimator_(new ProbeBitrateEstimator(nullptr)), bitrate_estimator_(new DelayBasedBwe(nullptr)), stream_generator_(new test::StreamGenerator(1e6, // Capacity. clock_.TimeInMicroseconds())), @@ -198,10 +200,15 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_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); + acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets); DelayBasedBwe::Result result = bitrate_estimator_->IncomingPacketFeedbackVector( packets, acknowledged_bitrate_estimator_->bitrate(), + probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate(), Timestamp::ms(clock_.TimeInMilliseconds())); const uint32_t kDummySsrc = 0; if (result.updated) { @@ -232,12 +239,17 @@ bool DelayBasedBweTest::GenerateAndProcessFrame(uint32_t ssrc, for (auto& packet : packets) { RTC_CHECK_GE(packet.arrival_time_ms + arrival_time_offset_ms_, 0); packet.arrival_time_ms += arrival_time_offset_ms_; + + 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); DelayBasedBwe::Result result = bitrate_estimator_->IncomingPacketFeedbackVector( packets, acknowledged_bitrate_estimator_->bitrate(), + probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate(), Timestamp::ms(clock_.TimeInMilliseconds())); const uint32_t kDummySsrc = 0; if (result.updated) { 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 7ba18c3675..444e0cd67d 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 @@ -172,6 +172,7 @@ class DelayBasedBweTest : public ::testing::Test { SimulatedClock clock_; // Time at the receiver. test::TestBitrateObserver bitrate_observer_; std::unique_ptr acknowledged_bitrate_estimator_; + const std::unique_ptr probe_bitrate_estimator_; std::unique_ptr bitrate_estimator_; std::unique_ptr stream_generator_; int64_t arrival_time_offset_ms_; 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 945a9c2eb4..93fb9e1b44 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -140,6 +140,7 @@ GoogCcNetworkController::GoogCcNetworkController(RtcEventLog* event_log, bandwidth_estimation_( absl::make_unique(event_log_)), alr_detector_(absl::make_unique()), + probe_bitrate_estimator_(new ProbeBitrateEstimator(event_log)), delay_based_bwe_(new DelayBasedBwe(event_log_)), acknowledged_bitrate_estimator_( absl::make_unique()), @@ -208,6 +209,7 @@ NetworkControlUpdate GoogCcNetworkController::OnNetworkRouteChange( } acknowledged_bitrate_estimator_.reset(new AcknowledgedBitrateEstimator()); + probe_bitrate_estimator_.reset(new ProbeBitrateEstimator(event_log_)); delay_based_bwe_.reset(new DelayBasedBwe(event_log_)); if (msg.constraints.starting_rate) delay_based_bwe_->SetStartBitrate(*msg.constraints.starting_rate); @@ -484,10 +486,18 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback( auto acknowledged_bitrate = acknowledged_bitrate_estimator_->bitrate(); bandwidth_estimation_->IncomingPacketFeedbackVector(report, acknowledged_bitrate); + for (const auto& feedback : received_feedback_vector) { + if (feedback.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) { + probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(feedback); + } + } + absl::optional probe_bitrate = + probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate(); DelayBasedBwe::Result result; result = delay_based_bwe_->IncomingPacketFeedbackVector( - received_feedback_vector, acknowledged_bitrate, report.feedback_time); + received_feedback_vector, acknowledged_bitrate, probe_bitrate, + report.feedback_time); NetworkControlUpdate update; if (result.updated) { diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.h b/modules/congestion_controller/goog_cc/goog_cc_network_control.h index 60a75ad8e4..382f027d5f 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h @@ -73,6 +73,7 @@ class GoogCcNetworkController : public NetworkControllerInterface { std::unique_ptr bandwidth_estimation_; std::unique_ptr alr_detector_; + std::unique_ptr probe_bitrate_estimator_; std::unique_ptr delay_based_bwe_; std::unique_ptr acknowledged_bitrate_estimator_; diff --git a/modules/congestion_controller/include/send_side_congestion_controller.h b/modules/congestion_controller/include/send_side_congestion_controller.h index 88530a22c1..746e590a59 100644 --- a/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/modules/congestion_controller/include/send_side_congestion_controller.h @@ -152,6 +152,8 @@ class SendSideCongestionController bool pacer_paused_; rtc::CriticalSection bwe_lock_; int min_bitrate_bps_ RTC_GUARDED_BY(bwe_lock_); + std::unique_ptr probe_bitrate_estimator_ + RTC_GUARDED_BY(bwe_lock_); std::unique_ptr delay_based_bwe_ RTC_GUARDED_BY(bwe_lock_); bool in_cwnd_experiment_; int64_t accepted_queue_ms_; diff --git a/modules/congestion_controller/send_side_congestion_controller.cc b/modules/congestion_controller/send_side_congestion_controller.cc index 36e11dcbc1..147f103e78 100644 --- a/modules/congestion_controller/send_side_congestion_controller.cc +++ b/modules/congestion_controller/send_side_congestion_controller.cc @@ -143,6 +143,7 @@ SendSideCongestionController::SendSideCongestionController( pause_pacer_(false), pacer_paused_(false), min_bitrate_bps_(congestion_controller::GetMinBitrateBps()), + probe_bitrate_estimator_(new ProbeBitrateEstimator(event_log_)), delay_based_bwe_(new DelayBasedBwe(event_log_)), in_cwnd_experiment_(CwndExperimentEnabled()), accepted_queue_ms_(kDefaultAcceptedQueueMs), @@ -258,6 +259,7 @@ void SendSideCongestionController::OnNetworkRouteChanged( rtc::CritScope cs(&bwe_lock_); transport_overhead_bytes_per_packet_ = network_route.packet_overhead; min_bitrate_bps_ = min_bitrate_bps; + probe_bitrate_estimator_.reset(new ProbeBitrateEstimator(event_log_)); delay_based_bwe_.reset(new DelayBasedBwe(event_log_)); acknowledged_bitrate_estimator_.reset(new AcknowledgedBitrateEstimator()); if (bitrate_bps > 0) { @@ -416,8 +418,15 @@ void SendSideCongestionController::OnTransportFeedback( DelayBasedBwe::Result result; { rtc::CritScope cs(&bwe_lock_); + for (const auto& packet : feedback_vector) { + if (packet.send_time_ms != PacketFeedback::kNoSendTime && + packet.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) { + probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(packet); + } + } result = delay_based_bwe_->IncomingPacketFeedbackVector( feedback_vector, acknowledged_bitrate_estimator_->bitrate(), + probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate(), Timestamp::ms(clock_->TimeInMilliseconds())); } if (result.updated) { diff --git a/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/modules/remote_bitrate_estimator/test/estimators/send_side.cc index 2907563947..289ff70ee2 100644 --- a/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -32,6 +32,7 @@ SendSideBweSender::SendSideBweSender(int kbps, &event_log_)), acknowledged_bitrate_estimator_( absl::make_unique()), + probe_bitrate_estimator_(new ProbeBitrateEstimator(nullptr)), bwe_(new DelayBasedBwe(nullptr)), feedback_observer_(bitrate_controller_.get()), clock_(clock), @@ -79,8 +80,14 @@ void SendSideBweSender::GiveFeedback(const FeedbackPacket& feedback) { PacketFeedbackComparator()); acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( packet_feedback_vector); + for (PacketFeedback& packet : packet_feedback_vector) { + if (packet.send_time_ms != PacketFeedback::kNoSendTime && + packet.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) + probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(packet); + } DelayBasedBwe::Result result = bwe_->IncomingPacketFeedbackVector( packet_feedback_vector, acknowledged_bitrate_estimator_->bitrate(), + probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate(), Timestamp::ms(clock_->TimeInMilliseconds())); if (result.updated) bitrate_controller_->OnDelayBasedBweResult(result); diff --git a/modules/remote_bitrate_estimator/test/estimators/send_side.h b/modules/remote_bitrate_estimator/test/estimators/send_side.h index 6e939c1f03..5b45e66fbc 100644 --- a/modules/remote_bitrate_estimator/test/estimators/send_side.h +++ b/modules/remote_bitrate_estimator/test/estimators/send_side.h @@ -39,6 +39,7 @@ class SendSideBweSender : public BweSender, public RemoteBitrateObserver { protected: std::unique_ptr bitrate_controller_; std::unique_ptr acknowledged_bitrate_estimator_; + std::unique_ptr probe_bitrate_estimator_; std::unique_ptr bwe_; RtcpBandwidthObserver* feedback_observer_;