From 20ad2544b423d2cab523889fc126fa6e851ea844 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 11 Oct 2018 18:48:49 +0200 Subject: [PATCH] Adds tracking of allocated but unacknowledged bitrate. This adds tracking of traffic for streams that are part of bitrate allocation but without packet feedback to send side congestion controller. This is part of a series of CLs that allows GoogCC to track sent bitrate that is included in bitrate allocation but without transport feedback. Bug: webrtc:9796 Change-Id: I13e994461c26638d76e8f2f115e6d375e4403116 Reviewed-on: https://webrtc-review.googlesource.com/c/104940 Commit-Queue: Sebastian Jansson Reviewed-by: Christoffer Rodbro Cr-Commit-Position: refs/heads/master@{#25126} --- .../rtp/send_side_congestion_controller.cc | 42 ++++++++++--------- ...end_side_congestion_controller_unittest.cc | 5 ++- .../rtp/send_time_history.cc | 23 +++++++++- .../rtp/send_time_history.h | 5 +++ .../rtp/transport_feedback_adapter.cc | 6 +++ .../rtp/transport_feedback_adapter.h | 1 + 6 files changed, 60 insertions(+), 22 deletions(-) diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc index 953214e47b..a962fd7b7b 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc @@ -547,26 +547,28 @@ void SendSideCongestionController::SignalNetworkState(NetworkState state) { void SendSideCongestionController::OnSentPacket( const rtc::SentPacket& sent_packet) { - // We're not interested in packets without an id, which may be stun packets, - // etc, sent on the same transport. - if (sent_packet.packet_id == -1) - return; - transport_feedback_adapter_.OnSentPacket(sent_packet.packet_id, - sent_packet.send_time_ms); - MaybeUpdateOutstandingData(); - auto packet = transport_feedback_adapter_.GetPacket(sent_packet.packet_id); - if (packet.has_value()) { - SentPacket msg; - msg.size = DataSize::bytes(packet->payload_size); - msg.send_time = Timestamp::ms(packet->send_time_ms); - msg.sequence_number = packet->long_sequence_number; - msg.data_in_flight = - DataSize::bytes(transport_feedback_adapter_.GetOutstandingBytes()); - task_queue_->PostTask([this, msg]() { - RTC_DCHECK_RUN_ON(task_queue_); - if (controller_) - control_handler_->PostUpdates(controller_->OnSentPacket(msg)); - }); + if (sent_packet.packet_id != -1) { + transport_feedback_adapter_.OnSentPacket(sent_packet.packet_id, + sent_packet.send_time_ms); + MaybeUpdateOutstandingData(); + auto packet = transport_feedback_adapter_.GetPacket(sent_packet.packet_id); + if (packet.has_value()) { + SentPacket msg; + msg.size = DataSize::bytes(packet->payload_size); + msg.send_time = Timestamp::ms(packet->send_time_ms); + msg.sequence_number = packet->long_sequence_number; + msg.prior_unacked_data = DataSize::bytes(packet->unacknowledged_data); + msg.data_in_flight = + DataSize::bytes(transport_feedback_adapter_.GetOutstandingBytes()); + task_queue_->PostTask([this, msg]() { + RTC_DCHECK_RUN_ON(task_queue_); + if (controller_) + control_handler_->PostUpdates(controller_->OnSentPacket(msg)); + }); + } + } else if (sent_packet.info.included_in_allocation) { + transport_feedback_adapter_.AddUntracked(sent_packet.info.packet_size_bytes, + sent_packet.send_time_ms); } } diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc index d610afe13d..66ec5783c8 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc @@ -108,8 +108,11 @@ class SendSideCongestionControllerTest : public ::testing::Test { controller_->AddPacket(ssrc, packet_feedback.sequence_number, packet_feedback.payload_size, packet_feedback.pacing_info); + rtc::PacketInfo packet_info; + packet_info.included_in_feedback = true; controller_->OnSentPacket(rtc::SentPacket(packet_feedback.sequence_number, - packet_feedback.send_time_ms)); + packet_feedback.send_time_ms, + packet_info)); } // Allows us to track the target bitrate, without prescribing the exact diff --git a/modules/congestion_controller/rtp/send_time_history.cc b/modules/congestion_controller/rtp/send_time_history.cc index 73682cd5b2..205f86a420 100644 --- a/modules/congestion_controller/rtp/send_time_history.cc +++ b/modules/congestion_controller/rtp/send_time_history.cc @@ -15,6 +15,7 @@ #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/checks.h" +#include "rtc_base/logging.h" #include "system_wrappers/include/clock.h" namespace webrtc { @@ -41,8 +42,19 @@ void SendTimeHistory::AddAndRemoveOld(const PacketFeedback& packet) { PacketFeedback packet_copy = packet; packet_copy.long_sequence_number = unwrapped_seq_num; history_.insert(std::make_pair(unwrapped_seq_num, packet_copy)); - if (packet.send_time_ms >= 0) + if (packet.send_time_ms >= 0) { AddPacketBytes(packet_copy); + last_send_time_ms_ = std::max(last_send_time_ms_, packet.send_time_ms); + } +} + +void SendTimeHistory::AddUntracked(size_t packet_size, int64_t send_time_ms) { + if (send_time_ms < last_send_time_ms_) { + RTC_LOG(LS_WARNING) << "ignoring untracked data for out of order packet."; + } + pending_untracked_size_ += packet_size; + last_untracked_send_time_ms_ = + std::max(last_untracked_send_time_ms_, send_time_ms); } bool SendTimeHistory::OnSentPacket(uint16_t sequence_number, @@ -53,8 +65,17 @@ bool SendTimeHistory::OnSentPacket(uint16_t sequence_number, return false; bool packet_retransmit = it->second.send_time_ms >= 0; it->second.send_time_ms = send_time_ms; + last_send_time_ms_ = std::max(last_send_time_ms_, send_time_ms); if (!packet_retransmit) AddPacketBytes(it->second); + if (pending_untracked_size_ > 0) { + if (send_time_ms < last_untracked_send_time_ms_) + RTC_LOG(LS_WARNING) + << "appending acknowledged data for out of order packet. (Diff: " + << last_untracked_send_time_ms_ - send_time_ms << " ms.)"; + it->second.unacknowledged_data += pending_untracked_size_; + pending_untracked_size_ = 0; + } return true; } diff --git a/modules/congestion_controller/rtp/send_time_history.h b/modules/congestion_controller/rtp/send_time_history.h index 7cc6ad7641..98d3d5cc82 100644 --- a/modules/congestion_controller/rtp/send_time_history.h +++ b/modules/congestion_controller/rtp/send_time_history.h @@ -29,6 +29,8 @@ class SendTimeHistory { // Cleanup old entries, then add new packet info with provided parameters. void AddAndRemoveOld(const PacketFeedback& packet); + void AddUntracked(size_t packet_size, int64_t send_time_ms); + // Updates packet info identified by |sequence_number| with |send_time_ms|. // Return false if not found. bool OnSentPacket(uint16_t sequence_number, int64_t send_time_ms); @@ -52,6 +54,9 @@ class SendTimeHistory { void UpdateAckedSeqNum(int64_t acked_seq_num); const Clock* const clock_; const int64_t packet_age_limit_ms_; + size_t pending_untracked_size_ = 0; + int64_t last_send_time_ms_ = -1; + int64_t last_untracked_send_time_ms_ = -1; SequenceNumberUnwrapper seq_num_unwrapper_; std::map history_; absl::optional last_ack_seq_num_; diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.cc b/modules/congestion_controller/rtp/transport_feedback_adapter.cc index d1b4855e36..f5d2c38336 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.cc @@ -77,6 +77,12 @@ void TransportFeedbackAdapter::AddPacket(uint32_t ssrc, } } +void TransportFeedbackAdapter::AddUntracked(size_t packet_size, + int64_t send_time_ms) { + rtc::CritScope cs(&lock_); + send_time_history_.AddUntracked(packet_size, send_time_ms); +} + void TransportFeedbackAdapter::OnSentPacket(uint16_t sequence_number, int64_t send_time_ms) { rtc::CritScope cs(&lock_); diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.h b/modules/congestion_controller/rtp/transport_feedback_adapter.h index e2477ca31f..ee957752ce 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.h +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.h @@ -42,6 +42,7 @@ class TransportFeedbackAdapter { uint16_t sequence_number, size_t length, const PacedPacketInfo& pacing_info); + void AddUntracked(size_t packet_size, int64_t send_time_ms); void OnSentPacket(uint16_t sequence_number, int64_t send_time_ms); // TODO(holmer): This method should return DelayBasedBwe::Result so that we