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}

webrtc/modules/audio_coding/BUILD.gn

@@ -2114,6 +2114,7 @@ if (rtc_include_tests) {
       "neteq/mock/mock_external_decoder_pcm16b.h",
       "neteq/mock/mock_packet_buffer.h",
       "neteq/mock/mock_red_payload_splitter.h",
+      "neteq/mock/mock_statistics_calculator.h",
       "neteq/nack_tracker_unittest.cc",
       "neteq/neteq_external_decoder_unittest.cc",
       "neteq/neteq_impl_unittest.cc",

webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h

@@ -48,12 +48,13 @@ class MockPacketBuffer : public PacketBuffer {
       const Packet*());
   MOCK_METHOD0(GetNextPacket,
       rtc::Optional<Packet>());
-  MOCK_METHOD0(DiscardNextPacket,
+  MOCK_METHOD1(DiscardNextPacket, int(StatisticsCalculator* stats));
-      int());
+  MOCK_METHOD3(DiscardOldPackets,
-  MOCK_METHOD2(DiscardOldPackets,
+               void(uint32_t timestamp_limit,
-      int(uint32_t timestamp_limit, uint32_t horizon_samples));
+                    uint32_t horizon_samples,
-  MOCK_METHOD1(DiscardAllOldPackets,
+                    StatisticsCalculator* stats));
-      int(uint32_t timestamp_limit));
+  MOCK_METHOD2(DiscardAllOldPackets,
+               void(uint32_t timestamp_limit, StatisticsCalculator* stats));
   MOCK_CONST_METHOD0(NumPacketsInBuffer,
       size_t());
   MOCK_METHOD1(IncrementWaitingTimes,

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_

webrtc/modules/audio_coding/neteq/neteq_impl.cc

@@ -217,7 +217,7 @@ void NetEqImpl::SetCodecs(const std::map<int, SdpAudioFormat>& codecs) {
   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 @@ int NetEqImpl::RemovePayloadType(uint8_t rtp_payload_type) {
   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 @@ int NetEqImpl::GetDecision(Operations* operation,
   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 @@ int NetEqImpl::GetDecision(Operations* operation,
            (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 @@ int NetEqImpl::ExtractPackets(size_t required_samples,
     // 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);

webrtc/modules/audio_coding/neteq/neteq_unittest.cc

@@ -482,10 +482,10 @@ TEST_F(NetEqDecodingTest, MAYBE_TestOpusBitExactness) {
       "6237dd113ad80d7764fe4c90b55b2ec035eae64e");
 
   const std::string network_stats_checksum =
-      PlatformChecksum("0869a450a819b14bf2a91eb6f3629a3421d17606",
+      PlatformChecksum("7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
-                       "0869a450a819b14bf2a91eb6f3629a3421d17606",
+                       "7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
-                       "0869a450a819b14bf2a91eb6f3629a3421d17606",
+                       "7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
-                       "0869a450a819b14bf2a91eb6f3629a3421d17606");
+                       "7a29daca4dfa4fb6eaec06d7e63c1729da7708f6");
 
   const std::string rtcp_stats_checksum = PlatformChecksum(
       "e37c797e3de6a64dda88c9ade7a013d022a2e1e0",

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 @@ rtc::Optional<Packet> PacketBuffer::GetNextPacket() {
   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,
+void PacketBuffer::DiscardOldPackets(uint32_t timestamp_limit,
-                                    uint32_t horizon_samples) {
+                                     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) {
+void PacketBuffer::DiscardAllOldPackets(uint32_t timestamp_limit,
-  return DiscardOldPackets(timestamp_limit, 0);
+                                        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 {

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 @@ class PacketBuffer {
   // 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 void DiscardOldPackets(uint32_t timestamp_limit,
-  virtual int DiscardOldPackets(uint32_t timestamp_limit,
+                                 uint32_t horizon_samples,
-                                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.

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 @@ TEST(PacketBuffer, DiscardPackets) {
   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;
+  constexpr int kDiscardPackets = 5;
-  for (int i = 0; i < 10; ++i) {
+  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 @@ TEST(PacketBuffer, Failures) {
             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,

webrtc/modules/audio_coding/neteq/statistics_calculator.h

@@ -64,7 +64,7 @@ class StatisticsCalculator {
   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);