From 4e7aa43ea0fd7106cd39036798877301398966a6 Mon Sep 17 00:00:00 2001 From: Bjorn Volcker Date: Tue, 7 Jul 2015 11:50:05 +0200 Subject: [PATCH] audio_processing: Adds two UMA histograms logging delay jumps in AEC We have two histograms today that trigger on large jumps in either platform reported stream delays (WebRTC.Audio.PlatformReportedStreamDelayJump) or the system delay in the AEC (WebRTC.Audio.AecSystemDelayJump). The latter is the internal buffer size in the AEC. The sizes of such jumps are of relevance since it can harm the AEC and even put it in a complete failure state. It is hard, not to say impossible, to tell how frequent it is. Therefore, two complementary histograms are added; number of jumps in each metric. This way we get a quick way to determine how often a jump occurs in general and also how frequent it is within a call. This is solved by adding a counter for each metric. The counter is activated either upon an event trigger or if we know for sure when the AEC is running. Unfortunately, we can't rely on the destructor at the end of a call so we add a public API for the user to take on the action of calling it at the end of a call. Tested locally by building ToT chromium including changes and three triggered jumps (200, 50 and 60 ms). The stats picked up the 60 and 200 ms jumps as expected. BUG=488124 R=asapersson@webrtc.org, pbos@webrtc.org Review URL: https://codereview.webrtc.org/1229443003. Cr-Commit-Position: refs/heads/master@{#9544} --- talk/media/webrtc/fakewebrtcvoiceengine.h | 1 + .../audio_processing/audio_processing_impl.cc | 40 ++++++++++++++++++- .../audio_processing/audio_processing_impl.h | 3 ++ .../include/audio_processing.h | 4 ++ .../include/mock_audio_processing.h | 1 + 5 files changed, 47 insertions(+), 2 deletions(-) diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h index 971f9f07c..419170b24 100644 --- a/talk/media/webrtc/fakewebrtcvoiceengine.h +++ b/talk/media/webrtc/fakewebrtcvoiceengine.h @@ -152,6 +152,7 @@ class FakeAudioProcessing : public webrtc::AudioProcessing { WEBRTC_STUB(StartDebugRecording, (const char filename[kMaxFilenameSize])); WEBRTC_STUB(StartDebugRecording, (FILE* handle)); WEBRTC_STUB(StopDebugRecording, ()); + WEBRTC_VOID_STUB(UpdateHistogramsOnCallEnd, ()); webrtc::EchoCancellation* echo_cancellation() const override { return NULL; } webrtc::EchoControlMobile* echo_control_mobile() const override { return NULL; diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc index fd6b25814..87b82a6a3 100644 --- a/webrtc/modules/audio_processing/audio_processing_impl.cc +++ b/webrtc/modules/audio_processing/audio_processing_impl.cc @@ -177,6 +177,8 @@ AudioProcessingImpl::AudioProcessingImpl(const Config& config, was_stream_delay_set_(false), last_stream_delay_ms_(0), last_aec_system_delay_ms_(0), + stream_delay_jumps_(-1), + aec_system_delay_jumps_(-1), output_will_be_muted_(false), key_pressed_(false), #if defined(WEBRTC_ANDROID) || defined(WEBRTC_IOS) @@ -1003,11 +1005,25 @@ void AudioProcessingImpl::MaybeUpdateHistograms() { static const int kMinDiffDelayMs = 60; if (echo_cancellation()->is_enabled()) { + // Activate delay_jumps_ counters if we know echo_cancellation is runnning. + // If a stream has echo we know that the echo_cancellation is in process. + if (stream_delay_jumps_ == -1 && echo_cancellation()->stream_has_echo()) { + stream_delay_jumps_ = 0; + } + if (aec_system_delay_jumps_ == -1 && + echo_cancellation()->stream_has_echo()) { + aec_system_delay_jumps_ = 0; + } + // Detect a jump in platform reported system delay and log the difference. const int diff_stream_delay_ms = stream_delay_ms_ - last_stream_delay_ms_; if (diff_stream_delay_ms > kMinDiffDelayMs && last_stream_delay_ms_ != 0) { RTC_HISTOGRAM_COUNTS("WebRTC.Audio.PlatformReportedStreamDelayJump", diff_stream_delay_ms, kMinDiffDelayMs, 1000, 100); + if (stream_delay_jumps_ == -1) { + stream_delay_jumps_ = 0; // Activate counter if needed. + } + stream_delay_jumps_++; } last_stream_delay_ms_ = stream_delay_ms_; @@ -1022,13 +1038,33 @@ void AudioProcessingImpl::MaybeUpdateHistograms() { RTC_HISTOGRAM_COUNTS("WebRTC.Audio.AecSystemDelayJump", diff_aec_system_delay_ms, kMinDiffDelayMs, 1000, 100); + if (aec_system_delay_jumps_ == -1) { + aec_system_delay_jumps_ = 0; // Activate counter if needed. + } + aec_system_delay_jumps_++; } last_aec_system_delay_ms_ = aec_system_delay_ms; - // TODO(bjornv): Consider also logging amount of jumps. This gives a better - // indication of how frequent jumps are. } } +void AudioProcessingImpl::UpdateHistogramsOnCallEnd() { + CriticalSectionScoped crit_scoped(crit_); + if (stream_delay_jumps_ > -1) { + RTC_HISTOGRAM_ENUMERATION( + "WebRTC.Audio.NumOfPlatformReportedStreamDelayJumps", + stream_delay_jumps_, 51); + } + stream_delay_jumps_ = -1; + last_stream_delay_ms_ = 0; + + if (aec_system_delay_jumps_ > -1) { + RTC_HISTOGRAM_ENUMERATION("WebRTC.Audio.NumOfAecSystemDelayJumps", + aec_system_delay_jumps_, 51); + } + aec_system_delay_jumps_ = -1; + last_aec_system_delay_ms_ = 0; +} + #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP int AudioProcessingImpl::WriteMessageToDebugFile() { int32_t size = event_msg_->ByteSize(); diff --git a/webrtc/modules/audio_processing/audio_processing_impl.h b/webrtc/modules/audio_processing/audio_processing_impl.h index 6f237ae4d..bbd171915 100644 --- a/webrtc/modules/audio_processing/audio_processing_impl.h +++ b/webrtc/modules/audio_processing/audio_processing_impl.h @@ -134,6 +134,7 @@ class AudioProcessingImpl : public AudioProcessing { int StartDebugRecording(FILE* handle) override; int StartDebugRecordingForPlatformFile(rtc::PlatformFile handle) override; int StopDebugRecording() override; + void UpdateHistogramsOnCallEnd() override; EchoCancellation* echo_cancellation() const override; EchoControlMobile* echo_control_mobile() const override; GainControl* gain_control() const override; @@ -210,6 +211,8 @@ class AudioProcessingImpl : public AudioProcessing { bool was_stream_delay_set_; int last_stream_delay_ms_; int last_aec_system_delay_ms_; + int stream_delay_jumps_; + int aec_system_delay_jumps_; bool output_will_be_muted_ GUARDED_BY(crit_); diff --git a/webrtc/modules/audio_processing/include/audio_processing.h b/webrtc/modules/audio_processing/include/audio_processing.h index 9affbdd10..6fa1c96c0 100644 --- a/webrtc/modules/audio_processing/include/audio_processing.h +++ b/webrtc/modules/audio_processing/include/audio_processing.h @@ -378,6 +378,10 @@ class AudioProcessing { // cannot be resumed in the same file (without overwriting it). virtual int StopDebugRecording() = 0; + // Use to send UMA histograms at end of a call. Note that all histogram + // specific member variables are reset. + virtual void UpdateHistogramsOnCallEnd() = 0; + // These provide access to the component interfaces and should never return // NULL. The pointers will be valid for the lifetime of the APM instance. // The memory for these objects is entirely managed internally. diff --git a/webrtc/modules/audio_processing/include/mock_audio_processing.h b/webrtc/modules/audio_processing/include/mock_audio_processing.h index 63161c891..480d0e34b 100644 --- a/webrtc/modules/audio_processing/include/mock_audio_processing.h +++ b/webrtc/modules/audio_processing/include/mock_audio_processing.h @@ -243,6 +243,7 @@ class MockAudioProcessing : public AudioProcessing { int(FILE* handle)); MOCK_METHOD0(StopDebugRecording, int()); + MOCK_METHOD0(UpdateHistogramsOnCallEnd, void()); virtual MockEchoCancellation* echo_cancellation() const { return echo_cancellation_.get(); }