NetEq/Stats: Don't let concealed_samples decrease

When NetEq performs a merge operation, it will usually have to correct
the stats for number of concealment samples produced, sometimes with
decreasing it.

This does not make sense in the context of the stats spec, and
stats-consuming applications may not be prepared for it. With this
change, only positive corrections are allowed for the
concealed_samples value. This will sometimes lead to a small positive
bias, but it will be negligible over time.

Bug: webrtc:8253
Change-Id: Ie9de311ab16401f1a4b435f6269725901b8cf561
Reviewed-on: https://webrtc-review.googlesource.com/1583
Commit-Queue: Henrik Lundin <[email protected]>
Reviewed-by: Gustaf Ullberg <[email protected]>
Cr-Commit-Position: refs/heads/master@{#19941}

modules/audio_coding/BUILD.gn

@@ -2141,6 +2141,7 @@ if (rtc_include_tests) {
       "neteq/post_decode_vad_unittest.cc",
       "neteq/random_vector_unittest.cc",
       "neteq/red_payload_splitter_unittest.cc",
+      "neteq/statistics_calculator_unittest.cc",
       "neteq/sync_buffer_unittest.cc",
       "neteq/tick_timer_unittest.cc",
       "neteq/time_stretch_unittest.cc",

modules/audio_coding/neteq/neteq_unittest.cc

@@ -368,6 +368,8 @@ void NetEqDecodingTest::DecodeAndCompare(
 
   packet_ = rtp_source_->NextPacket();
   int i = 0;
+  uint64_t last_concealed_samples = 0;
+  uint64_t last_total_samples_received = 0;
   while (packet_) {
     std::ostringstream ss;
     ss << "Lap number " << i++ << " in DecodeAndCompare while loop";
@@ -387,6 +389,20 @@ void NetEqDecodingTest::DecodeAndCompare(
       EXPECT_EQ(current_network_stats.current_buffer_size_ms,
                 neteq_->CurrentDelayMs());
 
+      // Verify that liftime stats and network stats report similar loss
+      // concealment rates.
+      auto lifetime_stats = neteq_->GetLifetimeStatistics();
+      const uint64_t delta_concealed_samples =
+          lifetime_stats.concealed_samples - last_concealed_samples;
+      last_concealed_samples = lifetime_stats.concealed_samples;
+      const uint64_t delta_total_samples_received =
+          lifetime_stats.total_samples_received - last_total_samples_received;
+      last_total_samples_received = lifetime_stats.total_samples_received;
+      // The tolerance is 1% but expressed in Q14.
+      EXPECT_NEAR(
+          (delta_concealed_samples << 14) / delta_total_samples_received,
+          current_network_stats.expand_rate, (2 << 14) / 100.0);
+
       // Process RTCPstat.
       RtcpStatistics current_rtcp_stats;
       neteq_->GetRtcpStatistics(&current_rtcp_stats);

modules/audio_coding/neteq/statistics_calculator.cc

@@ -154,32 +154,45 @@ void StatisticsCalculator::ResetMcu() {
 void StatisticsCalculator::ExpandedVoiceSamples(size_t num_samples,
                                                 bool is_new_concealment_event) {
   expanded_speech_samples_ += num_samples;
-  lifetime_stats_.concealed_samples += num_samples;
+  ConcealedSamplesCorrection(num_samples);
   lifetime_stats_.concealment_events += is_new_concealment_event;
 }
 
 void StatisticsCalculator::ExpandedNoiseSamples(size_t num_samples,
                                                 bool is_new_concealment_event) {
   expanded_noise_samples_ += num_samples;
-  lifetime_stats_.concealed_samples += num_samples;
+  ConcealedSamplesCorrection(num_samples);
   lifetime_stats_.concealment_events += is_new_concealment_event;
 }
 
 void StatisticsCalculator::ExpandedVoiceSamplesCorrection(int num_samples) {
   expanded_speech_samples_ =
       AddIntToSizeTWithLowerCap(num_samples, expanded_speech_samples_);
-  lifetime_stats_.concealed_samples += num_samples;
+  ConcealedSamplesCorrection(num_samples);
 }
 
 void StatisticsCalculator::ExpandedNoiseSamplesCorrection(int num_samples) {
   expanded_noise_samples_ =
       AddIntToSizeTWithLowerCap(num_samples, expanded_noise_samples_);
-  lifetime_stats_.concealed_samples += num_samples;
+  ConcealedSamplesCorrection(num_samples);
+}
+
+void StatisticsCalculator::ConcealedSamplesCorrection(int num_samples) {
+  if (num_samples < 0) {
+    // Store negative correction to subtract from future positive additions.
+    // See also the function comment in the header file.
+    concealed_samples_correction_ -= num_samples;
+    return;
+  }
+
+  const size_t canceled_out =
+      std::min(static_cast<size_t>(num_samples), concealed_samples_correction_);
+  concealed_samples_correction_ -= canceled_out;
+  lifetime_stats_.concealed_samples += num_samples - canceled_out;
 }
 
 void StatisticsCalculator::PreemptiveExpandedSamples(size_t num_samples) {
   preemptive_samples_ += num_samples;
-  lifetime_stats_.concealed_samples += num_samples;
 }
 
 void StatisticsCalculator::AcceleratedSamples(size_t num_samples) {

modules/audio_coding/neteq/statistics_calculator.h

@@ -159,11 +159,18 @@ class StatisticsCalculator {
     int counter_ = 0;
   };
 
+  // Corrects the concealed samples counter in lifetime_stats_. The value of
+  // num_samples_ is added directly to the stat if the correction is positive.
+  // If the correction is negative, it is cached and will be subtracted against
+  // future additions to the counter. This is meant to be called from
+  // Expanded{Voice,Noise}Samples{Correction}.
+  void ConcealedSamplesCorrection(int num_samples);
+
   // Calculates numerator / denominator, and returns the value in Q14.
   static uint16_t CalculateQ14Ratio(size_t numerator, uint32_t denominator);
 
-  // TODO(steveanton): Add unit tests for the lifetime stats.
   NetEqLifetimeStatistics lifetime_stats_;
+  size_t concealed_samples_correction_ = 0;
   size_t preemptive_samples_;
   size_t accelerate_samples_;
   size_t added_zero_samples_;

modules/audio_coding/neteq/statistics_calculator_unittest.cc

@@ -0,0 +1,66 @@
+/*
+ *  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.
+ */
+
+#include "modules/audio_coding/neteq/statistics_calculator.h"
+
+#include "test/gtest.h"
+
+namespace webrtc {
+
+TEST(LifetimeStatistics, TotalSamplesReceived) {
+  StatisticsCalculator stats;
+  for (int i = 0; i < 10; ++i) {
+    stats.IncreaseCounter(480, 48000);  // 10 ms at 48 kHz.
+  }
+  EXPECT_EQ(10 * 480u, stats.GetLifetimeStatistics().total_samples_received);
+}
+
+TEST(LifetimeStatistics, SamplesConcealed) {
+  StatisticsCalculator stats;
+  stats.ExpandedVoiceSamples(100, false);
+  stats.ExpandedNoiseSamples(17, false);
+  EXPECT_EQ(100u + 17u, stats.GetLifetimeStatistics().concealed_samples);
+}
+
+// This test verifies that a negative correction of concealed_samples does not
+// result in a decrease in the stats value (because stats-consuming applications
+// would not expect the value to decrease). Instead, the correction should be
+// made to future increments to the stat.
+TEST(LifetimeStatistics, SamplesConcealedCorrection) {
+  StatisticsCalculator stats;
+  stats.ExpandedVoiceSamples(100, false);
+  EXPECT_EQ(100u, stats.GetLifetimeStatistics().concealed_samples);
+  stats.ExpandedVoiceSamplesCorrection(-10);
+  // Do not subtract directly, but keep the correction for later.
+  EXPECT_EQ(100u, stats.GetLifetimeStatistics().concealed_samples);
+  stats.ExpandedVoiceSamplesCorrection(20);
+  // The total correction is 20 - 10.
+  EXPECT_EQ(110u, stats.GetLifetimeStatistics().concealed_samples);
+
+  // Also test correction done to the next ExpandedVoiceSamples call.
+  stats.ExpandedVoiceSamplesCorrection(-17);
+  EXPECT_EQ(110u, stats.GetLifetimeStatistics().concealed_samples);
+  stats.ExpandedVoiceSamples(100, false);
+  EXPECT_EQ(110u + 100u - 17u, stats.GetLifetimeStatistics().concealed_samples);
+}
+
+// This test verifies that neither "accelerate" nor "pre-emptive expand" reults
+// in a modification to concealed_samples stats. Only PLC operations (i.e.,
+// "expand" and "merge") should affect the stat.
+TEST(LifetimeStatistics, NoUpdateOnTimeStretch) {
+  StatisticsCalculator stats;
+  stats.ExpandedVoiceSamples(100, false);
+  stats.AcceleratedSamples(4711);
+  stats.PreemptiveExpandedSamples(17);
+  stats.ExpandedVoiceSamples(100, false);
+  EXPECT_EQ(200u, stats.GetLifetimeStatistics().concealed_samples);
+}
+
+}  // namespace webrtc