zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

In RemoteEstimatorProxy treat zero arrival time as valid

Browse Source 
Inserting packet with zero arrival time may trigger inconsistent state in the internal map where packet sometimes treated as received, but sometimes treated as not received.

Bug: chromium:1346959
Change-Id: I0809e41a873103dcd62528358e64794c1d3cb28f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269382
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37609}
This commit is contained in:
Danil Chapovalov
2022-07-25 13:27:59 +02:00
committed by WebRTC LUCI CQ
parent 5edefa87f7
commit fc22dfa727
3 changed files with 10 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
modules/remote_bitrate_estimator/packet_arrival_map.cc
View File

@ -19,6 +19,7 @@ constexpr size_t PacketArrivalTimeMap::kMaxNumberOfPackets;
void PacketArrivalTimeMap::AddPacket(int64_t sequence_number,
Timestamp arrival_time) {
RTC_DCHECK_GE(arrival_time, Timestamp::Zero());
if (!has_seen_packet_) {
// First packet.
has_seen_packet_ = true;
@ -45,7 +46,7 @@ void PacketArrivalTimeMap::AddPacket(int64_t sequence_number,
}
arrival_times_.insert(arrival_times_.begin(), missing_packets,
Timestamp::Zero());
Timestamp::MinusInfinity());
arrival_times_[0] = arrival_time;
begin_sequence_number_ = sequence_number;
return;
@ -64,7 +65,7 @@ void PacketArrivalTimeMap::AddPacket(int64_t sequence_number,
// Also trim the buffer to remove leading non-received packets, to
// ensure that the buffer only spans received packets.
while (packets_to_remove < arrival_times_.size() &&
arrival_times_[packets_to_remove] == Timestamp::Zero()) {
arrival_times_[packets_to_remove].IsInfinite()) {
++packets_to_remove;
}
@ -81,7 +82,7 @@ void PacketArrivalTimeMap::AddPacket(int64_t sequence_number,
size_t missing_gap_packets = pos - arrival_times_.size();
if (missing_gap_packets > 0) {
arrival_times_.insert(arrival_times_.end(), missing_gap_packets,
Timestamp::Zero());
Timestamp::MinusInfinity());
}
RTC_DCHECK_EQ(arrival_times_.size(), pos);
arrival_times_.push_back(arrival_time);
@ -100,7 +101,7 @@ void PacketArrivalTimeMap::RemoveOldPackets(int64_t sequence_number,
bool PacketArrivalTimeMap::has_received(int64_t sequence_number) const {
int64_t pos = sequence_number - begin_sequence_number_;
if (pos >= 0 && pos < static_cast<int64_t>(arrival_times_.size()) &&
arrival_times_[pos] != Timestamp::Zero()) {
arrival_times_[pos].IsFinite()) {
return true;
}
return false;

8
modules/remote_bitrate_estimator/packet_arrival_map_test.cc
View File

@ -43,7 +43,7 @@ TEST(PacketArrivalMapTest, InsertsFirstItemIntoMap) {
TEST(PacketArrivalMapTest, InsertsWithGaps) {
PacketArrivalTimeMap map;
map.AddPacket(42, Timestamp::Millis(10));
map.AddPacket(42, Timestamp::Zero());
map.AddPacket(45, Timestamp::Millis(11));
EXPECT_EQ(map.begin_sequence_number(), 42);
EXPECT_EQ(map.end_sequence_number(), 46);
@ -55,9 +55,9 @@ TEST(PacketArrivalMapTest, InsertsWithGaps) {
EXPECT_TRUE(map.has_received(45));
EXPECT_FALSE(map.has_received(46));
EXPECT_EQ(map.get(42), Timestamp::Millis(10));
EXPECT_EQ(map.get(43), Timestamp::Zero());
EXPECT_EQ(map.get(44), Timestamp::Zero());
EXPECT_EQ(map.get(42), Timestamp::Zero());
EXPECT_LT(map.get(43), Timestamp::Zero());
EXPECT_LT(map.get(44), Timestamp::Zero());
EXPECT_EQ(map.get(45), Timestamp::Millis(11));
EXPECT_EQ(map.clamp(-100), 42);

2
modules/remote_bitrate_estimator/remote_estimator_proxy.cc
View File

@ -292,7 +292,7 @@ RemoteEstimatorProxy::MaybeBuildFeedbackPacket(
for (int64_t seq = start_seq; seq < end_seq; ++seq) {
Timestamp arrival_time = packet_arrival_times_.get(seq);
if (arrival_time == Timestamp::Zero()) {
if (arrival_time.IsInfinite()) {
// Packet not received.
continue;
}