From bf395c1fc0a29b54fac4b6f6e9f6c117762faa15 Mon Sep 17 00:00:00 2001 From: Bjorn Volcker Date: Wed, 25 Mar 2015 22:45:56 +0100 Subject: [PATCH] Add WebRTC Media Constraint to force using Delay Agnostic AEC on Android If built-in Echo Cancellation is available on a device it is automatically enabled. The reason is that it in most cases performs better than the WebRTC software echo control for mobile. The drawback is that we can not develop, test and rollout the delay agnostic AEC (DA-AEC) on Android as for desktops. This CL includes - adding a media constraint to enable/disable DA-AEC. - automatically turning on echo cancellation if DA-AEC is enabled. - a fix in the AEC that enables delay estimation when DA-AEC is enabled, but delay metrics is disabled. - sets the Config struct ReportedDelay, which controls DA-AEC internally in the AEC. The test code to verify that it works in AppRTCDemo can be found here: https://webrtc-codereview.appspot.com/50479004/ BUG=4472 TESTED=locally on N7, N6, Android One R=glaznev@webrtc.org, perkj@webrtc.org Review URL: https://webrtc-codereview.appspot.com/48699004 Cr-Commit-Position: refs/heads/master@{#8861} --- talk/app/webrtc/localaudiosource.cc | 2 ++ talk/app/webrtc/localaudiosource_unittest.cc | 4 +++ talk/app/webrtc/mediaconstraintsinterface.cc | 2 ++ talk/app/webrtc/mediaconstraintsinterface.h | 1 + talk/media/base/mediachannel.h | 4 +++ talk/media/webrtc/webrtcvoiceengine.cc | 32 +++++++++++++++++-- talk/media/webrtc/webrtcvoiceengine.h | 9 +++--- .../webrtc/webrtcvoiceengine_unittest.cc | 27 ++++++++++++++++ .../modules/audio_processing/aec/aec_core.c | 4 ++- 9 files changed, 77 insertions(+), 8 deletions(-) diff --git a/talk/app/webrtc/localaudiosource.cc b/talk/app/webrtc/localaudiosource.cc index dcd2110724..bd57cd7efb 100644 --- a/talk/app/webrtc/localaudiosource.cc +++ b/talk/app/webrtc/localaudiosource.cc @@ -60,6 +60,8 @@ void FromConstraints(const MediaConstraintsInterface::Constraints& constraints, else if (iter->key == MediaConstraintsInterface::kExperimentalEchoCancellation) options->experimental_aec.Set(value); + else if (iter->key == MediaConstraintsInterface::kDAEchoCancellation) + options->delay_agnostic_aec.Set(value); else if (iter->key == MediaConstraintsInterface::kAutoGainControl) options->auto_gain_control.Set(value); else if (iter->key == diff --git a/talk/app/webrtc/localaudiosource_unittest.cc b/talk/app/webrtc/localaudiosource_unittest.cc index 02df099859..c838109f71 100644 --- a/talk/app/webrtc/localaudiosource_unittest.cc +++ b/talk/app/webrtc/localaudiosource_unittest.cc @@ -46,6 +46,8 @@ TEST(LocalAudioSourceTest, SetValidOptions) { constraints.AddMandatory(MediaConstraintsInterface::kEchoCancellation, false); constraints.AddOptional( MediaConstraintsInterface::kExperimentalEchoCancellation, true); + constraints.AddOptional( + MediaConstraintsInterface::kDAEchoCancellation, false); constraints.AddOptional(MediaConstraintsInterface::kAutoGainControl, true); constraints.AddOptional( MediaConstraintsInterface::kExperimentalAutoGainControl, true); @@ -61,6 +63,8 @@ TEST(LocalAudioSourceTest, SetValidOptions) { EXPECT_FALSE(value); EXPECT_TRUE(source->options().experimental_aec.Get(&value)); EXPECT_TRUE(value); + EXPECT_TRUE(source->options().delay_agnostic_aec.Get(&value)); + EXPECT_FALSE(value); EXPECT_TRUE(source->options().auto_gain_control.Get(&value)); EXPECT_TRUE(value); EXPECT_TRUE(source->options().experimental_agc.Get(&value)); diff --git a/talk/app/webrtc/mediaconstraintsinterface.cc b/talk/app/webrtc/mediaconstraintsinterface.cc index 323ecac94b..509c9db4f6 100644 --- a/talk/app/webrtc/mediaconstraintsinterface.cc +++ b/talk/app/webrtc/mediaconstraintsinterface.cc @@ -50,6 +50,8 @@ const char MediaConstraintsInterface::kEchoCancellation[] = "googEchoCancellation"; const char MediaConstraintsInterface::kExperimentalEchoCancellation[] = "googEchoCancellation2"; +const char MediaConstraintsInterface::kDAEchoCancellation[] = + "googDAEchoCancellation"; const char MediaConstraintsInterface::kAutoGainControl[] = "googAutoGainControl"; const char MediaConstraintsInterface::kExperimentalAutoGainControl[] = diff --git a/talk/app/webrtc/mediaconstraintsinterface.h b/talk/app/webrtc/mediaconstraintsinterface.h index c8d53ea2b8..a3d5858321 100644 --- a/talk/app/webrtc/mediaconstraintsinterface.h +++ b/talk/app/webrtc/mediaconstraintsinterface.h @@ -75,6 +75,7 @@ class MediaConstraintsInterface { // These keys are google specific. static const char kEchoCancellation[]; // googEchoCancellation static const char kExperimentalEchoCancellation[]; // googEchoCancellation2 + static const char kDAEchoCancellation[]; // googDAEchoCancellation static const char kAutoGainControl[]; // googAutoGainControl static const char kExperimentalAutoGainControl[]; // googAutoGainControl2 static const char kNoiseSuppression[]; // googNoiseSuppression diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index 03363fe0fb..70fd7f019f 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -156,6 +156,7 @@ struct AudioOptions { adjust_agc_delta.SetFrom(change.adjust_agc_delta); experimental_agc.SetFrom(change.experimental_agc); experimental_aec.SetFrom(change.experimental_aec); + delay_agnostic_aec.SetFrom(change.delay_agnostic_aec); experimental_ns.SetFrom(change.experimental_ns); aec_dump.SetFrom(change.aec_dump); tx_agc_target_dbov.SetFrom(change.tx_agc_target_dbov); @@ -184,6 +185,7 @@ struct AudioOptions { conference_mode == o.conference_mode && experimental_agc == o.experimental_agc && experimental_aec == o.experimental_aec && + delay_agnostic_aec == o.delay_agnostic_aec && experimental_ns == o.experimental_ns && adjust_agc_delta == o.adjust_agc_delta && aec_dump == o.aec_dump && @@ -214,6 +216,7 @@ struct AudioOptions { ost << ToStringIfSet("agc_delta", adjust_agc_delta); ost << ToStringIfSet("experimental_agc", experimental_agc); ost << ToStringIfSet("experimental_aec", experimental_aec); + ost << ToStringIfSet("delay_agnostic_aec", delay_agnostic_aec); ost << ToStringIfSet("experimental_ns", experimental_ns); ost << ToStringIfSet("aec_dump", aec_dump); ost << ToStringIfSet("tx_agc_target_dbov", tx_agc_target_dbov); @@ -252,6 +255,7 @@ struct AudioOptions { Settable adjust_agc_delta; Settable experimental_agc; Settable experimental_aec; + Settable delay_agnostic_aec; Settable experimental_ns; Settable aec_dump; // Note that tx_agc_* only applies to non-experimental AGC. diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 099a55b5d0..888776cf72 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -250,6 +250,7 @@ static AudioOptions GetDefaultEngineOptions() { options.adjust_agc_delta.Set(0); options.experimental_agc.Set(false); options.experimental_aec.Set(false); + options.delay_agnostic_aec.Set(false); options.experimental_ns.Set(false); options.aec_dump.Set(false); return options; @@ -806,6 +807,19 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { options.experimental_ns.Set(false); #endif + // Delay Agnostic AEC automatically turns on EC if not set except on iOS + // where the feature is not supported. + bool use_delay_agnostic_aec = false; +#if !defined(IOS) + if (options.delay_agnostic_aec.Get(&use_delay_agnostic_aec)) { + if (use_delay_agnostic_aec) { + options.echo_cancellation.Set(true); + options.experimental_aec.Set(true); + ec_mode = webrtc::kEcConference; + } + } +#endif + LOG(LS_INFO) << "Applying audio options: " << options.ToString(); webrtc::VoEAudioProcessing* voep = voe_wrapper_->processing(); @@ -818,10 +832,14 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { // in combination with Open SL ES audio. const bool built_in_aec = voe_wrapper_->hw()->BuiltInAECIsAvailable(); if (built_in_aec) { + // Enabled built-in EC if the device has one and delay agnostic AEC is not + // enabled. + const bool enable_built_in_aec = echo_cancellation & + !use_delay_agnostic_aec; // Set mode of built-in EC according to the audio options. - voe_wrapper_->hw()->EnableBuiltInAEC(echo_cancellation); - if (echo_cancellation) { - // Disable internal software EC if device has its own built-in EC, + voe_wrapper_->hw()->EnableBuiltInAEC(enable_built_in_aec); + if (enable_built_in_aec) { + // Disable internal software EC if built-in EC is enabled, // i.e., replace the software EC with the built-in EC. options.echo_cancellation.Set(false); echo_cancellation = false; @@ -948,6 +966,14 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { webrtc::Config config; + delay_agnostic_aec_.SetFrom(options.delay_agnostic_aec); + 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)); + } + experimental_aec_.SetFrom(options.experimental_aec); bool experimental_aec; if (experimental_aec_.Get(&experimental_aec)) { diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 700a411f9d..ffbace5f0d 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -290,11 +290,12 @@ class WebRtcVoiceEngine rtc::CriticalSection signal_media_critical_; - // Cache received experimental_aec and experimental_ns values, and apply them - // in case they are missing in the audio options. We need to do this because - // SetExtraOptions() will revert to defaults for options which are not - // provided. + // Cache received experimental_aec, delay_agnostic_aec and experimental_ns + // values, and apply them in case they are missing in the audio options. We + // need to do this because SetExtraOptions() will revert to defaults for + // options which are not provided. Settable experimental_aec_; + Settable delay_agnostic_aec_; Settable experimental_ns_; }; diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 749ff9cb3d..9c1090fa46 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -2834,6 +2834,33 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { EXPECT_EQ(ec_mode, webrtc::kEcConference); EXPECT_EQ(ns_mode, webrtc::kNsHighSuppression); + // Turn on delay agnostic aec and make sure nothing change w.r.t. echo + // control. + options.delay_agnostic_aec.Set(true); + ASSERT_TRUE(engine_.SetOptions(options)); + voe_.GetEcStatus(ec_enabled, ec_mode); + voe_.GetEcMetricsStatus(ec_metrics_enabled); + voe_.GetAecmMode(aecm_mode, cng_enabled); + EXPECT_TRUE(ec_enabled); + EXPECT_TRUE(ec_metrics_enabled); + EXPECT_EQ(ec_mode, webrtc::kEcConference); + + // Turn off echo cancellation and delay agnostic aec. + options.delay_agnostic_aec.Set(false); + options.experimental_aec.Set(false); + options.echo_cancellation.Set(false); + ASSERT_TRUE(engine_.SetOptions(options)); + voe_.GetEcStatus(ec_enabled, ec_mode); + EXPECT_FALSE(ec_enabled); + // Turning delay agnostic aec back on should also turn on echo cancellation. + options.delay_agnostic_aec.Set(true); + ASSERT_TRUE(engine_.SetOptions(options)); + voe_.GetEcStatus(ec_enabled, ec_mode); + voe_.GetEcMetricsStatus(ec_metrics_enabled); + EXPECT_TRUE(ec_enabled); + EXPECT_TRUE(ec_metrics_enabled); + EXPECT_EQ(ec_mode, webrtc::kEcConference); + // Turn off AGC options.auto_gain_control.Set(false); ASSERT_TRUE(engine_.SetOptions(options)); diff --git a/webrtc/modules/audio_processing/aec/aec_core.c b/webrtc/modules/audio_processing/aec/aec_core.c index ccd5b922f2..fc523bf9c7 100644 --- a/webrtc/modules/audio_processing/aec/aec_core.c +++ b/webrtc/modules/audio_processing/aec/aec_core.c @@ -1899,7 +1899,9 @@ void WebRtcAec_SetConfigCore(AecCore* self, if (self->metricsMode) { InitMetrics(self); } - self->delay_logging_enabled = delay_logging; + // 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; if (self->delay_logging_enabled) { memset(self->delay_histogram, 0, sizeof(self->delay_histogram)); }