From 0d8385770b53f3fef11ed2f5d98af712dc2ba89d Mon Sep 17 00:00:00 2001 From: "henrik.lundin" Date: Thu, 13 Oct 2016 03:35:55 -0700 Subject: [PATCH] NetEq: Convert AverageIAT from int to float calculations With this change, the calculations inside AverageIAT are changed to be in double-precision floating point instead of in fixed point. Also, the method's name is changed to EstimatedClockDriftPpm to better reflect what it returns. A few unit tests had to be updated because of minor numerical differences. Also removing the UBSan suppression related to this issue. BUG=webrtc:5889 Review-Url: https://codereview.webrtc.org/2408653002 Cr-Commit-Position: refs/heads/master@{#14628} --- tools/ubsan/blacklist.txt | 5 ---- .../audio_coding/neteq/delay_manager.cc | 25 ++++++++----------- .../audio_coding/neteq/delay_manager.h | 2 +- .../neteq/mock/mock_delay_manager.h | 2 -- .../audio_coding/neteq/neteq_unittest.cc | 20 +++++++-------- .../neteq/statistics_calculator.cc | 3 ++- 6 files changed, 23 insertions(+), 34 deletions(-) diff --git a/tools/ubsan/blacklist.txt b/tools/ubsan/blacklist.txt index 0d887e51bb..403c86645a 100644 --- a/tools/ubsan/blacklist.txt +++ b/tools/ubsan/blacklist.txt @@ -31,8 +31,3 @@ fun:*WebRtcSpl_AddSatW32* fun:*WebRtcSpl_SubSatW32* fun:*WebRtcSpl_DivW32HiLow* fun:*GmmProbability* - -############################################################################# -# Ignore errors in peerconnection_unittests. -# https://bugs.chromium.org/p/webrtc/issues/detail?id=5889 -src:*/webrtc/modules/audio_coding/neteq/delay_manager.cc diff --git a/webrtc/modules/audio_coding/neteq/delay_manager.cc b/webrtc/modules/audio_coding/neteq/delay_manager.cc index a6efdab8dc..bc27bd1552 100644 --- a/webrtc/modules/audio_coding/neteq/delay_manager.cc +++ b/webrtc/modules/audio_coding/neteq/delay_manager.cc @@ -319,22 +319,17 @@ void DelayManager::Reset() { last_pack_cng_or_dtmf_ = 1; } -int DelayManager::AverageIAT() const { - int32_t sum_q24 = 0; - // Using an int for the upper limit of the following for-loop so the - // loop-counter can be int. Otherwise we need a cast where |sum_q24| is - // updated. - const int iat_vec_size = static_cast(iat_vector_.size()); - assert(iat_vector_.size() == 65); // Algorithm is hard-coded for this size. - for (int i = 0; i < iat_vec_size; ++i) { - // Shift 6 to fit worst case: 2^30 * 64. - sum_q24 += (iat_vector_[i] >> 6) * i; +double DelayManager::EstimatedClockDriftPpm() const { + double sum = 0.0; + // Calculate the expected value based on the probabilities in |iat_vector_|. + for (size_t i = 0; i < iat_vector_.size(); ++i) { + sum += static_cast(iat_vector_[i]) * i; } - // Subtract the nominal inter-arrival time 1 = 2^24 in Q24. - sum_q24 -= (1 << 24); - // Multiply with 1000000 / 2^24 = 15625 / 2^18 to get in parts-per-million. - // Shift 7 to Q17 first, then multiply with 15625 and shift another 11. - return ((sum_q24 >> 7) * 15625) >> 11; + // The probabilities in |iat_vector_| 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 { diff --git a/webrtc/modules/audio_coding/neteq/delay_manager.h b/webrtc/modules/audio_coding/neteq/delay_manager.h index 2f928f175b..78715720b8 100644 --- a/webrtc/modules/audio_coding/neteq/delay_manager.h +++ b/webrtc/modules/audio_coding/neteq/delay_manager.h @@ -72,7 +72,7 @@ class DelayManager { // 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 int AverageIAT() const; + 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 diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h b/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h index 9a0c0e6bd5..7a66b68c63 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h @@ -35,8 +35,6 @@ class MockDelayManager : public DelayManager { int(int length_ms)); MOCK_METHOD0(Reset, void()); - MOCK_CONST_METHOD0(AverageIAT, - int()); MOCK_CONST_METHOD0(PeakFound, bool()); MOCK_METHOD1(UpdateCounters, diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc index 11fd9b440f..b958e27a64 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc @@ -448,10 +448,10 @@ TEST_F(NetEqDecodingTest, MAYBE_TestBitExactness) { "52797b781758a1d2303140b80b9c5030c9093d6b"); const std::string network_stats_checksum = PlatformChecksum( - "9c5bb9e74a583be89313b158a19ea10d41bf9de6", - "e948ec65cf18852ba2a197189a3186635db34c3b", - "9c5bb9e74a583be89313b158a19ea10d41bf9de6", - "9c5bb9e74a583be89313b158a19ea10d41bf9de6"); + "f59b3dfdb9b1b8bbb61abedd7c8cf3fc47c21f5f", + "c8b2a93842e48d014f7e6efe10ae96cb3892b129", + "f59b3dfdb9b1b8bbb61abedd7c8cf3fc47c21f5f", + "f59b3dfdb9b1b8bbb61abedd7c8cf3fc47c21f5f"); const std::string rtcp_stats_checksum = PlatformChecksum( "b8880bf9fed2487efbddcb8d94b9937a29ae521d", @@ -484,10 +484,10 @@ TEST_F(NetEqDecodingTest, MAYBE_TestOpusBitExactness) { "9d7d52bc94e941d106aa518f324f16a58d231586"); const std::string network_stats_checksum = PlatformChecksum( - "191af29ed3b8b6dd4c4cc94dc3f33bdf48f055ef", - "191af29ed3b8b6dd4c4cc94dc3f33bdf48f055ef", - "191af29ed3b8b6dd4c4cc94dc3f33bdf48f055ef", - "191af29ed3b8b6dd4c4cc94dc3f33bdf48f055ef"); + "d8379381d5a619f0616bb3c0a8a9eea1704a8ab8", + "d8379381d5a619f0616bb3c0a8a9eea1704a8ab8", + "d8379381d5a619f0616bb3c0a8a9eea1704a8ab8", + "d8379381d5a619f0616bb3c0a8a9eea1704a8ab8"); const std::string rtcp_stats_checksum = PlatformChecksum( "e37c797e3de6a64dda88c9ade7a013d022a2e1e0", @@ -577,7 +577,7 @@ TEST_F(NetEqDecodingTest, TestAverageInterArrivalTimeNegative) { NetEqNetworkStatistics network_stats; ASSERT_EQ(0, neteq_->NetworkStatistics(&network_stats)); - EXPECT_EQ(-103196, network_stats.clockdrift_ppm); + EXPECT_EQ(-103192, network_stats.clockdrift_ppm); } TEST_F(NetEqDecodingTest, TestAverageInterArrivalTimePositive) { @@ -605,7 +605,7 @@ TEST_F(NetEqDecodingTest, TestAverageInterArrivalTimePositive) { NetEqNetworkStatistics network_stats; ASSERT_EQ(0, neteq_->NetworkStatistics(&network_stats)); - EXPECT_EQ(110946, network_stats.clockdrift_ppm); + EXPECT_EQ(110953, network_stats.clockdrift_ppm); } void NetEqDecodingTest::LongCngWithClockDrift(double drift_factor, diff --git a/webrtc/modules/audio_coding/neteq/statistics_calculator.cc b/webrtc/modules/audio_coding/neteq/statistics_calculator.cc index d16a11bc63..e9bceb72d9 100644 --- a/webrtc/modules/audio_coding/neteq/statistics_calculator.cc +++ b/webrtc/modules/audio_coding/neteq/statistics_calculator.cc @@ -223,7 +223,8 @@ void StatisticsCalculator::GetNetworkStatistics( stats->preferred_buffer_size_ms = (delay_manager.TargetLevel() >> 8) * ms_per_packet; stats->jitter_peaks_found = delay_manager.PeakFound(); - stats->clockdrift_ppm = delay_manager.AverageIAT(); + stats->clockdrift_ppm = + rtc::saturated_cast(delay_manager.EstimatedClockDriftPpm()); stats->packet_loss_rate = CalculateQ14Ratio(lost_timestamps_, timestamps_since_last_report_);