From 50c8a39b51c30e50382f7dd2dc09c8b9f7726349 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Thu, 11 May 2023 13:32:59 +0200 Subject: [PATCH 1/2] Revert "AudioDeviceBroker: fix deadlock with setA2dpSuspended" This reverts commit 2bd07ff1daed36dc1ffd30134e50887996250927. Bug: 281786703 Test: make Change-Id: I5ed93786daf6bd34a249d231b2831aab158d6476 --- .../android/server/audio/AudioDeviceBroker.java | 17 ----------------- .../com/android/server/audio/AudioService.java | 4 ++-- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index bc4e8df2a4ad3..e3273b174fed4 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -971,10 +971,6 @@ import java.util.concurrent.atomic.AtomicBoolean; } } - /*package*/ void postSetA2dpSuspended(boolean enable, String eventSource) { - sendILMsgNoDelay(MSG_IL_SET_A2DP_SUSPENDED, SENDMSG_QUEUE, (enable ? 1 : 0), eventSource); - } - /*package*/ void setA2dpSuspended(boolean enable, boolean internal, String eventSource) { if (AudioService.DEBUG_COMM_RTE) { Log.v(TAG, "setA2dpSuspended source: " + eventSource + ", enable: " @@ -1003,11 +999,6 @@ import java.util.concurrent.atomic.AtomicBoolean; } } - /*package*/ void postSetLeAudioSuspended(boolean enable, String eventSource) { - sendILMsgNoDelay( - MSG_IL_SET_LEAUDIO_SUSPENDED, SENDMSG_QUEUE, (enable ? 1 : 0), eventSource); - } - /*package*/ void setLeAudioSuspended(boolean enable, boolean internal, String eventSource) { if (AudioService.DEBUG_COMM_RTE) { Log.v(TAG, "setLeAudioSuspended source: " + eventSource + ", enable: " @@ -1821,12 +1812,6 @@ import java.util.concurrent.atomic.AtomicBoolean; final int capturePreset = msg.arg1; mDeviceInventory.onSaveClearPreferredDevicesForCapturePreset(capturePreset); } break; - case MSG_IL_SET_A2DP_SUSPENDED: { - setA2dpSuspended((msg.arg1 == 1), false /*internal*/, (String) msg.obj); - } break; - case MSG_IL_SET_LEAUDIO_SUSPENDED: { - setLeAudioSuspended((msg.arg1 == 1), false /*internal*/, (String) msg.obj); - } break; case MSG_L_NOTIFY_PREFERRED_AUDIOPROFILE_APPLIED: { final BluetoothDevice btDevice = (BluetoothDevice) msg.obj; BtHelper.onNotifyPreferredAudioProfileApplied(btDevice); @@ -1905,8 +1890,6 @@ import java.util.concurrent.atomic.AtomicBoolean; private static final int MSG_IL_SAVE_NDEF_DEVICE_FOR_STRATEGY = 47; private static final int MSG_IL_SAVE_REMOVE_NDEF_DEVICE_FOR_STRATEGY = 48; private static final int MSG_IL_BTLEAUDIO_TIMEOUT = 49; - private static final int MSG_IL_SET_A2DP_SUSPENDED = 50; - private static final int MSG_IL_SET_LEAUDIO_SUSPENDED = 51; private static final int MSG_L_NOTIFY_PREFERRED_AUDIOPROFILE_APPLIED = 52; diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 20393250b5b78..3487fc2c14bee 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -6412,7 +6412,7 @@ public class AudioService extends IAudioService.Stub final String eventSource = new StringBuilder("setA2dpSuspended(").append(enable) .append(") from u/pid:").append(Binder.getCallingUid()).append("/") .append(Binder.getCallingPid()).toString(); - mDeviceBroker.postSetA2dpSuspended(enable, eventSource); + mDeviceBroker.setA2dpSuspended(enable, false /*internal*/, eventSource); } /** @see AudioManager#setA2dpSuspended(boolean) */ @@ -6422,7 +6422,7 @@ public class AudioService extends IAudioService.Stub final String eventSource = new StringBuilder("setLeAudioSuspended(").append(enable) .append(") from u/pid:").append(Binder.getCallingUid()).append("/") .append(Binder.getCallingPid()).toString(); - mDeviceBroker.postSetLeAudioSuspended(enable, eventSource); + mDeviceBroker.setLeAudioSuspended(enable, false /*internal*/, eventSource); } /** @see AudioManager#isBluetoothScoOn() From 7282eb5e2e1a3d32cbe2b886e71c64ee0dbd9828 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Thu, 11 May 2023 13:33:51 +0200 Subject: [PATCH 2/2] Revert "AudioService: synchronize BT_SCO on and A2DP or LE suspended states" This reverts commit c0184adb64974c8a1f947a923832909ecec465db. Bug 281786703 Test: make Change-Id: I81bda9127c951850adcda43c010fe3d560395dd4 --- media/java/android/media/AudioManager.java | 14 +- media/java/android/media/IAudioService.aidl | 6 - .../server/audio/AudioDeviceBroker.java | 160 +----------------- .../server/audio/AudioDeviceInventory.java | 10 +- .../android/server/audio/AudioService.java | 20 --- .../com/android/server/audio/BtHelper.java | 4 +- 6 files changed, 17 insertions(+), 197 deletions(-) diff --git a/media/java/android/media/AudioManager.java b/media/java/android/media/AudioManager.java index 4759689335e96..b1d2e33df3f79 100644 --- a/media/java/android/media/AudioManager.java +++ b/media/java/android/media/AudioManager.java @@ -3730,12 +3730,7 @@ public class AudioManager { @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) @RequiresPermission(Manifest.permission.BLUETOOTH_STACK) public void setA2dpSuspended(boolean enable) { - final IAudioService service = getService(); - try { - service.setA2dpSuspended(enable); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + AudioSystem.setParameters("A2dpSuspended=" + enable); } /** @@ -3748,12 +3743,7 @@ public class AudioManager { @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) @RequiresPermission(Manifest.permission.BLUETOOTH_STACK) public void setLeAudioSuspended(boolean enable) { - final IAudioService service = getService(); - try { - service.setLeAudioSuspended(enable); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + AudioSystem.setParameters("LeAudioSuspended=" + enable); } /** diff --git a/media/java/android/media/IAudioService.aidl b/media/java/android/media/IAudioService.aidl index 7ce189ba85d58..fe5afc5a717e6 100644 --- a/media/java/android/media/IAudioService.aidl +++ b/media/java/android/media/IAudioService.aidl @@ -231,12 +231,6 @@ interface IAudioService { void setBluetoothScoOn(boolean on); - @EnforcePermission("BLUETOOTH_STACK") - void setA2dpSuspended(boolean on); - - @EnforcePermission("BLUETOOTH_STACK") - void setLeAudioSuspended(boolean enable); - boolean isBluetoothScoOn(); void setBluetoothA2dpOn(boolean on); diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index e3273b174fed4..71401f4372325 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -209,7 +209,6 @@ import java.util.concurrent.atomic.AtomicBoolean; private void init() { setupMessaging(mContext); - initAudioHalBluetoothState(); initRoutingStrategyIds(); mPreferredCommunicationDevice = null; updateActiveCommunicationDevice(); @@ -865,100 +864,10 @@ import java.util.concurrent.atomic.AtomicBoolean; } } - // Current Bluetooth SCO audio active state indicated by BtHelper via setBluetoothScoOn(). - @GuardedBy("mDeviceStateLock") + /** + * Current Bluetooth SCO audio active state indicated by BtHelper via setBluetoothScoOn(). + */ private boolean mBluetoothScoOn; - // value of BT_SCO parameter currently applied to audio HAL. - @GuardedBy("mDeviceStateLock") - private boolean mBluetoothScoOnApplied; - - // A2DP suspend state requested by AudioManager.setA2dpSuspended() API. - @GuardedBy("mDeviceStateLock") - private boolean mBluetoothA2dpSuspendedExt; - // A2DP suspend state requested by AudioDeviceInventory. - @GuardedBy("mDeviceStateLock") - private boolean mBluetoothA2dpSuspendedInt; - // value of BT_A2dpSuspendedSCO parameter currently applied to audio HAL. - @GuardedBy("mDeviceStateLock") - private boolean mBluetoothA2dpSuspendedApplied; - - // LE Audio suspend state requested by AudioManager.setLeAudioSuspended() API. - @GuardedBy("mDeviceStateLock") - private boolean mBluetoothLeSuspendedExt; - // LE Audio suspend state requested by AudioDeviceInventory. - @GuardedBy("mDeviceStateLock") - private boolean mBluetoothLeSuspendedInt; - // value of LeAudioSuspended parameter currently applied to audio HAL. - @GuardedBy("mDeviceStateLock") - private boolean mBluetoothLeSuspendedApplied; - - private void initAudioHalBluetoothState() { - mBluetoothScoOnApplied = false; - AudioSystem.setParameters("BT_SCO=off"); - mBluetoothA2dpSuspendedApplied = false; - AudioSystem.setParameters("A2dpSuspended=false"); - mBluetoothLeSuspendedApplied = false; - AudioSystem.setParameters("LeAudioSuspended=false"); - } - - @GuardedBy("mDeviceStateLock") - private void updateAudioHalBluetoothState() { - if (mBluetoothScoOn != mBluetoothScoOnApplied) { - if (AudioService.DEBUG_COMM_RTE) { - Log.v(TAG, "updateAudioHalBluetoothState() mBluetoothScoOn: " - + mBluetoothScoOn + ", mBluetoothScoOnApplied: " + mBluetoothScoOnApplied); - } - if (mBluetoothScoOn) { - if (!mBluetoothA2dpSuspendedApplied) { - AudioSystem.setParameters("A2dpSuspended=true"); - mBluetoothA2dpSuspendedApplied = true; - } - if (!mBluetoothLeSuspendedApplied) { - AudioSystem.setParameters("LeAudioSuspended=true"); - mBluetoothLeSuspendedApplied = true; - } - AudioSystem.setParameters("BT_SCO=on"); - } else { - AudioSystem.setParameters("BT_SCO=off"); - } - mBluetoothScoOnApplied = mBluetoothScoOn; - } - if (!mBluetoothScoOnApplied) { - if ((mBluetoothA2dpSuspendedExt || mBluetoothA2dpSuspendedInt) - != mBluetoothA2dpSuspendedApplied) { - if (AudioService.DEBUG_COMM_RTE) { - Log.v(TAG, "updateAudioHalBluetoothState() mBluetoothA2dpSuspendedExt: " - + mBluetoothA2dpSuspendedExt - + ", mBluetoothA2dpSuspendedInt: " + mBluetoothA2dpSuspendedInt - + ", mBluetoothA2dpSuspendedApplied: " - + mBluetoothA2dpSuspendedApplied); - } - mBluetoothA2dpSuspendedApplied = - mBluetoothA2dpSuspendedExt || mBluetoothA2dpSuspendedInt; - if (mBluetoothA2dpSuspendedApplied) { - AudioSystem.setParameters("A2dpSuspended=true"); - } else { - AudioSystem.setParameters("A2dpSuspended=false"); - } - } - if ((mBluetoothLeSuspendedExt || mBluetoothLeSuspendedInt) - != mBluetoothLeSuspendedApplied) { - if (AudioService.DEBUG_COMM_RTE) { - Log.v(TAG, "updateAudioHalBluetoothState() mBluetoothLeSuspendedExt: " - + mBluetoothLeSuspendedExt - + ", mBluetoothLeSuspendedInt: " + mBluetoothLeSuspendedInt - + ", mBluetoothLeSuspendedApplied: " + mBluetoothLeSuspendedApplied); - } - mBluetoothLeSuspendedApplied = - mBluetoothLeSuspendedExt || mBluetoothLeSuspendedInt; - if (mBluetoothLeSuspendedApplied) { - AudioSystem.setParameters("LeAudioSuspended=true"); - } else { - AudioSystem.setParameters("LeAudioSuspended=false"); - } - } - } - } /*package*/ void setBluetoothScoOn(boolean on, String eventSource) { if (AudioService.DEBUG_COMM_RTE) { @@ -966,67 +875,10 @@ import java.util.concurrent.atomic.AtomicBoolean; } synchronized (mDeviceStateLock) { mBluetoothScoOn = on; - updateAudioHalBluetoothState(); postUpdateCommunicationRouteClient(eventSource); } } - /*package*/ void setA2dpSuspended(boolean enable, boolean internal, String eventSource) { - if (AudioService.DEBUG_COMM_RTE) { - Log.v(TAG, "setA2dpSuspended source: " + eventSource + ", enable: " - + enable + ", internal: " + internal - + ", mBluetoothA2dpSuspendedInt: " + mBluetoothA2dpSuspendedInt - + ", mBluetoothA2dpSuspendedExt: " + mBluetoothA2dpSuspendedExt); - } - synchronized (mDeviceStateLock) { - if (internal) { - mBluetoothA2dpSuspendedInt = enable; - } else { - mBluetoothA2dpSuspendedExt = enable; - } - updateAudioHalBluetoothState(); - } - } - - /*package*/ void clearA2dpSuspended() { - if (AudioService.DEBUG_COMM_RTE) { - Log.v(TAG, "clearA2dpSuspended"); - } - synchronized (mDeviceStateLock) { - mBluetoothA2dpSuspendedInt = false; - mBluetoothA2dpSuspendedExt = false; - updateAudioHalBluetoothState(); - } - } - - /*package*/ void setLeAudioSuspended(boolean enable, boolean internal, String eventSource) { - if (AudioService.DEBUG_COMM_RTE) { - Log.v(TAG, "setLeAudioSuspended source: " + eventSource + ", enable: " - + enable + ", internal: " + internal - + ", mBluetoothLeSuspendedInt: " + mBluetoothA2dpSuspendedInt - + ", mBluetoothLeSuspendedExt: " + mBluetoothA2dpSuspendedExt); - } - synchronized (mDeviceStateLock) { - if (internal) { - mBluetoothLeSuspendedInt = enable; - } else { - mBluetoothLeSuspendedExt = enable; - } - updateAudioHalBluetoothState(); - } - } - - /*package*/ void clearLeAudioSuspended() { - if (AudioService.DEBUG_COMM_RTE) { - Log.v(TAG, "clearLeAudioSuspended"); - } - synchronized (mDeviceStateLock) { - mBluetoothLeSuspendedInt = false; - mBluetoothLeSuspendedExt = false; - updateAudioHalBluetoothState(); - } - } - /*package*/ AudioRoutesInfo startWatchingRoutes(IAudioRoutesObserver observer) { synchronized (mDeviceStateLock) { return mDeviceInventory.startWatchingRoutes(observer); @@ -2168,6 +2020,12 @@ import java.util.concurrent.atomic.AtomicBoolean; "updateCommunicationRoute, preferredCommunicationDevice: " + preferredCommunicationDevice + " eventSource: " + eventSource))); + if (preferredCommunicationDevice == null + || preferredCommunicationDevice.getType() != AudioDeviceInfo.TYPE_BLUETOOTH_SCO) { + AudioSystem.setParameters("BT_SCO=off"); + } else { + AudioSystem.setParameters("BT_SCO=on"); + } if (preferredCommunicationDevice == null) { AudioDeviceAttributes defaultDevice = getDefaultCommunicationDevice(); if (defaultDevice != null) { diff --git a/services/core/java/com/android/server/audio/AudioDeviceInventory.java b/services/core/java/com/android/server/audio/AudioDeviceInventory.java index 773df3720ed36..1fda71264a286 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceInventory.java +++ b/services/core/java/com/android/server/audio/AudioDeviceInventory.java @@ -1477,7 +1477,7 @@ public class AudioDeviceInventory { } // Reset A2DP suspend state each time a new sink is connected - mDeviceBroker.clearA2dpSuspended(); + mAudioSystem.setParameters("A2dpSuspended=false"); // The convention for head tracking sensors associated with A2DP devices is to // use a UUID derived from the MAC address as follows: @@ -1750,8 +1750,7 @@ public class AudioDeviceInventory { private void makeA2dpDeviceUnavailableLater(String address, int delayMs) { // prevent any activity on the A2DP audio output to avoid unwanted // reconnection of the sink. - mDeviceBroker.setA2dpSuspended( - true /*enable*/, true /*internal*/, "makeA2dpDeviceUnavailableLater"); + mAudioSystem.setParameters("A2dpSuspended=true"); // retrieve DeviceInfo before removing device final String deviceKey = DeviceInfo.makeDeviceListKey(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, address); @@ -1898,7 +1897,7 @@ public class AudioDeviceInventory { "LE Audio device addr=" + address + " now available").printLog(TAG)); } // Reset LEA suspend state each time a new sink is connected - mDeviceBroker.clearLeAudioSuspended(); + mAudioSystem.setParameters("LeAudioSuspended=false"); UUID sensorUuid = UuidUtils.uuidFromAudioDeviceAttributes(ada); mConnectedDevices.put(DeviceInfo.makeDeviceListKey(device, address), @@ -1953,8 +1952,7 @@ public class AudioDeviceInventory { private void makeLeAudioDeviceUnavailableLater(String address, int device, int delayMs) { // prevent any activity on the LEA output to avoid unwanted // reconnection of the sink. - mDeviceBroker.setLeAudioSuspended( - true /*enable*/, true /*internal*/, "makeLeAudioDeviceUnavailableLater"); + mAudioSystem.setParameters("LeAudioSuspended=true"); // the device will be made unavailable later, so consider it disconnected right away mConnectedDevices.remove(DeviceInfo.makeDeviceListKey(device, address)); // send the delayed message to make the device unavailable later diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 3487fc2c14bee..8777d60a343bc 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -6405,26 +6405,6 @@ public class AudioService extends IAudioService.Stub mDeviceBroker.setBluetoothScoOn(on, eventSource); } - /** @see AudioManager#setA2dpSuspended(boolean) */ - @android.annotation.EnforcePermission(android.Manifest.permission.BLUETOOTH_STACK) - public void setA2dpSuspended(boolean enable) { - super.setA2dpSuspended_enforcePermission(); - final String eventSource = new StringBuilder("setA2dpSuspended(").append(enable) - .append(") from u/pid:").append(Binder.getCallingUid()).append("/") - .append(Binder.getCallingPid()).toString(); - mDeviceBroker.setA2dpSuspended(enable, false /*internal*/, eventSource); - } - - /** @see AudioManager#setA2dpSuspended(boolean) */ - @android.annotation.EnforcePermission(android.Manifest.permission.BLUETOOTH_STACK) - public void setLeAudioSuspended(boolean enable) { - super.setLeAudioSuspended_enforcePermission(); - final String eventSource = new StringBuilder("setLeAudioSuspended(").append(enable) - .append(") from u/pid:").append(Binder.getCallingUid()).append("/") - .append(Binder.getCallingPid()).toString(); - mDeviceBroker.setLeAudioSuspended(enable, false /*internal*/, eventSource); - } - /** @see AudioManager#isBluetoothScoOn() * Note that it doesn't report internal state, but state seen by apps (which may have * called setBluetoothScoOn() */ diff --git a/services/core/java/com/android/server/audio/BtHelper.java b/services/core/java/com/android/server/audio/BtHelper.java index e46c3cc4a2853..bfa6c36e1c570 100644 --- a/services/core/java/com/android/server/audio/BtHelper.java +++ b/services/core/java/com/android/server/audio/BtHelper.java @@ -445,8 +445,8 @@ public class BtHelper { /*package*/ synchronized void resetBluetoothSco() { mScoAudioState = SCO_STATE_INACTIVE; broadcastScoConnectionState(AudioManager.SCO_AUDIO_STATE_DISCONNECTED); - mDeviceBroker.clearA2dpSuspended(); - mDeviceBroker.clearLeAudioSuspended(); + AudioSystem.setParameters("A2dpSuspended=false"); + AudioSystem.setParameters("LeAudioSuspended=false"); mDeviceBroker.setBluetoothScoOn(false, "resetBluetoothSco"); }