From 65024d9620ada4c28802e9b3565597b2a3dd0abd Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Fri, 30 Aug 2019 15:37:07 +0200 Subject: [PATCH] Remove clock drift metric from NetEq. This metric is not used anywhere and is not calculated correctly when the delay manager is in relative arrival delay mode. Bug: webrtc:10333 Change-Id: Iac79ab40b79b17802ad9d626c130e82f761bae26 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150786 Reviewed-by: Henrik Lundin Reviewed-by: Oskar Sundbom Commit-Queue: Jakob Ivarsson Cr-Commit-Position: refs/heads/master@{#29037} --- audio/audio_receive_stream_unittest.cc | 2 +- modules/audio_coding/acm2/acm_receiver.cc | 1 - .../include/audio_coding_module_typedefs.h | 2 - modules/audio_coding/neteq/delay_manager.cc | 15 ---- modules/audio_coding/neteq/delay_manager.h | 8 -- modules/audio_coding/neteq/include/neteq.h | 2 - .../neteq/neteq_network_stats_unittest.cc | 8 +- modules/audio_coding/neteq/neteq_unittest.cc | 77 +++---------------- .../neteq/statistics_calculator.cc | 2 - .../neteq/tools/neteq_stats_getter.cc | 2 - 10 files changed, 13 insertions(+), 106 deletions(-) diff --git a/audio/audio_receive_stream_unittest.cc b/audio/audio_receive_stream_unittest.cc index d6c6afe022..a14e8e142e 100644 --- a/audio/audio_receive_stream_unittest.cc +++ b/audio/audio_receive_stream_unittest.cc @@ -70,7 +70,7 @@ const std::pair kReceiveCodec = { const NetworkStatistics kNetworkStats = { 123, 456, false, 789012, 3456, 123, 456, 789, 543, 432, 321, 123, 101, 0, {}, 789, 12, 345, 678, 901, - 0, -1, -1, -1, -1, -1, 0, 0, 0, 0}; + 0, -1, -1, -1, -1, 0, 0, 0, 0}; const AudioDecodingCallStats kAudioDecodeStats = MakeAudioDecodeStatsForTest(); struct ConfigHelper { diff --git a/modules/audio_coding/acm2/acm_receiver.cc b/modules/audio_coding/acm2/acm_receiver.cc index 6e7e631ef5..ad98fee05d 100644 --- a/modules/audio_coding/acm2/acm_receiver.cc +++ b/modules/audio_coding/acm2/acm_receiver.cc @@ -243,7 +243,6 @@ void AcmReceiver::GetNetworkStatistics(NetworkStatistics* acm_stat) const { acm_stat->currentAccelerateRate = neteq_stat.accelerate_rate; acm_stat->currentSecondaryDecodedRate = neteq_stat.secondary_decoded_rate; acm_stat->currentSecondaryDiscardedRate = neteq_stat.secondary_discarded_rate; - acm_stat->clockDriftPPM = neteq_stat.clockdrift_ppm; acm_stat->addedSamples = neteq_stat.added_zero_samples; acm_stat->meanWaitingTimeMs = neteq_stat.mean_waiting_time_ms; acm_stat->medianWaitingTimeMs = neteq_stat.median_waiting_time_ms; diff --git a/modules/audio_coding/include/audio_coding_module_typedefs.h b/modules/audio_coding/include/audio_coding_module_typedefs.h index 95314a3766..2f40acde07 100644 --- a/modules/audio_coding/include/audio_coding_module_typedefs.h +++ b/modules/audio_coding/include/audio_coding_module_typedefs.h @@ -119,8 +119,6 @@ struct NetworkStatistics { // primary data, obsoleting the secondary data. It can also be caused by early // or late arrival of secondary data. uint16_t currentSecondaryDiscardedRate; - // clock-drift in parts-per-million (negative or positive) - int32_t clockDriftPPM; // average packet waiting time in the jitter buffer (ms) int meanWaitingTimeMs; // median packet waiting time in the jitter buffer (ms) diff --git a/modules/audio_coding/neteq/delay_manager.cc b/modules/audio_coding/neteq/delay_manager.cc index 3d6e5a315b..5d6dc0066b 100644 --- a/modules/audio_coding/neteq/delay_manager.cc +++ b/modules/audio_coding/neteq/delay_manager.cc @@ -447,21 +447,6 @@ void DelayManager::Reset() { last_pack_cng_or_dtmf_ = 1; } -double DelayManager::EstimatedClockDriftPpm() const { - double sum = 0.0; - // Calculate the expected value based on the probabilities in - // |histogram_|. - auto buckets = histogram_->buckets(); - for (size_t i = 0; i < buckets.size(); ++i) { - sum += static_cast(buckets[i]) * i; - } - // The probabilities in |histogram_| are in Q30. Divide by 1 << 30 to - // convert to Q0; subtract the nominal inter-arrival time (1) to make a zero - // clockdrift represent as 0; mulitply by 1000000 to produce parts-per-million - // (ppm). - return (sum / (1 << 30) - 1) * 1e6; -} - bool DelayManager::PeakFound() const { return peak_detector_.peak_found(); } diff --git a/modules/audio_coding/neteq/delay_manager.h b/modules/audio_coding/neteq/delay_manager.h index 44d27f88e6..642de4db78 100644 --- a/modules/audio_coding/neteq/delay_manager.h +++ b/modules/audio_coding/neteq/delay_manager.h @@ -81,14 +81,6 @@ class DelayManager { // Resets the DelayManager and the associated DelayPeakDetector. virtual void Reset(); - // Calculates the average inter-arrival time deviation from the histogram. - // The result is returned as parts-per-million deviation from the nominal - // inter-arrival time. That is, if the average inter-arrival time is equal to - // the nominal frame time, the return value is zero. A positive value - // corresponds to packet spacing being too large, while a negative value means - // that the packets arrive with less spacing than expected. - virtual double EstimatedClockDriftPpm() const; - // Returns true if peak-mode is active. That is, delay peaks were observed // recently. This method simply asks for the same information from the // DelayPeakDetector object. diff --git a/modules/audio_coding/neteq/include/neteq.h b/modules/audio_coding/neteq/include/neteq.h index ef144e69f0..0224b37765 100644 --- a/modules/audio_coding/neteq/include/neteq.h +++ b/modules/audio_coding/neteq/include/neteq.h @@ -51,8 +51,6 @@ struct NetEqNetworkStatistics { // decoding (in Q14). uint16_t secondary_discarded_rate; // Fraction of discarded FEC/RED data (in // Q14). - 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. diff --git a/modules/audio_coding/neteq/neteq_network_stats_unittest.cc b/modules/audio_coding/neteq/neteq_network_stats_unittest.cc index 20e5a5ada5..709b14399a 100644 --- a/modules/audio_coding/neteq/neteq_network_stats_unittest.cc +++ b/modules/audio_coding/neteq/neteq_network_stats_unittest.cc @@ -147,7 +147,6 @@ class NetEqNetworkStatsTest { logic accelerate_rate; logic secondary_decoded_rate; logic secondary_discarded_rate; - logic clockdrift_ppm; logic added_zero_samples; NetEqNetworkStatistics stats_ref; }; @@ -216,7 +215,6 @@ class NetEqNetworkStatsTest { CHECK_NETEQ_NETWORK_STATS(accelerate_rate); CHECK_NETEQ_NETWORK_STATS(secondary_decoded_rate); CHECK_NETEQ_NETWORK_STATS(secondary_discarded_rate); - CHECK_NETEQ_NETWORK_STATS(clockdrift_ppm); CHECK_NETEQ_NETWORK_STATS(added_zero_samples); #undef CHECK_NETEQ_NETWORK_STATS @@ -266,9 +264,8 @@ class NetEqNetworkStatsTest { kEqual, // accelerate_rate kEqual, // decoded_fec_rate kEqual, // discarded_fec_rate - kIgnore, // clockdrift_ppm kEqual, // added_zero_samples - {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; RunTest(50, expects); // Next we introduce packet losses. @@ -298,9 +295,8 @@ class NetEqNetworkStatsTest { kEqual, // accelerate_rate kEqual, // decoded_fec_rate kEqual, // discard_fec_rate - kIgnore, // clockdrift_ppm kEqual, // added_zero_samples - {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; RunTest(50, expects); SetPacketLossRate(1); diff --git a/modules/audio_coding/neteq/neteq_unittest.cc b/modules/audio_coding/neteq/neteq_unittest.cc index 479fe7e34e..0db6fc7b73 100644 --- a/modules/audio_coding/neteq/neteq_unittest.cc +++ b/modules/audio_coding/neteq/neteq_unittest.cc @@ -93,7 +93,6 @@ void Convert(const webrtc::NetEqNetworkStatistics& stats_raw, stats->set_accelerate_rate(stats_raw.accelerate_rate); stats->set_secondary_decoded_rate(stats_raw.secondary_decoded_rate); stats->set_secondary_discarded_rate(stats_raw.secondary_discarded_rate); - stats->set_clockdrift_ppm(stats_raw.clockdrift_ppm); stats->set_added_zero_samples(stats_raw.added_zero_samples); stats->set_mean_waiting_time_ms(stats_raw.mean_waiting_time_ms); stats->set_median_waiting_time_ms(stats_raw.median_waiting_time_ms); @@ -463,10 +462,10 @@ TEST_F(NetEqDecodingTest, MAYBE_TestBitExactness) { "4116ac2a6e75baac3194b712d6fabe28b384275e"); const std::string network_stats_checksum = - PlatformChecksum("3689c9f0ab9e50cefab3e44c37c3d7aa0de82ca4", - "0a596217fccd8d90eff7d1666b8cc63143eeda12", "not used", - "3689c9f0ab9e50cefab3e44c37c3d7aa0de82ca4", - "3689c9f0ab9e50cefab3e44c37c3d7aa0de82ca4"); + PlatformChecksum("5e5230b2d5042eccd197dac29edade1cc233586c", + "2183564f11b53259ba7f86f48f4df3d7d653c678", "not used", + "5e5230b2d5042eccd197dac29edade1cc233586c", + "5e5230b2d5042eccd197dac29edade1cc233586c"); DecodeAndCompare(input_rtp_file, output_checksum, network_stats_checksum, absl::GetFlag(FLAGS_gen_ref)); @@ -497,12 +496,12 @@ TEST_F(NetEqDecodingTest, MAYBE_TestOpusBitExactness) { // The neon implementation may differ. const std::string stats_maybe_neon = "a71dce66c7bea85ba22d4e29a5298f606f810444|" - "889cae8977da9ad7563864726f4eeb5ae66ac7da"; + "6b8c29e39c82f5479f59726744d0cf3e88e725d3"; const std::string network_stats_checksum = PlatformChecksum( - "0b3d34baffaf651812ffaf06ea1b5ce45ea1c47a", stats_maybe_neon, - "7c64e1e915bace7c4bf583484efd64eaf234552f", - "0b3d34baffaf651812ffaf06ea1b5ce45ea1c47a", - "0b3d34baffaf651812ffaf06ea1b5ce45ea1c47a"); + "87d2d3e5ca7f1b3fb7a501ffaa51ae29aea74544", stats_maybe_neon, + "c876f2a04c4f0a91da7f084f80e87871b7c5a4a1", + "87d2d3e5ca7f1b3fb7a501ffaa51ae29aea74544", + "87d2d3e5ca7f1b3fb7a501ffaa51ae29aea74544"); DecodeAndCompare(input_rtp_file, output_checksum, network_stats_checksum, absl::GetFlag(FLAGS_gen_ref)); @@ -526,7 +525,7 @@ TEST_F(NetEqDecodingTest, MAYBE_TestOpusDtxBitExactness) { "da9f9a2d94e0c2d67342fad4965d7b91cda50b25", maybe_sse, maybe_sse); const std::string network_stats_checksum = - "bab58dc587d956f326056d7340c96eb9d2d3cc21"; + "8caf49765f35b6862066d3f17531ce44d8e25f60"; DecodeAndCompare(input_rtp_file, output_checksum, network_stats_checksum, absl::GetFlag(FLAGS_gen_ref)); @@ -583,62 +582,6 @@ TEST_F(NetEqDecodingTestFaxMode, TestFrameWaitingTimeStatistics) { EXPECT_EQ(-1, stats.max_waiting_time_ms); } -TEST_F(NetEqDecodingTest, TestAverageInterArrivalTimeNegative) { - const int kNumFrames = 3000; // Needed for convergence. - int frame_index = 0; - const size_t kSamples = 10 * 16; - const size_t kPayloadBytes = kSamples * 2; - while (frame_index < kNumFrames) { - // Insert one packet each time, except every 10th time where we insert two - // packets at once. This will create a negative clock-drift of approx. 10%. - int num_packets = (frame_index % 10 == 0 ? 2 : 1); - for (int n = 0; n < num_packets; ++n) { - uint8_t payload[kPayloadBytes] = {0}; - RTPHeader rtp_info; - PopulateRtpInfo(frame_index, frame_index * kSamples, &rtp_info); - ASSERT_EQ(0, neteq_->InsertPacket(rtp_info, payload, 0)); - ++frame_index; - } - - // Pull out data once. - bool muted; - ASSERT_EQ(0, neteq_->GetAudio(&out_frame_, &muted)); - ASSERT_EQ(kBlockSize16kHz, out_frame_.samples_per_channel_); - } - - NetEqNetworkStatistics network_stats; - ASSERT_EQ(0, neteq_->NetworkStatistics(&network_stats)); - EXPECT_EQ(-103192, network_stats.clockdrift_ppm); -} - -TEST_F(NetEqDecodingTest, TestAverageInterArrivalTimePositive) { - const int kNumFrames = 5000; // Needed for convergence. - int frame_index = 0; - const size_t kSamples = 10 * 16; - const size_t kPayloadBytes = kSamples * 2; - for (int i = 0; i < kNumFrames; ++i) { - // Insert one packet each time, except every 10th time where we don't insert - // any packet. This will create a positive clock-drift of approx. 11%. - int num_packets = (i % 10 == 9 ? 0 : 1); - for (int n = 0; n < num_packets; ++n) { - uint8_t payload[kPayloadBytes] = {0}; - RTPHeader rtp_info; - PopulateRtpInfo(frame_index, frame_index * kSamples, &rtp_info); - ASSERT_EQ(0, neteq_->InsertPacket(rtp_info, payload, 0)); - ++frame_index; - } - - // Pull out data once. - bool muted; - ASSERT_EQ(0, neteq_->GetAudio(&out_frame_, &muted)); - ASSERT_EQ(kBlockSize16kHz, out_frame_.samples_per_channel_); - } - - NetEqNetworkStatistics network_stats; - ASSERT_EQ(0, neteq_->NetworkStatistics(&network_stats)); - EXPECT_EQ(110953, network_stats.clockdrift_ppm); -} - void NetEqDecodingTest::LongCngWithClockDrift(double drift_factor, double network_freeze_ms, bool pull_audio_during_freeze, diff --git a/modules/audio_coding/neteq/statistics_calculator.cc b/modules/audio_coding/neteq/statistics_calculator.cc index d0fd26eb10..76f661937c 100644 --- a/modules/audio_coding/neteq/statistics_calculator.cc +++ b/modules/audio_coding/neteq/statistics_calculator.cc @@ -390,8 +390,6 @@ void StatisticsCalculator::PopulateDelayManagerStats( stats->preferred_buffer_size_ms = (delay_manager.TargetLevel() >> 8) * ms_per_packet; stats->jitter_peaks_found = delay_manager.PeakFound(); - stats->clockdrift_ppm = - rtc::saturated_cast(delay_manager.EstimatedClockDriftPpm()); } NetEqLifetimeStatistics StatisticsCalculator::GetLifetimeStatistics() const { diff --git a/modules/audio_coding/neteq/tools/neteq_stats_getter.cc b/modules/audio_coding/neteq/tools/neteq_stats_getter.cc index 91dd47c37d..4e34f7905d 100644 --- a/modules/audio_coding/neteq/tools/neteq_stats_getter.cc +++ b/modules/audio_coding/neteq/tools/neteq_stats_getter.cc @@ -110,7 +110,6 @@ NetEqStatsGetter::Stats NetEqStatsGetter::AverageStats() const { a.accelerate_rate += b.accelerate_rate / 16384.0; a.secondary_decoded_rate += b.secondary_decoded_rate / 16384.0; a.secondary_discarded_rate += b.secondary_discarded_rate / 16384.0; - a.clockdrift_ppm += b.clockdrift_ppm; a.added_zero_samples += b.added_zero_samples; a.mean_waiting_time_ms += b.mean_waiting_time_ms; a.median_waiting_time_ms += b.median_waiting_time_ms; @@ -131,7 +130,6 @@ NetEqStatsGetter::Stats NetEqStatsGetter::AverageStats() const { sum_stats.accelerate_rate /= stats_.size(); sum_stats.secondary_decoded_rate /= stats_.size(); sum_stats.secondary_discarded_rate /= stats_.size(); - sum_stats.clockdrift_ppm /= stats_.size(); sum_stats.added_zero_samples /= stats_.size(); sum_stats.mean_waiting_time_ms /= stats_.size(); sum_stats.median_waiting_time_ms /= stats_.size();