Make the argument speech_probability non-optional in
InputVolumeController::Process() and
MonoInputVolumeController::Process().
Additional clean-up: Remove the flag enabled in the
config. Add unit tests for MonoInputVolumeController.
Bug: webrtc:7494
Change-Id: Ie28af77dc628bf71d09ce1ff033d39031f77a21e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283700
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38710}
Make speech probability threshold configurable by replacing
kSpeechProbabilitySilenceThreshold with speech_probability_threshold in
InputVolumeController::Config.
Make the processing more robust against outliers in speech probability
estimaton by computing an aggregate speech activity over a speech
segment. In MonoInputVolumeController::Process(), use the passed
non-empty speech probabilities to compute the speech activity over the
speech segment and only allow updates for segments with a high enough
ratio of speech frames. Pass RMS error and speech probability for every
frame in Process(): If rms_error_dbfs is empty, volume updates are not
allowed; if speech_probability is empty, the frame counts as a non-
speech frame.
Remove startup_min_volume from the config since it's no longer used
after https://webrtc-review.googlesource.com/c/src/+/282821.
Bug: webrtc:7494
Change-Id: I0ab81b03371496315348f552133aa9909bd36f26
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283523
Commit-Queue: Hanna Silen <silen@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38685}
Replace kUpdateInputVolumeWaitFrames with
update_input_volume_wait_frames in InputVolumeController::Config.
Also, fix an off-by-one error in the frame count to give a better
readability for non-zero wait frames. Now
update_input_volume_wait_frames_ = 100 allows updates every 100 frames
instead of every 101 frames. Effectively, this makes
update_input_volume_wait_frames = 0 and 1 to behave similarly (i.e.,
they now both allow updates after every frame).
Bug: webrtc:7494
Change-Id: I597f7e88895a4dcd365dc6dee526acb9d971b2fc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282863
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38648}
Replace the use of MonoInputController::min_mic_level() with
MonoInputVolumeController::clipped_level_min() when estimating input
volume adjustment from clipping prediction. The adjustment is later
capped in MonoInputVolumeController::HandleClipping() using
clipped_level_min_ so no audio changes are expected from this change.
Bug: webrtc:7494
Change-Id: Ie26d0aa5cce3eeef06f70a281504889519bb5aca
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282840
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38593}
Remove function declarations, members, and friend tests that are
no longer used. Reorder the member variables.
Bug: webrtc:7494
Change-Id: I8c24e2f4b9d9846e6d3fef4e2c998aa26f49f8c9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282180
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38570}
Replace the use of speech level target and digital gain maximum with speech level target range parameters.
Bug: webrtc:7494
Change-Id: I703756c5a3fbd330ed585e3f5b4ac3141d9ea6e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280943
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38563}
In InputVolumeController/MonoInputVolumeController, set
min_digital_gain_db_ and disable_digital_adaptive_ to fixed values
ahead of replacing speech level target as well as digital gain
minimum and maximum with target range parameters.
In InputVolumeController, remove digital_adaptive_follows and
min_digital_gain_db from the config as they are no longer needed.
Bug: webrtc:7494
Change-Id: I1378b6e182224c41038c6d8c649e7a28961f73d4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280962
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38554}
Modify unit tests ahead of changes that will replace the minimum
digital gain with a fixed value 0 and always enable digital gain
compensation.
Bug: webrtc:7494
Change-Id: I9df95667b831d5b68e70aaba22f631b398edf8e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280960
Commit-Queue: Hanna Silen <silen@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38553}
Rename constants and arguments reflecting the old naming with RMS error
overriding the error calculated by the analog AGC. Rename the related
unit tests and helper functions.
Bug: webrtc:7494
Change-Id: I9a1d972e9ff7ab5cdd43ca3568379d511801adee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280481
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38552}
Add InputVolumeController as a member in GainController2 (not created
by default). Add a method GainController2::Analyze() to update the
applied input volume and run the pre-processing steps in
InputVolumeController. Add a call InputVolumeController::Process() in
GainController2::Process().
Bug: webrtc:7494
Change-Id: Idf4111ac5e19a620b6421c7f23fd642f169c7b5a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279822
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38548}
Implement InputVolumeController and RecommendedInputVolumeEstimator based on the copy of agc classes AgcManagerDirect and MonoAgc.
Copies of the original files created in https://webrtc-review.googlesource.com/c/src/+/278624.
Bug: webrtc:7494
Change-Id: I74acee57b0db5cc8a6b666be9ba619c6c98a1773
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278625
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38533}
Copy AgcManagerDirect files from agc to agc2. Rename the newly
created files and classes ahead of refactoring. Add a build
target.
This change is done to enable creating a class
InputVolumeController based on AgcManagerDirect. The added
temporary dependency on files in agc will be removed
in https://webrtc-review.googlesource.com/c/src/+/278625.
The exact copy of the files happened in the 1st patchset and it
has been verified as follows:
Checksum check:
```
$ git checkout main && git pull
# Go back to the tree state before [1] landed
$ git new-branch tmp
$ git reset --hard 2235776597
$ cd modules/audio_processing/agc/
$ md5 agc_manager_direct*
MD5 (agc_manager_direct.cc) = e661481a85f72596cae4599b62907f5b
MD5 (agc_manager_direct.h) = bf68280e2d0f689b4ebcd665b5db6052
MD5 (agc_manager_direct_unittest.cc) = 6bf0bf45ff5e940b1a3bb37154f09269
```
Patchset 1 (see [2])
```
$ cd modules/audio_processing/agc2/
$ md5 input_volume_controlle*
MD5 (input_volume_controller.cc) = e661481a85f72596cae4599b62907f5b
MD5 (input_volume_controller.h) = bf68280e2d0f689b4ebcd665b5db6052
MD5 (input_volume_controller_unittest.cc) = 6bf0bf45ff5e940b1a3bb37154f09269
```
[1] https://webrtc-review.googlesource.com/c/src/+/278781
[2] https://webrtc-review.googlesource.com/c/src/+/278624/1
Bug: webrtc:7494
Change-Id: I7804da899d18adf556b089c76a567ce27c299a62
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278624
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38512}
This is a reland of commit 8d7273357d
Root cause:
audioproc_f doesn't call `metrics::Enable()` and therefore the stats
reporter crashed when `metrics::HistogramFactoryGetCountsLinear()`
returned a nullptr.
Bug fix:
Added `InputVolumeStatsReporter::cannot_log_stats_`, a const flag
that is set to true if any histogram factory returns a nullptr.
When true, the class does nothing.
This CL also includes other code readability improvements that were
not part of the original CL.
Original change's description:
> APM: log both applied and recommended input volume stats
>
> This CL replaces the existing `WebRTC.Audio.ApmAnalogGain.*` stats
> with `WebRTC.Audio.Apm.AppliedInputVolume.*` and adds the
> `WebRTC.Audio.Apm.RecommendedInputVolume.*` stats.
>
> Bug: webrtc:7494
> Change-Id: I70be710d20b1589fc814cbce3d3329ac1500686f
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280220
> Reviewed-by: Hanna Silen <silen@webrtc.org>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38468}
Bug: webrtc:7494
Change-Id: I8373d16beb06b84f439d2c2274ededea7c5e95b0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280661
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38484}
This CL replaces the existing `WebRTC.Audio.ApmAnalogGain.*` stats
with `WebRTC.Audio.Apm.AppliedInputVolume.*` and adds the
`WebRTC.Audio.Apm.RecommendedInputVolume.*` stats.
Bug: webrtc:7494
Change-Id: I70be710d20b1589fc814cbce3d3329ac1500686f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280220
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38468}
Adopt the new naming convention, which replaces "analog gain" and
"mic level" with "input volume", in the input volume stats reporter.
Bug: webrtc:7494
Change-Id: Ia24876151f51dd1dcc4e4f9db56c64d11ae3b442
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279840
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38467}
Add a buffer class to store speech probabilities and to estimate speech
activity. Follows the implementation of speech activity computation in
LoudnessHistogram but uses floats for computations.
Bug: webrtc:7494
Change-Id: I6ee72ec52919904ea4e1fbe51d61993aa7813c9f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277801
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38309}
Just applied a short sed script. See bug description for
the motiviation for this change.
This is the command that was used to generate the changes:
$ find . -type f \( -iname '*.cc' -o -iname '*.h' \) -print0 | \
xargs -0 sed -i -e 's/(const override)/(const, override)/'
Bug: webrtc:13090
Change-Id: Iec7d280f9d55263a972dbb3bd644ebfcd2eb38cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249088
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Ali Tofigh <alito@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35801}
New limiter tuning to more quickly go back to 0 dB after the limiter
kicks in and the input peak level goes back to normal.
Bug: webrtc:7494
Change-Id: I1050957ca4caf12c4562b899b16c306957dce169
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/237701
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35384}
Add implementation of RTC_DCHECK_NOTREACHED equal to the RTC_NOTREACHED.
The new macros will replace the old one when old one's usage will be
removed. The idea of the renaming to provide a clear signal that this
is debug build only macros and will be stripped in the production build.
Bug: webrtc:9065
Change-Id: I4c35d8b03e74a4b3fd1ae75dba2f9c05643101db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/237802
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35348}
The class has also been renamed to better reflect its purpose.
Bug: webrtc:7494
Change-Id: I223a364ab4f8b8a5fef765848bf05675d045cefd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/236343
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35277}
Bit exactness verified with audioproc_f on a collection of AEC dumps
and Wav files (42 recordings in total).
Bug: webrtc:7494
Change-Id: Id9849c4463791f5a203afe31efc163efb4d4458e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234583
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35248}
Not passing the sample rate to the `VoiceActivityDetectorWrapper` ctor
yet since that would require an unnecessary refactoring of `AdaptiveAgc`
which will soon be removed.
Instead, to ensure correct initialization until the child CL [1] lands,
`VoiceActivityDetectorWrapper::initialized_` is temporarily added.
Bit exactness verified with audioproc_f on a collection of AEC dumps
and Wav files (42 recordings in total).
[1] https://webrtc-review.googlesource.com/c/src/+/234583
Bug: webrtc:7494
Change-Id: I4b4be7b8106ba36c958d91bf263a7b30271a1ee3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234587
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35213}
Internal refactoring of AGC2 to decouple the VAD, its wrapper and the
peak and RMS level measurements.
Bit exactness verified with audioproc_f on a collection of AEC dumps
and Wav files (42 recordings in total).
Bug: webrtc:7494
Change-Id: Ib560f1fcaa601557f4f30e47025c69e91b1b62e0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234524
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35208}
When `AudioProcessingImpl::ApplyConfig()` is called, AGC2 is initialized
and then the new config is applied. That is error prone and for example
breaks bit exactness in [1].
Changes:
- `GainController2` must be created by passing configuration,
sample rate and number of channels
- `GainController2::ApplyConfig()` removed
Bit exactness verified with audioproc_f on a collection of AEC dumps
and Wav files (42 recordings in total).
[1] https://webrtc-review.googlesource.com/c/src/+/234587.
Bug: webrtc:7494
Change-Id: I251e03603394a4fc8769b9b5c197a157893676a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235060
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35206}
Internal refactoring of AGC2. This CL is needed in preparation for its
child CL to correctly show the upcoming changes in the diff.
Bug: webrtc:7494
Change-Id: If7f837e064243d5ffe09e21fc68f489bb00dfdc5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234527
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35170}
This CL does *not* change the behavior of the AGC2 adaptive digital
controller - bitexactness verified with audioproc_f on a collection of
AEC dumps and Wav files (42 recordings in total).
Tested: compiled Chrome with this patch and made an appr.tc test call
Bug: webrtc:7494
Change-Id: Ia8a9f6fbc3a3459b888a2eed87e108f0d39cfe99
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/233520
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35140}
Instead of using two different headroom parameters, namely
`kHeadroomDbfs` and `kSaturationProtectorExtraHeadroomDb`, only use
the former that now also accounts for the deleted one - i.e., it equals
the sum of the two headrooms. In this way, tuning AGC2 will be easier.
This CL does *not* change the behavior of the AGC2 adaptive digital
controller - bitexactness verified with audioproc_f on a collection of
AEC dumps and Wav files (42 recordings in total).
The unit tests changes in agc2/saturation_protector_unittest.cc are
required since `extra_headroom_db` is removed and the changes in
agc2/adaptive_digital_gain_applier_unittest.cc are required because
`AdaptiveDigitalGainApplier` depends on `kHeadroomDbfs` which has been
updated as stated above.
Bug: webrtc:7494
Change-Id: I0a2a710bbede0caa53938090a004d185fdefaeb9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232905
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35109}
This CL also includes the following changes:
- `AudioProcessing::Config::GainController2::noise_estimator`
deprecated
- `EnergyToDbfs()` optimized by removing unnecessary `sqrt`
- Unit test minor fix, incorrect type was used
Bug: webrtc:7494
Change-Id: I88a6672d6f7cd03fcf6a3031883522d256880140
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230940
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34893}
Add the option to run the adaptive digital controller of AGC2 without
side-effects - i.e., no gain applied.
Tested: adapation verified during a video call in chromium
Bug: webrtc:7494
Change-Id: I4776f6012907d76a17a3bca89991da97dc38657f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215964
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33875}
- Bug fix: the desired initial gain quickly dropped to 0 dB hence
starting a call with a too low level
- New tuning to make AGC2 more robust to VAD mistakes
- Smarter max gain increase speed: to deal with an increased threshold
of adjacent speech frames, the gain applier temporarily allows a
faster gain increase to deal with a longer time spent waiting for
enough speech frames in a row to be observed
- Saturation protector isolated from `AdaptiveModeLevelEstimator` to
simplify the unit tests for the latter (non bit-exact change)
- AGC2 adaptive digital config: unnecessary params deprecated
- Code readability improvements
- Data dumps clean-up and better naming
Bug: webrtc:7494
Change-Id: I4e36059bdf2566cc2a7e1a7e95b7430ba9ae9844
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215140
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33736}
The current noise level estimator has a bug due to which the estimated
level decays to the lower bound in a few seconds when speech is observed.
Instead of fixing the current implementation, which is based on a
stationarity classifier, an alternative, lightweight, noise floor
estimator has been added and tuned for AGC2.
Tested on several AEC dumps including HW mute, music and fast talking.
Bug: webrtc:7494
Change-Id: Iae4cff9fc955a716878f830957e893cd5bc59446
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214133
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33733}
Done in preparation for the child CL which adds an alternative
implementation.
Bug: webrtc:7494
Change-Id: I4963376afc917eae434a0d0ccee18f21880eefe0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214125
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33646}
Code style improvements done in preparation for a bug fix (TODO added)
which requires changes in the unit tests.
Note that one expected value in the unit tests has been adjusted since
the white noise generator is now instanced in each separate test and
therefore, even if the seed remained the same, the generated sequences
differ.
Bug: webrtc:7494
Change-Id: I497513b84f50b5c66cf6241a09946ce853eb1cd2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214122
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33636}
We need to be able build chromium with rtc_include_tests = true. It
reveals a lot of targets that are not compatible with chromium but
aren't marked so.
`rtc_include_tests=true` has been considered a way to disable targets for the Chromium build, causing an overload on rtc_include_tests while the meaning of the two GN args (rtc_include_tests and build_with_chromium) should be kept separated.
Bug: webrtc:12404
Change-Id: I2f72825445916eae7c20ef9338672d6a07a9b9ff
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/203890
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Andrey Logvin <landrey@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33124}
- The injection of the AGC2 level estimator into `AgcManagerDirect`
is not used anymore
- `ExperimentalAgc::enabled_agc2_level_estimator` can also be removed
- 3 ctors of `ExperimentalAgc` are unused
- `AgcManagerDirectStandaloneTest::AgcMinMicLevelExperiment` can be
split into separate unit tests (better code clarity)
Bug: webrtc:7494
Change-Id: I5843147c38cf7cb5ee484b0a72fe13dcf363efaf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/202025
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33027}
Results: RNN VAD realtime factor improved from 140x to 195x (+55x)
Test device: Pixel 2 XL
Benchmark setup: max clock speed forced on all the cores by
setting "performance" as scaling governor
Bug: webrtc:10480
Change-Id: I3e92f643853ad1fe990db909c578ce78ee826c03
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/198842
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32888}
The implementations for the fully connected layer can be simlpified by
using `VectorMath:DotProduct()`. In this way, it is also possible to
remove (nearly) duplicated SIMD code, reduce the binary size and more
easily maintain the code.
This CL also forces unoptimized code for the output layer of the VAD,
which is a FC 24x1 layer. A slight improvement of the realtime has
been measured (delta ~ +5x).
Bug: webrtc:10480
Change-Id: Iee93bd59f7905ebf96275dbbfeb3c921baf4e8db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195580
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32806}
Using `VectorMath::DotProduct()` in GatedRecurrentLayer to reuse existing
SIMD optimizations. Results:
- When SSE2/AVX2 is avilable, the GRU layer takes 40% of the unoptimized
code
- The realtime factor for the VAD improved as follows
- SSE2: from 570x to 630x
- AVX2: from 610x to 680x
This CL also improved the GRU layer benchmark by (i) benchmarking a GRU
layer havibng the same size of that used in the VAD and (ii) by prefetching
a long input sequence.
Bug: webrtc:10480
Change-Id: I9716b15661e4c6b81592b4cf7c172d90e41b5223
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195545
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32803}
Handle the case in which neither WEBRTC_ARCH_X86_FAMILY nor
WEBRTC_HAS_NEON are defined.
Bug: webrtc:10480
Change-Id: I241583911d8e5645dfbd39b60337dd20b2d9f046
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/196525
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32786}
Refactoring done to more easily and cleanly add SIMD optimizations and
to remove `GatedRecurrentLayer` from the RNN VAD api.
Bug: webrtc:10480
Change-Id: Ie1dffdd9b19c57c03a0b634f6818c0780456a66c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195445
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32770}
Refactoring done to more easily and cleanly add SIMD optimizations and
to remove `FullyConnectedLayer` from the RNN VAD api.
Minor improvements (readability, API):
- `FullyConnectedLayer` gets the ActivationFunction enum and not
a function view anymore
- SSE2 optimization moved into `FullyConnectedLayer::ComputeOutputSse2`
- layer name added for improved logs
Bug: webrtc:10480
Change-Id: Ida4903a67655e19ef0464f378c433c1f6e96dca7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195444
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32766}
Plus a few minor code readability improvements.
Bug: webrtc:10480
Change-Id: I590d8e203b1d05959a8c15373841e37abe83237e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195334
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32764}
This CL brings a large improvement to the RNN VAD CPU performance
by finally using `VectorMath::DotProduct()` for pitch search.
The realtime factor improved from about 390x to 570x for SSE2
(+180x, 45% faster) and to 610x for AVX2 (+235x, 60% faster).
RNN VAD benchmark results:
```
+-----+-------+------+------+
| run | none* | SSE2 | AVX2 |
+-----+-------+------+------+
| 1 | 393x | 572x | 618x |
| 2 | 388x | 568x | 607x |
| 3 | 393x | 564x | 599x |
+-----+-------+------+------+
```
*: baseline, no SIMD used for pitch search, but SSE2 used for the RNN
Results obtained as follows:
1. Force SSE2 in `DISABLED_RnnVadPerformance` for the RNN part in
order to measure the baseline correctly:
```
RnnBasedVad rnn_vad({/*sse2=*/true, /*avx2=*/true, /*neon=*/false});
```
2. Run the test:
```
$ ./out/release/modules_unittests \
--gtest_filter=*RnnVadTest*DISABLED_RnnVadPerformance* \
--gtest_also_run_disabled_tests --logs
```
Bug: webrtc:10480
Change-Id: I89a2bd420265540026944b9c0f1fdd4bfda7f475
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195001
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32755}
This CL adds a new library for the RNN VAD that provides (optimized)
vector math ops. The scheme is the same of the `VectorMath` class of AEC3
to ensure correct builds across different platforms.
Bug: webrtc:10480
Change-Id: I96bcfbf930ca27388ab5f2d52c022ddb73acf8e6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/194326
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32741}
In preparation for adding AVX2 code, a safe scheme to support
different SIMD optimizations is added.
Safety features:
- AVX2 kill switch to stop using it even if supported by the
architecture
- struct indicating the available CPU features propagated from
AGC2 to each component; in this way
- better control over the unit tests
- no need to propagate individual kill switches but just
set to false features that are turned off
Note that (i) this CL does not change the performance of the RNN VAD
and (ii) no AVX2 optimization is added yet.
Bug: webrtc:10480
Change-Id: I0e61f3311ecd140f38369cf68b6e5954f3dc1f5a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/193140
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32739}
Add inverted lags index to simplify the loop in
`FindBestPitchPeriod48kHz()`. Instead of looping over 294 items,
only loop over the relevant ones (up to 10) by keeping track of
the relevant indexes.
The benchmark has shown a slight improvement (about +6x).
Benchmarked as follows:
```
out/release/modules_unittests \
--gtest_filter=*RnnVadTest.DISABLED_RnnVadPerformance* \
--gtest_also_run_disabled_tests --logs
```
Results:
| baseline | this CL
------+----------------------+------------------------
run 1 | 22.8319 +/- 1.46554 | 22.1951 +/- 0.747611
| 389.367x | 400.539x
------+----------------------+------------------------
run 2 | 22.4286 +/- 0.726449 | 22.2718 +/- 0.963738
| 396.369x | 399.16x
------+----------------------+------------------------
run 2 | 22.5688 +/- 0.831341 | 22.4166 +/- 0.953362
| 393.906x | 396.581x
This CL also moved `PitchPseudoInterpolationInvLagAutoCorr()`
into `FindBestPitchPeriod48kHz()`.
Bug: webrtc:10480
Change-Id: Id4e6d755045c3198a80fa94a0a7463577d909b7e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191764
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32590}
This reverts commit 9da3e177fd.
Reason for revert: bug in ComputePitchPeriod48kHz()
Original change's description:
> RNN VAD: pitch search optimizations (part 1)
>
> TL;DR this CL improves efficiency and includes several code
> readability improvements mainly triggered by the comments to
> patch set #10.
>
> Highlights:
> - Split `FindBestPitchPeriods()` into 12 and 24 kHz versions
> to hard-code the input size and simplify the 24 kHz version
> - Loop in `ComputePitchPeriod48kHz()` (new name for
> `RefinePitchPeriod48kHz()`) removed since the lags for which
> we need to compute the auto correlation are a few
> - `ComputePitchGainThreshold()` was only used in unit tests; it's been
> moved into the anon ns and the test removed
>
> This CL makes `ComputePitchPeriod48kHz()` is about 10% faster (measured
> with https://webrtc-review.googlesource.com/c/src/+/191320/4/modules/audio_processing/agc2/rnn_vad/pitch_search_internal_unittest.cc).
> The realtime factor has improved by about +14%.
>
> Benchmarked as follows:
> ```
> out/release/modules_unittests \
> --gtest_filter=*RnnVadTest.DISABLED_RnnVadPerformance* \
> --gtest_also_run_disabled_tests --logs
> ```
>
> Results:
>
> | baseline | this CL
> ------+----------------------+------------------------
> run 1 | 24.0231 +/- 0.591016 | 23.568 +/- 0.990788
> | 370.06x | 377.207x
> ------+----------------------+------------------------
> run 2 | 24.0485 +/- 0.957498 | 23.3714 +/- 0.857523
> | 369.67x | 380.379x
> ------+----------------------+------------------------
> run 2 | 25.4091 +/- 2.6123 | 23.709 +/- 1.04477
> | 349.875x | 374.963x
>
> Bug: webrtc:10480
> Change-Id: I9a3e9164b2442114b928de506c92a547c273882f
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191320
> Reviewed-by: Per Åhgren <peah@webrtc.org>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32568}
TBR=alessiob@webrtc.org,peah@webrtc.org
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10480
Change-Id: I2a91f4f29566f872a7dfa220b31c6c625ed075db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/192660
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32581}
This reverts commit 2f7d1c62e2.
Reason for revert: bug in ancestor CL https://webrtc-review.googlesource.com/c/src/+/191320
Original change's description:
> RNN VAD: pitch search optimizations (part 2)
>
> This CL brings a large improvement to the VAD by precomputing the
> energy for the sliding frame `y` in the pitch buffer instead of
> computing them twice in two different places. The realtime factor
> has improved by about +16x.
>
> There is room for additional improvement (TODOs added), but that will
> be done in a follow up CL since the change won't be bit-exact and
> careful testing is needed.
>
> Benchmarked as follows:
> ```
> out/release/modules_unittests \
> --gtest_filter=*RnnVadTest.DISABLED_RnnVadPerformance* \
> --gtest_also_run_disabled_tests --logs
> ```
>
> Results:
>
> | baseline | this CL
> ------+----------------------+------------------------
> run 1 | 23.568 +/- 0.990788 | 22.8319 +/- 1.46554
> | 377.207x | 389.367x
> ------+----------------------+------------------------
> run 2 | 23.3714 +/- 0.857523 | 22.4286 +/- 0.726449
> | 380.379x | 396.369x
> ------+----------------------+------------------------
> run 2 | 23.709 +/- 1.04477 | 22.5688 +/- 0.831341
> | 374.963x | 393.906x
>
> Bug: webrtc:10480
> Change-Id: I599a4dda2bde16dc6c2f42cf89e96afbd4630311
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191484
> Reviewed-by: Per Åhgren <peah@webrtc.org>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32571}
TBR=alessiob@webrtc.org,peah@webrtc.org
Change-Id: I53e478d8d58912c7a5fae4ad8a8d1342a9a48091
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10480
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/192620
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32580}
This reverts commit ea89f2a447.
Reason for revert: bug in ancestor CL https://webrtc-review.googlesource.com/c/src/+/191320
Original change's description:
> RNN VAD: pitch search optimizations (part 3)
>
> `ComputeSlidingFrameSquareEnergies()` which computes the energy of a
> sliding 20 ms frame in the pitch buffer has been switched from backward
> to forward.
>
> The benchmark has shown a slight improvement (about +6x).
>
> This change is not bit exact but all the tolerance tests still pass
> except for one single case in `RnnVadTest,PitchSearchWithinTolerance`
> for which the tolerance has been slightly increased. Note that the pitch
> estimation is still bit-exact.
>
> Benchmarked as follows:
> ```
> out/release/modules_unittests \
> --gtest_filter=*RnnVadTest.DISABLED_RnnVadPerformance* \
> --gtest_also_run_disabled_tests --logs
> ```
>
> Results:
>
> | baseline | this CL
> ------+----------------------+------------------------
> run 1 | 22.8319 +/- 1.46554 | 22.087 +/- 0.552932
> | 389.367x | 402.499x
> ------+----------------------+------------------------
> run 2 | 22.4286 +/- 0.726449 | 22.216 +/- 0.916222
> | 396.369x | 400.162x
> ------+----------------------+------------------------
> run 2 | 22.5688 +/- 0.831341 | 22.4902 +/- 1.04881
> | 393.906x | 395.283x
>
> Bug: webrtc:10480
> Change-Id: I1fd54077a32e25e46196c8e18f003cd0ffd503e1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191703
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32572}
TBR=alessiob@webrtc.org,kwiberg@webrtc.org
Change-Id: I57a8f937ade0a35e1ccf0e229c391cc3a10e7c48
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10480
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/192621
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32578}
`ComputeSlidingFrameSquareEnergies()` which computes the energy of a
sliding 20 ms frame in the pitch buffer has been switched from backward
to forward.
The benchmark has shown a slight improvement (about +6x).
This change is not bit exact but all the tolerance tests still pass
except for one single case in `RnnVadTest,PitchSearchWithinTolerance`
for which the tolerance has been slightly increased. Note that the pitch
estimation is still bit-exact.
Benchmarked as follows:
```
out/release/modules_unittests \
--gtest_filter=*RnnVadTest.DISABLED_RnnVadPerformance* \
--gtest_also_run_disabled_tests --logs
```
Results:
| baseline | this CL
------+----------------------+------------------------
run 1 | 22.8319 +/- 1.46554 | 22.087 +/- 0.552932
| 389.367x | 402.499x
------+----------------------+------------------------
run 2 | 22.4286 +/- 0.726449 | 22.216 +/- 0.916222
| 396.369x | 400.162x
------+----------------------+------------------------
run 2 | 22.5688 +/- 0.831341 | 22.4902 +/- 1.04881
| 393.906x | 395.283x
Bug: webrtc:10480
Change-Id: I1fd54077a32e25e46196c8e18f003cd0ffd503e1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191703
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32572}
This CL brings a large improvement to the VAD by precomputing the
energy for the sliding frame `y` in the pitch buffer instead of
computing them twice in two different places. The realtime factor
has improved by about +16x.
There is room for additional improvement (TODOs added), but that will
be done in a follow up CL since the change won't be bit-exact and
careful testing is needed.
Benchmarked as follows:
```
out/release/modules_unittests \
--gtest_filter=*RnnVadTest.DISABLED_RnnVadPerformance* \
--gtest_also_run_disabled_tests --logs
```
Results:
| baseline | this CL
------+----------------------+------------------------
run 1 | 23.568 +/- 0.990788 | 22.8319 +/- 1.46554
| 377.207x | 389.367x
------+----------------------+------------------------
run 2 | 23.3714 +/- 0.857523 | 22.4286 +/- 0.726449
| 380.379x | 396.369x
------+----------------------+------------------------
run 2 | 23.709 +/- 1.04477 | 22.5688 +/- 0.831341
| 374.963x | 393.906x
Bug: webrtc:10480
Change-Id: I599a4dda2bde16dc6c2f42cf89e96afbd4630311
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191484
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32571}
TL;DR this CL improves efficiency and includes several code
readability improvements mainly triggered by the comments to
patch set #10.
Highlights:
- Split `FindBestPitchPeriods()` into 12 and 24 kHz versions
to hard-code the input size and simplify the 24 kHz version
- Loop in `ComputePitchPeriod48kHz()` (new name for
`RefinePitchPeriod48kHz()`) removed since the lags for which
we need to compute the auto correlation are a few
- `ComputePitchGainThreshold()` was only used in unit tests; it's been
moved into the anon ns and the test removed
This CL makes `ComputePitchPeriod48kHz()` is about 10% faster (measured
with https://webrtc-review.googlesource.com/c/src/+/191320/4/modules/audio_processing/agc2/rnn_vad/pitch_search_internal_unittest.cc).
The realtime factor has improved by about +14%.
Benchmarked as follows:
```
out/release/modules_unittests \
--gtest_filter=*RnnVadTest.DISABLED_RnnVadPerformance* \
--gtest_also_run_disabled_tests --logs
```
Results:
| baseline | this CL
------+----------------------+------------------------
run 1 | 24.0231 +/- 0.591016 | 23.568 +/- 0.990788
| 370.06x | 377.207x
------+----------------------+------------------------
run 2 | 24.0485 +/- 0.957498 | 23.3714 +/- 0.857523
| 369.67x | 380.379x
------+----------------------+------------------------
run 2 | 25.4091 +/- 2.6123 | 23.709 +/- 1.04477
| 349.875x | 374.963x
Bug: webrtc:10480
Change-Id: I9a3e9164b2442114b928de506c92a547c273882f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191320
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32568}
And use it in a few places that were using RTC_CHECK(false) or FATAL()
to do the exact same job. There should be no change in behavior.
Bug: none
Change-Id: I36d5e6bcf35fd41534e08a8c879fa0811b4f1967
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191963
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32567}
This CL includes two changes:
1. the type for (inverted) lags and pitch periods changed from
size_t to int to reduce the chance of bugs with pitch period
manipulations
2. CheckLowerPitchPeriodsAndComputePitchGain() is optimized by
replacing an unnecessary if statement inside the loop with the
predetermined number of loops
Bug: webrtc:10480
Change-Id: I38432699254b37a2c0111279c28be8dc65b87e9b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139252
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Reviewed-by: Fredrik Hernqvist <fhernqvist@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32521}
"warning: control reaches end of non-void function [-Wreturn-type]"
Reported by gcc (8.3)
In all the reported cases, the end of function is never actually
reached. Add RTC_CHECK(false) to ensure the compiler is aware that
this path is a dead-end.
Bug: webrtc:12008
Change-Id: I7f816fde3d1897ed2774057c7e05da66e1895e60
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189784
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Fabien VALLÉE <fabien.vallee@netgem.com>
Cr-Commit-Position: refs/heads/master@{#32503}
This CL removes the circular shift to compute the convolution in
`ComputeLpResidual()`, which is now 4x faster (benchmarked with the
`RnnVadTest.DISABLED_ComputeLpResidualBenchmark` unit test).
Note that the `RnnVadTest.LpResidualPipelineBitExactness` unit test
is still passing.
Bug: webrtc:10480
Change-Id: Ia7767d9b57378c12c8ff31f58fea03905be5c5de
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189964
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32491}
