zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Add one-stop-shop for built-in AEC toggling in APM"

Browse Source 
This reverts commit 771b50ca0b92477ce8aabba025e959790dd1a949.

Reason for revert: Introduces error-prone config.

Original change's description:
> Add one-stop-shop for built-in AEC toggling in APM
> 
> This does not change what AEC functionality is available.
> However, a client that only uses this interface - and not the submodule
> pointer accessors - gets simpler code, and is guaranteed not to run any
> two AECs in tandem.
> 
> The submodule interface EchoControlMobile is being deprecated in
> https://webrtc-review.googlesource.com/c/src/+/89392
> 
> Bug: webrtc:9535
> Change-Id: Id9326074e566be6d8768010fc421c457beff402c
> Reviewed-on: https://webrtc-review.googlesource.com/89386
> Commit-Queue: Sam Zackrisson <saza@webrtc.org>
> Reviewed-by: Per Åhgren <peah@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#24066}

TBR=saza@webrtc.org,peah@webrtc.org

Change-Id: I43283a1b22538a4caa77313499989146b2ce67f1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9535
Reviewed-on: https://webrtc-review.googlesource.com/90060
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24067}
This commit is contained in:
Sam Zackrisson
2018-07-23 14:48:07 +00:00
committed by Commit Bot
parent 771b50ca0b
commit 2a959d96c9
7 changed files with 30 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
modules/audio_processing/audio_processing_impl.cc
View File

@ -385,10 +385,8 @@ AudioProcessingImpl::AudioProcessingImpl(
rtc::CritScope cs_capture(&crit_capture_); rtc::CritScope cs_capture(&crit_capture_);
// Mark Echo Controller enabled if a factory is injected. // Mark Echo Controller enabled if a factory is injected.
if (echo_control_factory_) { capture_nonlocked_.echo_controller_enabled =
config_.echo_cancellation.enabled = true; static_cast<bool>(echo_control_factory_);
capture_nonlocked_.echo_controller_enabled = true;
}
public_submodules_->echo_cancellation.reset( public_submodules_->echo_cancellation.reset(
new EchoCancellationImpl(&crit_render_, &crit_capture_)); new EchoCancellationImpl(&crit_render_, &crit_capture_));
@ -493,15 +491,6 @@ int AudioProcessingImpl::MaybeInitialize(
} }
int AudioProcessingImpl::InitializeLocked() { int AudioProcessingImpl::InitializeLocked() {
// Ensure at most one built-in AEC is active, by arbitrarily preferring AEC2.
if (public_submodules_->echo_cancellation->is_enabled()) {
echo_control_mobile()->Enable(false);
config_.echo_cancellation.enabled = true;
config_.echo_cancellation.mobile_mode = false;
} else if (public_submodules_->echo_control_mobile->is_enabled()) {
config_.echo_cancellation.enabled = true;
config_.echo_cancellation.mobile_mode = true;
}
UpdateActiveSubmoduleStates(); UpdateActiveSubmoduleStates();
const int render_audiobuffer_num_output_frames = const int render_audiobuffer_num_output_frames =
@ -677,13 +666,6 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) {
rtc::CritScope cs_render(&crit_render_); rtc::CritScope cs_render(&crit_render_);
rtc::CritScope cs_capture(&crit_capture_); rtc::CritScope cs_capture(&crit_capture_);
capture_nonlocked_.echo_controller_enabled =
config_.echo_cancellation.enabled && private_submodules_->echo_controller;
echo_cancellation()->Enable(config_.echo_cancellation.enabled &&
!config_.echo_cancellation.mobile_mode);
echo_control_mobile()->Enable(config_.echo_cancellation.enabled &&
config_.echo_cancellation.mobile_mode);
InitializeLowCutFilter(); InitializeLowCutFilter();
RTC_LOG(LS_INFO) << "Highpass filter activated: " RTC_LOG(LS_INFO) << "Highpass filter activated: "
@ -1187,8 +1169,8 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
HandleCaptureRuntimeSettings(); HandleCaptureRuntimeSettings();
// Ensure that not both the AEC and AECM are active at the same time. // Ensure that not both the AEC and AECM are active at the same time.
// TODO(bugs.webrtc.org/9535): Simplify once the public API Enable functions // TODO(peah): Simplify once the public API Enable functions for these
// for these are moved to APM. // are moved to APM.
RTC_DCHECK(!(public_submodules_->echo_cancellation->is_enabled() && RTC_DCHECK(!(public_submodules_->echo_cancellation->is_enabled() &&
public_submodules_->echo_control_mobile->is_enabled())); public_submodules_->echo_control_mobile->is_enabled()));
@ -1215,7 +1197,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
levels.peak, 1, RmsLevel::kMinLevelDb, 64); levels.peak, 1, RmsLevel::kMinLevelDb, 64);
} }
if (capture_nonlocked_.echo_controller_enabled) { if (private_submodules_->echo_controller) {
// Detect and flag any change in the analog gain. // Detect and flag any change in the analog gain.
int analog_mic_level = gain_control()->stream_analog_level(); int analog_mic_level = gain_control()->stream_analog_level();
capture_.echo_path_gain_change = capture_.echo_path_gain_change =
@ -1239,7 +1221,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
capture_buffer->SplitIntoFrequencyBands(); capture_buffer->SplitIntoFrequencyBands();
} }
if (capture_nonlocked_.echo_controller_enabled) { if (private_submodules_->echo_controller) {
// Force down-mixing of the number of channels after the detection of // Force down-mixing of the number of channels after the detection of
// capture signal saturation. // capture signal saturation.
// TODO(peah): Look into ensuring that this kind of tampering with the // TODO(peah): Look into ensuring that this kind of tampering with the
@ -1249,7 +1231,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
// TODO(peah): Move the AEC3 low-cut filter to this place. // TODO(peah): Move the AEC3 low-cut filter to this place.
if (private_submodules_->low_cut_filter && if (private_submodules_->low_cut_filter &&
!capture_nonlocked_.echo_controller_enabled) { !private_submodules_->echo_controller) {
private_submodules_->low_cut_filter->Process(capture_buffer); private_submodules_->low_cut_filter->Process(capture_buffer);
} }
RETURN_ON_ERR( RETURN_ON_ERR(
@ -1259,11 +1241,11 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
// Ensure that the stream delay was set before the call to the // Ensure that the stream delay was set before the call to the
// AEC ProcessCaptureAudio function. // AEC ProcessCaptureAudio function.
if (public_submodules_->echo_cancellation->is_enabled() && if (public_submodules_->echo_cancellation->is_enabled() &&
!was_stream_delay_set()) { !private_submodules_->echo_controller && !was_stream_delay_set()) {
return AudioProcessing::kStreamParameterNotSetError; return AudioProcessing::kStreamParameterNotSetError;
} }
if (capture_nonlocked_.echo_controller_enabled) { if (private_submodules_->echo_controller) {
data_dumper_->DumpRaw("stream_delay", stream_delay_ms()); data_dumper_->DumpRaw("stream_delay", stream_delay_ms());
if (was_stream_delay_set()) { if (was_stream_delay_set()) {
@ -1303,7 +1285,8 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
return AudioProcessing::kStreamParameterNotSetError; return AudioProcessing::kStreamParameterNotSetError;
} }
if (public_submodules_->echo_control_mobile->is_enabled()) { if (!(private_submodules_->echo_controller ||
public_submodules_->echo_cancellation->is_enabled())) {
RETURN_ON_ERR(public_submodules_->echo_control_mobile->ProcessCaptureAudio( RETURN_ON_ERR(public_submodules_->echo_control_mobile->ProcessCaptureAudio(
capture_buffer, stream_delay_ms())); capture_buffer, stream_delay_ms()));
} }
@ -1521,7 +1504,7 @@ int AudioProcessingImpl::ProcessRenderStreamLocked() {
} }
// TODO(peah): Perform the queueing ínside QueueRenderAudiuo(). // TODO(peah): Perform the queueing ínside QueueRenderAudiuo().
if (capture_nonlocked_.echo_controller_enabled) { if (private_submodules_->echo_controller) {
private_submodules_->echo_controller->AnalyzeRender(render_buffer); private_submodules_->echo_controller->AnalyzeRender(render_buffer);
} }
@ -1644,7 +1627,7 @@ AudioProcessing::AudioProcessingStatistics AudioProcessingImpl::GetStatistics()
const { const {
AudioProcessingStatistics stats; AudioProcessingStatistics stats;
EchoCancellation::Metrics metrics; EchoCancellation::Metrics metrics;
if (capture_nonlocked_.echo_controller_enabled) { if (private_submodules_->echo_controller) {
rtc::CritScope cs_capture(&crit_capture_); rtc::CritScope cs_capture(&crit_capture_);
auto ec_metrics = private_submodules_->echo_controller->GetMetrics(); auto ec_metrics = private_submodules_->echo_controller->GetMetrics();
float erl = static_cast<float>(ec_metrics.echo_return_loss); float erl = static_cast<float>(ec_metrics.echo_return_loss);
@ -1680,7 +1663,7 @@ AudioProcessingStats AudioProcessingImpl::GetStatistics(
AudioProcessingStats stats; AudioProcessingStats stats;
if (has_remote_tracks) { if (has_remote_tracks) {
EchoCancellation::Metrics metrics; EchoCancellation::Metrics metrics;
if (capture_nonlocked_.echo_controller_enabled) { if (private_submodules_->echo_controller) {
rtc::CritScope cs_capture(&crit_capture_); rtc::CritScope cs_capture(&crit_capture_);
auto ec_metrics = private_submodules_->echo_controller->GetMetrics(); auto ec_metrics = private_submodules_->echo_controller->GetMetrics();
stats.echo_return_loss = ec_metrics.echo_return_loss; stats.echo_return_loss = ec_metrics.echo_return_loss;
@ -1870,8 +1853,8 @@ void AudioProcessingImpl::MaybeUpdateHistograms() {
static const int kMinDiffDelayMs = 60; static const int kMinDiffDelayMs = 60;
if (echo_cancellation()->is_enabled()) { if (echo_cancellation()->is_enabled()) {
// Activate delay_jumps_ counters if we know AEC2 is running. // Activate delay_jumps_ counters if we know echo_cancellation is running.
// If a stream has echo we know that echo cancellation is in process. // If a stream has echo we know that the echo_cancellation is in process.
if (capture_.stream_delay_jumps == -1 && if (capture_.stream_delay_jumps == -1 &&
echo_cancellation()->stream_has_echo()) { echo_cancellation()->stream_has_echo()) {
capture_.stream_delay_jumps = 0; capture_.stream_delay_jumps = 0;

2
modules/audio_processing/audio_processing_impl.h
View File

@ -393,8 +393,6 @@ class AudioProcessingImpl : public AudioProcessing {
int prev_analog_mic_level; int prev_analog_mic_level;
} capture_ RTC_GUARDED_BY(crit_capture_); } capture_ RTC_GUARDED_BY(crit_capture_);
// This struct requires EITHER crit_capture_ OR crit_render_ to read, and
// requires both locks to write.
struct ApmCaptureNonLockedState { struct ApmCaptureNonLockedState {
ApmCaptureNonLockedState(bool intelligibility_enabled) ApmCaptureNonLockedState(bool intelligibility_enabled)
: capture_processing_format(kSampleRate16kHz), : capture_processing_format(kSampleRate16kHz),

9
modules/audio_processing/audio_processing_unittest.cc
View File

@ -176,24 +176,23 @@ bool FrameDataAreEqual(const AudioFrame& frame1, const AudioFrame& frame2) {
} }
void EnableAllAPComponents(AudioProcessing* ap) { void EnableAllAPComponents(AudioProcessing* ap) {
AudioProcessing::Config apm_config;
#if defined(WEBRTC_AUDIOPROC_FIXED_PROFILE) #if defined(WEBRTC_AUDIOPROC_FIXED_PROFILE)
apm_config.echo_cancellation.enabled = true; EXPECT_NOERR(ap->echo_control_mobile()->Enable(true));
apm_config.echo_cancellation.mobile_mode = true;
EXPECT_NOERR(ap->gain_control()->set_mode(GainControl::kAdaptiveDigital)); EXPECT_NOERR(ap->gain_control()->set_mode(GainControl::kAdaptiveDigital));
EXPECT_NOERR(ap->gain_control()->Enable(true)); EXPECT_NOERR(ap->gain_control()->Enable(true));
#elif defined(WEBRTC_AUDIOPROC_FLOAT_PROFILE) #elif defined(WEBRTC_AUDIOPROC_FLOAT_PROFILE)
apm_config.echo_cancellation.enabled = true;
apm_config.echo_cancellation.mobile_mode = false;
EXPECT_NOERR(ap->echo_cancellation()->enable_drift_compensation(true)); EXPECT_NOERR(ap->echo_cancellation()->enable_drift_compensation(true));
EXPECT_NOERR(ap->echo_cancellation()->enable_metrics(true)); EXPECT_NOERR(ap->echo_cancellation()->enable_metrics(true));
EXPECT_NOERR(ap->echo_cancellation()->enable_delay_logging(true)); EXPECT_NOERR(ap->echo_cancellation()->enable_delay_logging(true));
EXPECT_NOERR(ap->echo_cancellation()->Enable(true));
EXPECT_NOERR(ap->gain_control()->set_mode(GainControl::kAdaptiveAnalog)); EXPECT_NOERR(ap->gain_control()->set_mode(GainControl::kAdaptiveAnalog));
EXPECT_NOERR(ap->gain_control()->set_analog_level_limits(0, 255)); EXPECT_NOERR(ap->gain_control()->set_analog_level_limits(0, 255));
EXPECT_NOERR(ap->gain_control()->Enable(true)); EXPECT_NOERR(ap->gain_control()->Enable(true));
#endif #endif
AudioProcessing::Config apm_config;
apm_config.high_pass_filter.enabled = true; apm_config.high_pass_filter.enabled = true;
ap->ApplyConfig(apm_config); ap->ApplyConfig(apm_config);

11
modules/audio_processing/include/audio_processing.h
View File

@ -253,13 +253,6 @@ class AudioProcessing : public rtc::RefCountInterface {
// by changing the default values in the AudioProcessing::Config struct. // by changing the default values in the AudioProcessing::Config struct.
// The config is applied by passing the struct to the ApplyConfig method. // The config is applied by passing the struct to the ApplyConfig method.
struct Config { struct Config {
// Configures whether acoustic echo cancellation is performed.
// Has a specific tuning for mobile devices.
struct EchoCancellation {
bool enabled = false;
bool mobile_mode = false;
} echo_cancellation;
struct ResidualEchoDetector { struct ResidualEchoDetector {
bool enabled = true; bool enabled = true;
} residual_echo_detector; } residual_echo_detector;
@ -803,7 +796,7 @@ class ProcessingConfig {
class EchoCancellation { class EchoCancellation {
public: public:
// EchoCancellation and EchoControlMobile may not be enabled simultaneously. // EchoCancellation and EchoControlMobile may not be enabled simultaneously.
// If both are enabled, one (unspecified) will automatically be disabled. // Enabling one will disable the other.
virtual int Enable(bool enable) = 0; virtual int Enable(bool enable) = 0;
virtual bool is_enabled() const = 0; virtual bool is_enabled() const = 0;
@ -907,7 +900,7 @@ class EchoCancellation {
class EchoControlMobile { class EchoControlMobile {
public: public:
// EchoCancellation and EchoControlMobile may not be enabled simultaneously. // EchoCancellation and EchoControlMobile may not be enabled simultaneously.
// If both are enabled, one (unspecified) will automatically be disabled. // Enabling one will disable the other.
virtual int Enable(bool enable) = 0; virtual int Enable(bool enable) = 0;
virtual bool is_enabled() const = 0; virtual bool is_enabled() const = 0;

12
modules/audio_processing/test/debug_dump_replayer.cc
View File

@ -202,10 +202,8 @@ void DebugDumpReplayer::ConfigureApm(const audioproc::Config& msg) {
// AEC configs. // AEC configs.
RTC_CHECK(msg.has_aec_enabled()); RTC_CHECK(msg.has_aec_enabled());
if (msg.aec_enabled()) { RTC_CHECK_EQ(AudioProcessing::kNoError,
apm_config.echo_cancellation.enabled = true; apm_->echo_cancellation()->Enable(msg.aec_enabled()));
apm_config.echo_cancellation.mobile_mode = false;
}
RTC_CHECK(msg.has_aec_drift_compensation_enabled()); RTC_CHECK(msg.has_aec_drift_compensation_enabled());
RTC_CHECK_EQ(AudioProcessing::kNoError, RTC_CHECK_EQ(AudioProcessing::kNoError,
@ -220,10 +218,8 @@ void DebugDumpReplayer::ConfigureApm(const audioproc::Config& msg) {
// AECM configs. // AECM configs.
RTC_CHECK(msg.has_aecm_enabled()); RTC_CHECK(msg.has_aecm_enabled());
if (msg.aecm_enabled()) { RTC_CHECK_EQ(AudioProcessing::kNoError,
apm_config.echo_cancellation.enabled = true; apm_->echo_control_mobile()->Enable(msg.aecm_enabled()));
apm_config.echo_cancellation.mobile_mode = true;
}
RTC_CHECK(msg.has_aecm_comfort_noise_enabled()); RTC_CHECK(msg.has_aecm_comfort_noise_enabled());
RTC_CHECK_EQ(AudioProcessing::kNoError, RTC_CHECK_EQ(AudioProcessing::kNoError,

9
modules/audio_processing/test/debug_dump_test.cc
View File

@ -368,9 +368,8 @@ TEST_F(DebugDumpTest, ToggleDelayAgnosticAec) {
generator.StartRecording(); generator.StartRecording();
generator.Process(100); generator.Process(100);
AudioProcessing::Config new_config; EchoCancellation* aec = generator.apm()->echo_cancellation();
new_config.echo_cancellation.enabled = true; EXPECT_EQ(AudioProcessing::kNoError, aec->Enable(!aec->is_enabled()));
generator.apm()->ApplyConfig(new_config);
generator.Process(100); generator.Process(100);
generator.StopRecording(); generator.StopRecording();
@ -408,7 +407,6 @@ TEST_F(DebugDumpTest, VerifyCombinedExperimentalStringInclusive) {
// Arbitrarily set clipping gain to 17, which will never be the default. // Arbitrarily set clipping gain to 17, which will never be the default.
config.Set<ExperimentalAgc>(new ExperimentalAgc(true, 0, 17)); config.Set<ExperimentalAgc>(new ExperimentalAgc(true, 0, 17));
bool enable_aec3 = true; bool enable_aec3 = true;
apm_config.echo_cancellation.enabled = true;
DebugDumpGenerator generator(config, apm_config, enable_aec3); DebugDumpGenerator generator(config, apm_config, enable_aec3);
generator.StartRecording(); generator.StartRecording();
generator.Process(100); generator.Process(100);
@ -465,8 +463,7 @@ TEST_F(DebugDumpTest, VerifyCombinedExperimentalStringExclusive) {
TEST_F(DebugDumpTest, VerifyAec3ExperimentalString) { TEST_F(DebugDumpTest, VerifyAec3ExperimentalString) {
Config config; Config config;
AudioProcessing::Config apm_config; AudioProcessing::Config apm_config;
apm_config.echo_cancellation.enabled = true; DebugDumpGenerator generator(config, apm_config, true);
DebugDumpGenerator generator(config, apm_config, true /* enable_aec3 */);
generator.StartRecording(); generator.StartRecording();
generator.Process(100); generator.Process(100);
generator.StopRecording(); generator.StopRecording();

2
test/fuzzers/audio_processing_configs_fuzzer.cc
View File

@ -128,7 +128,7 @@ std::unique_ptr<AudioProcessing> CreateApm(test::FuzzDataHelper* fuzz_data,
apm->AttachAecDump( apm->AttachAecDump(
absl::make_unique<testing::NiceMock<webrtc::test::MockAecDump>>()); absl::make_unique<testing::NiceMock<webrtc::test::MockAecDump>>());
webrtc::AudioProcessing::Config apm_config = apm->GetConfig(); webrtc::AudioProcessing::Config apm_config;
apm_config.residual_echo_detector.enabled = red; apm_config.residual_echo_detector.enabled = red;
apm_config.high_pass_filter.enabled = hpf; apm_config.high_pass_filter.enabled = hpf;
apm_config.gain_controller2.enabled = use_agc2_limiter; apm_config.gain_controller2.enabled = use_agc2_limiter;