From f377f9e0a20cf6e0f7593d83c206cdd8180ca048 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Thu, 28 May 2020 18:32:48 -0700 Subject: [PATCH] AudioService: more fixes for BT A2DP device switch Fix a couple of remaining problems left after commit 4599ee23. - Increase delay before unmuting media after handling last message in switch sequence. - Mute stream immediately instead of via a message. - Also mute when handling messages for media A2DP forced usage and route updates because of MediaRouter interaction while switching device. - Handle muting before enqueueing the messages in case the handler thread preempts the sender after the message is enqueued and before mute is handled. Bug: 144784716 Test: repro steps in bug. Change-Id: Iaae8e6d8eb3112e9770085e65afe7fb73119b502 --- .../server/audio/AudioDeviceBroker.java | 51 +++++++++++-------- .../server/audio/AudioDeviceInventory.java | 20 ++++---- .../android/server/audio/AudioService.java | 12 +---- 3 files changed, 43 insertions(+), 40 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index 67e27c41bbc78..5c6b481a60bda 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -63,7 +63,7 @@ import java.util.concurrent.atomic.AtomicBoolean; /*package*/ static final int BT_HEADSET_CNCT_TIMEOUT_MS = 3000; // Delay before checking it music should be unmuted after processing an A2DP message - private static final int BTA2DP_MUTE_CHECK_DELAY_MS = 50; + private static final int BTA2DP_MUTE_CHECK_DELAY_MS = 100; private final @NonNull AudioService mAudioService; private final @NonNull Context mContext; @@ -160,8 +160,10 @@ import java.util.concurrent.atomic.AtomicBoolean; synchronized (mDeviceStateLock) { AudioSystem.setParameters( "BT_SCO=" + (mForcedUseForComm == AudioSystem.FORCE_BT_SCO ? "on" : "off")); - onSetForceUse(AudioSystem.FOR_COMMUNICATION, mForcedUseForComm, "onAudioServerDied"); - onSetForceUse(AudioSystem.FOR_RECORD, mForcedUseForComm, "onAudioServerDied"); + onSetForceUse(AudioSystem.FOR_COMMUNICATION, mForcedUseForComm, + false /*fromA2dp*/, "onAudioServerDied"); + onSetForceUse(AudioSystem.FOR_RECORD, mForcedUseForComm, + false /*fromA2dp*/, "onAudioServerDied"); } // restore devices sendMsgNoDelay(MSG_RESTORE_DEVICES, SENDMSG_REPLACE); @@ -667,7 +669,7 @@ import java.util.concurrent.atomic.AtomicBoolean; // Handles request to override default use of A2DP for media. //@GuardedBy("mConnectedDevices") - /*package*/ void setBluetoothA2dpOnInt(boolean on, String source) { + /*package*/ void setBluetoothA2dpOnInt(boolean on, boolean fromA2dp, String source) { // for logging only final String eventSource = new StringBuilder("setBluetoothA2dpOn(").append(on) .append(") from u/pid:").append(Binder.getCallingUid()).append("/") @@ -679,6 +681,7 @@ import java.util.concurrent.atomic.AtomicBoolean; onSetForceUse( AudioSystem.FOR_MEDIA, mBluetoothA2dpEnabled ? AudioSystem.FORCE_NONE : AudioSystem.FORCE_NO_BT_A2DP, + fromA2dp, eventSource); } } @@ -705,8 +708,8 @@ import java.util.concurrent.atomic.AtomicBoolean; mBrokerHandler.removeMessages(MSG_BT_HEADSET_CNCT_FAILED); } - /*package*/ void postReportNewRoutes() { - sendMsgNoDelay(MSG_REPORT_NEW_ROUTES, SENDMSG_NOOP); + /*package*/ void postReportNewRoutes(boolean fromA2dp) { + sendMsgNoDelay(fromA2dp ? MSG_REPORT_NEW_ROUTES_A2DP : MSG_REPORT_NEW_ROUTES, SENDMSG_NOOP); } /*package*/ void postA2dpActiveDeviceChange( @@ -774,9 +777,9 @@ import java.util.concurrent.atomic.AtomicBoolean; // These methods are ALL synchronous, in response to message handling in BrokerHandler // Blocking in any of those will block the message queue - private void onSetForceUse(int useCase, int config, String eventSource) { + private void onSetForceUse(int useCase, int config, boolean fromA2dp, String eventSource) { if (useCase == AudioSystem.FOR_MEDIA) { - postReportNewRoutes(); + postReportNewRoutes(fromA2dp); } AudioService.sForceUseLogger.log( new AudioServiceEvents.ForceUseEvent(useCase, config, eventSource)); @@ -870,9 +873,11 @@ import java.util.concurrent.atomic.AtomicBoolean; break; case MSG_IIL_SET_FORCE_USE: // intended fall-through case MSG_IIL_SET_FORCE_BT_A2DP_USE: - onSetForceUse(msg.arg1, msg.arg2, (String) msg.obj); + onSetForceUse(msg.arg1, msg.arg2, + (msg.what == MSG_IIL_SET_FORCE_BT_A2DP_USE), (String) msg.obj); break; case MSG_REPORT_NEW_ROUTES: + case MSG_REPORT_NEW_ROUTES_A2DP: synchronized (mDeviceStateLock) { mDeviceInventory.onReportNewRoutes(); } @@ -1057,7 +1062,7 @@ import java.util.concurrent.atomic.AtomicBoolean; mDeviceInventory.onSaveRemovePreferredDevice(strategy); } break; case MSG_CHECK_MUTE_MUSIC: - checkMessagesMuteMusic(); + checkMessagesMuteMusic(0); break; default: Log.wtf(TAG, "Invalid message " + msg.what); @@ -1133,6 +1138,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private static final int MSG_L_SPEAKERPHONE_CLIENT_DIED = 35; private static final int MSG_CHECK_MUTE_MUSIC = 36; + private static final int MSG_REPORT_NEW_ROUTES_A2DP = 37; private static boolean isMessageHandledUnderWakelock(int msgId) { @@ -1224,6 +1230,10 @@ import java.util.concurrent.atomic.AtomicBoolean; Binder.restoreCallingIdentity(identity); } + if (MESSAGES_MUTE_MUSIC.contains(msg)) { + checkMessagesMuteMusic(msg); + } + synchronized (sLastDeviceConnectionMsgTimeLock) { long time = SystemClock.uptimeMillis() + delay; @@ -1245,13 +1255,9 @@ import java.util.concurrent.atomic.AtomicBoolean; default: break; } - mBrokerHandler.sendMessageAtTime(mBrokerHandler.obtainMessage(msg, arg1, arg2, obj), time); } - if (MESSAGES_MUTE_MUSIC.contains(msg)) { - checkMessagesMuteMusic(); - } } /** List of messages for which music is muted while processing is pending */ @@ -1264,19 +1270,24 @@ import java.util.concurrent.atomic.AtomicBoolean; MESSAGES_MUTE_MUSIC.add(MSG_L_A2DP_ACTIVE_DEVICE_CHANGE); MESSAGES_MUTE_MUSIC.add(MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_CONNECTION); MESSAGES_MUTE_MUSIC.add(MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT_DISCONNECTION); + MESSAGES_MUTE_MUSIC.add(MSG_IIL_SET_FORCE_BT_A2DP_USE); + MESSAGES_MUTE_MUSIC.add(MSG_REPORT_NEW_ROUTES_A2DP); } private AtomicBoolean mMusicMuted = new AtomicBoolean(false); /** Mutes or unmutes music according to pending A2DP messages */ - private void checkMessagesMuteMusic() { - boolean mute = false; - for (int msg : MESSAGES_MUTE_MUSIC) { - if (mBrokerHandler.hasMessages(msg)) { - mute = true; - break; + private void checkMessagesMuteMusic(int message) { + boolean mute = message != 0; + if (!mute) { + for (int msg : MESSAGES_MUTE_MUSIC) { + if (mBrokerHandler.hasMessages(msg)) { + mute = true; + break; + } } } + if (mute != mMusicMuted.getAndSet(mute)) { mAudioService.setMusicMute(mute); } diff --git a/services/core/java/com/android/server/audio/AudioDeviceInventory.java b/services/core/java/com/android/server/audio/AudioDeviceInventory.java index b1f8feeb37f0e..02a846e3dc825 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceInventory.java +++ b/services/core/java/com/android/server/audio/AudioDeviceInventory.java @@ -549,7 +549,7 @@ public class AudioDeviceInventory { synchronized (mDevicesLock) { if ((wdcs.mState == AudioService.CONNECTION_STATE_DISCONNECTED) && DEVICE_OVERRIDE_A2DP_ROUTE_ON_PLUG_SET.contains(wdcs.mType)) { - mDeviceBroker.setBluetoothA2dpOnInt(true, + mDeviceBroker.setBluetoothA2dpOnInt(true, false /*fromA2dp*/, "onSetWiredDeviceConnectionState state DISCONNECTED"); } @@ -562,7 +562,7 @@ public class AudioDeviceInventory { } if (wdcs.mState != AudioService.CONNECTION_STATE_DISCONNECTED) { if (DEVICE_OVERRIDE_A2DP_ROUTE_ON_PLUG_SET.contains(wdcs.mType)) { - mDeviceBroker.setBluetoothA2dpOnInt(false, + mDeviceBroker.setBluetoothA2dpOnInt(false, false /*fromA2dp*/, "onSetWiredDeviceConnectionState state not DISCONNECTED"); } mDeviceBroker.checkMusicActive(wdcs.mType, wdcs.mCaller); @@ -879,7 +879,7 @@ public class AudioDeviceInventory { int a2dpCodec) { // enable A2DP before notifying A2DP connection to avoid unnecessary processing in // audio policy manager - mDeviceBroker.setBluetoothA2dpOnInt(true, eventSource); + mDeviceBroker.setBluetoothA2dpOnInt(true, true /*fromA2dp*/, eventSource); // at this point there could be another A2DP device already connected in APM, but it // doesn't matter as this new one will overwrite the previous one final int res = mAudioSystem.setDeviceConnectionState(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, @@ -910,7 +910,7 @@ public class AudioDeviceInventory { mApmConnectedDevices.put(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, diKey); mDeviceBroker.postAccessoryPlugMediaUnmute(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP); - setCurrentAudioRouteNameIfPossible(name); + setCurrentAudioRouteNameIfPossible(name, true /*fromA2dp*/); } @GuardedBy("mDevicesLock") @@ -955,7 +955,7 @@ public class AudioDeviceInventory { } mApmConnectedDevices.remove(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP); // Remove A2DP routes as well - setCurrentAudioRouteNameIfPossible(null); + setCurrentAudioRouteNameIfPossible(null, true /*fromA2dp*/); mmi.record(); } @@ -1014,7 +1014,7 @@ public class AudioDeviceInventory { mDeviceBroker.postAccessoryPlugMediaUnmute(AudioSystem.DEVICE_OUT_HEARING_AID); mDeviceBroker.postApplyVolumeOnDevice(streamType, AudioSystem.DEVICE_OUT_HEARING_AID, "makeHearingAidDeviceAvailable"); - setCurrentAudioRouteNameIfPossible(name); + setCurrentAudioRouteNameIfPossible(name, false /*fromA2dp*/); new MediaMetrics.Item(mMetricsId + "makeHearingAidDeviceAvailable") .set(MediaMetrics.Property.ADDRESS, address != null ? address : "") .set(MediaMetrics.Property.DEVICE, @@ -1033,7 +1033,7 @@ public class AudioDeviceInventory { mConnectedDevices.remove( DeviceInfo.makeDeviceListKey(AudioSystem.DEVICE_OUT_HEARING_AID, address)); // Remove Hearing Aid routes as well - setCurrentAudioRouteNameIfPossible(null); + setCurrentAudioRouteNameIfPossible(null, false /*fromA2dp*/); new MediaMetrics.Item(mMetricsId + "makeHearingAidDeviceUnavailable") .set(MediaMetrics.Property.ADDRESS, address != null ? address : "") .set(MediaMetrics.Property.DEVICE, @@ -1042,14 +1042,14 @@ public class AudioDeviceInventory { } @GuardedBy("mDevicesLock") - private void setCurrentAudioRouteNameIfPossible(String name) { + private void setCurrentAudioRouteNameIfPossible(String name, boolean fromA2dp) { synchronized (mCurAudioRoutes) { if (TextUtils.equals(mCurAudioRoutes.bluetoothName, name)) { return; } if (name != null || !isCurrentDeviceConnected()) { mCurAudioRoutes.bluetoothName = name; - mDeviceBroker.postReportNewRoutes(); + mDeviceBroker.postReportNewRoutes(fromA2dp); } } } @@ -1236,7 +1236,7 @@ public class AudioDeviceInventory { } if (newConn != mCurAudioRoutes.mainType) { mCurAudioRoutes.mainType = newConn; - mDeviceBroker.postReportNewRoutes(); + mDeviceBroker.postReportNewRoutes(false /*fromA2dp*/); } } } diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index ed05ffffaa382..5427dd2328a6a 100755 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -5904,16 +5904,8 @@ public class AudioService extends IAudioService.Stub if (state != mIsMutedInternally) { changed = true; mIsMutedInternally = state; - - // Set the new mute volume. This propagates the values to - // the audio system, otherwise the volume won't be changed - // at the lower level. - sendMsg(mAudioHandler, - MSG_SET_ALL_VOLUMES, - SENDMSG_QUEUE, - 0, - 0, - this, 0); + // mute immediately to avoid delay and preemption when using a message. + applyAllVolumes(); } } if (changed) {