From e9e4a3de4a2782cee906eea70d9a3f403366df14 Mon Sep 17 00:00:00 2001 From: Jean-Michel Trivi Date: Wed, 22 Apr 2020 21:38:07 -0700 Subject: [PATCH] AudioService: fix A2DP (dis)connection during media playback Fix issue with A2DP connection/disconnection during media playback by modifying: - how messages related to A2DP connection/disconnection are removed from the AudioDeviceBroker message handler: have .equals() be used when checking which object to remove, and correct which class is used for the messages in the queue: * MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_*CONNECTED is using BtHelper.BluetoothA2dpDeviceInfo * MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_CONNECTION is using BtDeviceConnectionInfo - locking: use the AudioDeviceBroker main lock, mDeviceStateLock to synchronize posting events on the handler, and removing some of those messages to avoid race conditions the thread in which events are posted (e.g. BT's event thread) and the AudioDeviceBroker handler thread. Cleanup: annotation around audio formats from native defined in AudioSystem, moved conversions to AudioSystem. Bug: 142293357 Test: atest AudioDeviceBrokerTest Change-Id: I2e8dc813354829fb1b2c7ca95ad395a14f8b70cf --- media/java/android/media/AudioSystem.java | 47 +++++++- .../server/audio/AudioDeviceBroker.java | 101 +++++++++++++----- .../server/audio/AudioDeviceInventory.java | 28 ++--- .../com/android/server/audio/BtHelper.java | 39 +++---- 4 files changed, 151 insertions(+), 64 deletions(-) diff --git a/media/java/android/media/AudioSystem.java b/media/java/android/media/AudioSystem.java index 373f6e1269241..2d00010f51ee3 100644 --- a/media/java/android/media/AudioSystem.java +++ b/media/java/android/media/AudioSystem.java @@ -199,18 +199,61 @@ public class AudioSystem /** @hide */ public static final int AUDIO_FORMAT_LDAC = 0x23000000; + /** @hide */ + @IntDef(flag = false, prefix = "AUDIO_FORMAT_", value = { + AUDIO_FORMAT_INVALID, + AUDIO_FORMAT_DEFAULT, + AUDIO_FORMAT_AAC, + AUDIO_FORMAT_SBC, + AUDIO_FORMAT_APTX, + AUDIO_FORMAT_APTX_HD, + AUDIO_FORMAT_LDAC } + ) + @Retention(RetentionPolicy.SOURCE) + public @interface AudioFormatNativeEnumForBtCodec {} + /** * @hide * Convert audio format enum values to Bluetooth codec values */ - public static int audioFormatToBluetoothSourceCodec(int audioFormat) { + public static int audioFormatToBluetoothSourceCodec( + @AudioFormatNativeEnumForBtCodec int audioFormat) { switch (audioFormat) { case AUDIO_FORMAT_AAC: return BluetoothCodecConfig.SOURCE_CODEC_TYPE_AAC; case AUDIO_FORMAT_SBC: return BluetoothCodecConfig.SOURCE_CODEC_TYPE_SBC; case AUDIO_FORMAT_APTX: return BluetoothCodecConfig.SOURCE_CODEC_TYPE_APTX; case AUDIO_FORMAT_APTX_HD: return BluetoothCodecConfig.SOURCE_CODEC_TYPE_APTX_HD; case AUDIO_FORMAT_LDAC: return BluetoothCodecConfig.SOURCE_CODEC_TYPE_LDAC; - default: return BluetoothCodecConfig.SOURCE_CODEC_TYPE_INVALID; + default: + Log.e(TAG, "Unknown audio format 0x" + Integer.toHexString(audioFormat) + + " for conversion to BT codec"); + return BluetoothCodecConfig.SOURCE_CODEC_TYPE_INVALID; + } + } + + /** + * @hide + * Convert a Bluetooth codec to an audio format enum + * @param btCodec the codec to convert. + * @return the audio format, or {@link #AUDIO_FORMAT_DEFAULT} if unknown + */ + public static @AudioFormatNativeEnumForBtCodec int bluetoothCodecToAudioFormat(int btCodec) { + switch (btCodec) { + case BluetoothCodecConfig.SOURCE_CODEC_TYPE_SBC: + return AudioSystem.AUDIO_FORMAT_SBC; + case BluetoothCodecConfig.SOURCE_CODEC_TYPE_AAC: + return AudioSystem.AUDIO_FORMAT_AAC; + case BluetoothCodecConfig.SOURCE_CODEC_TYPE_APTX: + return AudioSystem.AUDIO_FORMAT_APTX; + case BluetoothCodecConfig.SOURCE_CODEC_TYPE_APTX_HD: + return AudioSystem.AUDIO_FORMAT_APTX_HD; + case BluetoothCodecConfig.SOURCE_CODEC_TYPE_LDAC: + return AudioSystem.AUDIO_FORMAT_LDAC; + default: + Log.e(TAG, "Unknown BT codec 0x" + Integer.toHexString(btCodec) + + " for conversion to audio format"); + // TODO returning DEFAULT is the current behavior, should this return INVALID? + return AudioSystem.AUDIO_FORMAT_DEFAULT; } } diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index 6067ccb2da27b..40b6f42309bd7 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -255,7 +255,21 @@ import java.io.PrintWriter; // redefine equality op so we can match messages intended for this device @Override public boolean equals(Object o) { - return mDevice.equals(o); + if (o == null) { + return false; + } + if (this == o) { + return true; + } + if (o instanceof BtDeviceConnectionInfo) { + return mDevice.equals(((BtDeviceConnectionInfo) o).mDevice); + } + return false; + } + + @Override + public String toString() { + return "BtDeviceConnectionInfo dev=" + mDevice.toString(); } } @@ -266,27 +280,45 @@ import java.io.PrintWriter; final BtDeviceConnectionInfo info = new BtDeviceConnectionInfo(device, state, profile, suppressNoisyIntent, a2dpVolume); - // when receiving a request to change the connection state of a device, this last request - // is the source of truth, so cancel all previous requests - removeAllA2dpConnectionEvents(device); + // operations of removing and posting messages related to A2DP device state change must be + // mutually exclusive + synchronized (mDeviceStateLock) { + // when receiving a request to change the connection state of a device, this last + // request is the source of truth, so cancel all previous requests that are already in + // the handler + removeScheduledA2dpEvents(device); - sendLMsgNoDelay( - state == BluetoothProfile.STATE_CONNECTED - ? MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_CONNECTION - : MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_DISCONNECTION, - SENDMSG_QUEUE, info); + sendLMsgNoDelay( + state == BluetoothProfile.STATE_CONNECTED + ? MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_CONNECTION + : MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_DISCONNECTION, + SENDMSG_QUEUE, info); + } } - /** remove all previously scheduled connection and disconnection events for the given device */ - private void removeAllA2dpConnectionEvents(@NonNull BluetoothDevice device) { - mBrokerHandler.removeMessages(MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_DISCONNECTION, - device); - mBrokerHandler.removeMessages(MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_CONNECTION, - device); - mBrokerHandler.removeMessages(MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_CONNECTED, - device); - mBrokerHandler.removeMessages(MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_DISCONNECTED, - device); + /** remove all previously scheduled connection and state change events for the given device */ + @GuardedBy("mDeviceStateLock") + private void removeScheduledA2dpEvents(@NonNull BluetoothDevice device) { + mBrokerHandler.removeEqualMessages(MSG_L_A2DP_DEVICE_CONFIG_CHANGE, device); + + final BtDeviceConnectionInfo connectionInfoToRemove = new BtDeviceConnectionInfo(device, + // the next parameters of the constructor will be ignored when finding the message + // to remove as the equality of the message's object is tested on the device itself + // (see BtDeviceConnectionInfo.equals() method override) + BluetoothProfile.STATE_CONNECTED, 0, false, -1); + mBrokerHandler.removeEqualMessages(MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_DISCONNECTION, + connectionInfoToRemove); + mBrokerHandler.removeEqualMessages(MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_CONNECTION, + connectionInfoToRemove); + + final BtHelper.BluetoothA2dpDeviceInfo devInfoToRemove = + new BtHelper.BluetoothA2dpDeviceInfo(device); + mBrokerHandler.removeEqualMessages(MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_CONNECTED, + devInfoToRemove); + mBrokerHandler.removeEqualMessages(MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_DISCONNECTED, + devInfoToRemove); + mBrokerHandler.removeEqualMessages(MSG_L_A2DP_ACTIVE_DEVICE_CHANGE, + devInfoToRemove); } private static final class HearingAidDeviceConnectionInfo { @@ -493,6 +525,7 @@ import java.io.PrintWriter; sendMsgNoDelay(MSG_BROADCAST_AUDIO_BECOMING_NOISY, SENDMSG_REPLACE); } + @GuardedBy("mDeviceStateLock") /*package*/ void postA2dpSinkConnection(@AudioService.BtProfileConnectionState int state, @NonNull BtHelper.BluetoothA2dpDeviceInfo btDeviceInfo, int delay) { sendILMsg(state == BluetoothA2dp.STATE_CONNECTED @@ -624,10 +657,12 @@ import java.io.PrintWriter; // must be called synchronized on mConnectedDevices /*package*/ boolean hasScheduledA2dpSinkConnectionState(BluetoothDevice btDevice) { - return (mBrokerHandler.hasMessages(MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_CONNECTED, - new BtHelper.BluetoothA2dpDeviceInfo(btDevice)) - || mBrokerHandler.hasMessages(MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_DISCONNECTED, - new BtHelper.BluetoothA2dpDeviceInfo(btDevice))); + final BtHelper.BluetoothA2dpDeviceInfo devInfoToCheck = + new BtHelper.BluetoothA2dpDeviceInfo(btDevice); + return (mBrokerHandler.hasEqualMessages( + MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_CONNECTED, devInfoToCheck) + || mBrokerHandler.hasEqualMessages( + MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_DISCONNECTED, devInfoToCheck)); } /*package*/ void setA2dpTimeout(String address, int a2dpCodec, int delayMs) { @@ -808,6 +843,9 @@ import java.io.PrintWriter; final BluetoothDevice btDevice = (BluetoothDevice) msg.obj; synchronized (mDeviceStateLock) { a2dpCodec = mBtHelper.getA2dpCodec(btDevice); + // TODO: name of method being called on AudioDeviceInventory is currently + // misleading (config change vs active device change), to be + // reconciliated once the BT side has been updated. mDeviceInventory.onBluetoothA2dpActiveDeviceChange( new BtHelper.BluetoothA2dpDeviceInfo(btDevice, -1, a2dpCodec), BtHelper.EVENT_DEVICE_CONFIG_CHANGE); @@ -900,7 +938,7 @@ import java.io.PrintWriter; case MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_DISCONNECTION: { final BtDeviceConnectionInfo info = (BtDeviceConnectionInfo) msg.obj; AudioService.sDeviceLogger.log((new AudioEventLogger.StringEvent( - "setBluetoothA2dpDeviceConnectionStateSuppressNoisyIntent " + "msg: setBluetoothA2dpDeviceConnectionStateSuppressNoisyIntent " + " state=" + info.mState // only querying address as this is the only readily available // field on the device @@ -917,7 +955,7 @@ import java.io.PrintWriter; final HearingAidDeviceConnectionInfo info = (HearingAidDeviceConnectionInfo) msg.obj; AudioService.sDeviceLogger.log((new AudioEventLogger.StringEvent( - "setHearingAidDeviceConnectionState state=" + info.mState + "msg: setHearingAidDeviceConnectionState state=" + info.mState + " addr=" + info.mDevice.getAddress() + " supprNoisy=" + info.mSupprNoisy + " src=" + info.mEventSource)).printLog(TAG)); @@ -962,13 +1000,19 @@ import java.io.PrintWriter; private static final int MSG_IL_SET_HEARING_AID_CONNECTION_STATE = 8; private static final int MSG_BT_HEADSET_CNCT_FAILED = 9; private static final int MSG_IL_BTA2DP_TIMEOUT = 10; + + // process change of A2DP device configuration, obj is BluetoothDevice private static final int MSG_L_A2DP_DEVICE_CONFIG_CHANGE = 11; + private static final int MSG_BROADCAST_AUDIO_BECOMING_NOISY = 12; private static final int MSG_REPORT_NEW_ROUTES = 13; private static final int MSG_II_SET_HEARING_AID_VOLUME = 14; private static final int MSG_I_SET_AVRCP_ABSOLUTE_VOLUME = 15; private static final int MSG_I_DISCONNECT_BT_SCO = 16; + + // process active A2DP device change, obj is BtHelper.BluetoothA2dpDeviceInfo private static final int MSG_L_A2DP_ACTIVE_DEVICE_CHANGE = 18; + private static final int MSG_DISCONNECT_A2DP = 19; private static final int MSG_DISCONNECT_A2DP_SINK = 20; private static final int MSG_DISCONNECT_BT_HEARING_AID = 21; @@ -977,13 +1021,18 @@ import java.io.PrintWriter; private static final int MSG_L_BT_SERVICE_CONNECTED_PROFILE_A2DP_SINK = 24; private static final int MSG_L_BT_SERVICE_CONNECTED_PROFILE_HEARING_AID = 25; private static final int MSG_L_BT_SERVICE_CONNECTED_PROFILE_HEADSET = 26; + + // process change of state, obj is BtHelper.BluetoothA2dpDeviceInfo private static final int MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_CONNECTED = 27; private static final int MSG_IL_SET_A2DP_SINK_CONNECTION_STATE_DISCONNECTED = 28; - // process external command to (dis)connect an A2DP device + + // process external command to (dis)connect an A2DP device, obj is BtDeviceConnectionInfo private static final int MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_CONNECTION = 29; private static final int MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_DISCONNECTION = 30; + // process external command to (dis)connect a hearing aid device private static final int MSG_L_HEARING_AID_DEVICE_CONNECTION_CHANGE_EXT = 31; + // a ScoClient died in BtHelper private static final int MSG_L_SCOCLIENT_DIED = 32; private static final int MSG_IL_SAVE_PREF_DEVICE_FOR_STRATEGY = 33; diff --git a/services/core/java/com/android/server/audio/AudioDeviceInventory.java b/services/core/java/com/android/server/audio/AudioDeviceInventory.java index 266dfc7e257ea..b1f8feeb37f0e 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceInventory.java +++ b/services/core/java/com/android/server/audio/AudioDeviceInventory.java @@ -289,11 +289,11 @@ public class AudioDeviceInventory { address = ""; } - final int a2dpCodec = btInfo.getCodec(); + final @AudioSystem.AudioFormatNativeEnumForBtCodec int a2dpCodec = btInfo.getCodec(); AudioService.sDeviceLogger.log(new AudioEventLogger.StringEvent( "A2DP sink connected: device addr=" + address + " state=" + state - + " codec=" + a2dpCodec + + " codec=" + AudioSystem.audioFormatToString(a2dpCodec) + " vol=" + a2dpVolume)); new MediaMetrics.Item(mMetricsId + "a2dp") @@ -422,7 +422,7 @@ public class AudioDeviceInventory { Log.d(TAG, "onBluetoothA2dpActiveDeviceChange btDevice=" + btDevice); } int a2dpVolume = btInfo.getVolume(); - final int a2dpCodec = btInfo.getCodec(); + @AudioSystem.AudioFormatNativeEnumForBtCodec final int a2dpCodec = btInfo.getCodec(); String address = btDevice.getAddress(); if (!BluetoothAdapter.checkBluetoothAddress(address)) { @@ -435,7 +435,8 @@ public class AudioDeviceInventory { synchronized (mDevicesLock) { if (mDeviceBroker.hasScheduledA2dpSinkConnectionState(btDevice)) { AudioService.sDeviceLogger.log(new AudioEventLogger.StringEvent( - "A2dp config change ignored (scheduled connection change)")); + "A2dp config change ignored (scheduled connection change)") + .printLog(TAG)); mmi.set(MediaMetrics.Property.EARLY_RETURN, "A2dp config change ignored") .record(); return; @@ -476,8 +477,9 @@ public class AudioDeviceInventory { if (res != AudioSystem.AUDIO_STATUS_OK) { AudioService.sDeviceLogger.log(new AudioEventLogger.StringEvent( - "APM handleDeviceConfigChange failed for A2DP device addr=" - + address + " codec=" + a2dpCodec).printLog(TAG)); + "APM handleDeviceConfigChange failed for A2DP device addr=" + address + + " codec=" + AudioSystem.audioFormatToString(a2dpCodec)) + .printLog(TAG)); int musicDevice = mDeviceBroker.getDeviceForStream(AudioSystem.STREAM_MUSIC); // force A2DP device disconnection in case of error so that AudioService state is @@ -488,8 +490,9 @@ public class AudioDeviceInventory { -1 /* a2dpVolume */); } else { AudioService.sDeviceLogger.log(new AudioEventLogger.StringEvent( - "APM handleDeviceConfigChange success for A2DP device addr=" - + address + " codec=" + a2dpCodec).printLog(TAG)); + "APM handleDeviceConfigChange success for A2DP device addr=" + address + + " codec=" + AudioSystem.audioFormatToString(a2dpCodec)) + .printLog(TAG)); } } mmi.record(); @@ -816,20 +819,17 @@ public class AudioDeviceInventory { if (AudioService.DEBUG_DEVICES) { Log.i(TAG, "setBluetoothA2dpDeviceConnectionState device: " + device - + " state: " + state + " delay(ms): " + delay + "codec:" + a2dpCodec + + " state: " + state + " delay(ms): " + delay + + " codec:" + Integer.toHexString(a2dpCodec) + " suppressNoisyIntent: " + suppressNoisyIntent); } final BtHelper.BluetoothA2dpDeviceInfo a2dpDeviceInfo = new BtHelper.BluetoothA2dpDeviceInfo(device, a2dpVolume, a2dpCodec); if (profile == BluetoothProfile.A2DP) { - if (delay == 0) { - onSetA2dpSinkConnectionState(a2dpDeviceInfo, state); - } else { - mDeviceBroker.postA2dpSinkConnection(state, + mDeviceBroker.postA2dpSinkConnection(state, a2dpDeviceInfo, delay); - } } else { //profile == BluetoothProfile.A2DP_SINK mDeviceBroker.postA2dpSourceConnection(state, a2dpDeviceInfo, diff --git a/services/core/java/com/android/server/audio/BtHelper.java b/services/core/java/com/android/server/audio/BtHelper.java index accb90cc3c0c0..0654f86c6a678 100644 --- a/services/core/java/com/android/server/audio/BtHelper.java +++ b/services/core/java/com/android/server/audio/BtHelper.java @@ -135,7 +135,7 @@ public class BtHelper { /*package*/ static class BluetoothA2dpDeviceInfo { private final @NonNull BluetoothDevice mBtDevice; private final int mVolume; - private final int mCodec; + private final @AudioSystem.AudioFormatNativeEnumForBtCodec int mCodec; BluetoothA2dpDeviceInfo(@NonNull BluetoothDevice btDevice) { this(btDevice, -1, AudioSystem.AUDIO_FORMAT_DEFAULT); @@ -155,15 +155,26 @@ public class BtHelper { return mVolume; } - public int getCodec() { + public @AudioSystem.AudioFormatNativeEnumForBtCodec int getCodec() { return mCodec; } // redefine equality op so we can match messages intended for this device @Override public boolean equals(Object o) { - return mBtDevice.equals(o); + if (o == null) { + return false; + } + if (this == o) { + return true; + } + if (o instanceof BluetoothA2dpDeviceInfo) { + return mBtDevice.equals(((BluetoothA2dpDeviceInfo) o).getBtDevice()); + } + return false; } + + } // A2DP device events @@ -249,7 +260,8 @@ public class BtHelper { mA2dp.setAvrcpAbsoluteVolume(index); } - /*package*/ synchronized int getA2dpCodec(@NonNull BluetoothDevice device) { + /*package*/ synchronized @AudioSystem.AudioFormatNativeEnumForBtCodec int getA2dpCodec( + @NonNull BluetoothDevice device) { if (mA2dp == null) { return AudioSystem.AUDIO_FORMAT_DEFAULT; } @@ -261,7 +273,7 @@ public class BtHelper { if (btCodecConfig == null) { return AudioSystem.AUDIO_FORMAT_DEFAULT; } - return mapBluetoothCodecToAudioFormat(btCodecConfig.getCodecType()); + return AudioSystem.bluetoothCodecToAudioFormat(btCodecConfig.getCodecType()); } // @GuardedBy("AudioDeviceBroker.mSetModeLock") @@ -967,23 +979,6 @@ public class BtHelper { return result; } - private int mapBluetoothCodecToAudioFormat(int btCodecType) { - switch (btCodecType) { - case BluetoothCodecConfig.SOURCE_CODEC_TYPE_SBC: - return AudioSystem.AUDIO_FORMAT_SBC; - case BluetoothCodecConfig.SOURCE_CODEC_TYPE_AAC: - return AudioSystem.AUDIO_FORMAT_AAC; - case BluetoothCodecConfig.SOURCE_CODEC_TYPE_APTX: - return AudioSystem.AUDIO_FORMAT_APTX; - case BluetoothCodecConfig.SOURCE_CODEC_TYPE_APTX_HD: - return AudioSystem.AUDIO_FORMAT_APTX_HD; - case BluetoothCodecConfig.SOURCE_CODEC_TYPE_LDAC: - return AudioSystem.AUDIO_FORMAT_LDAC; - default: - return AudioSystem.AUDIO_FORMAT_DEFAULT; - } - } - /** * Returns the String equivalent of the btCodecType. *