Merging TransportFeedbackAdapter and SendTimeHistory.
They were already tightly coupled, merging them makes the relations clearer.
We also remove the kill switch for removing duplicate feedback events since
there has been no need to use it.
The potential to account for bytes sent in AddNewPacket was also removed
since it is not used by TransportFeedbackAdapter.
Bug: webrtc:9883
Change-Id: I51823e0ce838c22158637954749310e0d0eeff27
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156140
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29449}
diff --git a/modules/congestion_controller/rtp/BUILD.gn b/modules/congestion_controller/rtp/BUILD.gn
index 2c73779..d3e995e 100644
--- a/modules/congestion_controller/rtp/BUILD.gn
+++ b/modules/congestion_controller/rtp/BUILD.gn
@@ -43,8 +43,6 @@
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 @@
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 2d0356b..0000000
--- 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 <algorithm>
-#include <utility>
-
-#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<PacketFeedback> SendTimeHistory::GetPacket(
- uint16_t sequence_number) const {
- int64_t unwrapped_seq_num =
- seq_num_unwrapper_.UnwrapWithoutUpdate(sequence_number);
- absl::optional<PacketFeedback> 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<int64_t> 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 9563fb8..0000000
--- 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 <map>
-#include <utility>
-
-#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<PacketFeedback> 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<int64_t> GetFirstUnackedSendTime() const;
-
- private:
- using RemoteAndLocalNetworkId = std::pair<uint16_t, uint16_t>;
-
- 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<int64_t, PacketFeedback> history_;
- absl::optional<int64_t> last_ack_seq_num_;
- std::map<RemoteAndLocalNetworkId, size_t> 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 604685c..0000000
--- 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 <algorithm>
-#include <limits>
-#include <random>
-#include <vector>
-
-#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<PacketFeedback> sent_packets;
- std::vector<PacketFeedback> 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<int64_t>(i),
- static_cast<uint16_t>(i), kPacketSize,
- kPacingInfo));
- received_packets.push_back(PacketFeedback(
- static_cast<int64_t>(i) + kTransmissionTime, 0,
- static_cast<uint16_t>(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<uint16_t>(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<uint16_t>(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<uint16_t>::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<uint16_t>(kMaxSeqNo - 2));
- EXPECT_FALSE(history_.GetFeedback(&packet, false));
- PacketFeedback packet2(0, static_cast<uint16_t>(kMaxSeqNo - 1));
- EXPECT_TRUE(history_.GetFeedback(&packet2, false));
- PacketFeedback packet3(0, static_cast<uint16_t>(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<uint16_t>(kMaxSeqNo - 2));
- EXPECT_FALSE(history_.GetFeedback(&packet6, false));
- PacketFeedback packet7(0, static_cast<uint16_t>(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 e77e0e7..1b667ae 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 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 @@
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 @@
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<PacketFeedback> 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 @@
}
{
rtc::CritScope cs(&lock_);
- absl::optional<int64_t> 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 @@
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<PacketFeedback> TransportFeedbackAdapter::GetPacketFeedbackVector(
@@ -220,7 +249,7 @@
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 @@
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 @@
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 d347f2d..edd3fb8 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 <deque>
+#include <map>
+#include <utility>
#include <vector>
#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 @@
DataSize GetOutstandingData() const;
private:
+ using RemoteAndLocalNetworkId = std::pair<uint16_t, uint16_t>;
+
+ enum class SendTimeHistoryStatus { kNotAdded, kOk, kDuplicate };
+
void OnTransportFeedback(const rtcp::TransportFeedback& feedback);
std::vector<PacketFeedback> 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<int64_t, PacketFeedback> 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<RemoteAndLocalNetworkId, size_t> in_flight_bytes_
+ RTC_GUARDED_BY(&lock_);
+
int64_t current_offset_ms_;
int64_t last_timestamp_us_;
std::vector<PacketFeedback> 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 3fdc21f..593c994 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 @@
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<SentPacket> 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<SentPacket> 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
