Implement packet discard rate in NetEq.
BUG=webrtc:7903
Change-Id: I819c9362671ca0b02c602d53e4dc39afdd8ec465
Reviewed-on: https://chromium-review.googlesource.com/555311
Commit-Queue: Minyue Li <minyue@webrtc.org>
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#18899}diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h b/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h
index 5a2f7e1..7967ab9 100644
--- a/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h
+++ b/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h
@@ -48,12 +48,13 @@
const Packet*());
MOCK_METHOD0(GetNextPacket,
rtc::Optional<Packet>());
- MOCK_METHOD0(DiscardNextPacket,
- int());
- MOCK_METHOD2(DiscardOldPackets,
- int(uint32_t timestamp_limit, uint32_t horizon_samples));
- MOCK_METHOD1(DiscardAllOldPackets,
- int(uint32_t timestamp_limit));
+ MOCK_METHOD1(DiscardNextPacket, int(StatisticsCalculator* stats));
+ MOCK_METHOD3(DiscardOldPackets,
+ void(uint32_t timestamp_limit,
+ uint32_t horizon_samples,
+ StatisticsCalculator* stats));
+ MOCK_METHOD2(DiscardAllOldPackets,
+ void(uint32_t timestamp_limit, StatisticsCalculator* stats));
MOCK_CONST_METHOD0(NumPacketsInBuffer,
size_t());
MOCK_METHOD1(IncrementWaitingTimes,
diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_statistics_calculator.h b/webrtc/modules/audio_coding/neteq/mock/mock_statistics_calculator.h
new file mode 100644
index 0000000..da4e3ed
--- /dev/null
+++ b/webrtc/modules/audio_coding/neteq/mock/mock_statistics_calculator.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_MOCK_MOCK_STATISTICS_CALCULATOR_H_
+#define WEBRTC_MODULES_AUDIO_CODING_NETEQ_MOCK_MOCK_STATISTICS_CALCULATOR_H_
+
+#include "webrtc/modules/audio_coding/neteq/statistics_calculator.h"
+
+#include "webrtc/test/gmock.h"
+
+namespace webrtc {
+
+class MockStatisticsCalculator : public StatisticsCalculator {
+ public:
+ // For current unittest, we mock only one method.
+ MOCK_METHOD1(PacketsDiscarded, void(size_t num_packets));
+};
+
+} // namespace webrtc
+#endif // WEBRTC_MODULES_AUDIO_CODING_NETEQ_MOCK_MOCK_STATISTICS_CALCULATOR_H_
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
index fb3cc11..bf4e4eb 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
@@ -217,7 +217,7 @@
const std::vector<int> changed_payload_types =
decoder_database_->SetCodecs(codecs);
for (const int pt : changed_payload_types) {
- packet_buffer_->DiscardPacketsWithPayloadType(pt);
+ packet_buffer_->DiscardPacketsWithPayloadType(pt, &stats_);
}
}
@@ -268,7 +268,7 @@
rtc::CritScope lock(&crit_sect_);
int ret = decoder_database_->Remove(rtp_payload_type);
if (ret == DecoderDatabase::kOK || ret == DecoderDatabase::kDecoderNotFound) {
- packet_buffer_->DiscardPacketsWithPayloadType(rtp_payload_type);
+ packet_buffer_->DiscardPacketsWithPayloadType(rtp_payload_type, &stats_);
return kOK;
}
return kFail;
@@ -1063,7 +1063,8 @@
uint32_t end_timestamp = sync_buffer_->end_timestamp();
if (!new_codec_) {
const uint32_t five_seconds_samples = 5 * fs_hz_;
- packet_buffer_->DiscardOldPackets(end_timestamp, five_seconds_samples);
+ packet_buffer_->DiscardOldPackets(end_timestamp, five_seconds_samples,
+ &stats_);
}
const Packet* packet = packet_buffer_->PeekNextPacket();
@@ -1084,12 +1085,12 @@
(end_timestamp >= packet->timestamp ||
end_timestamp + generated_noise_samples > packet->timestamp)) {
// Don't use this packet, discard it.
- if (packet_buffer_->DiscardNextPacket() != PacketBuffer::kOK) {
+ if (packet_buffer_->DiscardNextPacket(&stats_) != PacketBuffer::kOK) {
assert(false); // Must be ok by design.
}
// Check buffer again.
if (!new_codec_) {
- packet_buffer_->DiscardOldPackets(end_timestamp, 5 * fs_hz_);
+ packet_buffer_->DiscardOldPackets(end_timestamp, 5 * fs_hz_, &stats_);
}
packet = packet_buffer_->PeekNextPacket();
}
@@ -2003,7 +2004,7 @@
// we could end up in the situation where we never decode anything, since
// all incoming packets are considered too old but the buffer will also
// never be flooded and flushed.
- packet_buffer_->DiscardAllOldPackets(timestamp_);
+ packet_buffer_->DiscardAllOldPackets(timestamp_, &stats_);
}
return rtc::dchecked_cast<int>(extracted_samples);
diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
index eed7740..5049a6b 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
@@ -482,10 +482,10 @@
"6237dd113ad80d7764fe4c90b55b2ec035eae64e");
const std::string network_stats_checksum =
- PlatformChecksum("0869a450a819b14bf2a91eb6f3629a3421d17606",
- "0869a450a819b14bf2a91eb6f3629a3421d17606",
- "0869a450a819b14bf2a91eb6f3629a3421d17606",
- "0869a450a819b14bf2a91eb6f3629a3421d17606");
+ PlatformChecksum("7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
+ "7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
+ "7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
+ "7a29daca4dfa4fb6eaec06d7e63c1729da7708f6");
const std::string rtcp_stats_checksum = PlatformChecksum(
"e37c797e3de6a64dda88c9ade7a013d022a2e1e0",
diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.cc b/webrtc/modules/audio_coding/neteq/packet_buffer.cc
index 78a71c7..cea6b3d 100644
--- a/webrtc/modules/audio_coding/neteq/packet_buffer.cc
+++ b/webrtc/modules/audio_coding/neteq/packet_buffer.cc
@@ -19,6 +19,7 @@
#include "webrtc/api/audio_codecs/audio_decoder.h"
#include "webrtc/base/logging.h"
#include "webrtc/modules/audio_coding/neteq/decoder_database.h"
+#include "webrtc/modules/audio_coding/neteq/statistics_calculator.h"
#include "webrtc/modules/audio_coding/neteq/tick_timer.h"
namespace webrtc {
@@ -206,41 +207,51 @@
return packet;
}
-int PacketBuffer::DiscardNextPacket() {
+int PacketBuffer::DiscardNextPacket(StatisticsCalculator* stats) {
if (Empty()) {
return kBufferEmpty;
}
// Assert that the packet sanity checks in InsertPacket method works.
RTC_DCHECK(!buffer_.front().empty());
buffer_.pop_front();
+ stats->PacketsDiscarded(1);
return kOK;
}
-int PacketBuffer::DiscardOldPackets(uint32_t timestamp_limit,
- uint32_t horizon_samples) {
+void PacketBuffer::DiscardOldPackets(uint32_t timestamp_limit,
+ uint32_t horizon_samples,
+ StatisticsCalculator* stats) {
+ // TODO(minyue): the following implementation is wrong. It won't discard
+ // old packets if the buffer_.front() is newer than timestamp_limit -
+ // horizon_samples. https://bugs.chromium.org/p/webrtc/issues/detail?id=7937
while (!Empty() && timestamp_limit != buffer_.front().timestamp &&
IsObsoleteTimestamp(buffer_.front().timestamp, timestamp_limit,
horizon_samples)) {
- if (DiscardNextPacket() != kOK) {
+ if (DiscardNextPacket(stats) != kOK) {
assert(false); // Must be ok by design.
}
}
- return 0;
}
-int PacketBuffer::DiscardAllOldPackets(uint32_t timestamp_limit) {
- return DiscardOldPackets(timestamp_limit, 0);
+void PacketBuffer::DiscardAllOldPackets(uint32_t timestamp_limit,
+ StatisticsCalculator* stats) {
+ DiscardOldPackets(timestamp_limit, 0, stats);
}
-void PacketBuffer::DiscardPacketsWithPayloadType(uint8_t payload_type) {
+void PacketBuffer::DiscardPacketsWithPayloadType(uint8_t payload_type,
+ StatisticsCalculator* stats) {
+ int packets_discarded = 0;
for (auto it = buffer_.begin(); it != buffer_.end(); /* */) {
const Packet& packet = *it;
if (packet.payload_type == payload_type) {
it = buffer_.erase(it);
+ ++packets_discarded;
} else {
++it;
}
}
+ if (packets_discarded > 0)
+ stats->PacketsDiscarded(packets_discarded);
}
size_t PacketBuffer::NumPacketsInBuffer() const {
diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.h b/webrtc/modules/audio_coding/neteq/packet_buffer.h
index a26d6c5..f937322 100644
--- a/webrtc/modules/audio_coding/neteq/packet_buffer.h
+++ b/webrtc/modules/audio_coding/neteq/packet_buffer.h
@@ -20,6 +20,7 @@
namespace webrtc {
class DecoderDatabase;
+class StatisticsCalculator;
class TickTimer;
// This is the actual buffer holding the packets before decoding.
@@ -92,22 +93,24 @@
// Discards the first packet in the buffer. The packet is deleted.
// Returns PacketBuffer::kBufferEmpty if the buffer is empty,
// PacketBuffer::kOK otherwise.
- virtual int DiscardNextPacket();
+ virtual int DiscardNextPacket(StatisticsCalculator* stats);
// Discards all packets that are (strictly) older than timestamp_limit,
// but newer than timestamp_limit - horizon_samples. Setting horizon_samples
// to zero implies that the horizon is set to half the timestamp range. That
// is, if a packet is more than 2^31 timestamps into the future compared with
// timestamp_limit (including wrap-around), it is considered old.
- // Returns number of packets discarded.
- virtual int DiscardOldPackets(uint32_t timestamp_limit,
- uint32_t horizon_samples);
+ virtual void DiscardOldPackets(uint32_t timestamp_limit,
+ uint32_t horizon_samples,
+ StatisticsCalculator* stats);
// Discards all packets that are (strictly) older than timestamp_limit.
- virtual int DiscardAllOldPackets(uint32_t timestamp_limit);
+ virtual void DiscardAllOldPackets(uint32_t timestamp_limit,
+ StatisticsCalculator* stats);
// Removes all packets with a specific payload type from the buffer.
- virtual void DiscardPacketsWithPayloadType(uint8_t payload_type);
+ virtual void DiscardPacketsWithPayloadType(uint8_t payload_type,
+ StatisticsCalculator* stats);
// Returns the number of packets in the buffer, including duplicates and
// redundant packets.
diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc
index e8795a9..82f7246 100644
--- a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc
@@ -10,15 +10,17 @@
// Unit tests for PacketBuffer class.
+#include "webrtc/modules/audio_coding/neteq/packet_buffer.h"
#include "webrtc/api/audio_codecs/builtin_audio_decoder_factory.h"
#include "webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h"
+#include "webrtc/modules/audio_coding/neteq/mock/mock_statistics_calculator.h"
#include "webrtc/modules/audio_coding/neteq/packet.h"
-#include "webrtc/modules/audio_coding/neteq/packet_buffer.h"
#include "webrtc/modules/audio_coding/neteq/tick_timer.h"
#include "webrtc/test/gmock.h"
#include "webrtc/test/gtest.h"
using ::testing::Return;
+using ::testing::StrictMock;
using ::testing::_;
namespace webrtc {
@@ -299,22 +301,42 @@
PacketList list;
const int payload_len = 10;
+ constexpr int kTotalPackets = 10;
// Insert 10 small packets.
- for (int i = 0; i < 10; ++i) {
+ for (int i = 0; i < kTotalPackets; ++i) {
buffer.InsertPacket(gen.NextPacket(payload_len));
}
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
+ StrictMock<MockStatisticsCalculator> mock_stats;
+ uint32_t current_ts = start_ts;
+
// Discard them one by one and make sure that the right packets are at the
// front of the buffer.
- uint32_t current_ts = start_ts;
- for (int i = 0; i < 10; ++i) {
+ constexpr int kDiscardPackets = 5;
+ for (int i = 0; i < kDiscardPackets; ++i) {
uint32_t ts;
EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&ts));
EXPECT_EQ(current_ts, ts);
- EXPECT_EQ(PacketBuffer::kOK, buffer.DiscardNextPacket());
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1)).Times(1);
+ EXPECT_EQ(PacketBuffer::kOK, buffer.DiscardNextPacket(&mock_stats));
current_ts += ts_increment;
}
+
+ constexpr int kRemainingPackets = kTotalPackets - kDiscardPackets;
+ // This will not discard any packets because the oldest packet is newer than
+ // the indicated horizon_samples.
+ buffer.DiscardOldPackets(start_ts + kTotalPackets * ts_increment,
+ kRemainingPackets * ts_increment, &mock_stats);
+ uint32_t ts;
+ EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&ts));
+ EXPECT_EQ(current_ts, ts);
+
+ // Discard all remaining packets.
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1)).Times(kRemainingPackets);
+ buffer.DiscardAllOldPackets(start_ts + kTotalPackets * ts_increment,
+ &mock_stats);
+
EXPECT_TRUE(buffer.Empty());
}
@@ -452,8 +474,12 @@
buffer->NextHigherTimestamp(0, &temp_ts));
EXPECT_EQ(NULL, buffer->PeekNextPacket());
EXPECT_FALSE(buffer->GetNextPacket());
- EXPECT_EQ(PacketBuffer::kBufferEmpty, buffer->DiscardNextPacket());
- EXPECT_EQ(0, buffer->DiscardAllOldPackets(0)); // 0 packets discarded.
+
+ StrictMock<MockStatisticsCalculator> mock_stats;
+ // Discarding packets will not invoke mock_stats.PacketDiscarded() because the
+ // packet buffer is empty.
+ EXPECT_EQ(PacketBuffer::kBufferEmpty, buffer->DiscardNextPacket(&mock_stats));
+ buffer->DiscardAllOldPackets(0, &mock_stats);
// Insert one packet to make the buffer non-empty.
EXPECT_EQ(PacketBuffer::kOK,
diff --git a/webrtc/modules/audio_coding/neteq/statistics_calculator.h b/webrtc/modules/audio_coding/neteq/statistics_calculator.h
index fb869ab..320a74a 100644
--- a/webrtc/modules/audio_coding/neteq/statistics_calculator.h
+++ b/webrtc/modules/audio_coding/neteq/statistics_calculator.h
@@ -64,7 +64,7 @@
void AddZeros(size_t num_samples);
// Reports that |num_packets| packets were discarded.
- void PacketsDiscarded(size_t num_packets);
+ virtual void PacketsDiscarded(size_t num_packets);
// Reports that |num_samples| were lost.
void LostSamples(size_t num_samples);