From defc21e0aa6b090111d6bd8ffe43ddafcb657c7d Mon Sep 17 00:00:00 2001 From: henrika Date: Tue, 11 Oct 2016 01:29:09 -0700 Subject: [PATCH] Removes usage of hardware AGC and any related APIs on Android. Compromise solution where WebRtcAudioUtils.setWebRtcBasedAutomaticGainControl() is marked as deprecated and where as many APIs as possible that touches the HW AGC are removed. Some basic architecture is saved to ensure that we can restore usage of the HW AGC if ever required for future devices. The AppRTCMobile demo does still contain an AGC check box but it is now grayed out. BUG=b/30387905 Review-Url: https://codereview.webrtc.org/2402883003 Cr-Commit-Position: refs/heads/master@{#14596} --- .../android/audio_manager_unittest.cc | 3 +- .../audio_device/android/audio_record_jni.cc | 12 +-- .../audio_device/android/audio_record_jni.h | 2 - .../voiceengine/WebRtcAudioEffects.java | 101 +----------------- .../voiceengine/WebRtcAudioManager.java | 9 +- .../webrtc/voiceengine/WebRtcAudioRecord.java | 9 -- .../webrtc/voiceengine/WebRtcAudioUtils.java | 39 +++---- 7 files changed, 28 insertions(+), 147 deletions(-) diff --git a/webrtc/modules/audio_device/android/audio_manager_unittest.cc b/webrtc/modules/audio_device/android/audio_manager_unittest.cc index 76ee305dfa..2507976522 100644 --- a/webrtc/modules/audio_device/android/audio_manager_unittest.cc +++ b/webrtc/modules/audio_device/android/audio_manager_unittest.cc @@ -113,8 +113,7 @@ TEST_F(AudioManagerTest, IsAcousticEchoCancelerSupported) { } TEST_F(AudioManagerTest, IsAutomaticGainControlSupported) { - PRINT("%sAutomatic Gain Control support: %s\n", kTag, - audio_manager()->IsAutomaticGainControlSupported() ? "Yes" : "No"); + EXPECT_FALSE(audio_manager()->IsAutomaticGainControlSupported()); } TEST_F(AudioManagerTest, IsNoiseSuppressorSupported) { diff --git a/webrtc/modules/audio_device/android/audio_record_jni.cc b/webrtc/modules/audio_device/android/audio_record_jni.cc index 8ce13861a4..a02bbd5f0c 100644 --- a/webrtc/modules/audio_device/android/audio_record_jni.cc +++ b/webrtc/modules/audio_device/android/audio_record_jni.cc @@ -37,7 +37,6 @@ AudioRecordJni::JavaAudioRecord::JavaAudioRecord( start_recording_(native_reg->GetMethodId("startRecording", "()Z")), stop_recording_(native_reg->GetMethodId("stopRecording", "()Z")), enable_built_in_aec_(native_reg->GetMethodId("enableBuiltInAEC", "(Z)Z")), - enable_built_in_agc_(native_reg->GetMethodId("enableBuiltInAGC", "(Z)Z")), enable_built_in_ns_(native_reg->GetMethodId("enableBuiltInNS", "(Z)Z")) {} AudioRecordJni::JavaAudioRecord::~JavaAudioRecord() {} @@ -62,11 +61,6 @@ bool AudioRecordJni::JavaAudioRecord::EnableBuiltInAEC(bool enable) { static_cast(enable)); } -bool AudioRecordJni::JavaAudioRecord::EnableBuiltInAGC(bool enable) { - return audio_record_->CallBooleanMethod(enable_built_in_agc_, - static_cast(enable)); -} - bool AudioRecordJni::JavaAudioRecord::EnableBuiltInNS(bool enable) { return audio_record_->CallBooleanMethod(enable_built_in_ns_, static_cast(enable)); @@ -201,9 +195,9 @@ int32_t AudioRecordJni::EnableBuiltInAEC(bool enable) { } int32_t AudioRecordJni::EnableBuiltInAGC(bool enable) { - ALOGD("EnableBuiltInAGC%s", GetThreadInfo().c_str()); - RTC_DCHECK(thread_checker_.CalledOnValidThread()); - return j_audio_record_->EnableBuiltInAGC(enable) ? 0 : -1; + // TODO(henrika): possibly remove when no longer used by any client. + FATAL() << "Should never be called"; + return -1; } int32_t AudioRecordJni::EnableBuiltInNS(bool enable) { diff --git a/webrtc/modules/audio_device/android/audio_record_jni.h b/webrtc/modules/audio_device/android/audio_record_jni.h index de58468c9a..3a1a94aa41 100644 --- a/webrtc/modules/audio_device/android/audio_record_jni.h +++ b/webrtc/modules/audio_device/android/audio_record_jni.h @@ -55,7 +55,6 @@ class AudioRecordJni { bool StartRecording(); bool StopRecording(); bool EnableBuiltInAEC(bool enable); - bool EnableBuiltInAGC(bool enable); bool EnableBuiltInNS(bool enable); private: @@ -64,7 +63,6 @@ class AudioRecordJni { jmethodID start_recording_; jmethodID stop_recording_; jmethodID enable_built_in_aec_; - jmethodID enable_built_in_agc_; jmethodID enable_built_in_ns_; }; diff --git a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java index 14295c2161..e02158816c 100644 --- a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java +++ b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java @@ -24,8 +24,8 @@ import java.util.List; import java.util.UUID; // This class wraps control of three different platform effects. Supported -// effects are: AcousticEchoCanceler (AEC), AutomaticGainControl (AGC) and -// NoiseSuppressor (NS). Calling enable() will active all effects that are +// effects are: AcousticEchoCanceler (AEC) and NoiseSuppressor (NS). +// Calling enable() will active all effects that are // supported by the device if the corresponding |shouldEnableXXX| member is set. class WebRtcAudioEffects { private static final boolean DEBUG = false; @@ -36,8 +36,6 @@ class WebRtcAudioEffects { // The implementor field will be set to "The Android Open Source Project". private static final UUID AOSP_ACOUSTIC_ECHO_CANCELER = UUID.fromString("bb392ec0-8d4d-11e0-a896-0002a5d5c51b"); - private static final UUID AOSP_AUTOMATIC_GAIN_CONTROL = - UUID.fromString("aa8130e0-66fc-11e0-bad0-0002a5d5c51b"); private static final UUID AOSP_NOISE_SUPPRESSOR = UUID.fromString("c06c8400-8e06-11e0-9cb6-0002a5d5c51b"); @@ -49,16 +47,14 @@ class WebRtcAudioEffects { // Contains the audio effect objects. Created in enable() and destroyed // in release(). private AcousticEchoCanceler aec = null; - private AutomaticGainControl agc = null; private NoiseSuppressor ns = null; // Affects the final state given to the setEnabled() method on each effect. // The default state is set to "disabled" but each effect can also be enabled - // by calling setAEC(), setAGC() and setNS(). + // by calling setAEC() and setNS(). // To enable an effect, both the shouldEnableXXX member and the static // canUseXXX() must be true. private boolean shouldEnableAec = false; - private boolean shouldEnableAgc = false; private boolean shouldEnableNs = false; // Checks if the device implements Acoustic Echo Cancellation (AEC). @@ -70,15 +66,6 @@ class WebRtcAudioEffects { return WebRtcAudioUtils.runningOnJellyBeanOrHigher() && isAcousticEchoCancelerEffectAvailable(); } - // Checks if the device implements Automatic Gain Control (AGC). - // Returns true if the device implements AGC, false otherwise. - public static boolean isAutomaticGainControlSupported() { - // Note: we're using isAutomaticGainControlEffectAvailable() instead of - // AutomaticGainControl.isAvailable() to avoid the expensive getEffects() - // OS API call. - return WebRtcAudioUtils.runningOnJellyBeanOrHigher() && isAutomaticGainControlEffectAvailable(); - } - // Checks if the device implements Noise Suppression (NS). // Returns true if the device implements NS, false otherwise. public static boolean isNoiseSuppressorSupported() { @@ -98,16 +85,6 @@ class WebRtcAudioEffects { return isBlacklisted; } - // Returns true if the device is blacklisted for HW AGC usage. - public static boolean isAutomaticGainControlBlacklisted() { - List blackListedModels = WebRtcAudioUtils.getBlackListedModelsForAgcUsage(); - boolean isBlacklisted = blackListedModels.contains(Build.MODEL); - if (isBlacklisted) { - Logging.w(TAG, Build.MODEL + " is blacklisted for HW AGC usage!"); - } - return isBlacklisted; - } - // Returns true if the device is blacklisted for HW NS usage. public static boolean isNoiseSuppressorBlacklisted() { List blackListedModels = WebRtcAudioUtils.getBlackListedModelsForNsUsage(); @@ -131,19 +108,6 @@ class WebRtcAudioEffects { return false; } - // Returns true if the platform AGC should be excluded based on its UUID. - // AudioEffect.queryEffects() can throw IllegalStateException. - @TargetApi(18) - private static boolean isAutomaticGainControlExcludedByUUID() { - for (Descriptor d : getAvailableEffects()) { - if (d.type.equals(AudioEffect.EFFECT_TYPE_AGC) - && d.uuid.equals(AOSP_AUTOMATIC_GAIN_CONTROL)) { - return true; - } - } - return false; - } - // Returns true if the platform NS should be excluded based on its UUID. // AudioEffect.queryEffects() can throw IllegalStateException. @TargetApi(18) @@ -162,12 +126,6 @@ class WebRtcAudioEffects { return isEffectTypeAvailable(AudioEffect.EFFECT_TYPE_AEC); } - // Returns true if the device supports Automatic Gain Control (AGC). - @TargetApi(18) - private static boolean isAutomaticGainControlEffectAvailable() { - return isEffectTypeAvailable(AudioEffect.EFFECT_TYPE_AGC); - } - // Returns true if the device supports Noise Suppression (NS). @TargetApi(18) private static boolean isNoiseSuppressorEffectAvailable() { @@ -184,16 +142,6 @@ class WebRtcAudioEffects { return canUseAcousticEchoCanceler; } - // Returns true if all conditions for supporting the HW AGC are fulfilled. - // It will not be possible to enable the HW AGC if this method returns false. - public static boolean canUseAutomaticGainControl() { - boolean canUseAutomaticGainControl = isAutomaticGainControlSupported() - && !WebRtcAudioUtils.useWebRtcBasedAutomaticGainControl() - && !isAutomaticGainControlBlacklisted() && !isAutomaticGainControlExcludedByUUID(); - Logging.d(TAG, "canUseAutomaticGainControl: " + canUseAutomaticGainControl); - return canUseAutomaticGainControl; - } - // Returns true if all conditions for supporting the HW NS are fulfilled. // It will not be possible to enable the HW NS if this method returns false. public static boolean canUseNoiseSuppressor() { @@ -236,25 +184,6 @@ class WebRtcAudioEffects { return true; } - // Call this method to enable or disable the platform AGC. It modifies - // |shouldEnableAgc| which is used in enable() where the actual state - // of the AGC effect is modified. Returns true if HW AGC is supported and - // false otherwise. - public boolean setAGC(boolean enable) { - Logging.d(TAG, "setAGC(" + enable + ")"); - if (!canUseAutomaticGainControl()) { - Logging.w(TAG, "Platform AGC is not supported"); - shouldEnableAgc = false; - return false; - } - if (agc != null && (enable != shouldEnableAgc)) { - Logging.e(TAG, "Platform AGC state can't be modified while recording"); - return false; - } - shouldEnableAgc = enable; - return true; - } - // Call this method to enable or disable the platform NS. It modifies // |shouldEnableNs| which is used in enable() where the actual state // of the NS effect is modified. Returns true if HW NS is supported and @@ -277,7 +206,6 @@ class WebRtcAudioEffects { public void enable(int audioSession) { Logging.d(TAG, "enable(audioSession=" + audioSession + ")"); assertTrue(aec == null); - assertTrue(agc == null); assertTrue(ns == null); // Add logging of supported effects but filter out "VoIP effects", i.e., @@ -309,24 +237,6 @@ class WebRtcAudioEffects { } } - if (isAutomaticGainControlSupported()) { - // Create an AutomaticGainControl and attach it to the AudioRecord on - // the specified audio session. - agc = AutomaticGainControl.create(audioSession); - if (agc != null) { - boolean enabled = agc.getEnabled(); - boolean enable = shouldEnableAgc && canUseAutomaticGainControl(); - if (agc.setEnabled(enable) != AudioEffect.SUCCESS) { - Logging.e(TAG, "Failed to set the AutomaticGainControl state"); - } - Logging.d(TAG, "AutomaticGainControl: was " + (enabled ? "enabled" : "disabled") - + ", enable: " + enable + ", is now: " - + (agc.getEnabled() ? "enabled" : "disabled")); - } else { - Logging.e(TAG, "Failed to create the AutomaticGainControl instance"); - } - } - if (isNoiseSuppressorSupported()) { // Create an NoiseSuppressor and attach it to the AudioRecord on the // specified audio session. @@ -354,10 +264,6 @@ class WebRtcAudioEffects { aec.release(); aec = null; } - if (agc != null) { - agc.release(); - agc = null; - } if (ns != null) { ns.release(); ns = null; @@ -377,7 +283,6 @@ class WebRtcAudioEffects { return false; return (AudioEffect.EFFECT_TYPE_AEC.equals(type) && isAcousticEchoCancelerSupported()) - || (AudioEffect.EFFECT_TYPE_AGC.equals(type) && isAutomaticGainControlSupported()) || (AudioEffect.EFFECT_TYPE_NS.equals(type) && isNoiseSuppressorSupported()); } diff --git a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java index 19ee09a466..fb0516fa06 100644 --- a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java +++ b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java @@ -192,7 +192,9 @@ public class WebRtcAudioManager { channels = CHANNELS; sampleRate = getNativeOutputSampleRate(); hardwareAEC = isAcousticEchoCancelerSupported(); - hardwareAGC = isAutomaticGainControlSupported(); + // TODO(henrika): use of hardware AGC is no longer supported. Currently + // hardcoded to false. To be removed. + hardwareAGC = false; hardwareNS = isNoiseSuppressorSupported(); lowLatencyOutput = isLowLatencyOutputSupported(); lowLatencyInput = isLowLatencyInputSupported(); @@ -278,7 +280,7 @@ public class WebRtcAudioManager { return framesPerBuffer == null ? DEFAULT_FRAME_PER_BUFFER : Integer.parseInt(framesPerBuffer); } - // Returns true if the device supports an audio effect (AEC, AGC or NS). + // Returns true if the device supports an audio effect (AEC or NS). // Four conditions must be fulfilled if functions are to return true: // 1) the platform must support the built-in (HW) effect, // 2) explicit use (override) of a WebRTC based version must not be set, @@ -287,9 +289,6 @@ public class WebRtcAudioManager { private static boolean isAcousticEchoCancelerSupported() { return WebRtcAudioEffects.canUseAcousticEchoCanceler(); } - private static boolean isAutomaticGainControlSupported() { - return WebRtcAudioEffects.canUseAutomaticGainControl(); - } private static boolean isNoiseSuppressorSupported() { return WebRtcAudioEffects.canUseNoiseSuppressor(); } diff --git a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java index aa9608d7f5..939caf7282 100644 --- a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java +++ b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java @@ -138,15 +138,6 @@ public class WebRtcAudioRecord { return effects.setAEC(enable); } - private boolean enableBuiltInAGC(boolean enable) { - Logging.d(TAG, "enableBuiltInAGC(" + enable + ')'); - if (effects == null) { - Logging.e(TAG, "Built-in AGC is not supported on this platform"); - return false; - } - return effects.setAGC(enable); - } - private boolean enableBuiltInNS(boolean enable) { Logging.d(TAG, "enableBuiltInNS(" + enable + ')'); if (effects == null) { diff --git a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java index 420633d422..677569c694 100644 --- a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java +++ b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java @@ -39,9 +39,6 @@ public final class WebRtcAudioUtils { "ONE A2005", // OnePlus 2 "MotoG3", // Moto G (3rd Generation) }; - private static final String[] BLACKLISTED_AGC_MODELS = new String[] { - "Nexus 10", "Nexus 9", - }; private static final String[] BLACKLISTED_NS_MODELS = new String[] { "Nexus 10", "Nexus 9", "ONE A2005", // OnePlus 2 @@ -54,9 +51,9 @@ public final class WebRtcAudioUtils { // Set to true if setDefaultSampleRateHz() has been called. private static boolean isDefaultSampleRateOverridden = false; - // By default, utilize hardware based audio effects when available. + // By default, utilize hardware based audio effects for AEC and NS when + // available. private static boolean useWebRtcBasedAcousticEchoCanceler = false; - private static boolean useWebRtcBasedAutomaticGainControl = false; private static boolean useWebRtcBasedNoiseSuppressor = false; // Call these methods if any hardware based effect shall be replaced by a @@ -64,12 +61,13 @@ public final class WebRtcAudioUtils { public static synchronized void setWebRtcBasedAcousticEchoCanceler(boolean enable) { useWebRtcBasedAcousticEchoCanceler = enable; } - public static synchronized void setWebRtcBasedAutomaticGainControl(boolean enable) { - useWebRtcBasedAutomaticGainControl = enable; - } public static synchronized void setWebRtcBasedNoiseSuppressor(boolean enable) { useWebRtcBasedNoiseSuppressor = enable; } + public static synchronized void setWebRtcBasedAutomaticGainControl(boolean enable) { + // TODO(henrika): deprecated; remove when no longer used by any client. + Logging.w(TAG, "setWebRtcBasedAutomaticGainControl() is deprecated"); + } public static synchronized boolean useWebRtcBasedAcousticEchoCanceler() { if (useWebRtcBasedAcousticEchoCanceler) { @@ -77,20 +75,19 @@ public final class WebRtcAudioUtils { } return useWebRtcBasedAcousticEchoCanceler; } - public static synchronized boolean useWebRtcBasedAutomaticGainControl() { - if (useWebRtcBasedAutomaticGainControl) { - Logging.w(TAG, "Overriding default behavior; now using WebRTC AGC!"); - } - return useWebRtcBasedAutomaticGainControl; - } public static synchronized boolean useWebRtcBasedNoiseSuppressor() { if (useWebRtcBasedNoiseSuppressor) { Logging.w(TAG, "Overriding default behavior; now using WebRTC NS!"); } return useWebRtcBasedNoiseSuppressor; } + // TODO(henrika): deprecated; remove when no longer used by any client. + public static synchronized boolean useWebRtcBasedAutomaticGainControl() { + // Always return true here to avoid trying to use any built-in AGC. + return true; + } - // Returns true if the device supports an audio effect (AEC, AGC or NS). + // Returns true if the device supports an audio effect (AEC or NS). // Four conditions must be fulfilled if functions are to return true: // 1) the platform must support the built-in (HW) effect, // 2) explicit use (override) of a WebRTC based version must not be set, @@ -99,12 +96,14 @@ public final class WebRtcAudioUtils { public static boolean isAcousticEchoCancelerSupported() { return WebRtcAudioEffects.canUseAcousticEchoCanceler(); } - public static boolean isAutomaticGainControlSupported() { - return WebRtcAudioEffects.canUseAutomaticGainControl(); - } public static boolean isNoiseSuppressorSupported() { return WebRtcAudioEffects.canUseNoiseSuppressor(); } + // TODO(henrika): deprecated; remove when no longer used by any client. + public static boolean isAutomaticGainControlSupported() { + // Always return false here to avoid trying to use any built-in AGC. + return false; + } // Call this method if the default handling of querying the native sample // rate shall be overridden. Can be useful on some devices where the @@ -126,10 +125,6 @@ public final class WebRtcAudioUtils { return Arrays.asList(WebRtcAudioUtils.BLACKLISTED_AEC_MODELS); } - public static List getBlackListedModelsForAgcUsage() { - return Arrays.asList(WebRtcAudioUtils.BLACKLISTED_AGC_MODELS); - } - public static List getBlackListedModelsForNsUsage() { return Arrays.asList(WebRtcAudioUtils.BLACKLISTED_NS_MODELS); }