Enable platform echo cancellation through the AudioRecord path.

media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.media;
 
 import android.bluetooth.BluetoothAdapter;
 import android.bluetooth.BluetoothManager;
 import android.content.BroadcastReceiver;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.PackageManager;
 import android.database.ContentObserver;
+import android.media.audiofx.AcousticEchoCanceler;
 import android.media.AudioFormat;
 import android.media.AudioManager;
 import android.media.AudioRecord;
 import android.media.AudioTrack;
 import android.net.Uri;
 import android.os.Build;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Process;
 import android.provider.Settings;
@@ skipping 78 matching lines @@
     static final int STATE_BLUETOOTH_TURNING_OFF = 6;
     // TODO(henrika): document the valid state transitions.
 
     // Use 44.1kHz as the default sampling rate.
     private static final int DEFAULT_SAMPLING_RATE = 44100;
     // Randomly picked up frame size which is close to return value on N4.
     // Return this value when getProperty(PROPERTY_OUTPUT_FRAMES_PER_BUFFER)
     // fails.
     private static final int DEFAULT_FRAME_PER_BUFFER = 256;
 
+    // List of device models which have been vetted for good quality platform
+    // echo cancellation.
+    private static final Set<String> PLATFORM_AEC_MODEL_WHITELIST =

[henrika (OOO until Aug 14), 2013/12/06 12:52:21]

Guess you can follow this example instead of using HashSet. 
private static final String[] DEVICE_NAMES = new String[] {

or do as tommi suggests below...

But I like when you can find the list up here.

[ajm, 2013/12/10 06:37:16]

Done.

+            new HashSet<String>(Arrays.asList(new String[] {
+                "Nexus 5",
+                "Nexus 7"
+            }));
+
     private final AudioManager mAudioManager;
     private final Context mContext;
     private final long mNativeAudioManagerAndroid;
 
     private boolean mHasBluetoothPermission = false;
     private boolean mIsInitialized = false;
     private boolean mSavedIsSpeakerphoneOn;
     private boolean mSavedIsMicrophoneMute;
 
     private Integer mAudioDeviceState = STATE_NO_DEVICE_SELECTED;
@@ skipping 183 matching lines @@
                     i++;
                 }
             }
             logd("getAudioInputDeviceNames: " + devices);
             return array;
         }
     }
 
     @CalledByNative
     private int getNativeOutputSampleRate() {
-        if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.JELLY_BEAN_MR1) {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
             String sampleRateString = mAudioManager.getProperty(
                     AudioManager.PROPERTY_OUTPUT_SAMPLE_RATE);
             return (sampleRateString == null ?
                     DEFAULT_SAMPLING_RATE : Integer.parseInt(sampleRateString));
         } else {
             return DEFAULT_SAMPLING_RATE;
         }
     }
 
   /**
@@ skipping 43 matching lines @@
     }
 
     @CalledByNative
     private int getAudioLowLatencyOutputFrameSize() {
         String framesPerBuffer =
                 mAudioManager.getProperty(AudioManager.PROPERTY_OUTPUT_FRAMES_PER_BUFFER);
         return (framesPerBuffer == null ?
                 DEFAULT_FRAME_PER_BUFFER : Integer.parseInt(framesPerBuffer));
     }
 
+    @CalledByNative
+    public static boolean isPlatformAECSupported() {

[henrika (OOO until Aug 14), 2013/12/06 12:52:21]

Would it make sense to name this API to something which makes it more clear that
we call AcousticEchoCanceler internally?

e.g. hasAcousticEchoCanceler()?

It is basically a thin wrapper on an existing Java API. We do so for e.g.
setSpeakerPhoneOn().

[ajm, 2013/12/06 19:06:43]

Except that most (all?) devices will actually report isAvailable()==true, since
Android has a fallback SW AEC (webrtc's AECM of all things!). The more important
criterion here is the whitelist. I'm open to changing the function name
(suggestions?) but I don't think we should imply it's simply a wrapper over
isAvailable().

[Ami GONE FROM CHROMIUM, 2013/12/06 19:22:53]

hasAcousticEchoCanceler fails to connote that this method applies a whitelist. 
My bikeshed is painted shouldUseAcousticEchoCanceler.

[ajm, 2013/12/10 06:37:16]

Sounds good.

+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN) {
+            // AcousticEchoCanceler was added in API level 16 (Jelly Bean).

[Ami GONE FROM CHROMIUM, 2013/12/06 19:22:53]

Wouldn't this be accounted for by l.397?

[ajm, 2013/12/10 06:37:16]

As I understand it, we'll get a crash if we try to execute anything unavailable
in the runtime SDK. Since Chrome's minSdkVersion is set to 14 (ICS), we need to
guard the new functionality.

I've added comments elsewhere to explain that this class should only be used on
Jelly Bean and up.

None of the devices I have ATM can be flashed to ICS. I'll see if I can pick up
an older device tomorrow to ensure this works fine.

+            return false;
+        }
+
+        boolean isAvailable = AcousticEchoCanceler.isAvailable();
+        boolean isWhitelisted = PLATFORM_AEC_MODEL_WHITELIST.contains(Build.MODEL);

[tommi (sloooow) - chröme, 2013/12/06 12:11:34]

nit: Since the list contains only two items, you probably don't need a HashSet
or static variable/array. You can also bail out sooner here if isAvailable()
returns false.
E.g.:

public static boolean isPlatformAECSupported() {
    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN ||
        !AcousticEchoCanceler.isAvailable();) {
        // AcousticEchoCanceler was added in API level 16 (Jelly Bean).
        return false;
    }
    
    return Build.MODE.equals("Nexus 5") ||
           Build.MODE.equals("Nexus 7")
}

[ajm, 2013/12/06 19:06:43]

My initial implementation had all conditions in the return line. I only changed
it for logging purposes. I'm open to giving up the logging and returning to that
if you prefer.

Regarding the whitelist:
- I hope this list will be expanded in the future. For example, I think Nexus 4
can be added, but some more testing is needed.
- I like the clarity of using .contains() here. Is the concern that the HashSet
consumes unneeded memory?

I'm open to changing to your "A || B || ..." suggestion, just want to understand
the motivation.

[ajm, 2013/12/10 06:37:16]

Changed as recommended now.

+        logd("AcousticEchoCanceler.isAvailable: " + isAvailable);
+        logd("Build.MODEL: " + Build.MODEL + " isWhitelisted: " + isWhitelisted);

[Ami GONE FROM CHROMIUM, 2013/12/06 19:22:53]

I don't think these are worth spamming logcat for.

[ajm, 2013/12/10 06:37:16]

Done.

+        return isAvailable && isWhitelisted;
+    }
+
     /** Sets the speaker phone mode. */
     public void setSpeakerphoneOn(boolean on) {
         boolean wasOn = mAudioManager.isSpeakerphoneOn();
         if (wasOn == on) {
             return;
         }
         mAudioManager.setSpeakerphoneOn(on);
     }
 
     /** Sets the microphone mute state. */
@@ skipping 205 matching lines @@
         }
     }
 
     private void logDeviceInfo() {
         Log.i(TAG, "Manufacturer:" + Build.MANUFACTURER +
                 " Board: " + Build.BOARD + " Device: " + Build.DEVICE +
                 " Model: " + Build.MODEL + " PRODUCT: " + Build.PRODUCT);
     }
 
     /** Trivial helper method for debug logging */
-    private void logd(String msg) {
+    private static void logd(String msg) {
         Log.d(TAG, msg);
     }
 
     /** Trivial helper method for error logging */
-    private void loge(String msg) {
+    private static void loge(String msg) {
         Log.e(TAG, msg);
     }
 
     private class SettingsObserver extends ContentObserver {
         SettingsObserver() {
             super(new Handler());
             mContentResolver.registerContentObserver(Settings.System.CONTENT_URI, true, this);
         }
 
         @Override
@@ skipping 19 matching lines @@
             synchronized(mSettingsObserverLock) {
                 mSettingsObserver = new SettingsObserver();
                 mSettingsObserverLock.notify();
             }
 
             // Listen for volume change.
             Looper.loop();
         }
     }
 }