From ba502233634bca12dc434f86078238c9f6de8f9d Mon Sep 17 00:00:00 2001 From: Sam Zackrisson Date: Fri, 4 Jan 2019 10:36:48 +0100 Subject: [PATCH] Make voiceengine/audio transport stop using voice_detection() interface Configuration for AudioProcessing::voice_detection() is moving into AudioProcessing::Config, to get rid of the pointer-to-submodule interfaces (such as voice_detection()). Bug: webrtc:9947 Change-Id: Ia64ae996a43d44423aa0d612a3f1185b52a3e534 Reviewed-on: https://webrtc-review.googlesource.com/c/116067 Reviewed-by: Niels Moller Reviewed-by: Oskar Sundbom Commit-Queue: Sam Zackrisson Cr-Commit-Position: refs/heads/master@{#26216} --- audio/audio_transport_impl.cc | 4 ++-- media/engine/apm_helpers.cc | 14 ----------- media/engine/apm_helpers.h | 1 - media/engine/apm_helpers_unittest.cc | 17 -------------- media/engine/webrtcvoiceengine.cc | 13 +++++------ media/engine/webrtcvoiceengine_unittest.cc | 27 +++++++++++++++++----- 6 files changed, 29 insertions(+), 47 deletions(-) diff --git a/audio/audio_transport_impl.cc b/audio/audio_transport_impl.cc index 539456e2f4..71b809cdb5 100644 --- a/audio/audio_transport_impl.cc +++ b/audio/audio_transport_impl.cc @@ -133,9 +133,9 @@ int32_t AudioTransportImpl::RecordedDataIsAvailable( // Typing detection (utilizes the APM/VAD decision). We let the VAD determine // if we're using this feature or not. - // TODO(solenberg): is_enabled() takes a lock. Work around that. + // TODO(solenberg): GetConfig() takes a lock. Work around that. bool typing_detected = false; - if (audio_processing_->voice_detection()->is_enabled()) { + if (audio_processing_->GetConfig().voice_detection.enabled) { if (audio_frame->vad_activity_ != AudioFrame::kVadUnknown) { bool vad_active = audio_frame->vad_activity_ == AudioFrame::kVadActive; typing_detected = typing_detection_.Process(key_pressed, vad_active); diff --git a/media/engine/apm_helpers.cc b/media/engine/apm_helpers.cc index fe81a30809..164ca3c88a 100644 --- a/media/engine/apm_helpers.cc +++ b/media/engine/apm_helpers.cc @@ -103,19 +103,5 @@ void SetNsStatus(AudioProcessing* apm, bool enable) { } RTC_LOG(LS_INFO) << "NS set to " << enable; } - -void SetTypingDetectionStatus(AudioProcessing* apm, bool enable) { - RTC_DCHECK(apm); - VoiceDetection* vd = apm->voice_detection(); - if (vd->Enable(enable)) { - RTC_LOG(LS_ERROR) << "Failed to enable/disable VAD: " << enable; - return; - } - if (vd->set_likelihood(VoiceDetection::kVeryLowLikelihood)) { - RTC_LOG(LS_ERROR) << "Failed to set low VAD likelihood."; - return; - } - RTC_LOG(LS_INFO) << "VAD set to " << enable << " for typing detection."; -} } // namespace apm_helpers } // namespace webrtc diff --git a/media/engine/apm_helpers.h b/media/engine/apm_helpers.h index f775d8a473..6881622b65 100644 --- a/media/engine/apm_helpers.h +++ b/media/engine/apm_helpers.h @@ -38,7 +38,6 @@ void SetEcStatus(AudioProcessing* apm, bool enable, EcModes mode); void SetEcMetricsStatus(AudioProcessing* apm, bool enable); void SetAecmMode(AudioProcessing* apm, bool enable_cng); void SetNsStatus(AudioProcessing* apm, bool enable); -void SetTypingDetectionStatus(AudioProcessing* apm, bool enable); } // namespace apm_helpers } // namespace webrtc diff --git a/media/engine/apm_helpers_unittest.cc b/media/engine/apm_helpers_unittest.cc index 70ac0c1c21..fa5b4d32ef 100644 --- a/media/engine/apm_helpers_unittest.cc +++ b/media/engine/apm_helpers_unittest.cc @@ -147,21 +147,4 @@ TEST(ApmHelpersTest, NsStatus_EnableDisable) { EXPECT_EQ(NoiseSuppression::kHigh, ns->level()); EXPECT_FALSE(ns->is_enabled()); } - -TEST(ApmHelpersTest, TypingDetectionStatus_DefaultMode) { - TestHelper helper; - VoiceDetection* vd = helper.apm()->voice_detection(); - EXPECT_FALSE(vd->is_enabled()); -} - -TEST(ApmHelpersTest, TypingDetectionStatus_EnableDisable) { - TestHelper helper; - VoiceDetection* vd = helper.apm()->voice_detection(); - apm_helpers::SetTypingDetectionStatus(helper.apm(), true); - EXPECT_EQ(VoiceDetection::kVeryLowLikelihood, vd->likelihood()); - EXPECT_TRUE(vd->is_enabled()); - apm_helpers::SetTypingDetectionStatus(helper.apm(), false); - EXPECT_EQ(VoiceDetection::kVeryLowLikelihood, vd->likelihood()); - EXPECT_FALSE(vd->is_enabled()); -} } // namespace webrtc diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc index 40abb66acb..2bb4b28221 100644 --- a/media/engine/webrtcvoiceengine.cc +++ b/media/engine/webrtcvoiceengine.cc @@ -497,13 +497,6 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { *options.audio_jitter_buffer_enable_rtx_handling; } - if (options.typing_detection) { - RTC_LOG(LS_INFO) << "Typing detection is enabled? " - << *options.typing_detection; - webrtc::apm_helpers::SetTypingDetectionStatus(apm(), - *options.typing_detection); - } - webrtc::Config config; if (options.delay_agnostic_aec) @@ -544,6 +537,12 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { apm_config.residual_echo_detector.enabled = *options.residual_echo_detector; } + if (options.typing_detection) { + RTC_LOG(LS_INFO) << "Typing detection is enabled? " + << *options.typing_detection; + apm_config.voice_detection.enabled = *options.typing_detection; + } + apm()->SetExtraOptions(config); apm()->ApplyConfig(apm_config); return true; diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index 9fd5dfb0ad..ff0d8c6b8a 100644 --- a/media/engine/webrtcvoiceengine_unittest.cc +++ b/media/engine/webrtcvoiceengine_unittest.cc @@ -170,7 +170,6 @@ class WebRtcVoiceEngineTestFake : public testing::Test { StrictMock>()), apm_gc_(*apm_->gain_control()), apm_ns_(*apm_->noise_suppression()), - apm_vd_(*apm_->voice_detection()), call_(), override_field_trials_(field_trials) { // AudioDeviceModule. @@ -186,7 +185,6 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_CALL(apm_gc_, set_analog_level_limits(0, 255)).WillOnce(Return(0)); EXPECT_CALL(apm_ns_, set_level(kDefaultNsLevel)).WillOnce(Return(0)); EXPECT_CALL(apm_ns_, Enable(true)).WillOnce(Return(0)); - EXPECT_CALL(apm_vd_, Enable(true)).WillOnce(Return(0)); // Init does not overwrite default AGC config. EXPECT_CALL(apm_gc_, target_level_dbfs()).WillOnce(Return(1)); EXPECT_CALL(apm_gc_, compression_gain_db()).WillRepeatedly(Return(5)); @@ -207,6 +205,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { // Default Options. EXPECT_TRUE(IsEchoCancellationEnabled()); EXPECT_TRUE(IsHighPassFilterEnabled()); + EXPECT_TRUE(IsTypingDetectionEnabled()); } bool SetupChannel() { @@ -742,12 +741,15 @@ class WebRtcVoiceEngineTestFake : public testing::Test { return engine_->GetApmConfigForTest().high_pass_filter.enabled; } + bool IsTypingDetectionEnabled() { + return engine_->GetApmConfigForTest().voice_detection.enabled; + } + protected: StrictMock adm_; rtc::scoped_refptr> apm_; webrtc::test::MockGainControl& apm_gc_; webrtc::test::MockNoiseSuppression& apm_ns_; - webrtc::test::MockVoiceDetection& apm_vd_; cricket::FakeCall call_; std::unique_ptr engine_; cricket::VoiceMediaChannel* channel_ = nullptr; @@ -2848,9 +2850,25 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { SetSendParameters(send_parameters_); EXPECT_TRUE(IsEchoCancellationEnabled()); EXPECT_TRUE(IsHighPassFilterEnabled()); + EXPECT_TRUE(IsTypingDetectionEnabled()); EXPECT_EQ(50u, GetRecvStreamConfig(kSsrcY).jitter_buffer_max_packets); EXPECT_FALSE(GetRecvStreamConfig(kSsrcY).jitter_buffer_fast_accelerate); + // Turn typing detection off. + send_parameters_.options.typing_detection = false; + SetSendParameters(send_parameters_); + EXPECT_FALSE(IsTypingDetectionEnabled()); + + // Leave typing detection unchanged, but non-default. + send_parameters_.options.typing_detection = absl::nullopt; + SetSendParameters(send_parameters_); + EXPECT_FALSE(IsTypingDetectionEnabled()); + + // Turn typing detection on. + send_parameters_.options.typing_detection = true; + SetSendParameters(send_parameters_); + EXPECT_TRUE(IsTypingDetectionEnabled()); + // Turn echo cancellation off send_parameters_.options.echo_cancellation = false; SetSendParameters(send_parameters_); @@ -2899,10 +2917,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { EXPECT_CALL(apm_gc_, Enable(true)).WillOnce(Return(0)); EXPECT_CALL(apm_ns_, set_level(kDefaultNsLevel)).WillOnce(Return(0)); EXPECT_CALL(apm_ns_, Enable(false)).WillOnce(Return(0)); - EXPECT_CALL(apm_vd_, Enable(false)).WillOnce(Return(0)); send_parameters_.options.noise_suppression = false; send_parameters_.options.highpass_filter = false; - send_parameters_.options.typing_detection = false; send_parameters_.options.stereo_swapping = true; SetSendParameters(send_parameters_); EXPECT_TRUE(IsEchoCancellationEnabled()); @@ -2913,7 +2929,6 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { EXPECT_CALL(apm_gc_, Enable(true)).WillOnce(Return(0)); EXPECT_CALL(apm_ns_, set_level(kDefaultNsLevel)).WillOnce(Return(0)); EXPECT_CALL(apm_ns_, Enable(false)).WillOnce(Return(0)); - EXPECT_CALL(apm_vd_, Enable(false)).WillOnce(Return(0)); SetSendParameters(send_parameters_); EXPECT_TRUE(IsEchoCancellationEnabled()); }