From a0b2ee74b2fa75d95508b268951373bdad5fdfa8 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Mon, 21 Mar 2022 21:13:53 +0100 Subject: [PATCH] audioservice: improve SCO audio control logic In AudioDeviceBroker, make sure all methods controlling the SCO audio state are called from the handler thread to propely serialize them. This is important in a scenario were multiple clients are interacting and try to control SCO audio activity independently. For instance when a VoIP call over SCO is interrupted by an incoming call and the call is rejected. Also make sure we properly restart the SCO audio link if requested (instead of just reapplying the route according to actual SCO audio state) when a change in audio mode owner happens or SCO audio is disconnected. Bug: 206739623 Test: repro steps in bug Change-Id: I5ddeaffac4fe26316f084505d692d2fc80cff467 Merged-In: I5ddeaffac4fe26316f084505d692d2fc80cff467 --- .../server/audio/AudioDeviceBroker.java | 175 +++++++++++++++--- .../com/android/server/audio/BtHelper.java | 137 ++++++++------ 2 files changed, 223 insertions(+), 89 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index f053e942fe6b2..b1e62483c472d 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -252,8 +252,8 @@ import java.util.concurrent.atomic.AtomicBoolean; return; } } - setCommunicationRouteForClient( - cb, pid, device, BtHelper.SCO_MODE_UNDEFINED, eventSource); + postSetCommunicationRouteForClient(new CommunicationClientInfo( + cb, pid, device, BtHelper.SCO_MODE_UNDEFINED, eventSource)); } } } @@ -283,8 +283,8 @@ import java.util.concurrent.atomic.AtomicBoolean; return false; } } - setCommunicationRouteForClient( - cb, pid, deviceAttr, BtHelper.SCO_MODE_UNDEFINED, eventSource); + postSetCommunicationRouteForClient(new CommunicationClientInfo( + cb, pid, deviceAttr, BtHelper.SCO_MODE_UNDEFINED, eventSource)); } } return true; @@ -348,26 +348,35 @@ import java.util.concurrent.atomic.AtomicBoolean; } /** - * Returns the device currently requested for communication use case. - * If the current audio mode owner is in the communication route client list, - * use this preference. - * Otherwise use first client's preference (first client corresponds to latest request). - * null is returned if no client is in the list. - * @return AudioDeviceAttributes the requested device for communication. + * Returns the communication client with the highest priority: + * - 1) the client which is currently also controlling the audio mode + * - 2) the first client in the stack if there is no audio mode owner + * - 3) no client otherwise + * @return CommunicationRouteClient the client driving the communication use case routing. */ - @GuardedBy("mDeviceStateLock") - private AudioDeviceAttributes requestedCommunicationDevice() { - AudioDeviceAttributes device = null; - for (CommunicationRouteClient cl : mCommunicationRouteClients) { - if (cl.getPid() == mModeOwnerPid) { - device = cl.getDevice(); + private CommunicationRouteClient topCommunicationRouteClient() { + for (CommunicationRouteClient crc : mCommunicationRouteClients) { + if (crc.getPid() == mModeOwnerPid) { + return crc; } } if (!mCommunicationRouteClients.isEmpty() && mModeOwnerPid == 0) { - device = mCommunicationRouteClients.get(0).getDevice(); + return mCommunicationRouteClients.get(0); } + return null; + } + /** + * Returns the device currently requested for communication use case. + * Use the device requested by the communication route client selected by + * {@link #topCommunicationRouteClient()} if any or none otherwise. + * @return AudioDeviceAttributes the requested device for communication. + */ + @GuardedBy("mDeviceStateLock") + private AudioDeviceAttributes requestedCommunicationDevice() { + CommunicationRouteClient crc = topCommunicationRouteClient(); + AudioDeviceAttributes device = crc != null ? crc.getDevice() : null; if (AudioService.DEBUG_COMM_RTE) { Log.v(TAG, "requestedCommunicationDevice, device: " + device + " mode owner pid: " + mModeOwnerPid); @@ -644,7 +653,7 @@ import java.util.concurrent.atomic.AtomicBoolean; } synchronized (mDeviceStateLock) { mBluetoothScoOn = on; - sendLMsgNoDelay(MSG_L_UPDATE_COMMUNICATION_ROUTE, SENDMSG_QUEUE, eventSource); + postUpdateCommunicationRouteClient(eventSource); } } @@ -699,7 +708,9 @@ import java.util.concurrent.atomic.AtomicBoolean; synchronized (mDeviceStateLock) { AudioDeviceAttributes device = new AudioDeviceAttributes(AudioSystem.DEVICE_OUT_BLUETOOTH_SCO, ""); - setCommunicationRouteForClient(cb, pid, device, scoAudioMode, eventSource); + + postSetCommunicationRouteForClient(new CommunicationClientInfo( + cb, pid, device, scoAudioMode, eventSource)); } } } @@ -717,8 +728,8 @@ import java.util.concurrent.atomic.AtomicBoolean; if (client == null || !client.requestsBluetoothSco()) { return; } - setCommunicationRouteForClient( - cb, pid, null, BtHelper.SCO_MODE_UNDEFINED, eventSource); + postSetCommunicationRouteForClient(new CommunicationClientInfo( + cb, pid, null, BtHelper.SCO_MODE_UNDEFINED, eventSource)); } } } @@ -960,6 +971,61 @@ import java.util.concurrent.atomic.AtomicBoolean; MSG_I_SAVE_CLEAR_PREF_DEVICES_FOR_CAPTURE_PRESET, SENDMSG_QUEUE, capturePreset); } + /*package*/ void postUpdateCommunicationRouteClient(String eventSource) { + sendLMsgNoDelay(MSG_L_UPDATE_COMMUNICATION_ROUTE_CLIENT, SENDMSG_QUEUE, eventSource); + } + + /*package*/ void postSetCommunicationRouteForClient(CommunicationClientInfo info) { + sendLMsgNoDelay(MSG_L_SET_COMMUNICATION_ROUTE_FOR_CLIENT, SENDMSG_QUEUE, info); + } + + /*package*/ void postScoAudioStateChanged(int state) { + sendIMsgNoDelay(MSG_I_SCO_AUDIO_STATE_CHANGED, SENDMSG_QUEUE, state); + } + + /*package*/ static final class CommunicationClientInfo { + final @NonNull IBinder mCb; + final int mPid; + final @NonNull AudioDeviceAttributes mDevice; + final int mScoAudioMode; + final @NonNull String mEventSource; + + CommunicationClientInfo(@NonNull IBinder cb, int pid, @NonNull AudioDeviceAttributes device, + int scoAudioMode, @NonNull String eventSource) { + mCb = cb; + mPid = pid; + mDevice = device; + mScoAudioMode = scoAudioMode; + mEventSource = eventSource; + } + + // redefine equality op so we can match messages intended for this client + @Override + public boolean equals(Object o) { + if (o == null) { + return false; + } + if (this == o) { + return true; + } + if (!(o instanceof CommunicationClientInfo)) { + return false; + } + + return mCb.equals(((CommunicationClientInfo) o).mCb) + && mPid == ((CommunicationClientInfo) o).mPid; + } + + @Override + public String toString() { + return "CommunicationClientInfo mCb=" + mCb.toString() + +"mPid=" + mPid + +"mDevice=" + mDevice.toString() + +"mScoAudioMode=" + mScoAudioMode + +"mEventSource=" + mEventSource; + } + } + //--------------------------------------------------------------------- // Method forwarding between the helper classes (BtHelper, AudioDeviceInventory) // only call from a "handle"* method or "on"* method @@ -1270,18 +1336,30 @@ import java.util.concurrent.atomic.AtomicBoolean; synchronized (mDeviceStateLock) { mModeOwnerPid = msg.arg1; if (msg.arg2 != AudioSystem.MODE_RINGTONE) { - onUpdateCommunicationRoute("setNewModeOwner"); + onUpdateCommunicationRouteClient("setNewModeOwner"); } } } break; - case MSG_L_COMMUNICATION_ROUTE_CLIENT_DIED: + + case MSG_L_SET_COMMUNICATION_ROUTE_FOR_CLIENT: synchronized (mSetModeLock) { synchronized (mDeviceStateLock) { - onCommunicationRouteClientDied((CommunicationRouteClient) msg.obj); + CommunicationClientInfo info = (CommunicationClientInfo) msg.obj; + setCommunicationRouteForClient(info.mCb, info.mPid, info.mDevice, + info.mScoAudioMode, info.mEventSource); } } break; + + case MSG_L_UPDATE_COMMUNICATION_ROUTE_CLIENT: + synchronized (mSetModeLock) { + synchronized (mDeviceStateLock) { + onUpdateCommunicationRouteClient((String) msg.obj); + } + } + break; + case MSG_L_UPDATE_COMMUNICATION_ROUTE: synchronized (mSetModeLock) { synchronized (mDeviceStateLock) { @@ -1289,6 +1367,23 @@ import java.util.concurrent.atomic.AtomicBoolean; } } break; + + case MSG_L_COMMUNICATION_ROUTE_CLIENT_DIED: + synchronized (mSetModeLock) { + synchronized (mDeviceStateLock) { + onCommunicationRouteClientDied((CommunicationRouteClient) msg.obj); + } + } + break; + + case MSG_I_SCO_AUDIO_STATE_CHANGED: + synchronized (mSetModeLock) { + synchronized (mDeviceStateLock) { + mBtHelper.onScoAudioStateChanged(msg.arg1); + } + } + break; + case MSG_TOGGLE_HDMI: synchronized (mDeviceStateLock) { mDeviceInventory.onToggleHdmi(); @@ -1495,6 +1590,9 @@ import java.util.concurrent.atomic.AtomicBoolean; private static final int MSG_L_UPDATE_COMMUNICATION_ROUTE = 39; private static final int MSG_IL_SET_PREF_DEVICES_FOR_STRATEGY = 40; private static final int MSG_I_REMOVE_PREF_DEVICES_FOR_STRATEGY = 41; + private static final int MSG_L_SET_COMMUNICATION_ROUTE_FOR_CLIENT = 42; + private static final int MSG_L_UPDATE_COMMUNICATION_ROUTE_CLIENT = 43; + private static final int MSG_I_SCO_AUDIO_STATE_CHANGED = 44; private static boolean isMessageHandledUnderWakelock(int msgId) { switch(msgId) { @@ -1720,9 +1818,8 @@ import java.util.concurrent.atomic.AtomicBoolean; return; } Log.w(TAG, "Communication client died"); - setCommunicationRouteForClient( - client.getBinder(), client.getPid(), null, BtHelper.SCO_MODE_UNDEFINED, - "onCommunicationRouteClientDied"); + removeCommunicationRouteClient(client.getBinder(), true); + onUpdateCommunicationRouteClient("onCommunicationRouteClientDied"); } /** @@ -1776,11 +1873,31 @@ import java.util.concurrent.atomic.AtomicBoolean; AudioSystem.setParameters("BT_SCO=on"); } if (preferredCommunicationDevice == null) { - postRemovePreferredDevicesForStrategy(mCommunicationStrategyId); + removePreferredDevicesForStrategySync(mCommunicationStrategyId); } else { - postSetPreferredDevicesForStrategy( + setPreferredDevicesForStrategySync( mCommunicationStrategyId, Arrays.asList(preferredCommunicationDevice)); } + onUpdatePhoneStrategyDevice(preferredCommunicationDevice); + } + + /** + * Select new communication device from communication route client at the top of the stack + * and restore communication route including restarting SCO audio if needed. + */ + // @GuardedBy("mSetModeLock") + @GuardedBy("mDeviceStateLock") + private void onUpdateCommunicationRouteClient(String eventSource) { + onUpdateCommunicationRoute(eventSource); + CommunicationRouteClient crc = topCommunicationRouteClient(); + if (AudioService.DEBUG_COMM_RTE) { + Log.v(TAG, "onUpdateCommunicationRouteClient, crc: " + + crc + " eventSource: " + eventSource); + } + if (crc != null) { + setCommunicationRouteForClient(crc.getBinder(), crc.getPid(), crc.getDevice(), + BtHelper.SCO_MODE_UNDEFINED, eventSource); + } } private void onUpdatePhoneStrategyDevice(AudioDeviceAttributes device) { diff --git a/services/core/java/com/android/server/audio/BtHelper.java b/services/core/java/com/android/server/audio/BtHelper.java index 52e8edff5ffa8..a9a8d1096dd84 100644 --- a/services/core/java/com/android/server/audio/BtHelper.java +++ b/services/core/java/com/android/server/audio/BtHelper.java @@ -99,9 +99,9 @@ public class BtHelper { // SCO audio mode is virtual voice call (BluetoothHeadset.startScoUsingVirtualVoiceCall()) /*package*/ static final int SCO_MODE_VIRTUAL_CALL = 0; // SCO audio mode is raw audio (BluetoothHeadset.connectAudio()) - private static final int SCO_MODE_RAW = 1; + private static final int SCO_MODE_RAW = 1; // SCO audio mode is Voice Recognition (BluetoothHeadset.startVoiceRecognition()) - private static final int SCO_MODE_VR = 2; + private static final int SCO_MODE_VR = 2; // max valid SCO audio mode values private static final int SCO_MODE_MAX = 2; @@ -300,68 +300,76 @@ public class BtHelper { BluetoothDevice btDevice = intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE); setBtScoActiveDevice(btDevice); } else if (action.equals(BluetoothHeadset.ACTION_AUDIO_STATE_CHANGED)) { - boolean broadcast = false; - int scoAudioState = AudioManager.SCO_AUDIO_STATE_ERROR; int btState = intent.getIntExtra(BluetoothProfile.EXTRA_STATE, -1); - Log.i(TAG, "receiveBtEvent ACTION_AUDIO_STATE_CHANGED: " + btState); - switch (btState) { - case BluetoothHeadset.STATE_AUDIO_CONNECTED: - scoAudioState = AudioManager.SCO_AUDIO_STATE_CONNECTED; - if (mScoAudioState != SCO_STATE_ACTIVE_INTERNAL - && mScoAudioState != SCO_STATE_DEACTIVATE_REQ) { - mScoAudioState = SCO_STATE_ACTIVE_EXTERNAL; - } else if (mDeviceBroker.isBluetoothScoRequested()) { - // broadcast intent if the connection was initated by AudioService - broadcast = true; - } - mDeviceBroker.setBluetoothScoOn(true, "BtHelper.receiveBtEvent"); - break; - case BluetoothHeadset.STATE_AUDIO_DISCONNECTED: - mDeviceBroker.setBluetoothScoOn(false, "BtHelper.receiveBtEvent"); - scoAudioState = AudioManager.SCO_AUDIO_STATE_DISCONNECTED; - // There are two cases where we want to immediately reconnect audio: - // 1) If a new start request was received while disconnecting: this was - // notified by requestScoState() setting state to SCO_STATE_ACTIVATE_REQ. - // 2) If audio was connected then disconnected via Bluetooth APIs and - // we still have pending activation requests by apps: this is indicated by - // state SCO_STATE_ACTIVE_EXTERNAL and BT SCO is requested. - if (mScoAudioState == SCO_STATE_ACTIVATE_REQ - || (mScoAudioState == SCO_STATE_ACTIVE_EXTERNAL - && mDeviceBroker.isBluetoothScoRequested())) { - if (mBluetoothHeadset != null && mBluetoothHeadsetDevice != null - && connectBluetoothScoAudioHelper(mBluetoothHeadset, - mBluetoothHeadsetDevice, mScoAudioMode)) { - mScoAudioState = SCO_STATE_ACTIVE_INTERNAL; - scoAudioState = AudioManager.SCO_AUDIO_STATE_CONNECTING; - broadcast = true; - break; - } - } - if (mScoAudioState != SCO_STATE_ACTIVE_EXTERNAL) { - broadcast = true; - } - mScoAudioState = SCO_STATE_INACTIVE; - break; - case BluetoothHeadset.STATE_AUDIO_CONNECTING: - if (mScoAudioState != SCO_STATE_ACTIVE_INTERNAL - && mScoAudioState != SCO_STATE_DEACTIVATE_REQ) { - mScoAudioState = SCO_STATE_ACTIVE_EXTERNAL; - } - break; - default: - break; - } - if (broadcast) { - broadcastScoConnectionState(scoAudioState); - //FIXME: this is to maintain compatibility with deprecated intent - // AudioManager.ACTION_SCO_AUDIO_STATE_CHANGED. Remove when appropriate. - Intent newIntent = new Intent(AudioManager.ACTION_SCO_AUDIO_STATE_CHANGED); - newIntent.putExtra(AudioManager.EXTRA_SCO_AUDIO_STATE, scoAudioState); - sendStickyBroadcastToAll(newIntent); - } + Log.i(TAG,"receiveBtEvent ACTION_AUDIO_STATE_CHANGED: "+btState); + mDeviceBroker.postScoAudioStateChanged(btState); } } + /** + * Exclusively called from AudioDeviceBroker when handling MSG_I_SCO_AUDIO_STATE_CHANGED + * as part of the serialization of the communication route selection + */ + // @GuardedBy("AudioDeviceBroker.mSetModeLock") + @GuardedBy("AudioDeviceBroker.mDeviceStateLock") + void onScoAudioStateChanged(int state) { + boolean broadcast = false; + int scoAudioState = AudioManager.SCO_AUDIO_STATE_ERROR; + switch (state) { + case BluetoothHeadset.STATE_AUDIO_CONNECTED: + scoAudioState = AudioManager.SCO_AUDIO_STATE_CONNECTED; + if (mScoAudioState != SCO_STATE_ACTIVE_INTERNAL + && mScoAudioState != SCO_STATE_DEACTIVATE_REQ) { + mScoAudioState = SCO_STATE_ACTIVE_EXTERNAL; + } else if (mDeviceBroker.isBluetoothScoRequested()) { + // broadcast intent if the connection was initated by AudioService + broadcast = true; + } + mDeviceBroker.setBluetoothScoOn(true, "BtHelper.receiveBtEvent"); + break; + case BluetoothHeadset.STATE_AUDIO_DISCONNECTED: + mDeviceBroker.setBluetoothScoOn(false, "BtHelper.receiveBtEvent"); + scoAudioState = AudioManager.SCO_AUDIO_STATE_DISCONNECTED; + // There are two cases where we want to immediately reconnect audio: + // 1) If a new start request was received while disconnecting: this was + // notified by requestScoState() setting state to SCO_STATE_ACTIVATE_REQ. + // 2) If audio was connected then disconnected via Bluetooth APIs and + // we still have pending activation requests by apps: this is indicated by + // state SCO_STATE_ACTIVE_EXTERNAL and BT SCO is requested. + if (mScoAudioState == SCO_STATE_ACTIVATE_REQ) { + if (mBluetoothHeadset != null && mBluetoothHeadsetDevice != null + && connectBluetoothScoAudioHelper(mBluetoothHeadset, + mBluetoothHeadsetDevice, mScoAudioMode)) { + mScoAudioState = SCO_STATE_ACTIVE_INTERNAL; + scoAudioState = AudioManager.SCO_AUDIO_STATE_CONNECTING; + broadcast = true; + break; + } + } + if (mScoAudioState != SCO_STATE_ACTIVE_EXTERNAL) { + broadcast = true; + } + mScoAudioState = SCO_STATE_INACTIVE; + break; + case BluetoothHeadset.STATE_AUDIO_CONNECTING: + if (mScoAudioState != SCO_STATE_ACTIVE_INTERNAL + && mScoAudioState != SCO_STATE_DEACTIVATE_REQ) { + mScoAudioState = SCO_STATE_ACTIVE_EXTERNAL; + } + break; + default: + break; + } + if(broadcast) { + broadcastScoConnectionState(scoAudioState); + //FIXME: this is to maintain compatibility with deprecated intent + // AudioManager.ACTION_SCO_AUDIO_STATE_CHANGED. Remove when appropriate. + Intent newIntent = new Intent(AudioManager.ACTION_SCO_AUDIO_STATE_CHANGED); + newIntent.putExtra(AudioManager.EXTRA_SCO_AUDIO_STATE, scoAudioState); + sendStickyBroadcastToAll(newIntent); + } + + } /** * * @return false if SCO isn't connected @@ -752,6 +760,15 @@ public class BtHelper { case SCO_STATE_ACTIVE_INTERNAL: Log.w(TAG, "requestScoState: already in ACTIVE mode, simply return"); break; + case SCO_STATE_ACTIVE_EXTERNAL: + /* Confirm SCO Audio connection to requesting app as it is already connected + * externally (i.e. through SCO APIs by Telecom service). + * Once SCO Audio is disconnected by the external owner, we will reconnect it + * automatically on behalf of the requesting app and the state will move to + * SCO_STATE_ACTIVE_INTERNAL. + */ + broadcastScoConnectionState(AudioManager.SCO_AUDIO_STATE_CONNECTED); + break; default: Log.w(TAG, "requestScoState: failed to connect in state " + mScoAudioState + ", scoAudioMode=" + scoAudioMode);