Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(809)

Issue 1229443003: audio_processing: Adds two UMA histograms logging delay jumps in AEC (Closed)

Created:
5 years, 5 months ago by bjornv1
Modified:
5 years, 5 months ago
CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, rwolff_gocast.it, yujie_mao (webrtc), Andrew MacDonald, hlundin-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, kwiberg-webrtc, aluebs-webrtc
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

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 Committed: https://chromium.googlesource.com/external/webrtc/+/4e7aa43ea0fd7106cd39036798877301398966a6

Patch Set 1 #

Total comments: 1

Patch Set 2 : Changed to use ENUMERATION #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -2 lines) Patch
M talk/media/webrtc/fakewebrtcvoiceengine.h View 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/modules/audio_processing/audio_processing_impl.h View 2 chunks +3 lines, -0 lines 0 comments Download
M webrtc/modules/audio_processing/audio_processing_impl.cc View 1 3 chunks +38 lines, -2 lines 0 comments Download
M webrtc/modules/audio_processing/include/audio_processing.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M webrtc/modules/audio_processing/include/mock_audio_processing.h View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
bjornv1
Dear reviewers, PTAL at this at your earliest convenience. pbos: OWNERS stamp in talk/ asapersson: ...
5 years, 5 months ago (2015-07-07 07:31:35 UTC) #2
pbos-webrtc
talk/ lgtm
5 years, 5 months ago (2015-07-07 08:11:17 UTC) #3
bjornv1
Åsa, PTAL. I've verified the change in Chromium.
5 years, 5 months ago (2015-07-07 08:55:47 UTC) #4
åsapersson
On 2015/07/07 08:55:47, bjornv1 wrote: > Åsa, PTAL. I've verified the change in Chromium. lgtm
5 years, 5 months ago (2015-07-07 09:29:17 UTC) #5
bjornv1
5 years, 5 months ago (2015-07-07 09:50:21 UTC) #6
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
4e7aa43ea0fd7106cd39036798877301398966a6 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698