From 188cae1b54b905d23b0232cabb34eb415e0f591f Mon Sep 17 00:00:00 2001 From: Jean-Michel Trivi Date: Fri, 6 Jan 2023 05:18:04 +0000 Subject: [PATCH] DO NOT MERGE setDeviceVolume: better logs, clears routing cache When a call to setDeviceVolume is received, clear the AudioSystemAdapter cache for the AudioAttributes to audio device. This will eliminate any chance of the cache being stale when the evaluation of whether to send the VOLUME_CHANGED_ACTION intent is performed. Clean up log calls to setDeviceVolume, use a dedicated logger event, and also log the current device selected by getDevicesForStream. Don't send volume update to VolumeController (sysUI) when a device was specified (== call comes from setDeviceVolume). Fix code path in AudioDeviceBroker where its calls to AudioSystem.setForceUse were going directly to AudioSystem, instead of going through the adapter (which causes a cache clear, whereas it didn't in Broker). This is done by injecting into AudioDeviceBroker the same AudioSystemAdapter that AudioService uses. Fix AudioService's obsolete use of ANALOG_DOCK, replaced by DIGITAL_DOCK. Bug: 264302812 Test: boot with device docked, adjust volume Change-Id: Ibfa002b51f02302686853a8dea562e75d12d8949 --- .../server/audio/AudioDeviceBroker.java | 17 ++++--- .../android/server/audio/AudioService.java | 45 +++++++++++++------ .../server/audio/AudioServiceEvents.java | 29 +++++++++++- .../server/audio/AudioSystemAdapter.java | 8 ++++ .../server/audio/AudioDeviceBrokerTest.java | 2 +- 5 files changed, 79 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index 736914ace2155..278c98f45c44b 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -82,6 +82,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private final @NonNull AudioService mAudioService; private final @NonNull Context mContext; + private final @NonNull AudioSystemAdapter mAudioSystem; /** ID for Communication strategy retrieved form audio policy manager */ private int mCommunicationStrategyId = -1; @@ -156,12 +157,14 @@ import java.util.concurrent.atomic.AtomicBoolean; public static final long USE_SET_COMMUNICATION_DEVICE = 243827847L; //------------------------------------------------------------------- - /*package*/ AudioDeviceBroker(@NonNull Context context, @NonNull AudioService service) { + /*package*/ AudioDeviceBroker(@NonNull Context context, @NonNull AudioService service, + @NonNull AudioSystemAdapter audioSystem) { mContext = context; mAudioService = service; mBtHelper = new BtHelper(this); mDeviceInventory = new AudioDeviceInventory(this); mSystemServer = SystemServerAdapter.getDefaultAdapter(mContext); + mAudioSystem = audioSystem; init(); } @@ -170,12 +173,14 @@ import java.util.concurrent.atomic.AtomicBoolean; * in system_server */ AudioDeviceBroker(@NonNull Context context, @NonNull AudioService service, @NonNull AudioDeviceInventory mockDeviceInventory, - @NonNull SystemServerAdapter mockSystemServer) { + @NonNull SystemServerAdapter mockSystemServer, + @NonNull AudioSystemAdapter audioSystem) { mContext = context; mAudioService = service; mBtHelper = new BtHelper(this); mDeviceInventory = mockDeviceInventory; mSystemServer = mockSystemServer; + mAudioSystem = audioSystem; init(); } @@ -450,7 +455,7 @@ import java.util.concurrent.atomic.AtomicBoolean; AudioAttributes attr = AudioProductStrategy.getAudioAttributesForStrategyWithLegacyStreamType( AudioSystem.STREAM_VOICE_CALL); - List devices = AudioSystem.getDevicesForAttributes( + List devices = mAudioSystem.getDevicesForAttributes( attr, false /* forVolume */); if (devices.isEmpty()) { if (mAudioService.isPlatformVoice()) { @@ -1225,7 +1230,7 @@ import java.util.concurrent.atomic.AtomicBoolean; Log.v(TAG, "onSetForceUse(useCase<" + useCase + ">, config<" + config + ">, fromA2dp<" + fromA2dp + ">, eventSource<" + eventSource + ">)"); } - AudioSystem.setForceUse(useCase, config); + mAudioSystem.setForceUse(useCase, config); } private void onSendBecomingNoisyIntent() { @@ -1863,9 +1868,9 @@ import java.util.concurrent.atomic.AtomicBoolean; if (preferredCommunicationDevice == null || preferredCommunicationDevice.getType() != AudioDeviceInfo.TYPE_BLUETOOTH_SCO) { - AudioSystem.setParameters("BT_SCO=off"); + mAudioSystem.setParameters("BT_SCO=off"); } else { - AudioSystem.setParameters("BT_SCO=on"); + mAudioSystem.setParameters("BT_SCO=on"); } if (preferredCommunicationDevice == null) { AudioDeviceAttributes defaultDevice = getDefaultCommunicationDevice(); diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index c804ef2cf8b47..148797f21834d 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -183,6 +183,7 @@ import com.android.server.LocalServices; import com.android.server.SystemService; import com.android.server.audio.AudioServiceEvents.DeviceVolumeEvent; import com.android.server.audio.AudioServiceEvents.PhoneStateEvent; +import com.android.server.audio.AudioServiceEvents.VolChangedBroadcastEvent; import com.android.server.audio.AudioServiceEvents.VolumeEvent; import com.android.server.pm.UserManagerInternal; import com.android.server.pm.UserManagerInternal.UserRestrictionsListener; @@ -1205,7 +1206,7 @@ public class AudioService extends IAudioService.Stub mUseFixedVolume = mContext.getResources().getBoolean( com.android.internal.R.bool.config_useFixedVolume); - mDeviceBroker = new AudioDeviceBroker(mContext, this); + mDeviceBroker = new AudioDeviceBroker(mContext, this, mAudioSystem); mRecordMonitor = new RecordingActivityMonitor(mContext); mRecordMonitor.registerRecordingCallback(mVoiceRecordingActivityMonitor, true); @@ -1637,7 +1638,7 @@ public class AudioService extends IAudioService.Stub synchronized (mSettingsLock) { final int forDock = mDockAudioMediaEnabled ? - AudioSystem.FORCE_ANALOG_DOCK : AudioSystem.FORCE_NONE; + AudioSystem.FORCE_DIGITAL_DOCK : AudioSystem.FORCE_NONE; mDeviceBroker.setForceUse_Async(AudioSystem.FOR_DOCK, forDock, "onAudioServerDied"); sendEncodedSurroundMode(mContentResolver, "onAudioServerDied"); sendEnabledSurroundFormats(mContentResolver, true); @@ -2258,9 +2259,10 @@ public class AudioService extends IAudioService.Stub SENDMSG_QUEUE, AudioSystem.FOR_DOCK, mDockAudioMediaEnabled ? - AudioSystem.FORCE_ANALOG_DOCK : AudioSystem.FORCE_NONE, + AudioSystem.FORCE_DIGITAL_DOCK : AudioSystem.FORCE_NONE, new String("readDockAudioSettings"), 0); + } @@ -3741,19 +3743,30 @@ public class AudioService extends IAudioService.Stub Objects.requireNonNull(ada); Objects.requireNonNull(callingPackage); - AudioService.sVolumeLogger.loglogi("setDeviceVolume" + " from:" + callingPackage + " " - + vi + " " + ada, TAG); - if (!vi.hasStreamType()) { Log.e(TAG, "Unsupported non-stream type based VolumeInfo", new Exception()); return; } + int index = vi.getVolumeIndex(); if (index == VolumeInfo.INDEX_NOT_SET && !vi.hasMuteCommand()) { throw new IllegalArgumentException( "changing device volume requires a volume index or mute command"); } + // force a cache clear to force reevaluating stream type to audio device selection + // that can interfere with the sending of the VOLUME_CHANGED_ACTION intent + // TODO change cache management to not rely only on invalidation, but on "do not trust" + // moments when routing is in flux. + mAudioSystem.clearRoutingCache(); + + // log the current device that will be used when evaluating the sending of the + // VOLUME_CHANGED_ACTION intent to see if the current device is the one being modified + final int currDev = getDeviceForStream(vi.getStreamType()); + + AudioService.sVolumeLogger.log(new DeviceVolumeEvent(vi.getStreamType(), index, ada, + currDev, callingPackage)); + // TODO handle unmuting of current audio device // if a stream is not muted but the VolumeInfo is for muting, set the volume index // for the device to min volume @@ -3837,11 +3850,11 @@ public class AudioService extends IAudioService.Stub return; } - final AudioEventLogger.Event event = (device == null) - ? new VolumeEvent(VolumeEvent.VOL_SET_STREAM_VOL, streamType, - index/*val1*/, flags/*val2*/, callingPackage) - : new DeviceVolumeEvent(streamType, index, device, callingPackage); - sVolumeLogger.log(event); + if (device == null) { + // call was already logged in setDeviceVolume() + sVolumeLogger.log(new VolumeEvent(VolumeEvent.VOL_SET_STREAM_VOL, streamType, + index/*val1*/, flags/*val2*/, callingPackage)); + } setStreamVolume(streamType, index, flags, device, callingPackage, callingPackage, attributionTag, Binder.getCallingUid(), callingOrSelfHasAudioSettingsPermission()); @@ -4242,7 +4255,11 @@ public class AudioService extends IAudioService.Stub maybeSendSystemAudioStatusCommand(false); } } - sendVolumeUpdate(streamType, oldIndex, index, flags, device); + if (ada == null) { + // only non-null when coming here from setDeviceVolume + // TODO change test to check early if device is current device or not + sendVolumeUpdate(streamType, oldIndex, index, flags, device); + } } @@ -7982,6 +7999,8 @@ public class AudioService extends IAudioService.Stub mVolumeChanged.putExtra(AudioManager.EXTRA_PREV_VOLUME_STREAM_VALUE, oldIndex); mVolumeChanged.putExtra(AudioManager.EXTRA_VOLUME_STREAM_TYPE_ALIAS, mStreamVolumeAlias[mStreamType]); + AudioService.sVolumeLogger.log(new VolChangedBroadcastEvent( + mStreamType, mStreamVolumeAlias[mStreamType], index)); sendBroadcastToAll(mVolumeChanged); } } @@ -10155,7 +10174,7 @@ public class AudioService extends IAudioService.Stub static final int LOG_NB_EVENTS_PHONE_STATE = 20; static final int LOG_NB_EVENTS_DEVICE_CONNECTION = 50; static final int LOG_NB_EVENTS_FORCE_USE = 20; - static final int LOG_NB_EVENTS_VOLUME = 40; + static final int LOG_NB_EVENTS_VOLUME = 100; static final int LOG_NB_EVENTS_DYN_POLICY = 10; static final int LOG_NB_EVENTS_SPATIAL = 30; diff --git a/services/core/java/com/android/server/audio/AudioServiceEvents.java b/services/core/java/com/android/server/audio/AudioServiceEvents.java index 30a9e0a70e968..c2c3f028abdb0 100644 --- a/services/core/java/com/android/server/audio/AudioServiceEvents.java +++ b/services/core/java/com/android/server/audio/AudioServiceEvents.java @@ -147,19 +147,42 @@ public class AudioServiceEvents { } } + static final class VolChangedBroadcastEvent extends AudioEventLogger.Event { + final int mStreamType; + final int mAliasStreamType; + final int mIndex; + + VolChangedBroadcastEvent(int stream, int alias, int index) { + mStreamType = stream; + mAliasStreamType = alias; + mIndex = index; + } + + @Override + public String eventToString() { + return new StringBuilder("sending VOLUME_CHANGED stream:") + .append(AudioSystem.streamToString(mStreamType)) + .append(" index:").append(mIndex) + .append(" alias:").append(AudioSystem.streamToString(mAliasStreamType)) + .toString(); + } + } + static final class DeviceVolumeEvent extends AudioEventLogger.Event { final int mStream; final int mVolIndex; final String mDeviceNativeType; final String mDeviceAddress; final String mCaller; + final int mDeviceForStream; DeviceVolumeEvent(int streamType, int index, @NonNull AudioDeviceAttributes device, - String callingPackage) { + int deviceForStream, String callingPackage) { mStream = streamType; mVolIndex = index; mDeviceNativeType = "0x" + Integer.toHexString(device.getInternalType()); mDeviceAddress = device.getAddress(); + mDeviceForStream = deviceForStream; mCaller = callingPackage; // log metrics new MediaMetrics.Item(MediaMetrics.Name.AUDIO_VOLUME_EVENT) @@ -180,7 +203,9 @@ public class AudioServiceEvents { .append(" index:").append(mVolIndex) .append(" device:").append(mDeviceNativeType) .append(" addr:").append(mDeviceAddress) - .append(") from ").append(mCaller).toString(); + .append(") from ").append(mCaller) + .append(" currDevForStream:Ox").append(Integer.toHexString(mDeviceForStream)) + .toString(); } } diff --git a/services/core/java/com/android/server/audio/AudioSystemAdapter.java b/services/core/java/com/android/server/audio/AudioSystemAdapter.java index c3754eb3e44c0..258837116cd6d 100644 --- a/services/core/java/com/android/server/audio/AudioSystemAdapter.java +++ b/services/core/java/com/android/server/audio/AudioSystemAdapter.java @@ -105,6 +105,13 @@ public class AudioSystemAdapter implements AudioSystem.RoutingUpdateCallback, } } + public void clearRoutingCache() { + if (DEBUG_CACHE) { + Log.d(TAG, "---- routing cache clear (from java) ----------"); + } + invalidateRoutingCache(); + } + /** * Implementation of AudioSystem.VolumeRangeInitRequestCallback */ @@ -337,6 +344,7 @@ public class AudioSystemAdapter implements AudioSystem.RoutingUpdateCallback, * @return */ public int setParameters(String keyValuePairs) { + invalidateRoutingCache(); return AudioSystem.setParameters(keyValuePairs); } diff --git a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java index dad9fe8648b20..31599eed539d8 100644 --- a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java @@ -74,7 +74,7 @@ public class AudioDeviceBrokerTest { mSpyDevInventory = spy(new AudioDeviceInventory(mSpyAudioSystem)); mSpySystemServer = spy(new NoOpSystemServerAdapter()); mAudioDeviceBroker = new AudioDeviceBroker(mContext, mMockAudioService, mSpyDevInventory, - mSpySystemServer); + mSpySystemServer, mSpyAudioSystem); mSpyDevInventory.setDeviceBroker(mAudioDeviceBroker); BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter();