From f393370f02562e9ea89d542978f4b616333715e1 Mon Sep 17 00:00:00 2001 From: "henrik.lundin" Date: Thu, 28 Apr 2016 01:53:52 -0700 Subject: [PATCH] NetEq: Introduce TickTimer in DelayPeakDetector Specifically, this change replaces peak_period_counter_ms_ with peak_period_stopwatch_ which obtains a Stopwatch object from TickTimer. Necessary plumbing to get the TickTimer through to the DelayPeakDetector is also included. BUG=webrtc:5608 NOTRY=True Review-Url: https://codereview.webrtc.org/1921163003 Cr-Commit-Position: refs/heads/master@{#12542} --- .../neteq/decision_logic_unittest.cc | 2 +- .../audio_coding/neteq/delay_manager.cc | 1 - .../neteq/delay_manager_unittest.cc | 16 ++---- .../audio_coding/neteq/delay_peak_detector.cc | 50 +++++++++---------- .../audio_coding/neteq/delay_peak_detector.h | 19 +++---- .../neteq/delay_peak_detector_unittest.cc | 20 +++++--- .../neteq/mock/mock_delay_peak_detector.h | 5 +- .../modules/audio_coding/neteq/neteq_impl.cc | 2 +- .../audio_coding/neteq/neteq_impl_unittest.cc | 3 +- 9 files changed, 55 insertions(+), 63 deletions(-) diff --git a/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc b/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc index ff8a0c4aa7..350821c547 100644 --- a/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc @@ -27,7 +27,7 @@ TEST(DecisionLogic, CreateAndDestroy) { DecoderDatabase decoder_database; TickTimer tick_timer; PacketBuffer packet_buffer(10, &tick_timer); - DelayPeakDetector delay_peak_detector; + DelayPeakDetector delay_peak_detector(&tick_timer); DelayManager delay_manager(240, &delay_peak_detector); BufferLevelFilter buffer_level_filter; DecisionLogic* logic = DecisionLogic::Create(fs_hz, output_size_samples, diff --git a/webrtc/modules/audio_coding/neteq/delay_manager.cc b/webrtc/modules/audio_coding/neteq/delay_manager.cc index af49f00f8a..e955f17837 100644 --- a/webrtc/modules/audio_coding/neteq/delay_manager.cc +++ b/webrtc/modules/audio_coding/neteq/delay_manager.cc @@ -342,7 +342,6 @@ bool DelayManager::PeakFound() const { void DelayManager::UpdateCounters(int elapsed_time_ms) { packet_iat_count_ms_ += elapsed_time_ms; - peak_detector_.IncrementCounter(elapsed_time_ms); max_timer_ms_ += elapsed_time_ms; } diff --git a/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc b/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc index f231c3da30..05d6f3e192 100644 --- a/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc @@ -39,16 +39,14 @@ class DelayManagerTest : public ::testing::Test { void IncreaseTime(int inc_ms); DelayManager* dm_; + TickTimer tick_timer_; MockDelayPeakDetector detector_; uint16_t seq_no_; uint32_t ts_; }; DelayManagerTest::DelayManagerTest() - : dm_(NULL), - seq_no_(0x1234), - ts_(0x12345678) { -} + : dm_(NULL), detector_(&tick_timer_), seq_no_(0x1234), ts_(0x12345678) {} void DelayManagerTest::SetUp() { EXPECT_CALL(detector_, Reset()) @@ -69,9 +67,8 @@ void DelayManagerTest::InsertNextPacket() { void DelayManagerTest::IncreaseTime(int inc_ms) { for (int t = 0; t < inc_ms; t += kTimeStepMs) { - EXPECT_CALL(detector_, IncrementCounter(kTimeStepMs)) - .Times(1); dm_->UpdateCounters(kTimeStepMs); + tick_timer_.Increment(); } } void DelayManagerTest::TearDown() { @@ -115,13 +112,6 @@ TEST_F(DelayManagerTest, PeakFound) { EXPECT_FALSE(dm_->PeakFound()); } -TEST_F(DelayManagerTest, UpdateCounters) { - // Expect DelayManager to pass on the counter update to the detector. - EXPECT_CALL(detector_, IncrementCounter(kTimeStepMs)) - .Times(1); - dm_->UpdateCounters(kTimeStepMs); -} - TEST_F(DelayManagerTest, UpdateNormal) { SetPacketAudioLength(kFrameSizeMs); // First packet arrival. diff --git a/webrtc/modules/audio_coding/neteq/delay_peak_detector.cc b/webrtc/modules/audio_coding/neteq/delay_peak_detector.cc index 712c7788ac..ce9133bdae 100644 --- a/webrtc/modules/audio_coding/neteq/delay_peak_detector.cc +++ b/webrtc/modules/audio_coding/neteq/delay_peak_detector.cc @@ -12,6 +12,9 @@ #include // max +#include "webrtc/base/checks.h" +#include "webrtc/base/safe_conversions.h" + namespace webrtc { // The DelayPeakDetector keeps track of severe inter-arrival times, called @@ -23,14 +26,15 @@ namespace webrtc { DelayPeakDetector::~DelayPeakDetector() = default; -DelayPeakDetector::DelayPeakDetector() - : peak_found_(false), - peak_detection_threshold_(0), - peak_period_counter_ms_(-1) { +DelayPeakDetector::DelayPeakDetector(const TickTimer* tick_timer) + : peak_found_(false), + peak_detection_threshold_(0), + tick_timer_(tick_timer) { + RTC_DCHECK(!peak_period_stopwatch_); } void DelayPeakDetector::Reset() { - peak_period_counter_ms_ = -1; // Indicate that next peak is the first. + peak_period_stopwatch_.reset(); peak_found_ = false; peak_history_.clear(); } @@ -55,38 +59,40 @@ int DelayPeakDetector::MaxPeakHeight() const { return max_height; } -int DelayPeakDetector::MaxPeakPeriod() const { - int max_period = -1; // Returns -1 for an empty history. - std::list::const_iterator it; - for (it = peak_history_.begin(); it != peak_history_.end(); ++it) { - max_period = std::max(max_period, it->period_ms); +uint64_t DelayPeakDetector::MaxPeakPeriod() const { + auto max_period_element = std::max_element( + peak_history_.begin(), peak_history_.end(), + [](Peak a, Peak b) { return a.period_ms < b.period_ms; }); + if (max_period_element == peak_history_.end()) { + return 0; // |peak_history_| is empty. } - return max_period; + RTC_DCHECK_GT(max_period_element->period_ms, 0u); + return max_period_element->period_ms; } bool DelayPeakDetector::Update(int inter_arrival_time, int target_level) { if (inter_arrival_time > target_level + peak_detection_threshold_ || inter_arrival_time > 2 * target_level) { // A delay peak is observed. - if (peak_period_counter_ms_ == -1) { + if (!peak_period_stopwatch_) { // This is the first peak. Reset the period counter. - peak_period_counter_ms_ = 0; - } else if (peak_period_counter_ms_ <= kMaxPeakPeriodMs) { + 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_counter_ms_; + 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_counter_ms_ = 0; - } else if (peak_period_counter_ms_ <= 2 * kMaxPeakPeriodMs) { + 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_counter_ms_ = 0; + 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. @@ -97,16 +103,10 @@ bool DelayPeakDetector::Update(int inter_arrival_time, int target_level) { return CheckPeakConditions(); } -void DelayPeakDetector::IncrementCounter(int inc_ms) { - if (peak_period_counter_ms_ >= 0) { - peak_period_counter_ms_ += inc_ms; - } -} - bool DelayPeakDetector::CheckPeakConditions() { size_t s = peak_history_.size(); if (s >= kMinPeaksToTrigger && - peak_period_counter_ms_ <= 2 * MaxPeakPeriod()) { + peak_period_stopwatch_->ElapsedMs() <= 2 * MaxPeakPeriod()) { peak_found_ = true; } else { peak_found_ = false; diff --git a/webrtc/modules/audio_coding/neteq/delay_peak_detector.h b/webrtc/modules/audio_coding/neteq/delay_peak_detector.h index 69433b4524..060e2e13d3 100644 --- a/webrtc/modules/audio_coding/neteq/delay_peak_detector.h +++ b/webrtc/modules/audio_coding/neteq/delay_peak_detector.h @@ -16,12 +16,13 @@ #include #include "webrtc/base/constructormagic.h" +#include "webrtc/modules/audio_coding/neteq/tick_timer.h" namespace webrtc { class DelayPeakDetector { public: - DelayPeakDetector(); + DelayPeakDetector(const TickTimer* tick_timer); virtual ~DelayPeakDetector(); virtual void Reset(); @@ -37,20 +38,15 @@ class DelayPeakDetector { // delay peaks have been observed recently. The unit is number of packets. virtual int MaxPeakHeight() const; - // Calculates and returns the maximum delay peak distance in ms. - // Returns -1 if no delay peaks have been observed recently. - virtual int MaxPeakPeriod() const; + // Calculates and returns the maximum delay peak distance in ms (strictly + // larger than 0), or 0 if no delay peaks have been observed recently. + virtual uint64_t MaxPeakPeriod() const; // Updates the DelayPeakDetector with a new inter-arrival time (in packets) // and the current target buffer level (needed to decide if a peak is observed // or not). Returns true if peak-mode is active, false if not. virtual bool Update(int inter_arrival_time, int target_level); - // Increments the |peak_period_counter_ms_| with |inc_ms|. Only increments - // the counter if it is non-negative. A negative denotes that no peak has - // been observed. - virtual void IncrementCounter(int inc_ms); - private: static const size_t kMaxNumPeaks = 8; static const size_t kMinPeaksToTrigger = 2; @@ -58,7 +54,7 @@ class DelayPeakDetector { static const int kMaxPeakPeriodMs = 10000; typedef struct { - int period_ms; + uint64_t period_ms; int peak_height_packets; } Peak; @@ -67,7 +63,8 @@ class DelayPeakDetector { std::list peak_history_; bool peak_found_; int peak_detection_threshold_; - int peak_period_counter_ms_; + const TickTimer* tick_timer_; + std::unique_ptr peak_period_stopwatch_; RTC_DISALLOW_COPY_AND_ASSIGN(DelayPeakDetector); }; diff --git a/webrtc/modules/audio_coding/neteq/delay_peak_detector_unittest.cc b/webrtc/modules/audio_coding/neteq/delay_peak_detector_unittest.cc index c40f3991b0..32b36b25ef 100644 --- a/webrtc/modules/audio_coding/neteq/delay_peak_detector_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/delay_peak_detector_unittest.cc @@ -17,22 +17,25 @@ namespace webrtc { TEST(DelayPeakDetector, CreateAndDestroy) { - DelayPeakDetector* detector = new DelayPeakDetector(); + TickTimer tick_timer; + DelayPeakDetector* detector = new DelayPeakDetector(&tick_timer); EXPECT_FALSE(detector->peak_found()); delete detector; } TEST(DelayPeakDetector, EmptyHistory) { - DelayPeakDetector detector; + TickTimer tick_timer; + DelayPeakDetector detector(&tick_timer); EXPECT_EQ(-1, detector.MaxPeakHeight()); - EXPECT_EQ(-1, detector.MaxPeakPeriod()); + EXPECT_EQ(0u, detector.MaxPeakPeriod()); } // Inject a series of packet arrivals into the detector. Three of the packets // have suffered delays. After the third delay peak, peak-mode is expected to // start. This should then continue until it is disengaged due to lack of peaks. TEST(DelayPeakDetector, TriggerPeakMode) { - DelayPeakDetector detector; + TickTimer tick_timer; + DelayPeakDetector detector(&tick_timer); const int kPacketSizeMs = 30; detector.SetPacketAudioLength(kPacketSizeMs); @@ -52,7 +55,7 @@ TEST(DelayPeakDetector, TriggerPeakMode) { // Third delay peak. Trigger peak-mode after this packet. arrival_times_ms[400] += kPeakDelayMs; // The second peak period is the longest, 200 packets. - const int kWorstPeakPeriod = 200 * kPacketSizeMs; + const uint64_t kWorstPeakPeriod = 200 * kPacketSizeMs; int peak_mode_start_ms = arrival_times_ms[400]; // Expect to disengage after no peaks are observed for two period times. int peak_mode_end_ms = peak_mode_start_ms + 2 * kWorstPeakPeriod; @@ -74,7 +77,7 @@ TEST(DelayPeakDetector, TriggerPeakMode) { } ++next; } - detector.IncrementCounter(10); + tick_timer.Increment(); time += 10; // Increase time 10 ms. } } @@ -83,7 +86,8 @@ TEST(DelayPeakDetector, TriggerPeakMode) { // 2, in order to raise the bar for delay peaks to inter-arrival times > 4. // The delay pattern has peaks with delay = 3, thus should not trigger. TEST(DelayPeakDetector, DoNotTriggerPeakMode) { - DelayPeakDetector detector; + TickTimer tick_timer; + DelayPeakDetector detector(&tick_timer); const int kPacketSizeMs = 30; detector.SetPacketAudioLength(kPacketSizeMs); @@ -114,7 +118,7 @@ TEST(DelayPeakDetector, DoNotTriggerPeakMode) { EXPECT_FALSE(detector.Update(iat_packets, kTargetBufferLevel)); ++next; } - detector.IncrementCounter(10); + tick_timer.Increment(); time += 10; // Increase time 10 ms. } } diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_delay_peak_detector.h b/webrtc/modules/audio_coding/neteq/mock/mock_delay_peak_detector.h index fa5cd7ed06..5564fba312 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_delay_peak_detector.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_delay_peak_detector.h @@ -19,15 +19,16 @@ namespace webrtc { class MockDelayPeakDetector : public DelayPeakDetector { public: + MockDelayPeakDetector(const TickTimer* tick_timer) + : DelayPeakDetector(tick_timer) {} virtual ~MockDelayPeakDetector() { Die(); } MOCK_METHOD0(Die, void()); MOCK_METHOD0(Reset, void()); MOCK_METHOD1(SetPacketAudioLength, void(int length_ms)); MOCK_METHOD0(peak_found, bool()); MOCK_CONST_METHOD0(MaxPeakHeight, int()); - MOCK_CONST_METHOD0(MaxPeakPeriod, int()); + MOCK_CONST_METHOD0(MaxPeakPeriod, uint64_t()); MOCK_METHOD2(Update, bool(int inter_arrival_time, int target_level)); - MOCK_METHOD1(IncrementCounter, void(int inc_ms)); }; } // namespace webrtc diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index 01325e33cd..7bdb23cbc4 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -58,7 +58,7 @@ NetEqImpl::Dependencies::Dependencies(const NetEq::Config& config) : tick_timer(new TickTimer), buffer_level_filter(new BufferLevelFilter), decoder_database(new DecoderDatabase), - delay_peak_detector(new DelayPeakDetector), + delay_peak_detector(new DelayPeakDetector(tick_timer.get())), delay_manager(new DelayManager(config.max_packets_in_buffer, delay_peak_detector.get())), dtmf_buffer(new DtmfBuffer(config.sample_rate_hz)), diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index 5cb9762b25..ed6dc20815 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -81,7 +81,8 @@ class NetEqImplTest : public ::testing::Test { decoder_database_ = deps.decoder_database.get(); if (use_mock_delay_peak_detector_) { - std::unique_ptr mock(new MockDelayPeakDetector); + std::unique_ptr mock( + new MockDelayPeakDetector(tick_timer_)); mock_delay_peak_detector_ = mock.get(); EXPECT_CALL(*mock_delay_peak_detector_, Reset()).Times(1); deps.delay_peak_detector = std::move(mock);