From ac0a503828408d2a318355cf255bcab311bd5648 Mon Sep 17 00:00:00 2001 From: Henrik Lundin Date: Mon, 25 Sep 2017 12:22:46 +0200 Subject: [PATCH] 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 Reviewed-by: Gustaf Ullberg Cr-Commit-Position: refs/heads/master@{#19941} --- modules/audio_coding/BUILD.gn | 1 + modules/audio_coding/neteq/neteq_unittest.cc | 16 +++++ .../neteq/statistics_calculator.cc | 23 +++++-- .../neteq/statistics_calculator.h | 9 ++- .../neteq/statistics_calculator_unittest.cc | 66 +++++++++++++++++++ 5 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 modules/audio_coding/neteq/statistics_calculator_unittest.cc diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn index 84f369f86d..0c4ee624cf 100644 --- a/modules/audio_coding/BUILD.gn +++ b/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", diff --git a/modules/audio_coding/neteq/neteq_unittest.cc b/modules/audio_coding/neteq/neteq_unittest.cc index 764a505611..5b9221793f 100644 --- a/modules/audio_coding/neteq/neteq_unittest.cc +++ b/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(¤t_rtcp_stats); diff --git a/modules/audio_coding/neteq/statistics_calculator.cc b/modules/audio_coding/neteq/statistics_calculator.cc index bbd6e249f2..09ced6a752 100644 --- a/modules/audio_coding/neteq/statistics_calculator.cc +++ b/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(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) { diff --git a/modules/audio_coding/neteq/statistics_calculator.h b/modules/audio_coding/neteq/statistics_calculator.h index cec02bbf2d..232ca9acd0 100644 --- a/modules/audio_coding/neteq/statistics_calculator.h +++ b/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_; diff --git a/modules/audio_coding/neteq/statistics_calculator_unittest.cc b/modules/audio_coding/neteq/statistics_calculator_unittest.cc new file mode 100644 index 0000000000..0cc868aeb3 --- /dev/null +++ b/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