From 61ac3a9971d53f46cee9774395604e3c1d7c9c41 Mon Sep 17 00:00:00 2001 From: dijunkun Date: Sat, 8 Feb 2025 17:59:30 +0800 Subject: [PATCH] [fix] fix timestamp in congestion control feedback --- .../rtp_channel/rtp_video_receiver.cpp | 9 ----- src/channel/video_channel_send.cpp | 7 ++-- src/channel/video_channel_send.h | 4 +- src/pc/peer_connection.h | 2 +- src/qos/congestion_control.cpp | 37 ++++++++++--------- src/qos/congestion_control_feedback.cpp | 3 -- .../congestion_control_feedback_generator.cc | 12 +----- src/qos/send_side_bandwidth_estimation.cc | 27 +++----------- src/qos/transport_feedback_adapter.cc | 3 -- src/transport/ice_transport.cpp | 5 +-- 10 files changed, 38 insertions(+), 71 deletions(-) diff --git a/src/channel/rtp_channel/rtp_video_receiver.cpp b/src/channel/rtp_channel/rtp_video_receiver.cpp index 7498e45..97c1ad5 100644 --- a/src/channel/rtp_channel/rtp_video_receiver.cpp +++ b/src/channel/rtp_channel/rtp_video_receiver.cpp @@ -430,15 +430,6 @@ void RtpVideoReceiver::SendCombinedRtcpPacket( RTCPSender rtcp_sender( [this](const uint8_t* buffer, size_t size) -> int { - webrtc::rtcp::CommonHeader rtcp_block; - // bool valid = true; - // if (!rtcp_block.Parse(buffer, size)) { - // valid = false; - // } - - webrtc::rtcp::CongestionControlFeedback feedback; - feedback.Parse(rtcp_block); - return data_send_func_((const char*)buffer, size); }, 1200); diff --git a/src/channel/video_channel_send.cpp b/src/channel/video_channel_send.cpp index 85cb7fc..236d33b 100644 --- a/src/channel/video_channel_send.cpp +++ b/src/channel/video_channel_send.cpp @@ -82,11 +82,12 @@ int VideoChannelSend::SendVideo(char* data, size_t size) { } void VideoChannelSend::OnCongestionControlFeedback( - int64_t recv_ts, const webrtc::rtcp::CongestionControlFeedback& feedback) { + Timestamp recv_ts, + const webrtc::rtcp::CongestionControlFeedback& feedback) { ++feedback_count_; std::optional feedback_msg = - transport_feedback_adapter_.ProcessCongestionControlFeedback( - feedback, webrtc::Timestamp::Micros(recv_ts)); + transport_feedback_adapter_.ProcessCongestionControlFeedback(feedback, + recv_ts); if (feedback_msg) { HandleTransportPacketsFeedback(*feedback_msg); } diff --git a/src/channel/video_channel_send.h b/src/channel/video_channel_send.h index 2c3dc9e..7215a47 100644 --- a/src/channel/video_channel_send.h +++ b/src/channel/video_channel_send.h @@ -8,6 +8,7 @@ #define _VIDEO_CHANNEL_SEND_H_ #include "api/transport/network_types.h" +#include "api/units/timestamp.h" #include "clock.h" #include "congestion_control.h" #include "congestion_control_feedback.h" @@ -32,7 +33,8 @@ class VideoChannelSend { int SendVideo(char* data, size_t size); void OnCongestionControlFeedback( - int64_t recv_ts, const webrtc::rtcp::CongestionControlFeedback& feedback); + Timestamp recv_ts, + const webrtc::rtcp::CongestionControlFeedback& feedback); void HandleTransportPacketsFeedback( const webrtc::TransportPacketsFeedback& feedback); diff --git a/src/pc/peer_connection.h b/src/pc/peer_connection.h index 9299a41..f607729 100644 --- a/src/pc/peer_connection.h +++ b/src/pc/peer_connection.h @@ -143,7 +143,7 @@ class PeerConnection { bool av1_encoding_ = false; bool enable_turn_ = false; bool trickle_ice_ = true; - bool reliable_ice_ = true; + bool reliable_ice_ = false; bool try_rejoin_with_turn_ = false; TraversalMode mode_ = TraversalMode::P2P; diff --git a/src/qos/congestion_control.cpp b/src/qos/congestion_control.cpp index c526845..9978aae 100644 --- a/src/qos/congestion_control.cpp +++ b/src/qos/congestion_control.cpp @@ -30,7 +30,8 @@ CongestionControl::CongestionControl() limit_pacingfactor_by_upper_link_capacity_estimate_(false), probe_controller_(new ProbeController()), congestion_window_pushback_controller_( - std::make_unique()), + // std::make_unique() + nullptr), bandwidth_estimation_(new SendSideBandwidthEstimation()), alr_detector_(new AlrDetector()), probe_bitrate_estimator_(new ProbeBitrateEstimator()), @@ -53,10 +54,10 @@ NetworkControlUpdate CongestionControl::OnTransportPacketsFeedback( return NetworkControlUpdate(); } - if (congestion_window_pushback_controller_) { - congestion_window_pushback_controller_->UpdateOutstandingData( - report.data_in_flight.bytes()); - } + // if (congestion_window_pushback_controller_) { + // congestion_window_pushback_controller_->UpdateOutstandingData( + // report.data_in_flight.bytes()); + // } TimeDelta max_feedback_rtt = TimeDelta::MinusInfinity(); TimeDelta min_propagation_rtt = TimeDelta::PlusInfinity(); Timestamp max_recv_time = Timestamp::MinusInfinity(); @@ -89,8 +90,9 @@ NetworkControlUpdate CongestionControl::OnTransportPacketsFeedback( std::accumulate(feedback_max_rtts_.begin(), feedback_max_rtts_.end(), static_cast(0)); int64_t mean_rtt_ms = sum_rtt_ms / feedback_max_rtts_.size(); - if (delay_based_bwe_) + if (delay_based_bwe_) { delay_based_bwe_->OnRttUpdate(TimeDelta::Millis(mean_rtt_ms)); + } } TimeDelta feedback_min_rtt = TimeDelta::PlusInfinity(); @@ -114,8 +116,6 @@ NetworkControlUpdate CongestionControl::OnTransportPacketsFeedback( if (report.feedback_time > next_loss_update_) { next_loss_update_ = report.feedback_time + TimeDelta::Millis(kLossUpdateInterval); - LOG_WARN("lost_packets_since_last_loss_update_ = [{}]", - lost_packets_since_last_loss_update_); bandwidth_estimation_->UpdatePacketsLost( lost_packets_since_last_loss_update_, expected_packets_since_last_loss_update_, report.feedback_time); @@ -132,9 +132,12 @@ NetworkControlUpdate CongestionControl::OnTransportPacketsFeedback( probe_controller_->SetAlrEndedTimeMs(now_ms); } previously_in_alr_ = alr_start_time.has_value(); + acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( report.SortedByReceiveTime()); auto acknowledged_bitrate = acknowledged_bitrate_estimator_->bitrate(); + // TODO: fix acknowledged_bitrate + // acknowledged_bitrate = DataRate::KilobitsPerSec(1000); bandwidth_estimation_->SetAcknowledgedRate(acknowledged_bitrate, report.feedback_time); for (const auto& feedback : report.SortedByReceiveTime()) { @@ -186,17 +189,17 @@ NetworkControlUpdate CongestionControl::OnTransportPacketsFeedback( // MaybeTriggerOnNetworkChanged(&update, report.feedback_time); // } - // recovered_from_overuse = result.recovered_from_overuse; + recovered_from_overuse = result.recovered_from_overuse; - // if (recovered_from_overuse) { - // probe_controller_->SetAlrStartTimeMs(alr_start_time); - // auto probes = probe_controller_->RequestProbe(report.feedback_time); - // update.probe_cluster_configs.insert(update.probe_cluster_configs.end(), - // probes.begin(), probes.end()); - // } + if (recovered_from_overuse) { + probe_controller_->SetAlrStartTimeMs(alr_start_time); + auto probes = probe_controller_->RequestProbe(report.feedback_time); + update.probe_cluster_configs.insert(update.probe_cluster_configs.end(), + probes.begin(), probes.end()); + } - // // No valid RTT could be because send-side BWE isn't used, in which case - // // we don't try to limit the outstanding packets. + // No valid RTT could be because send-side BWE isn't used, in which case + // we don't try to limit the outstanding packets. // if (rate_control_settings_.UseCongestionWindow() && // max_feedback_rtt.IsFinite()) { // UpdateCongestionWindowSize(); diff --git a/src/qos/congestion_control_feedback.cpp b/src/qos/congestion_control_feedback.cpp index daf131b..1784215 100644 --- a/src/qos/congestion_control_feedback.cpp +++ b/src/qos/congestion_control_feedback.cpp @@ -297,9 +297,6 @@ bool CongestionControlFeedback::Parse(const rtcp::CommonHeader& packet) { uint16_t seq_no = base_seqno + i; bool received = (packet_info & 0x8000); - TimeDelta arrival_time_offset = AtoToTimeDelta(packet_info); - LOG_ERROR("received:{} = [{} {}]", received, - ToString(arrival_time_offset), arrival_time_offset.IsFinite()); packets_.push_back( {ssrc, seq_no, received ? AtoToTimeDelta(packet_info) : TimeDelta::MinusInfinity(), diff --git a/src/qos/congestion_control_feedback_generator.cc b/src/qos/congestion_control_feedback_generator.cc index c6b0d6f..56d240c 100644 --- a/src/qos/congestion_control_feedback_generator.cc +++ b/src/qos/congestion_control_feedback_generator.cc @@ -45,20 +45,13 @@ void CongestionControlFeedbackGenerator::OnReceivedPacket( } feedback_trackers_[packet.Ssrc()].ReceivedPacket(packet); if (NextFeedbackTime() < packet.arrival_time()) { - SendFeedback(Timestamp::Micros( - std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()) - .count())); + SendFeedback(clock_->CurrentTime()); } } Timestamp CongestionControlFeedbackGenerator::NextFeedbackTime() const { if (!first_arrival_time_since_feedback_) { - return std::max(Timestamp::Micros( - std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()) - .count()) + - min_time_between_feedback_, + return std::max(clock_->CurrentTime() + min_time_between_feedback_, next_possible_feedback_send_time_); } @@ -94,7 +87,6 @@ void CongestionControlFeedbackGenerator::SendFeedback(Timestamp now) { for (auto& [unused, tracker] : feedback_trackers_) { tracker.AddPacketsToFeedback(now, rtcp_packet_info); } - marker_bit_seen_ = false; first_arrival_time_since_feedback_ = std::nullopt; diff --git a/src/qos/send_side_bandwidth_estimation.cc b/src/qos/send_side_bandwidth_estimation.cc index 8c36be2..31f186f 100644 --- a/src/qos/send_side_bandwidth_estimation.cc +++ b/src/qos/send_side_bandwidth_estimation.cc @@ -163,7 +163,7 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation() low_loss_threshold_(kDefaultLowLossThreshold), high_loss_threshold_(kDefaultHighLossThreshold), bitrate_threshold_(kDefaultBitrateThreshold), - disable_receiver_limit_caps_only_("Disabled") { + disable_receiver_limit_caps_only_(false) { // rtt_backoff_ = } @@ -204,7 +204,6 @@ void SendSideBandwidthEstimation::SetBitrates( void SendSideBandwidthEstimation::SetSendBitrate(DataRate bitrate, Timestamp at_time) { - LOG_ERROR("3"); // Reset to avoid being capped by the estimate. delay_based_limit_ = DataRate::PlusInfinity(); UpdateTargetBitrate(bitrate, at_time); @@ -245,7 +244,6 @@ DataRate SendSideBandwidthEstimation::GetEstimatedLinkCapacity() const { void SendSideBandwidthEstimation::UpdateReceiverEstimate(Timestamp at_time, DataRate bandwidth) { - LOG_ERROR("6"); // TODO(srte): Ensure caller passes PlusInfinity, not zero, to represent no // limitation. receiver_limit_ = bandwidth.IsZero() ? DataRate::PlusInfinity() : bandwidth; @@ -254,7 +252,6 @@ void SendSideBandwidthEstimation::UpdateReceiverEstimate(Timestamp at_time, void SendSideBandwidthEstimation::UpdateDelayBasedEstimate(Timestamp at_time, DataRate bitrate) { - LOG_ERROR("7"); link_capacity_.UpdateDelayBasedEstimate(at_time, bitrate); // TODO(srte): Ensure caller passes PlusInfinity, not zero, to represent no // limitation. @@ -345,9 +342,7 @@ void SendSideBandwidthEstimation::UpdateRtt(TimeDelta rtt, Timestamp at_time) { } void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) { - LOG_ERROR("1"); if (rtt_backoff_.IsRttAboveLimit()) { - LOG_ERROR("11"); if (at_time - time_last_decrease_ >= rtt_backoff_.drop_interval_ && current_target_ > rtt_backoff_.bandwidth_floor_) { time_last_decrease_ = at_time; @@ -362,7 +357,6 @@ void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) { ApplyTargetLimits(at_time); return; } - LOG_ERROR("111"); // We trust the REMB and/or delay-based estimate during the first 2 seconds if // we haven't had any packet loss reported, to allow startup bitrate probing. if (last_fraction_loss_ == 0 && IsInStartPhase(at_time)) { @@ -381,28 +375,23 @@ void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) { return; } } - LOG_ERROR("112"); UpdateMinHistory(at_time); if (last_loss_packet_report_.IsInfinite()) { // No feedback received. // TODO(srte): This is likely redundant in most cases. - LOG_ERROR("113"); ApplyTargetLimits(at_time); return; } TimeDelta time_since_loss_packet_report = at_time - last_loss_packet_report_; if (time_since_loss_packet_report < 1.2 * kMaxRtcpFeedbackInterval) { - LOG_ERROR("114"); // We only care about loss above a given bitrate threshold. float loss = last_fraction_loss_ / 256.0f; - LOG_ERROR("current_target_ = [{}], loss = [{}]", ToString(current_target_), - loss); // We only make decisions based on loss when the bitrate is above a // threshold. This is a crude way of handling loss which is uncorrelated // to congestion. + LOG_WARN("loss: [{}]", loss); if (current_target_ < bitrate_threshold_ || loss <= low_loss_threshold_) { - LOG_ERROR("115"); // Loss < 2%: Increase rate by 8% of the min bitrate in the last // kBweIncreaseInterval. // Note that by remembering the bitrate over the last second one can @@ -420,21 +409,18 @@ void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) { // (gives a little extra increase at low rates, negligible at higher // rates). new_bitrate += DataRate::BitsPerSec(1000); + LOG_WARN("1 new_bitrate: [{}]", ToString(new_bitrate).c_str()); UpdateTargetBitrate(new_bitrate, at_time); return; } else if (current_target_ > bitrate_threshold_) { - LOG_ERROR("116"); if (loss <= high_loss_threshold_) { - LOG_ERROR("117"); // Loss between 2% - 10%: Do nothing. } else { - LOG_ERROR("118"); // Loss > 10%: Limit the rate decreases to once a kBweDecreaseInterval // + rtt. if (!has_decreased_since_last_fraction_loss_ && (at_time - time_last_decrease_) >= (kBweDecreaseInterval + last_round_trip_time_)) { - LOG_ERROR("119"); time_last_decrease_ = at_time; // Reduce rate: @@ -445,13 +431,13 @@ void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) { static_cast(512 - last_fraction_loss_)) / 512.0); has_decreased_since_last_fraction_loss_ = true; + LOG_WARN("2 new_bitrate: [{}]", ToString(new_bitrate).c_str()); UpdateTargetBitrate(new_bitrate, at_time); return; } } } } - LOG_ERROR("120"); // TODO(srte): This is likely redundant in most cases. ApplyTargetLimits(at_time); } @@ -494,8 +480,9 @@ void SendSideBandwidthEstimation::UpdateMinHistory(Timestamp at_time) { DataRate SendSideBandwidthEstimation::GetUpperLimit() const { DataRate upper_limit = delay_based_limit_; - if (disable_receiver_limit_caps_only_) + if (disable_receiver_limit_caps_only_) { upper_limit = std::min(upper_limit, receiver_limit_); + } return std::min(upper_limit, max_bitrate_configured_); } @@ -512,7 +499,6 @@ void SendSideBandwidthEstimation::MaybeLogLowBitrateWarning(DataRate bitrate, void SendSideBandwidthEstimation::UpdateTargetBitrate(DataRate new_bitrate, Timestamp at_time) { new_bitrate = std::min(new_bitrate, GetUpperLimit()); - LOG_WARN("new_bitrate: [{}]", ToString(new_bitrate).c_str()); if (new_bitrate < min_bitrate_configured_) { MaybeLogLowBitrateWarning(new_bitrate, at_time); new_bitrate = min_bitrate_configured_; @@ -522,7 +508,6 @@ void SendSideBandwidthEstimation::UpdateTargetBitrate(DataRate new_bitrate, } void SendSideBandwidthEstimation::ApplyTargetLimits(Timestamp at_time) { - LOG_ERROR("2"); UpdateTargetBitrate(current_target_, at_time); } diff --git a/src/qos/transport_feedback_adapter.cc b/src/qos/transport_feedback_adapter.cc index ff8f2fd..243d509 100644 --- a/src/qos/transport_feedback_adapter.cc +++ b/src/qos/transport_feedback_adapter.cc @@ -262,7 +262,6 @@ TransportFeedbackAdapter::ProcessCongestionControlFeedback( int failed_lookups = 0; bool supports_ecn = true; std::vector packet_result_vector; - LOG_ERROR("20"); for (const rtcp::CongestionControlFeedback::PacketInfo& packet_info : feedback.packets()) { std::optional packet_feedback = RetrievePacketFeedback( @@ -278,9 +277,7 @@ TransportFeedbackAdapter::ProcessCongestionControlFeedback( } PacketResult result; result.sent_packet = packet_feedback->sent; - LOG_ERROR("21"); if (packet_info.arrival_time_offset.IsFinite()) { - LOG_ERROR("22"); result.receive_time = current_offset_ - packet_info.arrival_time_offset; supports_ecn &= packet_info.ecn != EcnMarking::kNotEct; } diff --git a/src/transport/ice_transport.cpp b/src/transport/ice_transport.cpp index b22e5ba..6e45026 100644 --- a/src/transport/ice_transport.cpp +++ b/src/transport/ice_transport.cpp @@ -300,7 +300,6 @@ bool IceTransport::ParseRtcpPacket(const uint8_t *buffer, size_t size, case RtcpPacket::PAYLOAD_TYPE::TCC: switch (rtcp_block.fmt()) { case webrtc::rtcp::CongestionControlFeedback::kFeedbackMessageType: - LOG_INFO("Receive congestion control feedback"); valid = HandleCongestionControlFeedback(rtcp_block, rtcp_packet_info); break; default: @@ -374,8 +373,8 @@ bool IceTransport::HandleCongestionControlFeedback( // rtcp_packet_info->congestion_control_feedback.emplace(std::move(feedback)); // } - video_channel_send_->OnCongestionControlFeedback( - std::chrono::system_clock::now().time_since_epoch().count(), feedback); + video_channel_send_->OnCongestionControlFeedback(clock_->CurrentTime(), + feedback); return true; }