SpatializerHelper: fix canBeSpatializedOnDevice NPE

Check whether routing is known when calling
canBeSpatializedOnDevice. Unknown routing is defined
by an empty list of devices for the requested
AudioAttributes.

Add test method in SpatializerHelper to force state.
Implement a unit test covering failure modes when
querying routing.

Test: atest com.android.server.audio.SpatializerHelperTest#testNoRoutingCanBeSpatialized
Bug: 267963974
Change-Id: Ib6d73fec4d57cf372361762b5c1d85329794de92
Merged-In: Ib6d73fec4d57cf372361762b5c1d85329794de92
(cherry picked from commit af85e232b7)
(cherry picked from commit d5026e8c67)

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));
+    }
 }