Fixed bug in SendTimeHistory, where deleting packets via the getter

would not update the oldest suence number.

BUG=
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/42589004

Cr-Commit-Position: refs/heads/master@{#8574}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8574 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
sprang@webrtc.org 2015-03-03 15:44:15 +00:00
parent 0561716ae2
commit b144b4b74e
3 changed files with 48 additions and 25 deletions

View File

@ -39,22 +39,23 @@ void SendTimeHistory::EraseOld(int64_t limit) {
while (!history_.empty()) {
auto it = history_.find(oldest_sequence_number_);
assert(it != history_.end());
if (it->second <= limit) {
// Packet too old, remove it.
history_.erase(it);
if (it->second > limit)
return; // Oldest packet within age limit, return.
// TODO(sprang): Warn if erasing (too many) old items?
} else {
// Oldest packet within age limit, return.
return;
history_.erase(it);
UpdateOldestSequenceNumber();
}
}
if (history_.empty())
return;
// After removing element from the map, update oldest_sequence_number_ to
void SendTimeHistory::UpdateOldestSequenceNumber() {
// After removing an element from the map, update oldest_sequence_number_ to
// the element with the lowest sequence number higher than the previous
// value (there might be gaps).
it = history_.upper_bound(oldest_sequence_number_);
if (history_.empty())
return;
auto it = history_.upper_bound(oldest_sequence_number_);
if (it == history_.end()) {
// No element with higher sequence number than oldest_sequence_number_
// found, check wrap around. Note that history_.upper_bound(0) will not
@ -66,7 +67,6 @@ void SendTimeHistory::EraseOld(int64_t limit) {
assert(it != history_.end());
oldest_sequence_number_ = it->first;
}
}
bool SendTimeHistory::GetSendTime(uint16_t sequence_number,
int64_t* timestamp,
@ -75,8 +75,11 @@ bool SendTimeHistory::GetSendTime(uint16_t sequence_number,
if (it == history_.end())
return false;
*timestamp = it->second;
if (remove)
if (remove) {
history_.erase(it);
if (sequence_number == oldest_sequence_number_)
UpdateOldestSequenceNumber();
}
return true;
}

View File

@ -29,6 +29,7 @@ class SendTimeHistory {
private:
void EraseOld(int64_t limit);
void UpdateOldestSequenceNumber();
const int64_t packet_age_limit_;
uint16_t oldest_sequence_number_; // Oldest may not be lowest.

View File

@ -127,4 +127,23 @@ TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) {
EXPECT_TRUE(history_.GetSendTime(1, &timestamp, false));
}
TEST_F(SendTimeHistoryTest, InterlievedGetAndRemove) {
const uint16_t kSeqNo = 1;
const int64_t kTimestamp = 2;
history_.AddAndRemoveOldSendTimes(kSeqNo, kTimestamp);
history_.AddAndRemoveOldSendTimes(kSeqNo + 1, kTimestamp + 1);
int64_t time = 0;
EXPECT_TRUE(history_.GetSendTime(kSeqNo, &time, true));
EXPECT_EQ(kTimestamp, time);
history_.AddAndRemoveOldSendTimes(kSeqNo + 2, kTimestamp + 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);
}
} // namespace webrtc