NetEq: Fix a bug in DelayPeakDetector causing asserts to trigger

In some situation, typically when incoming packets were reordered, the
DelayPeakDetector::Update method may be called twice (or more) with
non-zero inter_arrival_time argument, but without the TickTimer object
being updated in between (i.e., packets coming in more or less at the
same time). In these situations, a delay peak may be registered with
zero peak period. This could eventually trigger the DCHECK in
DelayPeakDetector::MaxPeakPeriod().

With this fix, the problem is solved by not registering peaks for which
the TickTimer object has not moved since the last peak.

The problem was originally introduced with
https://codereview.webrtc.org/1921163003.

BUG=webrtc:6021

Review-Url: https://codereview.webrtc.org/2085233002
Cr-Commit-Position: refs/heads/master@{#13257}
This commit is contained in:
henrik.lundin 2016-06-22 09:07:04 -07:00 committed by Commit bot
parent 66910708ac
commit d5f50a1b53
2 changed files with 40 additions and 20 deletions

View File

@ -77,27 +77,29 @@ bool DelayPeakDetector::Update(int inter_arrival_time, int target_level) {
if (!peak_period_stopwatch_) {
// This is the first peak. Reset the period counter.
peak_period_stopwatch_ = tick_timer_->GetNewStopwatch();
} else if (peak_period_stopwatch_->ElapsedMs() <= kMaxPeakPeriodMs) {
// This is not the first peak, and the period is valid.
// Store peak data in the vector.
Peak peak_data;
peak_data.period_ms = peak_period_stopwatch_->ElapsedMs();
peak_data.peak_height_packets = inter_arrival_time;
peak_history_.push_back(peak_data);
while (peak_history_.size() > kMaxNumPeaks) {
// Delete the oldest data point.
peak_history_.pop_front();
} else if (peak_period_stopwatch_->ElapsedMs() > 0) {
if (peak_period_stopwatch_->ElapsedMs() <= kMaxPeakPeriodMs) {
// This is not the first peak, and the period is valid.
// Store peak data in the vector.
Peak peak_data;
peak_data.period_ms = peak_period_stopwatch_->ElapsedMs();
peak_data.peak_height_packets = inter_arrival_time;
peak_history_.push_back(peak_data);
while (peak_history_.size() > kMaxNumPeaks) {
// Delete the oldest data point.
peak_history_.pop_front();
}
peak_period_stopwatch_ = tick_timer_->GetNewStopwatch();
} else if (peak_period_stopwatch_->ElapsedMs() <= 2 * kMaxPeakPeriodMs) {
// Invalid peak due to too long period. Reset period counter and start
// looking for next peak.
peak_period_stopwatch_ = tick_timer_->GetNewStopwatch();
} else {
// More than 2 times the maximum period has elapsed since the last peak
// was registered. It seams that the network conditions have changed.
// Reset the peak statistics.
Reset();
}
peak_period_stopwatch_ = tick_timer_->GetNewStopwatch();
} else if (peak_period_stopwatch_->ElapsedMs() <= 2 * kMaxPeakPeriodMs) {
// Invalid peak due to too long period. Reset period counter and start
// looking for next peak.
peak_period_stopwatch_ = tick_timer_->GetNewStopwatch();
} else {
// More than 2 times the maximum period has elapsed since the last peak
// was registered. It seams that the network conditions have changed.
// Reset the peak statistics.
Reset();
}
}
return CheckPeakConditions();

View File

@ -122,4 +122,22 @@ TEST(DelayPeakDetector, DoNotTriggerPeakMode) {
time += 10; // Increase time 10 ms.
}
}
// In situations with reordered packets, the DelayPeakDetector may be updated
// back-to-back (i.e., without the tick_timer moving) but still with non-zero
// inter-arrival time. This test is to make sure that this does not cause
// problems.
TEST(DelayPeakDetector, ZeroDistancePeaks) {
TickTimer tick_timer;
DelayPeakDetector detector(&tick_timer);
const int kPacketSizeMs = 30;
detector.SetPacketAudioLength(kPacketSizeMs);
const int kTargetBufferLevel = 2; // Define peaks to be iat > 4.
const int kInterArrivalTime = 3 * kTargetBufferLevel; // Will trigger a peak.
EXPECT_FALSE(detector.Update(kInterArrivalTime, kTargetBufferLevel));
EXPECT_FALSE(detector.Update(kInterArrivalTime, kTargetBufferLevel));
EXPECT_FALSE(detector.Update(kInterArrivalTime, kTargetBufferLevel));
}
} // namespace webrtc