diff --git a/webrtc/modules/remote_bitrate_estimator/BUILD.gn b/webrtc/modules/remote_bitrate_estimator/BUILD.gn index 938777e249..5f0be091df 100644 --- a/webrtc/modules/remote_bitrate_estimator/BUILD.gn +++ b/webrtc/modules/remote_bitrate_estimator/BUILD.gn @@ -10,10 +10,8 @@ source_set("remote_bitrate_estimator") { sources = [ "include/bwe_defines.h", "include/remote_bitrate_estimator.h", - "include/send_time_history.h", "rate_statistics.cc", "rate_statistics.h", - "send_time_history.cc", ] configs += [ "../../:common_inherited_config" ] @@ -29,6 +27,7 @@ source_set("rbe_components") { sources = [ "aimd_rate_control.cc", "aimd_rate_control.h", + "include/send_time_history.h", "inter_arrival.cc", "inter_arrival.h", "overuse_detector.cc", @@ -37,6 +36,7 @@ source_set("rbe_components") { "overuse_estimator.h", "remote_bitrate_estimator_abs_send_time.cc", "remote_bitrate_estimator_single_stream.cc", + "send_time_history.cc", ] configs += [ "../..:common_config" ] diff --git a/webrtc/modules/remote_bitrate_estimator/include/send_time_history.h b/webrtc/modules/remote_bitrate_estimator/include/send_time_history.h index 8835856353..b01a62bdd7 100644 --- a/webrtc/modules/remote_bitrate_estimator/include/send_time_history.h +++ b/webrtc/modules/remote_bitrate_estimator/include/send_time_history.h @@ -15,6 +15,7 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/basictypes.h" +#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" namespace webrtc { @@ -23,8 +24,12 @@ class SendTimeHistory { explicit SendTimeHistory(int64_t packet_age_limit); virtual ~SendTimeHistory(); - void AddAndRemoveOldSendTimes(uint16_t sequence_number, int64_t timestamp); - bool GetSendTime(uint16_t sequence_number, int64_t* timestamp, bool remove); + void AddAndRemoveOld(const PacketInfo& packet); + bool UpdateSendTime(uint16_t sequence_number, int64_t timestamp); + // Look up PacketInfo for a sent packet, based on the sequence number, and + // populate all fields except for receive_time. The packet parameter must + // thus be non-null and have the sequence_number field set. + bool GetInfo(PacketInfo* packet, bool remove); void Clear(); private: @@ -33,7 +38,7 @@ class SendTimeHistory { const int64_t packet_age_limit_; uint16_t oldest_sequence_number_; // Oldest may not be lowest. - std::map history_; + std::map history_; DISALLOW_COPY_AND_ASSIGN(SendTimeHistory); }; diff --git a/webrtc/modules/remote_bitrate_estimator/send_time_history.cc b/webrtc/modules/remote_bitrate_estimator/send_time_history.cc index f564573161..fa51daddb6 100644 --- a/webrtc/modules/remote_bitrate_estimator/send_time_history.cc +++ b/webrtc/modules/remote_bitrate_estimator/send_time_history.cc @@ -25,14 +25,23 @@ void SendTimeHistory::Clear() { history_.clear(); } -void SendTimeHistory::AddAndRemoveOldSendTimes(uint16_t sequence_number, - int64_t timestamp) { - EraseOld(timestamp - packet_age_limit_); +void SendTimeHistory::AddAndRemoveOld(const PacketInfo& packet) { + EraseOld(packet.send_time_ms - packet_age_limit_); if (history_.empty()) - oldest_sequence_number_ = sequence_number; + oldest_sequence_number_ = packet.sequence_number; - history_[sequence_number] = timestamp; + history_.insert( + std::pair(packet.sequence_number, packet)); +} + +bool SendTimeHistory::UpdateSendTime(uint16_t sequence_number, + int64_t send_time_ms) { + auto it = history_.find(sequence_number); + if (it == history_.end()) + return false; + it->second.send_time_ms = send_time_ms; + return true; } void SendTimeHistory::EraseOld(int64_t limit) { @@ -40,7 +49,7 @@ void SendTimeHistory::EraseOld(int64_t limit) { auto it = history_.find(oldest_sequence_number_); assert(it != history_.end()); - if (it->second > limit) + if (it->second.send_time_ms > limit) return; // Oldest packet within age limit, return. // TODO(sprang): Warn if erasing (too many) old items? @@ -68,16 +77,16 @@ void SendTimeHistory::UpdateOldestSequenceNumber() { oldest_sequence_number_ = it->first; } -bool SendTimeHistory::GetSendTime(uint16_t sequence_number, - int64_t* timestamp, - bool remove) { - auto it = history_.find(sequence_number); +bool SendTimeHistory::GetInfo(PacketInfo* packet, bool remove) { + auto it = history_.find(packet->sequence_number); if (it == history_.end()) return false; - *timestamp = it->second; + int64_t receive_time = packet->arrival_time_ms; + *packet = it->second; + packet->arrival_time_ms = receive_time; if (remove) { history_.erase(it); - if (sequence_number == oldest_sequence_number_) + if (packet->sequence_number == oldest_sequence_number_) UpdateOldestSequenceNumber(); } return true; diff --git a/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc b/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc index 8d708520c6..e3d2c77619 100644 --- a/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc @@ -17,6 +17,7 @@ #include "webrtc/system_wrappers/interface/clock.h" namespace webrtc { +namespace test { static const int kDefaultHistoryLengthMs = 1000; @@ -33,117 +34,191 @@ class SendTimeHistoryTest : public ::testing::Test { webrtc::SimulatedClock clock_; }; +// Help class extended so we can do EXPECT_EQ and collections. +class PacketInfo : public webrtc::PacketInfo { + public: + PacketInfo() : webrtc::PacketInfo(0, 0, 0, 0, false) {} + PacketInfo(int64_t arrival_time_ms, + int64_t send_time_ms, + uint16_t sequence_number, + size_t payload_size, + bool was_paced) + : webrtc::PacketInfo(arrival_time_ms, + send_time_ms, + sequence_number, + payload_size, + was_paced) {} + bool operator==(const PacketInfo& other) const { + return arrival_time_ms == other.arrival_time_ms && + send_time_ms == other.send_time_ms && + sequence_number == other.sequence_number && + payload_size == other.payload_size && was_paced == other.was_paced; + } +}; + TEST_F(SendTimeHistoryTest, AddRemoveOne) { - const uint16_t kSeqNo = 1; - const int64_t kTimestamp = 2; - history_.AddAndRemoveOldSendTimes(kSeqNo, kTimestamp); + const uint16_t kSeqNo = 10; + const PacketInfo kSentPacket = {0, 1, kSeqNo, 1, true}; + history_.AddAndRemoveOld(kSentPacket); - int64_t time = 0; - EXPECT_TRUE(history_.GetSendTime(kSeqNo, &time, false)); - EXPECT_EQ(kTimestamp, time); + PacketInfo received_packet = {0, 0, kSeqNo, 0, false}; + EXPECT_TRUE(history_.GetInfo(&received_packet, false)); + EXPECT_EQ(kSentPacket, received_packet); - time = 0; - EXPECT_TRUE(history_.GetSendTime(kSeqNo, &time, true)); - EXPECT_EQ(kTimestamp, time); + received_packet = {0, 0, kSeqNo, 0, false}; + EXPECT_TRUE(history_.GetInfo(&received_packet, true)); + EXPECT_EQ(kSentPacket, received_packet); - time = 0; - EXPECT_FALSE(history_.GetSendTime(kSeqNo, &time, true)); + received_packet = {0, 0, kSeqNo, 0, false}; + EXPECT_FALSE(history_.GetInfo(&received_packet, true)); +} + +TEST_F(SendTimeHistoryTest, UpdateSendTime) { + const uint16_t kSeqNo = 10; + const int64_t kSendTime = 1000; + const int64_t kSendTimeUpdated = 2000; + const PacketInfo kSentPacket = {0, kSendTime, kSeqNo, 1, true}; + const PacketInfo kUpdatedPacket = {0, kSendTimeUpdated, kSeqNo, 1, true}; + + history_.AddAndRemoveOld(kSentPacket); + PacketInfo info = {0, 0, kSeqNo, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); + EXPECT_EQ(kSentPacket, info); + + EXPECT_TRUE(history_.UpdateSendTime(kSeqNo, kSendTimeUpdated)); + + info = {0, 0, kSeqNo, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, true)); + EXPECT_EQ(kUpdatedPacket, info); + + EXPECT_FALSE(history_.UpdateSendTime(kSeqNo, kSendTimeUpdated)); +} + +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 bool kPaced = true; + const PacketInfo kSentPacket = {0, kSendTime, kSeqNo, kPayloadSize, kPaced}; + + history_.AddAndRemoveOld(kSentPacket); + + PacketInfo info = {kReceiveTime, 0, kSeqNo, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, true)); + EXPECT_EQ(kReceiveTime, info.arrival_time_ms); + EXPECT_EQ(kSendTime, info.send_time_ms); + EXPECT_EQ(kSeqNo, info.sequence_number); + EXPECT_EQ(kPayloadSize, info.payload_size); + EXPECT_EQ(kPaced, info.was_paced); } TEST_F(SendTimeHistoryTest, AddThenRemoveOutOfOrder) { - struct Timestamp { - Timestamp(uint16_t sequence_number, int64_t timestamp) - : sequence_number(sequence_number), timestamp(timestamp) {} - uint16_t sequence_number; - int64_t timestamp; - }; - std::vector timestamps; + 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 bool kPaced = true; for (size_t i = 0; i < num_items; ++i) { - timestamps.push_back( - Timestamp(static_cast(i), static_cast(i))); + sent_packets.push_back(PacketInfo(0, static_cast(i), + static_cast(i), kPacketSize, + kPaced)); + received_packets.push_back( + PacketInfo(static_cast(i) + kTransmissionTime, 0, + static_cast(i), kPacketSize, false)); } - std::vector randomized_timestamps = timestamps; - std::random_shuffle(randomized_timestamps.begin(), - randomized_timestamps.end()); + for (size_t i = 0; i < num_items; ++i) + history_.AddAndRemoveOld(sent_packets[i]); + std::random_shuffle(received_packets.begin(), received_packets.end()); for (size_t i = 0; i < num_items; ++i) { - history_.AddAndRemoveOldSendTimes(timestamps[i].sequence_number, - timestamps[i].timestamp); - } - for (size_t i = 0; i < num_items; ++i) { - int64_t timestamp; - EXPECT_TRUE(history_.GetSendTime(randomized_timestamps[i].sequence_number, - ×tamp, false)); - EXPECT_EQ(randomized_timestamps[i].timestamp, timestamp); - EXPECT_TRUE(history_.GetSendTime(randomized_timestamps[i].sequence_number, - ×tamp, true)); - } - for (size_t i = 0; i < num_items; ++i) { - int64_t timestamp; - EXPECT_FALSE( - history_.GetSendTime(timestamps[i].sequence_number, ×tamp, false)); + PacketInfo packet = received_packets[i]; + EXPECT_TRUE(history_.GetInfo(&packet, false)); + PacketInfo sent_packet = sent_packets[packet.sequence_number]; + sent_packet.arrival_time_ms = packet.arrival_time_ms; + EXPECT_EQ(sent_packet, packet); + EXPECT_TRUE(history_.GetInfo(&packet, true)); } + for (PacketInfo packet : sent_packets) + EXPECT_FALSE(history_.GetInfo(&packet, false)); } TEST_F(SendTimeHistoryTest, HistorySize) { const int kItems = kDefaultHistoryLengthMs / 100; + for (int i = 0; i < kItems; ++i) + history_.AddAndRemoveOld(PacketInfo(0, i * 100, i, 0, false)); for (int i = 0; i < kItems; ++i) { - history_.AddAndRemoveOldSendTimes(i, i * 100); + PacketInfo info = {0, 0, static_cast(i), 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); + EXPECT_EQ(i * 100, info.send_time_ms); } - int64_t timestamp; - for (int i = 0; i < kItems; ++i) { - EXPECT_TRUE(history_.GetSendTime(i, ×tamp, false)); - EXPECT_EQ(i * 100, timestamp); - } - history_.AddAndRemoveOldSendTimes(kItems, kItems * 100); - EXPECT_FALSE(history_.GetSendTime(0, ×tamp, false)); + history_.AddAndRemoveOld(PacketInfo(0, kItems * 100, kItems, 0, false)); + PacketInfo info = {0, 0, 0, 0, false}; + EXPECT_FALSE(history_.GetInfo(&info, false)); for (int i = 1; i < (kItems + 1); ++i) { - EXPECT_TRUE(history_.GetSendTime(i, ×tamp, false)); - EXPECT_EQ(i * 100, timestamp); + info = {0, 0, static_cast(i), 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); + EXPECT_EQ(i * 100, info.send_time_ms); } } TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) { - const int kMaxSeqNo = std::numeric_limits::max(); - history_.AddAndRemoveOldSendTimes(kMaxSeqNo - 2, 0); - history_.AddAndRemoveOldSendTimes(kMaxSeqNo - 1, 100); - history_.AddAndRemoveOldSendTimes(kMaxSeqNo, 200); - history_.AddAndRemoveOldSendTimes(0, 1000); - int64_t timestamp; - EXPECT_FALSE(history_.GetSendTime(kMaxSeqNo - 2, ×tamp, false)); - EXPECT_TRUE(history_.GetSendTime(kMaxSeqNo - 1, ×tamp, false)); - EXPECT_TRUE(history_.GetSendTime(kMaxSeqNo, ×tamp, false)); - EXPECT_TRUE(history_.GetSendTime(0, ×tamp, false)); + const uint16_t kMaxSeqNo = std::numeric_limits::max(); + history_.AddAndRemoveOld(PacketInfo(0, 0, kMaxSeqNo - 2, 0, false)); + history_.AddAndRemoveOld(PacketInfo(0, 100, kMaxSeqNo - 1, 0, false)); + history_.AddAndRemoveOld(PacketInfo(0, 200, kMaxSeqNo, 0, false)); + history_.AddAndRemoveOld(PacketInfo(0, kDefaultHistoryLengthMs, 0, 0, false)); + PacketInfo info = {0, 0, static_cast(kMaxSeqNo - 2), 0, false}; + EXPECT_FALSE(history_.GetInfo(&info, false)); + info = {0, 0, static_cast(kMaxSeqNo - 1), 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); + info = {0, 0, static_cast(kMaxSeqNo), 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); + info = {0, 0, 0, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); // Create a gap (kMaxSeqNo - 1) -> 0. - EXPECT_TRUE(history_.GetSendTime(kMaxSeqNo, ×tamp, true)); + info = {0, 0, kMaxSeqNo, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, true)); - history_.AddAndRemoveOldSendTimes(1, 1100); + history_.AddAndRemoveOld(PacketInfo(0, 1100, 1, 0, false)); - EXPECT_FALSE(history_.GetSendTime(kMaxSeqNo - 2, ×tamp, false)); - EXPECT_FALSE(history_.GetSendTime(kMaxSeqNo - 1, ×tamp, false)); - EXPECT_FALSE(history_.GetSendTime(kMaxSeqNo, ×tamp, false)); - EXPECT_TRUE(history_.GetSendTime(0, ×tamp, false)); - EXPECT_TRUE(history_.GetSendTime(1, ×tamp, false)); + info = {0, 0, static_cast(kMaxSeqNo - 2), 0, false}; + EXPECT_FALSE(history_.GetInfo(&info, false)); + info = {0, 0, static_cast(kMaxSeqNo - 1), 0, false}; + EXPECT_FALSE(history_.GetInfo(&info, false)); + info = {0, 0, kMaxSeqNo, 0, false}; + EXPECT_FALSE(history_.GetInfo(&info, false)); + info = {0, 0, 0, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); + info = {0, 0, 1, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, false)); } TEST_F(SendTimeHistoryTest, InterlievedGetAndRemove) { const uint16_t kSeqNo = 1; const int64_t kTimestamp = 2; + PacketInfo packets[3] = {{0, kTimestamp, kSeqNo, 0, false}, + {0, kTimestamp + 1, kSeqNo + 1, 0, false}, + {0, kTimestamp + 2, kSeqNo + 2, 0, false}}; - history_.AddAndRemoveOldSendTimes(kSeqNo, kTimestamp); - history_.AddAndRemoveOldSendTimes(kSeqNo + 1, kTimestamp + 1); + history_.AddAndRemoveOld(packets[0]); + history_.AddAndRemoveOld(packets[1]); - int64_t time = 0; - EXPECT_TRUE(history_.GetSendTime(kSeqNo, &time, true)); - EXPECT_EQ(kTimestamp, time); + PacketInfo info = {0, 0, packets[0].sequence_number, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, true)); + EXPECT_EQ(packets[0], info); - history_.AddAndRemoveOldSendTimes(kSeqNo + 2, kTimestamp + 2); + history_.AddAndRemoveOld(packets[2]); - EXPECT_TRUE(history_.GetSendTime(kSeqNo + 1, &time, true)); - EXPECT_EQ(kTimestamp + 1, time); - EXPECT_TRUE(history_.GetSendTime(kSeqNo + 2, &time, true)); - EXPECT_EQ(kTimestamp + 2, time); + info = {0, 0, packets[1].sequence_number, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, true)); + EXPECT_EQ(packets[1], info); + + info = {0, 0, packets[2].sequence_number, 0, false}; + EXPECT_TRUE(history_.GetInfo(&info, true)); + EXPECT_EQ(packets[2], info); } +} // namespace test } // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc index 07ca63c061..a84d6f0a5c 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc @@ -132,7 +132,8 @@ Packet::Packet() creation_time_us_(-1), send_time_us_(-1), sender_timestamp_us_(-1), - payload_size_(0) { + payload_size_(0), + paced_(false) { } Packet::Packet(int flow_id, int64_t send_time_us, size_t payload_size) @@ -140,7 +141,8 @@ Packet::Packet(int flow_id, int64_t send_time_us, size_t payload_size) creation_time_us_(send_time_us), send_time_us_(send_time_us), sender_timestamp_us_(send_time_us), - payload_size_(payload_size) { + payload_size_(payload_size), + paced_(false) { } Packet::~Packet() { diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc index 1f95f654eb..37b604dfe9 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -50,11 +50,9 @@ void FullBweSender::GiveFeedback(const FeedbackPacket& feedback) { static_cast(feedback); if (fb.packet_feedback_vector().empty()) return; - // TODO(sprang): Unconstify PacketInfo so we don't need temp copy? std::vector packet_feedback_vector(fb.packet_feedback_vector()); - for (PacketInfo& packet : packet_feedback_vector) { - if (!send_time_history_.GetSendTime(packet.sequence_number, - &packet.send_time_ms, true)) { + for (PacketInfo& packet_info : packet_feedback_vector) { + if (!send_time_history_.GetInfo(&packet_info, true)) { LOG(LS_WARNING) << "Ack arrived too late."; } } @@ -95,9 +93,10 @@ void FullBweSender::OnPacketsSent(const Packets& packets) { for (Packet* packet : packets) { if (packet->GetPacketType() == Packet::kMedia) { MediaPacket* media_packet = static_cast(packet); - send_time_history_.AddAndRemoveOldSendTimes( - media_packet->header().sequenceNumber, - media_packet->GetAbsSendTimeInMs()); + PacketInfo info(0, media_packet->sender_timestamp_ms(), + media_packet->header().sequenceNumber, + media_packet->payload_size(), packet->paced()); + send_time_history_.AddAndRemoveOld(info); } } } diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet.h b/webrtc/modules/remote_bitrate_estimator/test/packet.h index 9be894da5f..11885a4544 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet.h +++ b/webrtc/modules/remote_bitrate_estimator/test/packet.h @@ -42,6 +42,8 @@ class Packet { virtual void set_sender_timestamp_us(int64_t sender_timestamp_us) { sender_timestamp_us_ = sender_timestamp_us; } + virtual void set_paced(bool paced) { paced_ = paced; } + virtual bool paced() const { return paced_; } virtual int64_t creation_time_ms() const { return (creation_time_us_ + 500) / 1000; } @@ -56,6 +58,7 @@ class Packet { int64_t send_time_us_; // Time the packet left last processor touching it. int64_t sender_timestamp_us_; // Time the packet left the Sender. size_t payload_size_; // Size of the (non-existent, simulated) payload. + bool paced_; // True if sent through paced sender. }; class MediaPacket : public Packet { diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc index 85cbdef8cf..cde93a1cc0 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc @@ -271,6 +271,8 @@ void PacedVideoSender::QueuePackets(Packets* batch, } Packets to_transfer; to_transfer.splice(to_transfer.begin(), queue_, queue_.begin(), it); + for (Packet* packet : to_transfer) + packet->set_paced(true); bwe_->OnPacketsSent(to_transfer); batch->merge(to_transfer, DereferencingComparator); } diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index b1a21f09d5..3e31bd3933 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -149,7 +149,8 @@ class AdaptedSendTimeHistory : public SendTimeHistory, public SendTimeObserver { virtual ~AdaptedSendTimeHistory() {} void OnPacketSent(uint16_t sequence_number, int64_t send_time) override { - SendTimeHistory::AddAndRemoveOldSendTimes(sequence_number, send_time); + PacketInfo info(0, send_time, sequence_number, 0, false); + SendTimeHistory::AddAndRemoveOld(info); } };