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}
diff --git a/webrtc/modules/audio_coding/neteq/interface/neteq.h b/webrtc/modules/audio_coding/neteq/interface/neteq.h
index 865a8b3..e2ea00d 100644
--- a/webrtc/modules/audio_coding/neteq/interface/neteq.h
+++ b/webrtc/modules/audio_coding/neteq/interface/neteq.h
@@ -14,7 +14,6 @@
#include <string.h> // Provide access to size_t.
#include <string>
-#include <vector>
#include "webrtc/base/constructormagic.h"
#include "webrtc/common_types.h"
@@ -46,6 +45,12 @@
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 @@
// 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<int>* 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 d890acb..22e71f7 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
@@ -318,11 +318,6 @@
return 0;
}
-void NetEqImpl::WaitingTimes(std::vector<int>* 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 502204a..182d8b8 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 <vector>
-
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/thread_annotations.h"
@@ -154,11 +152,6 @@
// 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<int>* 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 03fde53..0c43024 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
@@ -507,46 +507,23 @@
ASSERT_EQ(kBlockSize16kHz, out_len);
}
- std::vector<int> 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<int>(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<uint8_t*>(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 c716fe4..5a4ef54 100644
--- a/webrtc/modules/audio_coding/neteq/statistics_calculator.cc
+++ b/webrtc/modules/audio_coding/neteq/statistics_calculator.cc
@@ -12,6 +12,7 @@
#include <assert.h>
#include <string.h> // memset
+#include <algorithm>
#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 @@
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 @@
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 @@
expanded_speech_samples_ = 0;
expanded_noise_samples_ = 0;
secondary_decoded_samples_ = 0;
+ waiting_times_.clear();
}
void StatisticsCalculator::ResetMcu() {
@@ -135,12 +139,6 @@
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::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;
+ DCHECK_LE(waiting_times_.size(), kLenWaitingTimes);
+ while (waiting_times_.size() >= kLenWaitingTimes) {
+ // Erase first value.
+ waiting_times_.pop_front();
}
- if (len_waiting_times_ < kLenWaitingTimes) {
- len_waiting_times_++;
- }
+ waiting_times_.push_back(waiting_time_ms);
}
void StatisticsCalculator::GetNetworkStatistics(
@@ -254,19 +249,35 @@
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<int>(sum / waiting_times_.size());
+ }
+
// Reset counters.
ResetMcu();
Reset();
}
-void StatisticsCalculator::WaitingTimes(std::vector<int>* 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 3bd3e55..5bb66b66 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 <deque>
#include <string>
-#include <vector>
#include "webrtc/base/constructormagic.h"
#include "webrtc/modules/audio_coding/neteq/interface/neteq.h"
@@ -29,7 +29,7 @@
public:
StatisticsCalculator();
- virtual ~StatisticsCalculator() {}
+ virtual ~StatisticsCalculator();
// Resets most of the counters.
void Reset();
@@ -37,9 +37,6 @@
// 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 @@
const DecisionLogic& decision_logic,
NetEqNetworkStatistics *stats);
- void WaitingTimes(std::vector<int>* 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 @@
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<int> waiting_times_;
uint32_t secondary_decoded_samples_;
PeriodicUmaCount delayed_packet_outage_counter_;
PeriodicUmaAverage excess_buffer_delay_;