From 74154e65e8dd04ada213cad2dd5f13a7dc43e5a8 Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Thu, 22 Aug 2019 15:00:16 +0200 Subject: [PATCH] Save delays in history for 2 seconds instead of fixed 100 packets. Storing a fixed amount of packets does not work well with DTX since the history could include up to 20 seconds of packets which can potentially be negative in the event of clock drift or delay shifts. Bug: webrtc:10333 Change-Id: Ifb8543b7e999e17845cb0e4171066862941f370e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149832 Reviewed-by: Minyue Li Reviewed-by: Ivo Creusen Commit-Queue: Jakob Ivarsson Cr-Commit-Position: refs/heads/master@{#28942} --- modules/audio_coding/neteq/delay_manager.cc | 29 ++++++++++++------- modules/audio_coding/neteq/delay_manager.h | 12 ++++++-- .../neteq/delay_manager_unittest.cc | 22 ++++++++++++++ 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/modules/audio_coding/neteq/delay_manager.cc b/modules/audio_coding/neteq/delay_manager.cc index b101759496..19e0af2b34 100644 --- a/modules/audio_coding/neteq/delay_manager.cc +++ b/modules/audio_coding/neteq/delay_manager.cc @@ -38,9 +38,8 @@ constexpr int kIatFactor = 32745; // 0.9993 in Q15. constexpr int kMaxIat = 64; // Max inter-arrival time to register. constexpr int kMaxReorderedPackets = 10; // Max number of consecutive reordered packets. -constexpr int kMaxHistoryPackets = - 100; // Max number of packets used to calculate relative packet arrival - // delay. +constexpr int kMaxHistoryMs = 2000; // Oldest packet to include in history to + // calculate relative packet arrival delay. constexpr int kDelayBuckets = 100; constexpr int kBucketSizeMs = 20; @@ -284,7 +283,7 @@ int DelayManager::Update(uint16_t sequence_number, if (reordered) { relative_delay = std::max(iat_delay, 0); } else { - UpdateDelayHistory(iat_delay); + UpdateDelayHistory(iat_delay, timestamp, sample_rate_hz); relative_delay = CalculateRelativePacketArrivalDelay(); } statistics_->RelativePacketArrivalDelay(relative_delay); @@ -325,9 +324,15 @@ int DelayManager::Update(uint16_t sequence_number, return 0; } -void DelayManager::UpdateDelayHistory(int iat_delay) { - delay_history_.push_back(iat_delay); - if (delay_history_.size() > kMaxHistoryPackets) { +void DelayManager::UpdateDelayHistory(int iat_delay_ms, + uint32_t timestamp, + int sample_rate_hz) { + PacketDelay delay; + delay.iat_delay_ms = iat_delay_ms; + delay.timestamp = timestamp; + delay_history_.push_back(delay); + while (timestamp - delay_history_.front().timestamp > + static_cast(kMaxHistoryMs * sample_rate_hz / 1000)) { delay_history_.pop_front(); } } @@ -338,8 +343,8 @@ int DelayManager::CalculateRelativePacketArrivalDelay() const { // smaller than zero, it means the reference packet is invalid, and we // move the reference. int relative_delay = 0; - for (int delay : delay_history_) { - relative_delay += delay; + for (const PacketDelay& delay : delay_history_) { + relative_delay += delay.iat_delay_ms; relative_delay = std::max(relative_delay, 0); } return relative_delay; @@ -382,7 +387,10 @@ int DelayManager::CalculateTargetLevel(int iat_packets, bool reordered) { int target_level; switch (histogram_mode_) { case RELATIVE_ARRIVAL_DELAY: { - target_level = 1 + bucket_index * kBucketSizeMs / packet_len_ms_; + target_level = 1; + if (packet_len_ms_ > 0) { + target_level += bucket_index * kBucketSizeMs / packet_len_ms_; + } base_target_level_ = target_level; break; } @@ -432,6 +440,7 @@ void DelayManager::Reset() { packet_len_ms_ = 0; // Packet size unknown. peak_detector_.Reset(); histogram_->Reset(); + delay_history_.clear(); base_target_level_ = 4; target_level_ = base_target_level_ << 8; packet_iat_stopwatch_ = tick_timer_->GetNewStopwatch(); diff --git a/modules/audio_coding/neteq/delay_manager.h b/modules/audio_coding/neteq/delay_manager.h index 851ed46147..44d27f88e6 100644 --- a/modules/audio_coding/neteq/delay_manager.h +++ b/modules/audio_coding/neteq/delay_manager.h @@ -151,7 +151,9 @@ class DelayManager { int MaxBufferTimeQ75() const; // Updates |delay_history_|. - void UpdateDelayHistory(int iat_delay); + void UpdateDelayHistory(int iat_delay_ms, + uint32_t timestamp, + int sample_rate_hz); // Calculate relative packet arrival delay from |delay_history_|. int CalculateRelativePacketArrivalDelay() const; @@ -203,7 +205,13 @@ class DelayManager { const bool frame_length_change_experiment_; const bool enable_rtx_handling_; int num_reordered_packets_ = 0; // Number of consecutive reordered packets. - std::deque delay_history_; + + struct PacketDelay { + int iat_delay_ms; + uint32_t timestamp; + }; + std::deque delay_history_; + // When current buffer level is more than // |deceleration_target_level_offset_ms_| below the target level, NetEq will // impose deceleration to increase the buffer level. The value is in Q8, and diff --git a/modules/audio_coding/neteq/delay_manager_unittest.cc b/modules/audio_coding/neteq/delay_manager_unittest.cc index 2d15e479e7..ab316e2f52 100644 --- a/modules/audio_coding/neteq/delay_manager_unittest.cc +++ b/modules/audio_coding/neteq/delay_manager_unittest.cc @@ -748,6 +748,28 @@ TEST_F(DelayManagerTest, RelativeArrivalDelayMode) { EXPECT_EQ(0, dm_->Update(seq_no_, ts_, kFs)); } +TEST_F(DelayManagerTest, MaxDelayHistory) { + histogram_mode_ = DelayManager::HistogramMode::RELATIVE_ARRIVAL_DELAY; + use_mock_histogram_ = true; + RecreateDelayManager(); + + SetPacketAudioLength(kFrameSizeMs); + InsertNextPacket(); + + // Insert 20 ms iat delay in the delay history. + IncreaseTime(2 * kFrameSizeMs); + EXPECT_CALL(*mock_histogram_, Add(1)); // 20ms delayed. + InsertNextPacket(); + + // Insert next packet with a timestamp difference larger than maximum history + // size. This removes the previously inserted iat delay from the history. + constexpr int kMaxHistoryMs = 2000; + IncreaseTime(kMaxHistoryMs + kFrameSizeMs); + ts_ += kFs * kMaxHistoryMs / 1000; + EXPECT_CALL(*mock_histogram_, Add(0)); // Not delayed. + EXPECT_EQ(0, dm_->Update(seq_no_, ts_, kFs)); +} + TEST_F(DelayManagerTest, RelativeArrivalDelayStatistic) { SetPacketAudioLength(kFrameSizeMs); InsertNextPacket();