From 3ffa72d0f03b75812c0ce367c1958ff9a80a6aeb Mon Sep 17 00:00:00 2001 From: Jonathan Yu Date: Fri, 7 Jul 2017 00:05:10 -0700 Subject: [PATCH] Add AudioFrame::ResetWithoutMuting() to address performance regression. Prior to https://codereview.webrtc.org/2750783004/ Reset() intentionally did not zero out the buffer. After that change, callers calling Reset() and then mutable_data() were performing a wasteful zeroing. This change adds ResetWithoutMuting() to match the old behavior and switches the sole non-test caller of Reset() to use ResetWithoutMuting() instead. Prior to this change (optimized, Linux): $ out/Default/webrtc_perf_tests --gtest_filter=NetEqPerformanceTest.Run* \ --gtest_repeat=10 | grep neteq_performance *RESULT neteq_performance: 10_pl_10_drift= 4051 ms *RESULT neteq_performance: 0_pl_0_drift= 1768 ms *RESULT neteq_performance: 10_pl_10_drift= 3666 ms *RESULT neteq_performance: 0_pl_0_drift= 1690 ms *RESULT neteq_performance: 10_pl_10_drift= 3685 ms *RESULT neteq_performance: 0_pl_0_drift= 1693 ms *RESULT neteq_performance: 10_pl_10_drift= 3720 ms *RESULT neteq_performance: 0_pl_0_drift= 1690 ms *RESULT neteq_performance: 10_pl_10_drift= 3780 ms *RESULT neteq_performance: 0_pl_0_drift= 1728 ms *RESULT neteq_performance: 10_pl_10_drift= 3733 ms *RESULT neteq_performance: 0_pl_0_drift= 1737 ms *RESULT neteq_performance: 10_pl_10_drift= 3781 ms *RESULT neteq_performance: 0_pl_0_drift= 1744 ms *RESULT neteq_performance: 10_pl_10_drift= 3712 ms *RESULT neteq_performance: 0_pl_0_drift= 1731 ms *RESULT neteq_performance: 10_pl_10_drift= 3681 ms *RESULT neteq_performance: 0_pl_0_drift= 1691 ms *RESULT neteq_performance: 10_pl_10_drift= 3681 ms *RESULT neteq_performance: 0_pl_0_drift= 1690 ms With this change: $ out/Default/webrtc_perf_tests --gtest_filter=NetEqPerformanceTest.Run* \ --gtest_repeat=10 | grep neteq_performance *RESULT neteq_performance: 10_pl_10_drift= 3824 ms *RESULT neteq_performance: 0_pl_0_drift= 1632 ms *RESULT neteq_performance: 10_pl_10_drift= 3502 ms *RESULT neteq_performance: 0_pl_0_drift= 1521 ms *RESULT neteq_performance: 10_pl_10_drift= 3520 ms *RESULT neteq_performance: 0_pl_0_drift= 1534 ms *RESULT neteq_performance: 10_pl_10_drift= 3517 ms *RESULT neteq_performance: 0_pl_0_drift= 1530 ms *RESULT neteq_performance: 10_pl_10_drift= 3521 ms *RESULT neteq_performance: 0_pl_0_drift= 1527 ms *RESULT neteq_performance: 10_pl_10_drift= 3511 ms *RESULT neteq_performance: 0_pl_0_drift= 1533 ms *RESULT neteq_performance: 10_pl_10_drift= 3518 ms *RESULT neteq_performance: 0_pl_0_drift= 1523 ms *RESULT neteq_performance: 10_pl_10_drift= 3503 ms *RESULT neteq_performance: 0_pl_0_drift= 1524 ms *RESULT neteq_performance: 10_pl_10_drift= 3514 ms *RESULT neteq_performance: 0_pl_0_drift= 1534 ms *RESULT neteq_performance: 10_pl_10_drift= 3501 ms *RESULT neteq_performance: 0_pl_0_drift= 1530 ms BUG=webrtc:7343,chromium:738852,chromium:738839 Change-Id: Idcbb276ca0ed27fff95164a73f1c1fa310175ee5 Reviewed-on: https://chromium-review.googlesource.com/563021 Reviewed-by: Henrik Lundin Reviewed-by: Olga Sharonova Reviewed-by: Tommi Commit-Queue: Tommi Cr-Commit-Position: refs/heads/master@{#18939} --- webrtc/modules/audio_coding/neteq/sync_buffer.cc | 2 +- webrtc/modules/include/module_common_types.h | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/webrtc/modules/audio_coding/neteq/sync_buffer.cc b/webrtc/modules/audio_coding/neteq/sync_buffer.cc index 29630b631f..49f80132a1 100644 --- a/webrtc/modules/audio_coding/neteq/sync_buffer.cc +++ b/webrtc/modules/audio_coding/neteq/sync_buffer.cc @@ -74,7 +74,7 @@ void SyncBuffer::GetNextAudioInterleaved(size_t requested_len, AudioFrame* output) { RTC_DCHECK(output); const size_t samples_to_read = std::min(FutureLength(), requested_len); - output->Reset(); + output->ResetWithoutMuting(); const size_t tot_samples_read = ReadInterleavedFromIndex(next_index_, samples_to_read, output->mutable_data()); diff --git a/webrtc/modules/include/module_common_types.h b/webrtc/modules/include/module_common_types.h index 36024f6108..23ed8f40bf 100644 --- a/webrtc/modules/include/module_common_types.h +++ b/webrtc/modules/include/module_common_types.h @@ -307,6 +307,11 @@ class AudioFrame { // Resets all members to their default state. void Reset(); + // Same as Reset(), but leaves mute state unchanged. Muting a frame requires + // the buffer to be zeroed on the next call to mutable_data(). Callers + // intending to write to the buffer immediately after Reset() can instead use + // ResetWithoutMuting() to skip this wasteful zeroing. + void ResetWithoutMuting(); void UpdateFrame(int id, uint32_t timestamp, const int16_t* data, size_t samples_per_channel, int sample_rate_hz, @@ -370,13 +375,17 @@ inline AudioFrame::AudioFrame() { } inline void AudioFrame::Reset() { + ResetWithoutMuting(); + muted_ = true; +} + +inline void AudioFrame::ResetWithoutMuting() { id_ = -1; // TODO(wu): Zero is a valid value for |timestamp_|. We should initialize // to an invalid value, or add a new member to indicate invalidity. timestamp_ = 0; elapsed_time_ms_ = -1; ntp_time_ms_ = -1; - muted_ = true; samples_per_channel_ = 0; sample_rate_hz_ = 0; num_channels_ = 0;