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}
Sample rates not divisible by 100, in particular 11025 Hz and 22050 Hz, have long been used with APM in Chrome, but the support has never been stated explicitly.
This CL makes minor modifications to the APM API to clarify how rates are handled when 10 ms is not an integer number of samples. Unit tests are also extended to cover this case better.
This does not update all references to 10 ms and implicit floor(sample_rate/100) computations, but it does at least take us closer to a correct API.
Note that not all code needs to support these sample rates. For example, audio processing submodules only need to operate on the native APM rates 16000, 32000, 48000.
Bug: chromium:1332484
Change-Id: I1dad15468f6ccb9c0d4d09c5819fe87f8388d5b8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268769
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37682}
This is the first step of migrating
AudioProcessing::CreateAndAttachAecDump() from using std::string to
absl::string_view.
Bug: webrtc:13579
Change-Id: I8fc373e7ac55fd8e96bb0b01d1a30e28883ac9a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269400
Commit-Queue: Ali Tofigh <alito@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37631}
It is now easier to fully test `AgcManagerDirect` with different values
for the used field trials. In particular, this CL adds tests for the
field trial named `WebRTC-Audio-2ndAgcMinMicLevelExperiment`.
1. `UnmutingRaisesTooLowVolume` and `MicVolumeIsLimited`
The expectations for the lowest input volume are not hard-coded anymore
since the parametrized tests use different values for the enforced
minimum.
2. `RecoveryAfterManualLevelChangeBelowMin`
The recovery behavior after manual input volume change depends on
whether the minimum input volume is overridden. When that's the case,
the minimum volume is applied immediately after the manual adjustment.
Hence, the existing test is left and a parametrized version of it has been added to test the "instant recovery" behavior. The latter test is
skipped when the minimum input volume is not overridden since that case
is covered by the existing test.
Bug: chromium:1275566
Change-Id: Ib0d4427b32b88f33138d4062b365916a3c47a406
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268900
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37577}
Stop using TEST_F; that will make it easier to switch to parametric
tests that are needed to correctly test `AgcManagerDirect`.
"Avoid fixtures where reasonable."
Source: https://abseil.io/tips/122
Bug: chromium:1275566
Change-Id: I2d73a0913eb2349144f63bd17ab4d6efa245e472
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268766
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37556}
This reverts commit d0a6fd239c.
Reason for revert: reland the bug fix
Original change's description:
> Revert "`AgcManagerDirect`: stop enforcing min mic level override with 0 level"
>
> This reverts commit e76daab8b3.
>
> Reason for revert: revert required to revert the parent CL
>
> Original change's description:
> > `AgcManagerDirect`: stop enforcing min mic level override with 0 level
> >
> > https://webrtc-review.googlesource.com/c/src/+/250141 introduced a bug
> > due to which the min mic level override is always enforced, if specified
> > even if the user manually adjusts the mic level to zero.
> >
> > This CL fixes that bug, the changes run behind a kill switch.
> >
> > TESTED=Test video call on Chromium on Mac; input volume not adjusted after zeroing it from the system preferences UI
> >
> > Bug: chromium:1275566
> > Change-Id: I18ce2e5970d3002b301f51f84544583c64982d57
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267844
> > Reviewed-by: Hanna Silen <silen@webrtc.org>
> > Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#37460}
>
> Bug: chromium:1275566
> Change-Id: I6d22d8f3fafdc7da3814827b9b69146a506595db
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268468
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37515}
Bug: chromium:1275566
Change-Id: I7198587dec2a153270e8beb714e9dacccdaae806
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268544
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37530}
This reverts commit c9cad23274.
Reason for revert: add back field trial
Original change's description:
> Min mic analog level: override minimum and behavior on Mac
>
> This CL removes the `WebRTC-Audio-AgcMinMicLevelExperiment` field trial
> and always enables the code path behind that flag on Mac. In summary,
> the analog AGC behaves as follows on Mac:
> 1. the minimum level is overridden to 20
> 2. the minimum is applied even when clipping is detected
> 3. when the level is manually adjusted to 0, the minimum level is
> enforced - i.e., 20
>
> Note that the 3rd property had been unintentionally added when the
> changes were added behind the aforementioned field trial. This will
> be fixed in a follow-up CL.
>
> Bug: chromium:1275566
> Change-Id: If184c4455a0780fcd94f55141af34460c152e3c3
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266488
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Reviewed-by: Hanna Silen <silen@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37459}
Bug: chromium:1275566
Change-Id: I00a37ad9e16efc49f721558d25af16efd5f3db8c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268540
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37521}
This reverts commit e76daab8b3.
Reason for revert: revert required to revert the parent CL
Original change's description:
> `AgcManagerDirect`: stop enforcing min mic level override with 0 level
>
> https://webrtc-review.googlesource.com/c/src/+/250141 introduced a bug
> due to which the min mic level override is always enforced, if specified
> even if the user manually adjusts the mic level to zero.
>
> This CL fixes that bug, the changes run behind a kill switch.
>
> TESTED=Test video call on Chromium on Mac; input volume not adjusted after zeroing it from the system preferences UI
>
> Bug: chromium:1275566
> Change-Id: I18ce2e5970d3002b301f51f84544583c64982d57
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267844
> Reviewed-by: Hanna Silen <silen@webrtc.org>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37460}
Bug: chromium:1275566
Change-Id: I6d22d8f3fafdc7da3814827b9b69146a506595db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268468
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37515}
https://webrtc-review.googlesource.com/c/src/+/250141 introduced a bug
due to which the min mic level override is always enforced, if specified
even if the user manually adjusts the mic level to zero.
This CL fixes that bug, the changes run behind a kill switch.
TESTED=Test video call on Chromium on Mac; input volume not adjusted after zeroing it from the system preferences UI
Bug: chromium:1275566
Change-Id: I18ce2e5970d3002b301f51f84544583c64982d57
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267844
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37460}
This CL removes the `WebRTC-Audio-AgcMinMicLevelExperiment` field trial
and always enables the code path behind that flag on Mac. In summary,
the analog AGC behaves as follows on Mac:
1. the minimum level is overridden to 20
2. the minimum is applied even when clipping is detected
3. when the level is manually adjusted to 0, the minimum level is
enforced - i.e., 20
Note that the 3rd property had been unintentionally added when the
changes were added behind the aforementioned field trial. This will
be fixed in a follow-up CL.
Bug: chromium:1275566
Change-Id: If184c4455a0780fcd94f55141af34460c152e3c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266488
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37459}
This CL duplicates a few lines of utility code from
//modules/audio_processing:audioproc_test_utils (which contains more
testonly things) and allows the possibility to remove testonly from
the unpack_aecdump tool.
Bug: b/237526033
Change-Id: If2e1dd4cc825429c496091cf8640c67069fb6e6f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267701
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37437}
