From 1bb8cf846d9a0bfe74fceae34ebef60f56d12fa4 Mon Sep 17 00:00:00 2001 From: Henrik Lundin Date: Tue, 25 Aug 2015 13:08:04 +0200 Subject: [PATCH] NetEq/ACM: Refactor how packet waiting times are calculated With this change, the aggregates for packet waiting times are calculated in NetEq's StatisticsCalculator insead of in AcmReceiver. This simplifies things somewhat, and avoids having to copy the raw data on polling. R=ivoc@webrtc.org, minyue@webrtc.org Review URL: https://codereview.webrtc.org/1296633002 . Cr-Commit-Position: refs/heads/master@{#9778} --- .../audio_coding/main/acm2/acm_receiver.cc | 30 ++------- .../audio_coding/neteq/interface/neteq.h | 12 ++-- .../modules/audio_coding/neteq/neteq_impl.cc | 5 -- .../modules/audio_coding/neteq/neteq_impl.h | 7 --- .../audio_coding/neteq/neteq_unittest.cc | 49 ++++----------- .../neteq/statistics_calculator.cc | 61 +++++++++++-------- .../neteq/statistics_calculator.h | 15 ++--- 7 files changed, 64 insertions(+), 115 deletions(-) diff --git a/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc index 1cefeb6626..2040ae1c4e 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc @@ -15,6 +15,7 @@ #include // sort #include +#include "webrtc/base/checks.h" #include "webrtc/base/format_macros.h" #include "webrtc/base/logging.h" #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h" @@ -652,31 +653,10 @@ void AcmReceiver::GetNetworkStatistics(NetworkStatistics* acm_stat) { acm_stat->currentSecondaryDecodedRate = neteq_stat.secondary_decoded_rate; acm_stat->clockDriftPPM = neteq_stat.clockdrift_ppm; acm_stat->addedSamples = neteq_stat.added_zero_samples; - - std::vector waiting_times; - neteq_->WaitingTimes(&waiting_times); - size_t size = waiting_times.size(); - if (size == 0) { - acm_stat->meanWaitingTimeMs = -1; - acm_stat->medianWaitingTimeMs = -1; - acm_stat->minWaitingTimeMs = -1; - acm_stat->maxWaitingTimeMs = -1; - } else { - std::sort(waiting_times.begin(), waiting_times.end()); - if ((size & 0x1) == 0) { - acm_stat->medianWaitingTimeMs = (waiting_times[size / 2 - 1] + - waiting_times[size / 2]) / 2; - } else { - acm_stat->medianWaitingTimeMs = waiting_times[size / 2]; - } - acm_stat->minWaitingTimeMs = waiting_times.front(); - acm_stat->maxWaitingTimeMs = waiting_times.back(); - double sum = 0; - for (size_t i = 0; i < size; ++i) { - sum += waiting_times[i]; - } - acm_stat->meanWaitingTimeMs = static_cast(sum / size); - } + acm_stat->meanWaitingTimeMs = neteq_stat.mean_waiting_time_ms; + acm_stat->medianWaitingTimeMs = neteq_stat.median_waiting_time_ms; + acm_stat->minWaitingTimeMs = neteq_stat.min_waiting_time_ms; + acm_stat->maxWaitingTimeMs = neteq_stat.max_waiting_time_ms; } int AcmReceiver::DecoderByPayloadType(uint8_t payload_type, diff --git a/webrtc/modules/audio_coding/neteq/interface/neteq.h b/webrtc/modules/audio_coding/neteq/interface/neteq.h index 865a8b38ed..e2ea00debb 100644 --- a/webrtc/modules/audio_coding/neteq/interface/neteq.h +++ b/webrtc/modules/audio_coding/neteq/interface/neteq.h @@ -14,7 +14,6 @@ #include // Provide access to size_t. #include -#include #include "webrtc/base/constructormagic.h" #include "webrtc/common_types.h" @@ -46,6 +45,12 @@ struct NetEqNetworkStatistics { int32_t clockdrift_ppm; // Average clock-drift in parts-per-million // (positive or negative). size_t added_zero_samples; // Number of zero samples added in "off" mode. + // Statistics for packet waiting times, i.e., the time between a packet + // arrives until it is decoded. + int mean_waiting_time_ms; + int median_waiting_time_ms; + int min_waiting_time_ms; + int max_waiting_time_ms; }; enum NetEqOutputType { @@ -227,11 +232,6 @@ class NetEq { // after the call. virtual int NetworkStatistics(NetEqNetworkStatistics* stats) = 0; - // Writes the last packet waiting times (in ms) to |waiting_times|. The number - // of values written is no more than 100, but may be smaller if the interface - // is polled again before 100 packets has arrived. - virtual void WaitingTimes(std::vector* waiting_times) = 0; - // Writes the current RTCP statistics to |stats|. The statistics are reset // and a new report period is started with the call. virtual void GetRtcpStatistics(RtcpStatistics* stats) = 0; diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index d890acb582..22e71f7f1d 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -318,11 +318,6 @@ int NetEqImpl::NetworkStatistics(NetEqNetworkStatistics* stats) { return 0; } -void NetEqImpl::WaitingTimes(std::vector* waiting_times) { - CriticalSectionScoped lock(crit_sect_.get()); - stats_.WaitingTimes(waiting_times); -} - void NetEqImpl::GetRtcpStatistics(RtcpStatistics* stats) { CriticalSectionScoped lock(crit_sect_.get()); if (stats) { diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.h b/webrtc/modules/audio_coding/neteq/neteq_impl.h index 502204ac85..182d8b8d89 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h @@ -11,8 +11,6 @@ #ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_NETEQ_IMPL_H_ #define WEBRTC_MODULES_AUDIO_CODING_NETEQ_NETEQ_IMPL_H_ -#include - #include "webrtc/base/constructormagic.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/thread_annotations.h" @@ -154,11 +152,6 @@ class NetEqImpl : public webrtc::NetEq { // after the call. int NetworkStatistics(NetEqNetworkStatistics* stats) override; - // Writes the last packet waiting times (in ms) to |waiting_times|. The number - // of values written is no more than 100, but may be smaller if the interface - // is polled again before 100 packets has arrived. - void WaitingTimes(std::vector* waiting_times) override; - // Writes the current RTCP statistics to |stats|. The statistics are reset // and a new report period is started with the call. void GetRtcpStatistics(RtcpStatistics* stats) override; diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc index 03fde53889..0c43024799 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc @@ -507,46 +507,23 @@ TEST_F(NetEqDecodingTestFaxMode, TestFrameWaitingTimeStatistics) { ASSERT_EQ(kBlockSize16kHz, out_len); } - std::vector waiting_times; - neteq_->WaitingTimes(&waiting_times); - EXPECT_EQ(num_frames, waiting_times.size()); + NetEqNetworkStatistics stats; + EXPECT_EQ(0, neteq_->NetworkStatistics(&stats)); // Since all frames are dumped into NetEQ at once, but pulled out with 10 ms // spacing (per definition), we expect the delay to increase with 10 ms for - // each packet. - for (size_t i = 0; i < waiting_times.size(); ++i) { - EXPECT_EQ(static_cast(i + 1) * 10, waiting_times[i]); - } + // each packet. Thus, we are calculating the statistics for a series from 10 + // to 300, in steps of 10 ms. + EXPECT_EQ(155, stats.mean_waiting_time_ms); + EXPECT_EQ(155, stats.median_waiting_time_ms); + EXPECT_EQ(10, stats.min_waiting_time_ms); + EXPECT_EQ(300, stats.max_waiting_time_ms); // Check statistics again and make sure it's been reset. - neteq_->WaitingTimes(&waiting_times); - int len = waiting_times.size(); - EXPECT_EQ(0, len); - - // Process > 100 frames, and make sure that that we get statistics - // only for 100 frames. Note the new SSRC, causing NetEQ to reset. - num_frames = 110; - for (size_t i = 0; i < num_frames; ++i) { - uint16_t payload[kSamples] = {0}; - WebRtcRTPHeader rtp_info; - rtp_info.header.sequenceNumber = i; - rtp_info.header.timestamp = i * kSamples; - rtp_info.header.ssrc = 0x1235; // Just an arbitrary SSRC. - rtp_info.header.payloadType = 94; // PCM16b WB codec. - rtp_info.header.markerBit = 0; - ASSERT_EQ(0, neteq_->InsertPacket( - rtp_info, - reinterpret_cast(payload), - kPayloadBytes, 0)); - size_t out_len; - int num_channels; - NetEqOutputType type; - ASSERT_EQ(0, neteq_->GetAudio(kMaxBlockSize, out_data_, &out_len, - &num_channels, &type)); - ASSERT_EQ(kBlockSize16kHz, out_len); - } - - neteq_->WaitingTimes(&waiting_times); - EXPECT_EQ(100u, waiting_times.size()); + EXPECT_EQ(0, neteq_->NetworkStatistics(&stats)); + EXPECT_EQ(-1, stats.mean_waiting_time_ms); + EXPECT_EQ(-1, stats.median_waiting_time_ms); + EXPECT_EQ(-1, stats.min_waiting_time_ms); + EXPECT_EQ(-1, stats.max_waiting_time_ms); } TEST_F(NetEqDecodingTest, TestAverageInterArrivalTimeNegative) { diff --git a/webrtc/modules/audio_coding/neteq/statistics_calculator.cc b/webrtc/modules/audio_coding/neteq/statistics_calculator.cc index c716fe4df5..5a4ef54889 100644 --- a/webrtc/modules/audio_coding/neteq/statistics_calculator.cc +++ b/webrtc/modules/audio_coding/neteq/statistics_calculator.cc @@ -12,6 +12,7 @@ #include #include // memset +#include #include "webrtc/base/checks.h" #include "webrtc/base/safe_conversions.h" @@ -21,6 +22,9 @@ namespace webrtc { +// Allocating the static const so that it can be passed by reference to DCHECK. +const size_t StatisticsCalculator::kLenWaitingTimes; + StatisticsCalculator::PeriodicUmaLogger::PeriodicUmaLogger( const std::string& uma_name, int report_interval_ms, @@ -107,8 +111,6 @@ StatisticsCalculator::StatisticsCalculator() discarded_packets_(0), lost_timestamps_(0), timestamps_since_last_report_(0), - len_waiting_times_(0), - next_waiting_time_index_(0), secondary_decoded_samples_(0), delayed_packet_outage_counter_( "WebRTC.Audio.DelayedPacketOutageEventsPerMinute", @@ -117,9 +119,10 @@ StatisticsCalculator::StatisticsCalculator() excess_buffer_delay_("WebRTC.Audio.AverageExcessBufferDelayMs", 60000, // 60 seconds report interval. 1000) { - memset(waiting_times_, 0, kLenWaitingTimes * sizeof(waiting_times_[0])); } +StatisticsCalculator::~StatisticsCalculator() = default; + void StatisticsCalculator::Reset() { preemptive_samples_ = 0; accelerate_samples_ = 0; @@ -127,6 +130,7 @@ void StatisticsCalculator::Reset() { expanded_speech_samples_ = 0; expanded_noise_samples_ = 0; secondary_decoded_samples_ = 0; + waiting_times_.clear(); } void StatisticsCalculator::ResetMcu() { @@ -135,12 +139,6 @@ void StatisticsCalculator::ResetMcu() { timestamps_since_last_report_ = 0; } -void StatisticsCalculator::ResetWaitingTimeStatistics() { - memset(waiting_times_, 0, kLenWaitingTimes * sizeof(waiting_times_[0])); - len_waiting_times_ = 0; - next_waiting_time_index_ = 0; -} - void StatisticsCalculator::ExpandedVoiceSamples(size_t num_samples) { expanded_speech_samples_ += num_samples; } @@ -196,15 +194,12 @@ void StatisticsCalculator::LogDelayedPacketOutageEvent(int outage_duration_ms) { void StatisticsCalculator::StoreWaitingTime(int waiting_time_ms) { excess_buffer_delay_.RegisterSample(waiting_time_ms); - assert(next_waiting_time_index_ < kLenWaitingTimes); - waiting_times_[next_waiting_time_index_] = waiting_time_ms; - next_waiting_time_index_++; - if (next_waiting_time_index_ >= kLenWaitingTimes) { - next_waiting_time_index_ = 0; - } - if (len_waiting_times_ < kLenWaitingTimes) { - len_waiting_times_++; + DCHECK_LE(waiting_times_.size(), kLenWaitingTimes); + while (waiting_times_.size() >= kLenWaitingTimes) { + // Erase first value. + waiting_times_.pop_front(); } + waiting_times_.push_back(waiting_time_ms); } void StatisticsCalculator::GetNetworkStatistics( @@ -254,19 +249,35 @@ void StatisticsCalculator::GetNetworkStatistics( CalculateQ14Ratio(secondary_decoded_samples_, timestamps_since_last_report_); + if (waiting_times_.size() == 0) { + stats->mean_waiting_time_ms = -1; + stats->median_waiting_time_ms = -1; + stats->min_waiting_time_ms = -1; + stats->max_waiting_time_ms = -1; + } else { + std::sort(waiting_times_.begin(), waiting_times_.end()); + // Find mid-point elements. If the size is odd, the two values + // |middle_left| and |middle_right| will both be the one middle element; if + // the size is even, they will be the the two neighboring elements at the + // middle of the list. + const int middle_left = waiting_times_[(waiting_times_.size() - 1) / 2]; + const int middle_right = waiting_times_[waiting_times_.size() / 2]; + // Calculate the average of the two. (Works also for odd sizes.) + stats->median_waiting_time_ms = (middle_left + middle_right) / 2; + stats->min_waiting_time_ms = waiting_times_.front(); + stats->max_waiting_time_ms = waiting_times_.back(); + double sum = 0; + for (auto time : waiting_times_) { + sum += time; + } + stats->mean_waiting_time_ms = static_cast(sum / waiting_times_.size()); + } + // Reset counters. ResetMcu(); Reset(); } -void StatisticsCalculator::WaitingTimes(std::vector* waiting_times) { - if (!waiting_times) { - return; - } - waiting_times->assign(waiting_times_, waiting_times_ + len_waiting_times_); - ResetWaitingTimeStatistics(); -} - uint16_t StatisticsCalculator::CalculateQ14Ratio(size_t numerator, uint32_t denominator) { if (numerator == 0) { diff --git a/webrtc/modules/audio_coding/neteq/statistics_calculator.h b/webrtc/modules/audio_coding/neteq/statistics_calculator.h index 3bd3e55d46..5bb66b6697 100644 --- a/webrtc/modules/audio_coding/neteq/statistics_calculator.h +++ b/webrtc/modules/audio_coding/neteq/statistics_calculator.h @@ -11,8 +11,8 @@ #ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_STATISTICS_CALCULATOR_H_ #define WEBRTC_MODULES_AUDIO_CODING_NETEQ_STATISTICS_CALCULATOR_H_ +#include #include -#include #include "webrtc/base/constructormagic.h" #include "webrtc/modules/audio_coding/neteq/interface/neteq.h" @@ -29,7 +29,7 @@ class StatisticsCalculator { public: StatisticsCalculator(); - virtual ~StatisticsCalculator() {} + virtual ~StatisticsCalculator(); // Resets most of the counters. void Reset(); @@ -37,9 +37,6 @@ class StatisticsCalculator { // Resets the counters that are not handled by Reset(). void ResetMcu(); - // Resets the waiting time statistics. - void ResetWaitingTimeStatistics(); - // Reports that |num_samples| samples were produced through expansion, and // that the expansion produced other than just noise samples. void ExpandedVoiceSamples(size_t num_samples); @@ -91,11 +88,9 @@ class StatisticsCalculator { const DecisionLogic& decision_logic, NetEqNetworkStatistics *stats); - void WaitingTimes(std::vector* waiting_times); - private: static const int kMaxReportPeriod = 60; // Seconds before auto-reset. - static const int kLenWaitingTimes = 100; + static const size_t kLenWaitingTimes = 100; class PeriodicUmaLogger { public: @@ -160,9 +155,7 @@ class StatisticsCalculator { size_t discarded_packets_; size_t lost_timestamps_; uint32_t timestamps_since_last_report_; - int waiting_times_[kLenWaitingTimes]; // Used as a circular buffer. - int len_waiting_times_; - int next_waiting_time_index_; + std::deque waiting_times_; uint32_t secondary_decoded_samples_; PeriodicUmaCount delayed_packet_outage_counter_; PeriodicUmaAverage excess_buffer_delay_;