Merge "SpatializerHelper: fix canBeSpatializedOnDevice NPE" into tm-qpr-dev am: e455bc8e0a

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/22201117

Change-Id: I31d15a7b043c4a45c86d270048ca537ebbd2dfa1
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

services/core/java/com/android/server/audio/SpatializerHelper.java

@@ -106,12 +106,12 @@ public class SpatializerHelper {
     };
 
     // Spatializer state machine
-    private static final int STATE_UNINITIALIZED = 0;
+    /*package*/ static final int STATE_UNINITIALIZED = 0;
-    private static final int STATE_NOT_SUPPORTED = 1;
+    /*package*/ static final int STATE_NOT_SUPPORTED = 1;
-    private static final int STATE_DISABLED_UNAVAILABLE = 3;
+    /*package*/ static final int STATE_DISABLED_UNAVAILABLE = 3;
-    private static final int STATE_ENABLED_UNAVAILABLE = 4;
+    /*package*/ static final int STATE_ENABLED_UNAVAILABLE = 4;
-    private static final int STATE_ENABLED_AVAILABLE = 5;
+    /*package*/ static final int STATE_ENABLED_AVAILABLE = 5;
-    private static final int STATE_DISABLED_AVAILABLE = 6;
+    /*package*/ static final int STATE_DISABLED_AVAILABLE = 6;
     private int mState = STATE_UNINITIALIZED;
 
     private boolean mFeatureEnabled = false;
@@ -147,9 +147,9 @@ public class SpatializerHelper {
             .setSampleRate(48000)
             .setChannelMask(AudioFormat.CHANNEL_OUT_5POINT1)
             .build();
-    // device array to store the routing for the default attributes and format, size 1 because
+    // device array to store the routing for the default attributes and format, initialized to
-    // media is never expected to be duplicated
+    // an empty list as routing hasn't been established yet
-    private static final AudioDeviceAttributes[] ROUTING_DEVICES = new AudioDeviceAttributes[1];
+    private static ArrayList<AudioDeviceAttributes> sRoutingDevices = new ArrayList<>(0);
 
     //---------------------------------------------------------------
     // audio device compatibility / enabled
@@ -184,11 +184,6 @@ public class SpatializerHelper {
         SADeviceState.sHeadTrackingEnabledDefault = headTrackingEnabledDefault;
     }
 
-    synchronized void initForTest(boolean hasBinaural, boolean hasTransaural) {
-        mBinauralSupported = hasBinaural;
-        mTransauralSupported = hasTransaural;
-    }
-
     synchronized void init(boolean effectExpected, @Nullable String settings) {
         loglogi("init effectExpected=" + effectExpected);
         if (!effectExpected) {
@@ -322,8 +317,7 @@ public class SpatializerHelper {
             return;
         }
         mState = STATE_DISABLED_UNAVAILABLE;
-        mASA.getDevicesForAttributes(
+        sRoutingDevices = getRoutingDevices(DEFAULT_ATTRIBUTES);
-                DEFAULT_ATTRIBUTES, false /* forVolume */).toArray(ROUTING_DEVICES);
         // note at this point mSpat is still not instantiated
     }
 
@@ -365,34 +359,35 @@ public class SpatializerHelper {
             case STATE_DISABLED_AVAILABLE:
                 break;
         }
-        mASA.getDevicesForAttributes(
+
-                DEFAULT_ATTRIBUTES, false /* forVolume */).toArray(ROUTING_DEVICES);
+        sRoutingDevices = getRoutingDevices(DEFAULT_ATTRIBUTES);
 
         // check validity of routing information
-        if (ROUTING_DEVICES[0] == null) {
+        if (sRoutingDevices.isEmpty()) {
-            logloge("onRoutingUpdated: device is null, no Spatial Audio");
+            logloge("onRoutingUpdated: no device, no Spatial Audio");
             setDispatchAvailableState(false);
             // not changing the spatializer level as this is likely a transient state
             return;
         }
+        final AudioDeviceAttributes currentDevice = sRoutingDevices.get(0);
 
         // is media routed to a new device?
-        if (isWireless(ROUTING_DEVICES[0].getType())) {
+        if (isWireless(currentDevice.getType())) {
-            addWirelessDeviceIfNew(ROUTING_DEVICES[0]);
+            addWirelessDeviceIfNew(currentDevice);
         }
 
         // find if media device enabled / available
-        final Pair<Boolean, Boolean> enabledAvailable = evaluateState(ROUTING_DEVICES[0]);
+        final Pair<Boolean, Boolean> enabledAvailable = evaluateState(currentDevice);
 
         boolean able = false;
         if (enabledAvailable.second) {
             // available for Spatial audio, check w/ effect
-            able = canBeSpatializedOnDevice(DEFAULT_ATTRIBUTES, DEFAULT_FORMAT, ROUTING_DEVICES);
+            able = canBeSpatializedOnDevice(DEFAULT_ATTRIBUTES, DEFAULT_FORMAT, sRoutingDevices);
             loglogi("onRoutingUpdated: can spatialize media 5.1:" + able
-                    + " on device:" + ROUTING_DEVICES[0]);
+                    + " on device:" + currentDevice);
             setDispatchAvailableState(able);
         } else {
-            loglogi("onRoutingUpdated: device:" + ROUTING_DEVICES[0]
+            loglogi("onRoutingUpdated: device:" + currentDevice
                     + " not available for Spatial Audio");
             setDispatchAvailableState(false);
         }
@@ -400,10 +395,10 @@ public class SpatializerHelper {
         boolean enabled = able && enabledAvailable.first;
         if (enabled) {
             loglogi("Enabling Spatial Audio since enabled for media device:"
-                    + ROUTING_DEVICES[0]);
+                    + currentDevice);
         } else {
             loglogi("Disabling Spatial Audio since disabled for media device:"
-                    + ROUTING_DEVICES[0]);
+                    + currentDevice);
         }
         if (mSpat != null) {
             byte level = enabled ? (byte) Spatializer.SPATIALIZER_IMMERSIVE_LEVEL_MULTICHANNEL
@@ -736,9 +731,13 @@ public class SpatializerHelper {
     }
 
     private synchronized boolean canBeSpatializedOnDevice(@NonNull AudioAttributes attributes,
-            @NonNull AudioFormat format, @NonNull AudioDeviceAttributes[] devices) {
+            @NonNull AudioFormat format, @NonNull ArrayList<AudioDeviceAttributes> devices) {
-        if (isDeviceCompatibleWithSpatializationModes(devices[0])) {
+        if (devices.isEmpty()) {
-            return AudioSystem.canBeSpatialized(attributes, format, devices);
+            return false;
+        }
+        if (isDeviceCompatibleWithSpatializationModes(devices.get(0))) {
+            AudioDeviceAttributes[] devArray = new AudioDeviceAttributes[devices.size()];
+            return AudioSystem.canBeSpatialized(attributes, format, devices.toArray(devArray));
         }
         return false;
     }
@@ -1014,10 +1013,13 @@ public class SpatializerHelper {
                 logd("canBeSpatialized false due to usage:" + attributes.getUsage());
                 return false;
         }
-        AudioDeviceAttributes[] devices = new AudioDeviceAttributes[1];
+
         // going through adapter to take advantage of routing cache
-        mASA.getDevicesForAttributes(
+        final ArrayList<AudioDeviceAttributes> devices = getRoutingDevices(attributes);
-                attributes, false /* forVolume */).toArray(devices);
+        if (devices.isEmpty()) {
+            logloge("canBeSpatialized got no device for " + attributes);
+            return false;
+        }
         final boolean able = canBeSpatializedOnDevice(attributes, format, devices);
         logd("canBeSpatialized usage:" + attributes.getUsage()
                 + " format:" + format.toLogFriendlyString() + " returning " + able);
@@ -1148,8 +1150,13 @@ public class SpatializerHelper {
         logDeviceState(deviceState, "setHeadTrackerEnabled");
 
         // check current routing to see if it affects the headtracking mode
-        if (ROUTING_DEVICES[0] != null && ROUTING_DEVICES[0].getType() == ada.getType()
+        if (sRoutingDevices.isEmpty()) {
-                && ROUTING_DEVICES[0].getAddress().equals(ada.getAddress())) {
+            logloge("setHeadTrackerEnabled: no device, bailing");
+            return;
+        }
+        final AudioDeviceAttributes currentDevice = sRoutingDevices.get(0);
+        if (currentDevice.getType() == ada.getType()
+                && currentDevice.getAddress().equals(ada.getAddress())) {
             setDesiredHeadTrackingMode(enabled ? mDesiredHeadTrackingModeWhenEnabled
                     : Spatializer.HEAD_TRACKING_MODE_DISABLED);
             if (enabled && !mHeadTrackerAvailable) {
@@ -1706,10 +1713,11 @@ public class SpatializerHelper {
 
     private int getHeadSensorHandleUpdateTracker() {
         int headHandle = -1;
-        final AudioDeviceAttributes currentDevice = ROUTING_DEVICES[0];
+        if (sRoutingDevices.isEmpty()) {
-        if (currentDevice == null) {
+            logloge("getHeadSensorHandleUpdateTracker: no device, no head tracker");
             return headHandle;
         }
+        final AudioDeviceAttributes currentDevice = sRoutingDevices.get(0);
         UUID routingDeviceUuid = mAudioService.getDeviceSensorUuid(currentDevice);
         // We limit only to Sensor.TYPE_HEAD_TRACKER here to avoid confusion
         // with gaming sensors. (Note that Sensor.TYPE_ROTATION_VECTOR
@@ -1743,6 +1751,23 @@ public class SpatializerHelper {
         return screenHandle;
     }
 
+    /**
+     * Returns routing for the given attributes
+     * @param aa AudioAttributes whose routing is being queried
+     * @return a non-null never-empty list of devices. If the routing query failed, the list
+     *     will contain null.
+     */
+    private @NonNull ArrayList<AudioDeviceAttributes> getRoutingDevices(AudioAttributes aa) {
+        final ArrayList<AudioDeviceAttributes> devices = mASA.getDevicesForAttributes(
+                aa, false /* forVolume */);
+        for (AudioDeviceAttributes ada : devices) {
+            if (ada == null) {
+                // invalid entry, reject this routing query by returning an empty list
+                return new ArrayList<>(0);
+            }
+        }
+        return devices;
+    }
 
     private static void loglogi(String msg) {
         AudioService.sSpatialLogger.loglogi(msg, TAG);
@@ -1759,4 +1784,13 @@ public class SpatializerHelper {
     /*package*/ void clearSADevices() {
         mSADevices.clear();
     }
+
+    /*package*/ synchronized void forceStateForTest(int state) {
+        mState = state;
+    }
+
+    /*package*/ synchronized void initForTest(boolean hasBinaural, boolean hasTransaural) {
+        mBinauralSupported = hasBinaural;
+        mTransauralSupported = hasTransaural;
+    }
 }

services/tests/servicestests/src/com/android/server/audio/SpatializerHelperTest.java

@@ -17,12 +17,17 @@ package com.android.server.audio;
 
 import com.android.server.audio.SpatializerHelper.SADeviceState;
 
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
 
+import android.media.AudioAttributes;
 import android.media.AudioDeviceAttributes;
 import android.media.AudioDeviceInfo;
+import android.media.AudioFormat;
 import android.media.AudioSystem;
 import android.util.Log;
 
@@ -36,6 +41,7 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.Spy;
 
+import java.util.ArrayList;
 import java.util.List;
 
 @MediumTest
@@ -49,16 +55,35 @@ public class SpatializerHelperTest {
 
     @Mock private AudioService mMockAudioService;
     @Spy private AudioSystemAdapter mSpyAudioSystem;
+    @Mock private AudioSystemAdapter mMockAudioSystem;
 
     @Before
     public void setUp() throws Exception {
         mMockAudioService = mock(AudioService.class);
-        mSpyAudioSystem = spy(new NoOpAudioSystemAdapter());
+    }
 
-        mSpatHelper = new SpatializerHelper(mMockAudioService, mSpyAudioSystem,
+    /**
+     * Initializes mSpatHelper, the SpatizerHelper instance under test, to use the mock or spy
+     * AudioSystemAdapter
+     * @param useSpyAudioSystem true to use the spy adapter, mSpyAudioSystem, or false to use
+     *                          the mock adapter, mMockAudioSystem.
+     */
+    private void setUpSpatHelper(boolean useSpyAudioSystem) {
+        final AudioSystemAdapter asAdapter;
+        if (useSpyAudioSystem) {
+            mSpyAudioSystem = spy(new NoOpAudioSystemAdapter());
+            asAdapter = mSpyAudioSystem;
+            mMockAudioSystem = null;
+        } else {
+            mSpyAudioSystem = null;
+            mMockAudioSystem = mock(NoOpAudioSystemAdapter.class);
+            asAdapter = mMockAudioSystem;
+        }
+        mSpatHelper = new SpatializerHelper(mMockAudioService, asAdapter,
                 true /*binauralEnabledDefault*/,
                 true /*transauralEnabledDefault*/,
                 false /*headTrackingEnabledDefault*/);
+
     }
 
     /**
@@ -68,6 +93,7 @@ public class SpatializerHelperTest {
      */
     @Test
     public void testSADeviceStateNullAddressCtor() throws Exception {
+        setUpSpatHelper(true /*useSpyAudioSystem*/);
         try {
             SADeviceState devState = new SADeviceState(AudioDeviceInfo.TYPE_BUILTIN_SPEAKER, null);
             devState = new SADeviceState(AudioDeviceInfo.TYPE_BLUETOOTH_A2DP, null);
@@ -78,6 +104,7 @@ public class SpatializerHelperTest {
     @Test
     public void testSADeviceStateStringSerialization() throws Exception {
         Log.i(TAG, "starting testSADeviceStateStringSerialization");
+        setUpSpatHelper(true /*useSpyAudioSystem*/);
         final SADeviceState devState = new SADeviceState(
                 AudioDeviceInfo.TYPE_BUILTIN_SPEAKER, "bla");
         devState.mHasHeadTracker = false;
@@ -93,6 +120,7 @@ public class SpatializerHelperTest {
     @Test
     public void testSADeviceSettings() throws Exception {
         Log.i(TAG, "starting testSADeviceSettings");
+        setUpSpatHelper(true /*useSpyAudioSystem*/);
         final AudioDeviceAttributes dev1 =
                 new AudioDeviceAttributes(AudioSystem.DEVICE_OUT_SPEAKER, "");
         final AudioDeviceAttributes dev2 =
@@ -143,4 +171,34 @@ public class SpatializerHelperTest {
         Log.i(TAG, "device settingsRestored: " + settingsRestored);
         Assert.assertEquals(settings, settingsRestored);
     }
+
+    /**
+     * Test that null devices for routing do not break canBeSpatialized
+     * @throws Exception
+     */
+    @Test
+    public void testNoRoutingCanBeSpatialized() throws Exception {
+        Log.i(TAG, "Starting testNoRoutingCanBeSpatialized");
+        setUpSpatHelper(false /*useSpyAudioSystem*/);
+        mSpatHelper.forceStateForTest(SpatializerHelper.STATE_ENABLED_AVAILABLE);
+
+        final ArrayList<AudioDeviceAttributes> emptyList = new ArrayList<>(0);
+        final ArrayList<AudioDeviceAttributes> listWithNull = new ArrayList<>(1);
+        listWithNull.add(null);
+        final AudioAttributes media = new AudioAttributes.Builder()
+                .setUsage(AudioAttributes.USAGE_MEDIA).build();
+        final AudioFormat spatialFormat = new AudioFormat.Builder()
+                .setEncoding(AudioFormat.ENCODING_PCM_16BIT)
+                .setChannelMask(AudioFormat.CHANNEL_OUT_5POINT1).build();
+
+        when(mMockAudioSystem.getDevicesForAttributes(any(AudioAttributes.class), anyBoolean()))
+                .thenReturn(emptyList);
+        Assert.assertFalse("can be spatialized on empty routing",
+                mSpatHelper.canBeSpatialized(media, spatialFormat));
+
+        when(mMockAudioSystem.getDevicesForAttributes(any(AudioAttributes.class), anyBoolean()))
+                .thenReturn(listWithNull);
+        Assert.assertFalse("can be spatialized on null routing",
+                mSpatHelper.canBeSpatialized(media, spatialFormat));
+    }
 }