diff --git a/modules/congestion_controller/rtp/BUILD.gn b/modules/congestion_controller/rtp/BUILD.gn index 2c7377990e..d3e995e8de 100644 --- a/modules/congestion_controller/rtp/BUILD.gn +++ b/modules/congestion_controller/rtp/BUILD.gn @@ -43,8 +43,6 @@ rtc_source_set("control_handler") { rtc_static_library("transport_feedback") { visibility = [ "*" ] sources = [ - "send_time_history.cc", - "send_time_history.h", "transport_feedback_adapter.cc", "transport_feedback_adapter.h", ] @@ -71,7 +69,6 @@ if (rtc_include_tests) { sources = [ "congestion_controller_unittests_helper.cc", "congestion_controller_unittests_helper.h", - "send_time_history_unittest.cc", "transport_feedback_adapter_unittest.cc", ] deps = [ diff --git a/modules/congestion_controller/rtp/send_time_history.cc b/modules/congestion_controller/rtp/send_time_history.cc deleted file mode 100644 index 2d0356b996..0000000000 --- a/modules/congestion_controller/rtp/send_time_history.cc +++ /dev/null @@ -1,169 +0,0 @@ -/* - * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/congestion_controller/rtp/send_time_history.h" - -#include -#include - -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "rtc_base/checks.h" -#include "rtc_base/logging.h" - -namespace webrtc { - -SendTimeHistory::SendTimeHistory(int64_t packet_age_limit_ms) - : packet_age_limit_ms_(packet_age_limit_ms) {} - -SendTimeHistory::~SendTimeHistory() {} - -void SendTimeHistory::RemoveOld(int64_t at_time_ms) { - while (!history_.empty() && - at_time_ms - history_.begin()->second.creation_time_ms > - packet_age_limit_ms_) { - // TODO(sprang): Warn if erasing (too many) old items? - RemovePacketBytes(history_.begin()->second); - history_.erase(history_.begin()); - } -} - -void SendTimeHistory::AddNewPacket(PacketFeedback packet) { - packet.long_sequence_number = - seq_num_unwrapper_.Unwrap(packet.sequence_number); - history_.insert(std::make_pair(packet.long_sequence_number, packet)); - if (packet.send_time_ms >= 0) { - AddPacketBytes(packet); - 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); -} - -SendTimeHistory::Status SendTimeHistory::OnSentPacket(uint16_t sequence_number, - int64_t send_time_ms) { - int64_t unwrapped_seq_num = seq_num_unwrapper_.Unwrap(sequence_number); - auto it = history_.find(unwrapped_seq_num); - if (it == history_.end()) - return Status::kNotAdded; - 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 packet_retransmit ? Status::kDuplicate : Status::kOk; -} - -absl::optional SendTimeHistory::GetPacket( - uint16_t sequence_number) const { - int64_t unwrapped_seq_num = - seq_num_unwrapper_.UnwrapWithoutUpdate(sequence_number); - absl::optional optional_feedback; - auto it = history_.find(unwrapped_seq_num); - if (it != history_.end()) - optional_feedback.emplace(it->second); - return optional_feedback; -} - -bool SendTimeHistory::GetFeedback(PacketFeedback* packet_feedback, - bool remove) { - RTC_DCHECK(packet_feedback); - int64_t unwrapped_seq_num = - seq_num_unwrapper_.Unwrap(packet_feedback->sequence_number); - UpdateAckedSeqNum(unwrapped_seq_num); - RTC_DCHECK_GE(*last_ack_seq_num_, 0); - auto it = history_.find(unwrapped_seq_num); - if (it == history_.end()) - return false; - - // Save arrival_time not to overwrite it. - int64_t arrival_time_ms = packet_feedback->arrival_time_ms; - *packet_feedback = it->second; - packet_feedback->arrival_time_ms = arrival_time_ms; - - if (remove) - history_.erase(it); - return true; -} - -DataSize SendTimeHistory::GetOutstandingData(uint16_t local_net_id, - uint16_t remote_net_id) const { - auto it = in_flight_bytes_.find({local_net_id, remote_net_id}); - if (it != in_flight_bytes_.end()) { - return DataSize::bytes(it->second); - } else { - return DataSize::Zero(); - } -} - -absl::optional SendTimeHistory::GetFirstUnackedSendTime() const { - if (!last_ack_seq_num_) - return absl::nullopt; - auto it = history_.find(*last_ack_seq_num_); - if (it == history_.end() || - it->second.send_time_ms == PacketFeedback::kNoSendTime) - return absl::nullopt; - return it->second.send_time_ms; -} - -void SendTimeHistory::AddPacketBytes(const PacketFeedback& packet) { - if (packet.send_time_ms < 0 || packet.payload_size == 0 || - (last_ack_seq_num_ && *last_ack_seq_num_ >= packet.long_sequence_number)) - return; - auto it = in_flight_bytes_.find({packet.local_net_id, packet.remote_net_id}); - if (it != in_flight_bytes_.end()) { - it->second += packet.payload_size; - } else { - in_flight_bytes_[{packet.local_net_id, packet.remote_net_id}] = - packet.payload_size; - } -} - -void SendTimeHistory::RemovePacketBytes(const PacketFeedback& packet) { - if (packet.send_time_ms < 0 || packet.payload_size == 0 || - (last_ack_seq_num_ && *last_ack_seq_num_ >= packet.long_sequence_number)) - return; - auto it = in_flight_bytes_.find({packet.local_net_id, packet.remote_net_id}); - if (it != in_flight_bytes_.end()) { - it->second -= packet.payload_size; - if (it->second == 0) - in_flight_bytes_.erase(it); - } -} - -void SendTimeHistory::UpdateAckedSeqNum(int64_t acked_seq_num) { - if (last_ack_seq_num_ && *last_ack_seq_num_ >= acked_seq_num) - return; - - auto unacked_it = history_.begin(); - if (last_ack_seq_num_) - unacked_it = history_.lower_bound(*last_ack_seq_num_); - - auto newly_acked_end = history_.upper_bound(acked_seq_num); - for (; unacked_it != newly_acked_end; ++unacked_it) { - RemovePacketBytes(unacked_it->second); - } - last_ack_seq_num_.emplace(acked_seq_num); -} -} // namespace webrtc diff --git a/modules/congestion_controller/rtp/send_time_history.h b/modules/congestion_controller/rtp/send_time_history.h deleted file mode 100644 index 9563fb8681..0000000000 --- a/modules/congestion_controller/rtp/send_time_history.h +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef MODULES_CONGESTION_CONTROLLER_RTP_SEND_TIME_HISTORY_H_ -#define MODULES_CONGESTION_CONTROLLER_RTP_SEND_TIME_HISTORY_H_ - -#include -#include - -#include "absl/types/optional.h" -#include "api/units/data_size.h" -#include "modules/include/module_common_types_public.h" -#include "rtc_base/constructor_magic.h" - -namespace webrtc { -struct PacketFeedback; - -class SendTimeHistory { - public: - enum class Status { kNotAdded, kOk, kDuplicate }; - - explicit SendTimeHistory(int64_t packet_age_limit_ms); - ~SendTimeHistory(); - - // Cleanup old entries, then add new packet info with provided parameters. - void RemoveOld(int64_t at_time_ms); - void AddNewPacket(PacketFeedback packet); - - void AddUntracked(size_t packet_size, int64_t send_time_ms); - - // Updates packet info identified by |sequence_number| with |send_time_ms|. - // Returns a PacketSendState indicating if the packet was not found, sent, - // or if it was previously already marked as sent. - Status OnSentPacket(uint16_t sequence_number, int64_t send_time_ms); - - // Retrieves packet info identified by |sequence_number|. - absl::optional GetPacket(uint16_t sequence_number) const; - - // Look up PacketFeedback for a sent packet, based on the sequence number, and - // populate all fields except for arrival_time. The packet parameter must - // thus be non-null and have the sequence_number field set. - bool GetFeedback(PacketFeedback* packet_feedback, bool remove); - - DataSize GetOutstandingData(uint16_t local_net_id, - uint16_t remote_net_id) const; - - absl::optional GetFirstUnackedSendTime() const; - - private: - using RemoteAndLocalNetworkId = std::pair; - - void AddPacketBytes(const PacketFeedback& packet); - void RemovePacketBytes(const PacketFeedback& packet); - void UpdateAckedSeqNum(int64_t acked_seq_num); - 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_; - std::map in_flight_bytes_; - - RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(SendTimeHistory); -}; - -} // namespace webrtc -#endif // MODULES_CONGESTION_CONTROLLER_RTP_SEND_TIME_HISTORY_H_ diff --git a/modules/congestion_controller/rtp/send_time_history_unittest.cc b/modules/congestion_controller/rtp/send_time_history_unittest.cc deleted file mode 100644 index 604685cee8..0000000000 --- a/modules/congestion_controller/rtp/send_time_history_unittest.cc +++ /dev/null @@ -1,260 +0,0 @@ -/* - * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/congestion_controller/rtp/send_time_history.h" - -#include -#include -#include -#include - -#include "api/transport/network_types.h" -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "system_wrappers/include/clock.h" -#include "test/gtest.h" - -namespace webrtc { -namespace test { - -static const int kDefaultHistoryLengthMs = 1000; - -class SendTimeHistoryTest : public ::testing::Test { - protected: - SendTimeHistoryTest() : clock_(0), history_(kDefaultHistoryLengthMs) {} - ~SendTimeHistoryTest() {} - - virtual void SetUp() {} - - virtual void TearDown() {} - - void AddPacketWithSendTime(uint16_t sequence_number, - size_t length, - int64_t send_time_ms, - const PacedPacketInfo& pacing_info) { - PacketFeedback packet(clock_.TimeInMilliseconds(), sequence_number, length, - 0, 0, pacing_info); - history_.RemoveOld(clock_.TimeInMilliseconds()); - history_.AddNewPacket(packet); - history_.OnSentPacket(sequence_number, send_time_ms); - } - - webrtc::SimulatedClock clock_; - SendTimeHistory history_; -}; - -TEST_F(SendTimeHistoryTest, SaveAndRestoreNetworkId) { - const PacedPacketInfo kPacingInfo(0, 5, 1200); - uint16_t sequence_number = 0; - int64_t now_ms = clock_.TimeInMilliseconds(); - for (int i = 1; i < 5; ++i) { - PacketFeedback packet(now_ms, sequence_number, 1000, i, i - 1, kPacingInfo); - history_.RemoveOld(clock_.TimeInMilliseconds()); - history_.AddNewPacket(packet); - history_.OnSentPacket(sequence_number, now_ms); - PacketFeedback restored(now_ms, sequence_number); - EXPECT_TRUE(history_.GetFeedback(&restored, sequence_number++)); - EXPECT_EQ(packet.local_net_id, restored.local_net_id); - EXPECT_EQ(packet.remote_net_id, restored.remote_net_id); - } -} - -TEST_F(SendTimeHistoryTest, AddRemoveOne) { - const uint16_t kSeqNo = 10; - // TODO(philipel): Fix PacedPacketInfo constructor? - const PacedPacketInfo kPacingInfo(0, 5, 1200); - const PacketFeedback kSentPacket(0, 1, kSeqNo, 1, kPacingInfo); - AddPacketWithSendTime(kSeqNo, 1, 1, kPacingInfo); - - PacketFeedback received_packet(0, 0, kSeqNo, 0, kPacingInfo); - EXPECT_TRUE(history_.GetFeedback(&received_packet, false)); - EXPECT_EQ(kSentPacket, received_packet); - - PacketFeedback received_packet2(0, 0, kSeqNo, 0, kPacingInfo); - EXPECT_TRUE(history_.GetFeedback(&received_packet2, true)); - EXPECT_EQ(kSentPacket, received_packet2); - - PacketFeedback received_packet3(0, 0, kSeqNo, 0, kPacingInfo); - EXPECT_FALSE(history_.GetFeedback(&received_packet3, true)); -} - -TEST_F(SendTimeHistoryTest, GetPacketReturnsSentPacket) { - const uint16_t kSeqNo = 10; - const PacedPacketInfo kPacingInfo(0, 5, 1200); - const PacketFeedback kSentPacket(0, -1, 1, kSeqNo, 123, 0, 0, kPacingInfo); - AddPacketWithSendTime(kSeqNo, 123, 1, kPacingInfo); - auto sent_packet = history_.GetPacket(kSeqNo); - EXPECT_EQ(kSentPacket, *sent_packet); -} - -TEST_F(SendTimeHistoryTest, GetPacketEmptyForRemovedPacket) { - const uint16_t kSeqNo = 10; - const PacedPacketInfo kPacingInfo(0, 5, 1200); - AddPacketWithSendTime(kSeqNo, 123, 1, kPacingInfo); - auto sent_packet = history_.GetPacket(kSeqNo); - PacketFeedback received_packet(0, 0, kSeqNo, 0, kPacingInfo); - EXPECT_TRUE(history_.GetFeedback(&received_packet, true)); - sent_packet = history_.GetPacket(kSeqNo); - EXPECT_FALSE(sent_packet.has_value()); -} - -TEST_F(SendTimeHistoryTest, PopulatesExpectedFields) { - const uint16_t kSeqNo = 10; - const int64_t kSendTime = 1000; - const int64_t kReceiveTime = 2000; - const size_t kPayloadSize = 42; - const PacedPacketInfo kPacingInfo(3, 10, 1212); - - AddPacketWithSendTime(kSeqNo, kPayloadSize, kSendTime, kPacingInfo); - - PacketFeedback packet_feedback(kReceiveTime, kSeqNo); - EXPECT_TRUE(history_.GetFeedback(&packet_feedback, true)); - EXPECT_EQ(kReceiveTime, packet_feedback.arrival_time_ms); - EXPECT_EQ(kSendTime, packet_feedback.send_time_ms); - EXPECT_EQ(kSeqNo, packet_feedback.sequence_number); - EXPECT_EQ(kPayloadSize, packet_feedback.payload_size); - EXPECT_EQ(kPacingInfo, packet_feedback.pacing_info); -} - -TEST_F(SendTimeHistoryTest, AddThenRemoveOutOfOrder) { - std::vector sent_packets; - std::vector received_packets; - const size_t num_items = 100; - const size_t kPacketSize = 400; - const size_t kTransmissionTime = 1234; - const PacedPacketInfo kPacingInfo(1, 2, 200); - for (size_t i = 0; i < num_items; ++i) { - sent_packets.push_back(PacketFeedback(0, static_cast(i), - static_cast(i), kPacketSize, - kPacingInfo)); - received_packets.push_back(PacketFeedback( - static_cast(i) + kTransmissionTime, 0, - static_cast(i), kPacketSize, PacedPacketInfo())); - } - for (size_t i = 0; i < num_items; ++i) { - PacketFeedback packet = sent_packets[i]; - packet.arrival_time_ms = PacketFeedback::kNotReceived; - packet.send_time_ms = PacketFeedback::kNoSendTime; - history_.RemoveOld(clock_.TimeInMilliseconds()); - history_.AddNewPacket(packet); - } - for (size_t i = 0; i < num_items; ++i) - history_.OnSentPacket(sent_packets[i].sequence_number, - sent_packets[i].send_time_ms); - std::shuffle(received_packets.begin(), received_packets.end(), - std::mt19937(std::random_device()())); - for (size_t i = 0; i < num_items; ++i) { - PacketFeedback packet = received_packets[i]; - EXPECT_TRUE(history_.GetFeedback(&packet, false)); - PacketFeedback sent_packet = sent_packets[packet.sequence_number]; - sent_packet.arrival_time_ms = packet.arrival_time_ms; - EXPECT_EQ(sent_packet, packet); - EXPECT_TRUE(history_.GetFeedback(&packet, true)); - } - for (PacketFeedback packet : sent_packets) - EXPECT_FALSE(history_.GetFeedback(&packet, false)); -} - -TEST_F(SendTimeHistoryTest, HistorySize) { - const int kItems = kDefaultHistoryLengthMs / 100; - for (int i = 0; i < kItems; ++i) { - clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(i, 0, i * 100, PacedPacketInfo()); - } - for (int i = 0; i < kItems; ++i) { - PacketFeedback packet(0, 0, static_cast(i), 0, PacedPacketInfo()); - EXPECT_TRUE(history_.GetFeedback(&packet, false)); - EXPECT_EQ(i * 100, packet.send_time_ms); - } - clock_.AdvanceTimeMilliseconds(101); - AddPacketWithSendTime(kItems, 0, kItems * 101, PacedPacketInfo()); - PacketFeedback packet(0, 0, 0, 0, PacedPacketInfo()); - EXPECT_FALSE(history_.GetFeedback(&packet, false)); - for (int i = 1; i < (kItems + 1); ++i) { - PacketFeedback packet2(0, 0, static_cast(i), 0, - PacedPacketInfo()); - EXPECT_TRUE(history_.GetFeedback(&packet2, false)); - int64_t expected_time_ms = (i == kItems) ? i * 101 : i * 100; - EXPECT_EQ(expected_time_ms, packet2.send_time_ms); - } -} - -TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) { - const uint16_t kMaxSeqNo = std::numeric_limits::max(); - AddPacketWithSendTime(kMaxSeqNo - 2, 0, 0, PacedPacketInfo()); - - clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(kMaxSeqNo - 1, 1, 100, PacedPacketInfo()); - - clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(kMaxSeqNo, 0, 200, PacedPacketInfo()); - - clock_.AdvanceTimeMilliseconds(kDefaultHistoryLengthMs - 200 + 1); - AddPacketWithSendTime(0, 0, kDefaultHistoryLengthMs, PacedPacketInfo()); - - PacketFeedback packet(0, static_cast(kMaxSeqNo - 2)); - EXPECT_FALSE(history_.GetFeedback(&packet, false)); - PacketFeedback packet2(0, static_cast(kMaxSeqNo - 1)); - EXPECT_TRUE(history_.GetFeedback(&packet2, false)); - PacketFeedback packet3(0, static_cast(kMaxSeqNo)); - EXPECT_TRUE(history_.GetFeedback(&packet3, false)); - PacketFeedback packet4(0, 0); - EXPECT_TRUE(history_.GetFeedback(&packet4, false)); - - // Create a gap (kMaxSeqNo - 1) -> 0. - PacketFeedback packet5(0, kMaxSeqNo); - EXPECT_TRUE(history_.GetFeedback(&packet5, true)); - - clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(1, 0, 1100, PacedPacketInfo()); - - PacketFeedback packet6(0, static_cast(kMaxSeqNo - 2)); - EXPECT_FALSE(history_.GetFeedback(&packet6, false)); - PacketFeedback packet7(0, static_cast(kMaxSeqNo - 1)); - EXPECT_FALSE(history_.GetFeedback(&packet7, false)); - PacketFeedback packet8(0, kMaxSeqNo); - EXPECT_FALSE(history_.GetFeedback(&packet8, false)); - PacketFeedback packet9(0, 0); - EXPECT_TRUE(history_.GetFeedback(&packet9, false)); - PacketFeedback packet10(0, 1); - EXPECT_TRUE(history_.GetFeedback(&packet10, false)); -} - -TEST_F(SendTimeHistoryTest, InterlievedGetAndRemove) { - const uint16_t kSeqNo = 1; - const int64_t kTimestamp = 2; - const PacedPacketInfo kPacingInfo1(1, 1, 100); - const PacedPacketInfo kPacingInfo2(2, 2, 200); - const PacedPacketInfo kPacingInfo3(3, 3, 300); - PacketFeedback packets[3] = { - {0, kTimestamp, kSeqNo, 0, kPacingInfo1}, - {0, kTimestamp + 1, kSeqNo + 1, 0, kPacingInfo2}, - {0, kTimestamp + 2, kSeqNo + 2, 0, kPacingInfo3}}; - - AddPacketWithSendTime(packets[0].sequence_number, packets[0].payload_size, - packets[0].send_time_ms, packets[0].pacing_info); - AddPacketWithSendTime(packets[1].sequence_number, packets[1].payload_size, - packets[1].send_time_ms, packets[1].pacing_info); - PacketFeedback packet(0, 0, packets[0].sequence_number, 0, PacedPacketInfo()); - EXPECT_TRUE(history_.GetFeedback(&packet, true)); - EXPECT_EQ(packets[0], packet); - - AddPacketWithSendTime(packets[2].sequence_number, packets[2].payload_size, - packets[2].send_time_ms, packets[2].pacing_info); - - PacketFeedback packet2(0, 0, packets[1].sequence_number, 0, kPacingInfo1); - EXPECT_TRUE(history_.GetFeedback(&packet2, true)); - EXPECT_EQ(packets[1], packet2); - - PacketFeedback packet3(0, 0, packets[2].sequence_number, 0, kPacingInfo2); - EXPECT_TRUE(history_.GetFeedback(&packet3, true)); - EXPECT_EQ(packets[2], packet3); -} -} // namespace test -} // namespace webrtc diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.cc b/modules/congestion_controller/rtp/transport_feedback_adapter.cc index e77e0e7c2f..1b667aef14 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.cc @@ -47,9 +47,7 @@ const int64_t kNoTimestamp = -1; const int64_t kSendTimeHistoryWindowMs = 60000; TransportFeedbackAdapter::TransportFeedbackAdapter() - : allow_duplicates_(field_trial::IsEnabled( - "WebRTC-TransportFeedbackAdapter-AllowDuplicates")), - send_time_history_(kSendTimeHistoryWindowMs), + : packet_age_limit_ms_(kSendTimeHistoryWindowMs), current_offset_ms_(kNoTimestamp), last_timestamp_us_(kNoTimestamp), local_net_id_(0), @@ -82,16 +80,25 @@ void TransportFeedbackAdapter::AddPacket(const RtpPacketSendInfo& packet_info, Timestamp creation_time) { { rtc::CritScope cs(&lock_); - PacketFeedback packet_feedback( - creation_time.ms(), packet_info.transport_sequence_number, - packet_info.length + overhead_bytes, local_net_id_, remote_net_id_, - packet_info.pacing_info); + PacketFeedback packet(creation_time.ms(), + packet_info.transport_sequence_number, + packet_info.length + overhead_bytes, local_net_id_, + remote_net_id_, packet_info.pacing_info); if (packet_info.has_rtp_sequence_number) { - packet_feedback.ssrc = packet_info.ssrc; - packet_feedback.rtp_sequence_number = packet_info.rtp_sequence_number; + packet.ssrc = packet_info.ssrc; + packet.rtp_sequence_number = packet_info.rtp_sequence_number; } - send_time_history_.RemoveOld(creation_time.ms()); - send_time_history_.AddNewPacket(std::move(packet_feedback)); + packet.long_sequence_number = + seq_num_unwrapper_.Unwrap(packet.sequence_number); + + while (!history_.empty() && + creation_time.ms() - history_.begin()->second.creation_time_ms > + packet_age_limit_ms_) { + // TODO(sprang): Warn if erasing (too many) old items? + RemoveInFlightPacketBytes(history_.begin()->second); + history_.erase(history_.begin()); + } + history_.insert(std::make_pair(packet.long_sequence_number, packet)); } { @@ -107,27 +114,43 @@ absl::optional TransportFeedbackAdapter::ProcessSentPacket( rtc::CritScope cs(&lock_); // TODO(srte): Only use one way to indicate that packet feedback is used. if (sent_packet.info.included_in_feedback || sent_packet.packet_id != -1) { - SendTimeHistory::Status send_status = send_time_history_.OnSentPacket( - sent_packet.packet_id, sent_packet.send_time_ms); - absl::optional packet; - if (allow_duplicates_ || - send_status != SendTimeHistory::Status::kDuplicate) { - packet = send_time_history_.GetPacket(sent_packet.packet_id); - } - - if (packet) { - 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 = - send_time_history_.GetOutstandingData(local_net_id_, remote_net_id_); - return msg; + int64_t unwrapped_seq_num = + seq_num_unwrapper_.Unwrap(sent_packet.packet_id); + auto it = history_.find(unwrapped_seq_num); + if (it != history_.end()) { + bool packet_retransmit = it->second.send_time_ms >= 0; + it->second.send_time_ms = sent_packet.send_time_ms; + last_send_time_ms_ = + std::max(last_send_time_ms_, sent_packet.send_time_ms); + // TODO(srte): Don't do this on retransmit. + if (pending_untracked_size_ > 0) { + if (sent_packet.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_ - sent_packet.send_time_ms + << " ms.)"; + it->second.unacknowledged_data += pending_untracked_size_; + pending_untracked_size_ = 0; + } + if (!packet_retransmit) { + AddInFlightPacketBytes(it->second); + auto packet = it->second; + 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 = GetOutstandingData(); + return msg; + } } } else if (sent_packet.info.included_in_allocation) { - send_time_history_.AddUntracked(sent_packet.info.packet_size_bytes, - sent_packet.send_time_ms); + if (sent_packet.send_time_ms < last_send_time_ms_) { + RTC_LOG(LS_WARNING) << "ignoring untracked data for out of order packet."; + } + pending_untracked_size_ += sent_packet.info.packet_size_bytes; + last_untracked_send_time_ms_ = + std::max(last_untracked_send_time_ms_, sent_packet.send_time_ms); } return absl::nullopt; } @@ -165,10 +188,11 @@ TransportFeedbackAdapter::ProcessTransportFeedback( } { rtc::CritScope cs(&lock_); - absl::optional first_unacked_send_time_ms = - send_time_history_.GetFirstUnackedSendTime(); - if (first_unacked_send_time_ms) - msg.first_unacked_send_time = Timestamp::ms(*first_unacked_send_time_ms); + auto it = history_.find(last_ack_seq_num_); + if (it != history_.end() && + it->second.send_time_ms != PacketFeedback::kNoSendTime) { + msg.first_unacked_send_time = Timestamp::ms(it->second.send_time_ms); + } } msg.feedback_time = feedback_receive_time; msg.prior_in_flight = prior_in_flight; @@ -185,7 +209,12 @@ void TransportFeedbackAdapter::SetNetworkIds(uint16_t local_id, DataSize TransportFeedbackAdapter::GetOutstandingData() const { rtc::CritScope cs(&lock_); - return send_time_history_.GetOutstandingData(local_net_id_, remote_net_id_); + auto it = in_flight_bytes_.find({local_net_id_, remote_net_id_}); + if (it != in_flight_bytes_.end()) { + return DataSize::bytes(it->second); + } else { + return DataSize::Zero(); + } } std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( @@ -220,7 +249,7 @@ std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( PacketFeedback packet_feedback(PacketFeedback::kNotReceived, seq_num); // Note: Element not removed from history because it might be reported // as received by another feedback. - if (!send_time_history_.GetFeedback(&packet_feedback, false)) + if (!GetFeedback(&packet_feedback, false)) ++failed_lookups; if (packet_feedback.local_net_id == local_net_id_ && packet_feedback.remote_net_id == remote_net_id_) { @@ -232,7 +261,7 @@ std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( offset_us += packet.delta_us(); timestamp_ms = current_offset_ms_ + (offset_us / 1000); PacketFeedback packet_feedback(timestamp_ms, packet.sequence_number()); - if (!send_time_history_.GetFeedback(&packet_feedback, true)) + if (!GetFeedback(&packet_feedback, true)) ++failed_lookups; if (packet_feedback.local_net_id == local_net_id_ && packet_feedback.remote_net_id == remote_net_id_) { @@ -256,4 +285,62 @@ TransportFeedbackAdapter::GetTransportFeedbackVector() const { return last_packet_feedback_vector_; } +bool TransportFeedbackAdapter::GetFeedback(PacketFeedback* packet_feedback, + bool remove) { + RTC_DCHECK(packet_feedback); + int64_t acked_seq_num = + seq_num_unwrapper_.Unwrap(packet_feedback->sequence_number); + + if (acked_seq_num > last_ack_seq_num_) { + // Returns history_.begin() if last_ack_seq_num_ < 0, since any valid + // sequence number is >= 0. + auto unacked_it = history_.lower_bound(last_ack_seq_num_); + auto newly_acked_end = history_.upper_bound(acked_seq_num); + for (; unacked_it != newly_acked_end; ++unacked_it) { + RemoveInFlightPacketBytes(unacked_it->second); + } + last_ack_seq_num_ = acked_seq_num; + } + + auto it = history_.find(acked_seq_num); + if (it == history_.end()) + return false; + + // Save arrival_time not to overwrite it. + int64_t arrival_time_ms = packet_feedback->arrival_time_ms; + *packet_feedback = it->second; + packet_feedback->arrival_time_ms = arrival_time_ms; + + if (remove) + history_.erase(it); + return true; +} + +void TransportFeedbackAdapter::AddInFlightPacketBytes( + const PacketFeedback& packet) { + RTC_DCHECK_NE(packet.send_time_ms, -1); + if (last_ack_seq_num_ >= packet.long_sequence_number) + return; + auto it = in_flight_bytes_.find({packet.local_net_id, packet.remote_net_id}); + if (it != in_flight_bytes_.end()) { + it->second += packet.payload_size; + } else { + in_flight_bytes_[{packet.local_net_id, packet.remote_net_id}] = + packet.payload_size; + } +} + +void TransportFeedbackAdapter::RemoveInFlightPacketBytes( + const PacketFeedback& packet) { + if (packet.send_time_ms < 0 || + last_ack_seq_num_ >= packet.long_sequence_number) + return; + auto it = in_flight_bytes_.find({packet.local_net_id, packet.remote_net_id}); + if (it != in_flight_bytes_.end()) { + it->second -= packet.payload_size; + if (it->second == 0) + in_flight_bytes_.erase(it); + } +} + } // namespace webrtc diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.h b/modules/congestion_controller/rtp/transport_feedback_adapter.h index d347f2dfae..edd3fb86c3 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.h +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.h @@ -12,10 +12,13 @@ #define MODULES_CONGESTION_CONTROLLER_RTP_TRANSPORT_FEEDBACK_ADAPTER_H_ #include +#include +#include #include #include "api/transport/network_types.h" -#include "modules/congestion_controller/rtp/send_time_history.h" +#include "modules/include/module_common_types_public.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/critical_section.h" #include "rtc_base/network/sent_packet.h" #include "rtc_base/thread_annotations.h" @@ -55,16 +58,40 @@ class TransportFeedbackAdapter { DataSize GetOutstandingData() const; private: + using RemoteAndLocalNetworkId = std::pair; + + enum class SendTimeHistoryStatus { kNotAdded, kOk, kDuplicate }; + void OnTransportFeedback(const rtcp::TransportFeedback& feedback); std::vector GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback, Timestamp feedback_time); - const bool allow_duplicates_; + // Look up PacketFeedback for a sent packet, based on the sequence number, and + // populate all fields except for arrival_time. The packet parameter must + // thus be non-null and have the sequence_number field set. + bool GetFeedback(PacketFeedback* packet_feedback, bool remove) + RTC_RUN_ON(&lock_); + void AddInFlightPacketBytes(const PacketFeedback& packet) RTC_RUN_ON(&lock_); + void RemoveInFlightPacketBytes(const PacketFeedback& packet) + RTC_RUN_ON(&lock_); rtc::CriticalSection lock_; - SendTimeHistory send_time_history_ RTC_GUARDED_BY(&lock_); + + const int64_t packet_age_limit_ms_; + size_t pending_untracked_size_ RTC_GUARDED_BY(&lock_) = 0; + int64_t last_send_time_ms_ RTC_GUARDED_BY(&lock_) = -1; + int64_t last_untracked_send_time_ms_ RTC_GUARDED_BY(&lock_) = -1; + SequenceNumberUnwrapper seq_num_unwrapper_ RTC_GUARDED_BY(&lock_); + std::map history_ RTC_GUARDED_BY(&lock_); + + // Sequence numbers are never negative, using -1 as it always < a real + // sequence number. + int64_t last_ack_seq_num_ RTC_GUARDED_BY(&lock_) = -1; + std::map in_flight_bytes_ + RTC_GUARDED_BY(&lock_); + int64_t current_offset_ms_; int64_t last_timestamp_us_; std::vector last_packet_feedback_vector_; diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc b/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc index 3fdc21fa8f..593c9940a8 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc @@ -425,36 +425,6 @@ TEST_F(TransportFeedbackAdapterTest, IgnoreDuplicatePacketSentCalls) { EXPECT_FALSE(duplicate_packet.has_value()); } -TEST_F(TransportFeedbackAdapterTest, AllowDuplicatePacketSentCallsWithTrial) { - // Allow duplicates if this field trial kill-switch is enabled. - webrtc::test::ScopedFieldTrials field_trial( - "WebRTC-TransportFeedbackAdapter-AllowDuplicates/Enabled/"); - // Re-run setup so the flags goes into effect. - SetUp(); - - const PacketFeedback packet(100, 200, 0, 1500, kPacingInfo0); - - // Add a packet and then mark it as sent. - RtpPacketSendInfo packet_info; - packet_info.ssrc = kSsrc; - packet_info.transport_sequence_number = packet.sequence_number; - packet_info.length = packet.payload_size; - packet_info.pacing_info = packet.pacing_info; - adapter_->AddPacket(packet_info, 0u, - Timestamp::ms(clock_.TimeInMilliseconds())); - absl::optional sent_packet = - adapter_->ProcessSentPacket(rtc::SentPacket( - packet.sequence_number, packet.send_time_ms, rtc::PacketInfo())); - EXPECT_TRUE(sent_packet.has_value()); - - // Call ProcessSentPacket() again with the same sequence number. This packet - // should still be allowed due to the field trial/ - absl::optional duplicate_packet = - adapter_->ProcessSentPacket(rtc::SentPacket( - packet.sequence_number, packet.send_time_ms, rtc::PacketInfo())); - EXPECT_TRUE(duplicate_packet.has_value()); -} - } // namespace test } // namespace webrtc_cc } // namespace webrtc