WebRTC Code Reviews
Help | Chromium Project | Sign in
(1625)

Issue 43109004: audio_processing/agc: Adds config to set minimum microphone volume at startup (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by Bjorn
Modified:
4 years, 1 month ago
Reviewers:
kwiberg, ajm
CC:
webrtc-reviews_webrtc.org, tterriberry, kwiberg, aluebs, ajm
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Project:
webrtc
Visibility:
Public.

Description

audio_processing/agc: Adds config to set minimum microphone volume at startup The AGC is currently bumping up the mic volume to 33% at startup if it is below that level. This is to avoid getting stuck in a poor state from which the AGC can not move, simply a too low input audio level. For some users, 33% is instead too loud. This CL gives the user the possibility to set that level at create time. - Extends the Config ExperimentalAgc with a startup_mic_volume for the user to set if desired. Note that the bump up does not apply to the legacy AGC and the "regular" AGC is controlled by ExperimentalAgc. - Without any actions, the same default value as previously is used. - In addition I removed a return value from InitializeExperimentalAgc() and InitializeTransient() This has been tested by building Chromium on Mac and verify through apprtc that 1) startup_mic_volume = 128 bumps up to 50%. 2) startup_mic_volume = 500 (out of range) bumps up to 100%. 3) startup_mic_volume = 0 bumps up to 4%, the AGC min level. BUG=4529 TESTED=locally R=andrew@webrtc.org, kwiberg@webrtc.org Committed: https://crrev.com/adc46c4cf7dd8a015b0757c99787d80525c123ab Cr-Commit-Position: refs/heads/master@{#9004}

Patch Set 1 #

Patch Set 2 : Clang format #

Total comments: 13

Patch Set 3 : Addressed review comments #

Total comments: 3

Messages

Total messages: 9 (0 generated)
Bjorn
https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h File webrtc/modules/audio_processing/include/audio_processing.h (right): https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h#newcode78 webrtc/modules/audio_processing/include/audio_processing.h:78: // [12, 255], since that is the operation range. ...
4 years, 2 months ago (2015-04-09 15:21:51 UTC) #1
ajm
Drive-by. https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h File webrtc/modules/audio_processing/include/audio_processing.h (right): https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h#newcode83 webrtc/modules/audio_processing/include/audio_processing.h:83: // ProcessStream() call. This makes the semantics around ...
4 years, 2 months ago (2015-04-09 15:47:41 UTC) #2
Bjorn
https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h File webrtc/modules/audio_processing/include/audio_processing.h (right): https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h#newcode83 webrtc/modules/audio_processing/include/audio_processing.h:83: // ProcessStream() call. On 2015/04/09 15:47:41, ajm wrote: > ...
4 years, 2 months ago (2015-04-09 16:07:21 UTC) #3
kwiberg
Changing return values from int to void arguably doesn't belong in this CL, but I ...
4 years, 2 months ago (2015-04-10 00:18:05 UTC) #4
Bjorn
https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/agc/agc_manager_direct.cc File webrtc/modules/audio_processing/agc/agc_manager_direct.cc (right): https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/agc/agc_manager_direct.cc#newcode65 webrtc/modules/audio_processing/agc/agc_manager_direct.cc:65: return mic_level; On 2015/04/10 00:18:05, kwiberg wrote: > return ...
4 years, 2 months ago (2015-04-10 05:56:34 UTC) #5
ajm
lgtm https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h File webrtc/modules/audio_processing/include/audio_processing.h (right): https://webrtc-codereview.appspot.com/43109004/diff/20001/webrtc/modules/audio_processing/include/audio_processing.h#newcode83 webrtc/modules/audio_processing/include/audio_processing.h:83: // ProcessStream() call. On 2015/04/10 05:56:33, Bjorn wrote: ...
4 years, 2 months ago (2015-04-13 16:19:54 UTC) #6
kwiberg
lgtm https://webrtc-codereview.appspot.com/43109004/diff/40001/webrtc/modules/audio_processing/include/audio_processing.h File webrtc/modules/audio_processing/include/audio_processing.h (right): https://webrtc-codereview.appspot.com/43109004/diff/40001/webrtc/modules/audio_processing/include/audio_processing.h#newcode78 webrtc/modules/audio_processing/include/audio_processing.h:78: // [12, 255]. Here, 255 maps to 100%. ...
4 years, 2 months ago (2015-04-15 09:05:39 UTC) #7
Bjorn
Committed patchset #3 (id:40001) manually as adc46c4cf7dd8a015b0757c99787d80525c123ab (presubmit successful).
4 years, 2 months ago (2015-04-15 09:42:53 UTC) #8
commit-bot
4 years, 2 months ago (2015-04-15 09:42:58 UTC) #9
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/adc46c4cf7dd8a015b0757c99787d80525c123ab
Cr-Commit-Position: refs/heads/master@{#9004}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 245c2c2-tainted