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 2235776597e2f47ec353ac911428eb9a54d64a10
$ 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 8d7273357d92fab881561d886ce8dfe94e6e2238
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}
Enable by default the look for the first echo.
Bug: webrtc:14205
Change-Id: Iae904679c1432f3a0766263907cf376903685b97
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278043
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Commit-Queue: Lionel Koenig <lionelk@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38452}
Tested: Chromium built with this change; verified that the
behavior at the beginning of the call has not changed with
both low (< 12) and high (> 12) input volumes.
Bug: webrtc:7494
Change-Id: Ie184c994d46bf6fd1cb209873383b911beb766e3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278787
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38420}
The parameter is unused and it will be removed in [1]. This CL
isolates the necessary unit test changes from [1].
[1] https://webrtc-review.googlesource.com/c/src/+/278787
Bug: webrtc:7494
Change-Id: Ic1179d335926fba8ff1b65b494b538cf849724bb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279100
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38416}
This CL:
- makes it easier to understand the (nontrivial) metric interpretation
- corrects the computation of BufferDelay to use 0 for absent delay
- deletes metric MaxSkewShiftCount, unused since https://webrtc-review.googlesource.com/c/src/+/119701
- updates the unit test to directly test metric reporting
Corresponding update to histograms.xml:
https://crrev.com/c/3944909
Previous revert:
https://webrtc-review.googlesource.com/c/src/+/279040
This CL is identical to the original, except:
- the test is updated to spam fewer EXPECT_EQ failures on failure (EXPECT_EQs moved out of inner loop)
- the test not resets metrics (metrics::Reset()) at the beginning, like other histogram tests
Bug: webrtc:8671, chromium:1349051
Change-Id: Ie802e1f9d03a22ff7018f522a63b19e0b6eec2e8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279046
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38376}
- Set the initial input volume to that forced by startup min volume
since the latter is removed in a follow-up CL
- Remove unwanted expectations
Bug: webrtc:7494
Change-Id: I2df28f5bfaf4e592dfeae5e03b157268473cc822
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278784
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38370}
This CL:
- makes it easier to understand the (nontrivial) metric interpretation
- corrects the computation of BufferDelay to use 0 for absent delay
- deletes metric MaxSkewShiftCount, unused since https://webrtc-review.googlesource.com/c/src/+/119701
- updates the unit test to directly test metric reporting
Corresponding update to histograms.xml:
https://crrev.com/c/3944909
Bug: webrtc:8671, chromium:1349051
Change-Id: If73b6fca4de7343bff2c53f72cedda458d36c599
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278782
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38362}
This CL adds #includes to header files in order to make them
self contained after the preprocessor pass.
Bug: b/251890128
Change-Id: I81c3ba38fb8ab8a2bbd151ba99aa871fae9f1b1b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278422
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38327}
Remove the evaluation of clipping prediction. The result is not used.
Bug: webrtc:7494
Change-Id: I18d2c1f50ed675a9653d518095f69ed263a34041
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278361
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38320}
Add passing optional speech level and speech probability to Process().
This enables computing an override for the RMS error from
Agc::GetRmsErrorDb(). Currently no speech level or probability are
passed outside the tests and no override happens elsewhere.
Bug: webrtc:7494
Change-Id: I0a7b1204aa51bcde8588963a5af023410405e83d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277560
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38318}
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}
MaybeInitializeCapture may overwrite the render configuration of a concurrent render reinitialization, leading to a second render reinitialization on the next render processing call.
See bug description for details.
Tested: Verified bitexactness offline (single-threaded) on a large number of aecdumps.
Bug: webrtc:14495
Change-Id: I9b70b454ce1c27859c3414c9c9ec89b7bbe35559
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277380
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38241}
Main changes:
- `AgcManagerDirectTestHelper::FirstProcess()` replaced by
`CallAgcSequence()`, which is API contract compliant
- `ExpectCheckVolumeAndReset()`, `SetVolumeAndProcess()` and
`ExpectInitialize() `removed
- TODOs added for the next batch of improvements
- `AgcManagerDirectTestHelper::mock_agc` now using `NiceMock`
- `AgcManagerDirect::(AnalyzePre)Process()` now receives a
const ref
- `AnalyzePreProcess(const float* const*,size_t )` removed
Bug: webrtc:7494
Change-Id: Ie5bbaa590586dd806b30494fb00ca9c742c241e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273490
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38056}
The current design of the modified getter is error-prone since the
returned value changes meaning based on when (which point in the code)
the getter is called - namely, before `ProcessStream()` is called the
getter returns the stream analog level, after it returns the
recommended level.
Plus, the new implementation, which essentially returns a local
member, removes the risks that the non-trivial implementation
is computationally expensive.
Bug: webrtc:7494, b/241923537
Change-Id: I6714444df27bcc055ae693974ecd1f77f79e6ec0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271580
Reviewed-by: Hanna Silen <silen@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38055}
The `recommended_stream_analog_level()` getter is used to retrieve
both the applied and the recommended input volume. This behavior is
error-prone since the caller must know what is returned based on
the point in the code (namely, before/after the AGC has changed
the last applied input volume into a recommended level).
This CL is a first step to make clarity on which input volume is
handled in different parts of APM. Next in the pipeline: make
`recommended_stream_analog_level()` a trivial getter that always
returns the recommended level.
Main changes:
- When `recommended_stream_analog_level()` is called but
`set_stream_analog_level()` is not called, APM logs an error
and returns a fall-back volume (which should not be applied
since, when `set_stream_analog_level()` is not called, no
external input volume is expected to be present
- When APM is used without calling the `*_stream_analog_level()`
methods (e.g., when the caller does not provide any input volume),
the recorded AEC dumps won't store `Stream::applied_input_level`
Other changes:
- Removed `AudioProcessingImpl::capture_::prev_analog_mic_level`
- Removed redundant code in `GainController2` around detecting
input volume changes (already done by APM)
- Adapted the `audioproc_f` and `unpack_aecdump` tools
- Data dumps clean-up: the applied and the recommended input
volumes are now recorded in an AGC implementation agnostic way
Bug: webrtc:7494, b/241923537
Change-Id: I3cb4a731fd9f3dc19bf6ac679b7ed8c969ea283b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271544
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38054}
Switching to an AGC implementation agnostic solution for the input
volume emulation functionality offered by the
`capture_levels_adjuster` sub-module.
This CL also fixes a (silent) bug due to which, when the input
volume is emulated via the capture adjuster sub-module, AGC2
reads an incorrect value for the applied input volume.
Tested: audioproc_f with `--analog_mic_gain_emulation 1` used
to verify bit-exactness for one Wav file and one AEC dump for
which the input volume varies.
Bug: webrtc:7494, b/241923537
Change-Id: Ide3085f9a5dfd85888ad812ebd56faa175fb2ba7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273902
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38053}
`EchoControllerObservesAnalogAgc1EchoPathGainChange` is incorrect
since it does not call `set_stream_analog_level()`,
`ProcessCapture()` and `recommended_stream_analog_level()`
according to the contract.
`EchoControllerObservesNoDigitalAgc2EchoPathGainChange` is
useless since AGC2 doesn't have any analog controller at the
moment and the test is not written to explictly trigger digital
gain adaptations.
Bug: webrtc:7494, b/241923537
Change-Id: I56203c736448ec060077b00b57e98cd4c29fa737
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271541
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38052}
Making it clear that the field is used to store the applied input
volume and not the recommended input volume.
Bug: webrtc:7494, b/241923537
Change-Id: Ib91bc1a12348f63e3a4ba6e068ed02e40786a87b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271342
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38051}
Fixing a bug due to which the analog controller could not be disabled.
AudioProcessing::Config::GainController1::AnalogGainController::enabled
was ignored and therefore `recommended_stream_analog_level_locked()` in
APM was returning the level recommended by `AgcManagerDirect`.
When the analog controller is disabled, `stream_analog_level()` now
returns the last value set via `set_stream_analog_level()`.
However, the analog controller code is still running and, in particular,
the existing metrics are reported as if the controller were enabled.
This choice was made to reduce the risks of adding bugs in the digital
compression gain selection part, which is tied to the analog
controller. The metric drawback will be solved in a follow-up CL.
Additional changes:
- log `WebRTC.Audio.GainController.Analog.Enabled` when
AGC1 is created or when its config changes
- first step to replace "analog level" with "input volume"
Bug: webrtc:7909, b/180019868
Change-Id: I28ce9556dd98f3dd9ad546799406c55478730435
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/270663
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38044}
Instead of showing individual byte differences, this CL detects
differences in the expected and actual byte streams of the evaluated
AEC dump and, if detected, parses the `audioproc::Event` proto lite
messages and calls EXPECT_EQ() for a subset of individual (sub-)fields.
Note that messages are parsed only if the byte streams of each message
pair do not match, so with no failures the test runs at no extra cost.
Plus, the the added funcionality can only be enabled for local
debugging by flipping the `kDumpWhenExpectMessageEqFails` flag - a
code change cannot land if the flag is set to true.
Note that `MessageDifferencer` (see [1]) could not be used because
it is not implemented for `MessageLite` protos.
[1] https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.util.message_differencer
Bug: b/241923537
Change-Id: I8e0eda3b1ecfe06945b6dad5ee8871f8200d76d7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/270922
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37765}
