From 0f133b99c655cbdb347b4a71ac872c071532189f Mon Sep 17 00:00:00 2001 From: "henrik.lundin" Date: Thu, 2 Jul 2015 00:17:55 -0700 Subject: [PATCH] Rename APM Config ReportedDelay to DelayAgnostic We use this Config struct for enabling/disabling the delay agnostic AEC. This change renames it to DelayAgnostic for readability reasons. NOTE: The logic is reversed in this CL. The old ReportedDelay config turned DA-AEC off, while the new DelayAgnostic turns it on. The old Config is kept in parallel with the new during a transition period. This is to avoid problems with API breakages. During this period, ReportedDelay is disabled or DelayAgnostic is enabled, DA-AEC is engaged in APM. BUG=webrtc:4651 R=bjornv@webrtc.org, tommi@webrtc.org Review URL: https://codereview.webrtc.org/1211053006 Cr-Commit-Position: refs/heads/master@{#9531} --- talk/media/webrtc/webrtcvoiceengine.cc | 4 +- .../modules/audio_processing/aec/aec_core.c | 16 ++++---- .../modules/audio_processing/aec/aec_core.h | 7 ++-- .../audio_processing/aec/aec_core_internal.h | 6 +-- .../audio_processing/aec/echo_cancellation.c | 2 +- .../aec/system_delay_unittest.cc | 38 +++++++++---------- .../echo_cancellation_impl.cc | 10 +++-- .../audio_processing/echo_cancellation_impl.h | 2 +- .../echo_cancellation_impl_unittest.cc | 16 ++++---- .../include/audio_processing.h | 8 ++++ .../test/audio_processing_unittest.cc | 2 +- .../audio_processing/test/process_test.cc | 5 ++- 12 files changed, 65 insertions(+), 51 deletions(-) diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 84a26c51b8..284afed4d1 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -844,8 +844,8 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { bool delay_agnostic_aec; if (delay_agnostic_aec_.Get(&delay_agnostic_aec)) { LOG(LS_INFO) << "Delay agnostic aec is enabled? " << delay_agnostic_aec; - config.Set( - new webrtc::ReportedDelay(!delay_agnostic_aec)); + config.Set( + new webrtc::DelayAgnostic(delay_agnostic_aec)); } extended_filter_aec_.SetFrom(options.extended_filter_aec); diff --git a/webrtc/modules/audio_processing/aec/aec_core.c b/webrtc/modules/audio_processing/aec/aec_core.c index a3f9f24cfd..70927074f8 100644 --- a/webrtc/modules/audio_processing/aec/aec_core.c +++ b/webrtc/modules/audio_processing/aec/aec_core.c @@ -1452,12 +1452,12 @@ AecCore* WebRtcAec_CreateAec() { return NULL; } #ifdef WEBRTC_ANDROID - aec->reported_delay_enabled = 0; // DA-AEC enabled by default. + aec->delay_agnostic_enabled = 1; // DA-AEC enabled by default. // DA-AEC assumes the system is causal from the beginning and will self adjust // the lookahead when shifting is required. WebRtc_set_lookahead(aec->delay_estimator, 0); #else - aec->reported_delay_enabled = 1; + aec->delay_agnostic_enabled = 0; WebRtc_set_lookahead(aec->delay_estimator, kLookaheadBlocks); #endif aec->extended_filter_enabled = 0; @@ -1786,7 +1786,7 @@ void WebRtcAec_ProcessFrames(AecCore* aec, WebRtcAec_MoveFarReadPtr(aec, -(aec->mult + 1)); } - if (aec->reported_delay_enabled) { + if (!aec->delay_agnostic_enabled) { // 2 a) Compensate for a possible change in the system delay. // TODO(bjornv): Investigate how we should round the delay difference; @@ -1900,18 +1900,18 @@ void WebRtcAec_SetConfigCore(AecCore* self, } // Turn on delay logging if it is either set explicitly or if delay agnostic // AEC is enabled (which requires delay estimates). - self->delay_logging_enabled = delay_logging || !self->reported_delay_enabled; + self->delay_logging_enabled = delay_logging || self->delay_agnostic_enabled; if (self->delay_logging_enabled) { memset(self->delay_histogram, 0, sizeof(self->delay_histogram)); } } -void WebRtcAec_enable_reported_delay(AecCore* self, int enable) { - self->reported_delay_enabled = enable; +void WebRtcAec_enable_delay_agnostic(AecCore* self, int enable) { + self->delay_agnostic_enabled = enable; } -int WebRtcAec_reported_delay_enabled(AecCore* self) { - return self->reported_delay_enabled; +int WebRtcAec_delay_agnostic_enabled(AecCore* self) { + return self->delay_agnostic_enabled; } void WebRtcAec_enable_extended_filter(AecCore* self, int enable) { diff --git a/webrtc/modules/audio_processing/aec/aec_core.h b/webrtc/modules/audio_processing/aec/aec_core.h index 2fa26db859..2530527560 100644 --- a/webrtc/modules/audio_processing/aec/aec_core.h +++ b/webrtc/modules/audio_processing/aec/aec_core.h @@ -103,10 +103,11 @@ void WebRtcAec_SetConfigCore(AecCore* self, int delay_logging); // Non-zero enables, zero disables. -void WebRtcAec_enable_reported_delay(AecCore* self, int enable); +void WebRtcAec_enable_delay_agnostic(AecCore* self, int enable); -// Returns non-zero if reported delay is enabled and zero if disabled. -int WebRtcAec_reported_delay_enabled(AecCore* self); +// Returns non-zero if delay agnostic (i.e., signal based delay estimation) is +// enabled and zero if disabled. +int WebRtcAec_delay_agnostic_enabled(AecCore* self); // Enables or disables extended filter mode. Non-zero enables, zero disables. void WebRtcAec_enable_extended_filter(AecCore* self, int enable); diff --git a/webrtc/modules/audio_processing/aec/aec_core_internal.h b/webrtc/modules/audio_processing/aec/aec_core_internal.h index 2f795896f5..796ea2c9c0 100644 --- a/webrtc/modules/audio_processing/aec/aec_core_internal.h +++ b/webrtc/modules/audio_processing/aec/aec_core_internal.h @@ -144,9 +144,9 @@ struct AecCore { float delay_quality_threshold; int frame_count; - // 0 = reported delay mode disabled (signal based delay correction enabled). - // otherwise enabled - int reported_delay_enabled; + // 0 = delay agnostic mode (signal based delay correction) disabled. + // Otherwise enabled. + int delay_agnostic_enabled; // 1 = extended filter mode enabled, 0 = disabled. int extended_filter_enabled; // Runtime selection of number of filter partitions. diff --git a/webrtc/modules/audio_processing/aec/echo_cancellation.c b/webrtc/modules/audio_processing/aec/echo_cancellation.c index 5dfe7ad4d5..b31a84a87a 100644 --- a/webrtc/modules/audio_processing/aec/echo_cancellation.c +++ b/webrtc/modules/audio_processing/aec/echo_cancellation.c @@ -238,7 +238,7 @@ int32_t WebRtcAec_Init(void* aecInst, int32_t sampFreq, int32_t scSampFreq) { // We skip the startup_phase completely (setting to 0) if DA-AEC is enabled, // but not extended_filter mode. aecpc->startup_phase = WebRtcAec_extended_filter_enabled(aecpc->aec) || - WebRtcAec_reported_delay_enabled(aecpc->aec); + !WebRtcAec_delay_agnostic_enabled(aecpc->aec); aecpc->bufSizeStart = 0; aecpc->checkBufSizeCtr = 0; aecpc->msInSndCardBuf = 0; diff --git a/webrtc/modules/audio_processing/aec/system_delay_unittest.cc b/webrtc/modules/audio_processing/aec/system_delay_unittest.cc index 31401d4f29..5e26a31898 100644 --- a/webrtc/modules/audio_processing/aec/system_delay_unittest.cc +++ b/webrtc/modules/audio_processing/aec/system_delay_unittest.cc @@ -136,7 +136,7 @@ void SystemDelayTest::RunStableStartup() { // Process(). int buffer_size = BufferFillUp(); - if (WebRtcAec_reported_delay_enabled(self_->aec) == 0) { + if (WebRtcAec_delay_agnostic_enabled(self_->aec) == 1) { // In extended_filter mode we set the buffer size after the first processed // 10 ms chunk. Hence, we don't need to wait for the reported system delay // values to become stable. @@ -198,8 +198,8 @@ TEST_F(SystemDelayTest, CorrectIncreaseWhenBufferFarend) { WebRtcAec_enable_extended_filter(self_->aec, extended_filter); EXPECT_EQ(extended_filter, WebRtcAec_extended_filter_enabled(self_->aec)); for (int da_aec = 0; da_aec <= 1; ++da_aec) { - WebRtcAec_enable_reported_delay(self_->aec, 1 - da_aec); - EXPECT_EQ(1 - da_aec, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, da_aec); + EXPECT_EQ(da_aec, WebRtcAec_delay_agnostic_enabled(self_->aec)); for (size_t i = 0; i < kNumSampleRates; i++) { Init(kSampleRateHz[i]); // Loop through a couple of calls to make sure the system delay @@ -225,8 +225,8 @@ TEST_F(SystemDelayTest, CorrectDelayAfterStableStartup) { WebRtcAec_enable_extended_filter(self_->aec, extended_filter); EXPECT_EQ(extended_filter, WebRtcAec_extended_filter_enabled(self_->aec)); for (int da_aec = 0; da_aec <= 1; ++da_aec) { - WebRtcAec_enable_reported_delay(self_->aec, 1 - da_aec); - EXPECT_EQ(1 - da_aec, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, da_aec); + EXPECT_EQ(da_aec, WebRtcAec_delay_agnostic_enabled(self_->aec)); for (size_t i = 0; i < kNumSampleRates; i++) { Init(kSampleRateHz[i]); RunStableStartup(); @@ -253,8 +253,8 @@ TEST_F(SystemDelayTest, CorrectDelayAfterUnstableStartup) { // it apply if DA-AEC is on because that overrides the startup procedure. WebRtcAec_enable_extended_filter(self_->aec, 0); EXPECT_EQ(0, WebRtcAec_extended_filter_enabled(self_->aec)); - WebRtcAec_enable_reported_delay(self_->aec, 1); - EXPECT_EQ(1, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, 0); + EXPECT_EQ(0, WebRtcAec_delay_agnostic_enabled(self_->aec)); // In an unstable system we would start processing after |kMaxConvergenceMs|. // On the last frame the AEC buffer is adjusted to 60% of the last reported @@ -302,8 +302,8 @@ TEST_F(SystemDelayTest, CorrectDelayAfterStableBufferBuildUp) { // it apply if DA-AEC is on because that overrides the startup procedure. WebRtcAec_enable_extended_filter(self_->aec, 0); EXPECT_EQ(0, WebRtcAec_extended_filter_enabled(self_->aec)); - WebRtcAec_enable_reported_delay(self_->aec, 1); - EXPECT_EQ(1, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, 0); + EXPECT_EQ(0, WebRtcAec_delay_agnostic_enabled(self_->aec)); // In this test we start by establishing the device buffer size during stable // conditions, but with an empty internal far-end buffer. Once that is done we @@ -367,8 +367,8 @@ TEST_F(SystemDelayTest, CorrectDelayWhenBufferUnderrun) { WebRtcAec_enable_extended_filter(self_->aec, extended_filter); EXPECT_EQ(extended_filter, WebRtcAec_extended_filter_enabled(self_->aec)); for (int da_aec = 0; da_aec <= 1; ++da_aec) { - WebRtcAec_enable_reported_delay(self_->aec, 1 - da_aec); - EXPECT_EQ(1 - da_aec, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, da_aec); + EXPECT_EQ(da_aec, WebRtcAec_delay_agnostic_enabled(self_->aec)); for (size_t i = 0; i < kNumSampleRates; i++) { Init(kSampleRateHz[i]); RunStableStartup(); @@ -397,8 +397,8 @@ TEST_F(SystemDelayTest, CorrectDelayDuringDrift) { WebRtcAec_enable_extended_filter(self_->aec, extended_filter); EXPECT_EQ(extended_filter, WebRtcAec_extended_filter_enabled(self_->aec)); for (int da_aec = 0; da_aec <= 1; ++da_aec) { - WebRtcAec_enable_reported_delay(self_->aec, 1 - da_aec); - EXPECT_EQ(1 - da_aec, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, da_aec); + EXPECT_EQ(da_aec, WebRtcAec_delay_agnostic_enabled(self_->aec)); for (size_t i = 0; i < kNumSampleRates; i++) { Init(kSampleRateHz[i]); RunStableStartup(); @@ -441,8 +441,8 @@ TEST_F(SystemDelayTest, ShouldRecoverAfterGlitch) { WebRtcAec_enable_extended_filter(self_->aec, extended_filter); EXPECT_EQ(extended_filter, WebRtcAec_extended_filter_enabled(self_->aec)); for (int da_aec = 0; da_aec <= 1; ++da_aec) { - WebRtcAec_enable_reported_delay(self_->aec, 1 - da_aec); - EXPECT_EQ(1 - da_aec, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, da_aec); + EXPECT_EQ(da_aec, WebRtcAec_delay_agnostic_enabled(self_->aec)); for (size_t i = 0; i < kNumSampleRates; i++) { Init(kSampleRateHz[i]); RunStableStartup(); @@ -498,8 +498,8 @@ TEST_F(SystemDelayTest, UnaffectedWhenSpuriousDeviceBufferValues) { // Should be DA-AEC independent. for (int da_aec = 0; da_aec <= 1; ++da_aec) { - WebRtcAec_enable_reported_delay(self_->aec, 1 - da_aec); - EXPECT_EQ(1 - da_aec, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, da_aec); + EXPECT_EQ(da_aec, WebRtcAec_delay_agnostic_enabled(self_->aec)); // This spurious device buffer data test aims at verifying that the system // delay is unaffected by large outliers. // The system is said to be in a non-causal state if the difference between @@ -552,8 +552,8 @@ TEST_F(SystemDelayTest, CorrectImpactWhenTogglingDeviceBufferValues) { WebRtcAec_enable_extended_filter(self_->aec, extended_filter); EXPECT_EQ(extended_filter, WebRtcAec_extended_filter_enabled(self_->aec)); for (int da_aec = 0; da_aec <= 1; ++da_aec) { - WebRtcAec_enable_reported_delay(self_->aec, 1 - da_aec); - EXPECT_EQ(1 - da_aec, WebRtcAec_reported_delay_enabled(self_->aec)); + WebRtcAec_enable_delay_agnostic(self_->aec, da_aec); + EXPECT_EQ(da_aec, WebRtcAec_delay_agnostic_enabled(self_->aec)); if (extended_filter == 0 && da_aec == 1) { continue; } diff --git a/webrtc/modules/audio_processing/echo_cancellation_impl.cc b/webrtc/modules/audio_processing/echo_cancellation_impl.cc index dff47cca59..96e77c5963 100644 --- a/webrtc/modules/audio_processing/echo_cancellation_impl.cc +++ b/webrtc/modules/audio_processing/echo_cancellation_impl.cc @@ -68,7 +68,7 @@ EchoCancellationImpl::EchoCancellationImpl(const AudioProcessing* apm, stream_has_echo_(false), delay_logging_enabled_(false), extended_filter_enabled_(false), - reported_delay_enabled_(true) { + delay_agnostic_enabled_(false) { } EchoCancellationImpl::~EchoCancellationImpl() {} @@ -329,7 +329,8 @@ int EchoCancellationImpl::Initialize() { void EchoCancellationImpl::SetExtraOptions(const Config& config) { extended_filter_enabled_ = config.Get().enabled; - reported_delay_enabled_ = config.Get().enabled; + delay_agnostic_enabled_ = config.Get().enabled || + !config.Get().enabled; Configure(); } @@ -363,8 +364,9 @@ int EchoCancellationImpl::ConfigureHandle(void* handle) const { WebRtcAec_enable_extended_filter( WebRtcAec_aec_core(static_cast(handle)), extended_filter_enabled_ ? 1 : 0); - WebRtcAec_enable_reported_delay(WebRtcAec_aec_core( - static_cast(handle)), reported_delay_enabled_ ? 1 : 0); + WebRtcAec_enable_delay_agnostic( + WebRtcAec_aec_core(static_cast(handle)), + delay_agnostic_enabled_ ? 1 : 0); return WebRtcAec_set_config(static_cast(handle), config); } diff --git a/webrtc/modules/audio_processing/echo_cancellation_impl.h b/webrtc/modules/audio_processing/echo_cancellation_impl.h index cf3ebf61af..9c2b32c473 100644 --- a/webrtc/modules/audio_processing/echo_cancellation_impl.h +++ b/webrtc/modules/audio_processing/echo_cancellation_impl.h @@ -75,7 +75,7 @@ class EchoCancellationImpl : public EchoCancellation, bool stream_has_echo_; bool delay_logging_enabled_; bool extended_filter_enabled_; - bool reported_delay_enabled_; + bool delay_agnostic_enabled_; }; } // namespace webrtc diff --git a/webrtc/modules/audio_processing/echo_cancellation_impl_unittest.cc b/webrtc/modules/audio_processing/echo_cancellation_impl_unittest.cc index 63b6db919f..b2e11981fa 100644 --- a/webrtc/modules/audio_processing/echo_cancellation_impl_unittest.cc +++ b/webrtc/modules/audio_processing/echo_cancellation_impl_unittest.cc @@ -48,7 +48,7 @@ TEST(EchoCancellationInternalTest, ExtendedFilter) { EXPECT_EQ(0, WebRtcAec_extended_filter_enabled(aec_core)); } -TEST(EchoCancellationInternalTest, ReportedDelay) { +TEST(EchoCancellationInternalTest, DelayAgnostic) { rtc::scoped_ptr ap(AudioProcessing::Create()); EXPECT_TRUE(ap->echo_cancellation()->aec_core() == NULL); @@ -58,24 +58,24 @@ TEST(EchoCancellationInternalTest, ReportedDelay) { AecCore* aec_core = ap->echo_cancellation()->aec_core(); ASSERT_TRUE(aec_core != NULL); // Enabled by default. - EXPECT_EQ(1, WebRtcAec_reported_delay_enabled(aec_core)); + EXPECT_EQ(0, WebRtcAec_delay_agnostic_enabled(aec_core)); Config config; - config.Set(new ReportedDelay(false)); + config.Set(new DelayAgnostic(true)); ap->SetExtraOptions(config); - EXPECT_EQ(0, WebRtcAec_reported_delay_enabled(aec_core)); + EXPECT_EQ(1, WebRtcAec_delay_agnostic_enabled(aec_core)); // Retains setting after initialization. EXPECT_EQ(ap->kNoError, ap->Initialize()); - EXPECT_EQ(0, WebRtcAec_reported_delay_enabled(aec_core)); + EXPECT_EQ(1, WebRtcAec_delay_agnostic_enabled(aec_core)); - config.Set(new ReportedDelay(true)); + config.Set(new DelayAgnostic(false)); ap->SetExtraOptions(config); - EXPECT_EQ(1, WebRtcAec_reported_delay_enabled(aec_core)); + EXPECT_EQ(0, WebRtcAec_delay_agnostic_enabled(aec_core)); // Retains setting after initialization. EXPECT_EQ(ap->kNoError, ap->Initialize()); - EXPECT_EQ(1, WebRtcAec_reported_delay_enabled(aec_core)); + EXPECT_EQ(0, WebRtcAec_delay_agnostic_enabled(aec_core)); } } // namespace webrtc diff --git a/webrtc/modules/audio_processing/include/audio_processing.h b/webrtc/modules/audio_processing/include/audio_processing.h index 2163a9ecf2..80b7b4d2f3 100644 --- a/webrtc/modules/audio_processing/include/audio_processing.h +++ b/webrtc/modules/audio_processing/include/audio_processing.h @@ -66,11 +66,19 @@ struct ExtendedFilter { // and not EchoControlMobile and is set with AudioProcessing::SetExtraOptions(). // Note that by disabling reported system delays the EchoCancellation may // regress in performance. +// TODO(henrik.lundin): Remove ReportedDelay once DelayAgnostic has +// propagated through to all channels +// (https://code.google.com/p/webrtc/issues/detail?id=4651). struct ReportedDelay { ReportedDelay() : enabled(true) {} explicit ReportedDelay(bool enabled) : enabled(enabled) {} bool enabled; }; +struct DelayAgnostic { + DelayAgnostic() : enabled(false) {} + explicit DelayAgnostic(bool enabled) : enabled(enabled) {} + bool enabled; +}; // Use to enable experimental gain control (AGC). At startup the experimental // AGC moves the microphone volume up to |startup_min_volume| if the current diff --git a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc index c19449ae25..c7c45f7eeb 100644 --- a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc +++ b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc @@ -893,7 +893,7 @@ TEST_F(ApmTest, DISABLED_EchoCancellationReportsCorrectDelays) { apm_->echo_cancellation()->enable_delay_logging(true)); EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true)); Config config; - config.Set(new ReportedDelay(true)); + config.Set(new DelayAgnostic(false)); apm_->SetExtraOptions(config); // Internally in the AEC the amount of lookahead the delay estimation can diff --git a/webrtc/modules/audio_processing/test/process_test.cc b/webrtc/modules/audio_processing/test/process_test.cc index 0f28a44492..715762f31e 100644 --- a/webrtc/modules/audio_processing/test/process_test.cc +++ b/webrtc/modules/audio_processing/test/process_test.cc @@ -261,7 +261,10 @@ void void_main(int argc, char* argv[]) { config.Set(new ExtendedFilter(true)); } else if (strcmp(argv[i], "--no_reported_delay") == 0) { - config.Set(new ReportedDelay(false)); + config.Set(new DelayAgnostic(true)); + + } else if (strcmp(argv[i], "--delay_agnostic") == 0) { + config.Set(new DelayAgnostic(true)); } else if (strcmp(argv[i], "-aecm") == 0) { ASSERT_EQ(apm->kNoError, apm->echo_control_mobile()->Enable(true));