From c4a891c6bcb9ab2b28c70bfd44c283cab1f43247 Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Fri, 21 Apr 2023 16:30:16 -0700 Subject: [PATCH 1/3] Revert "Generate an abort event when stopping" This reverts commit 3b660525f37bdb6c549d9ce483658d4a8f189438. This commit causes deadlocks. Bug: 236826280 Fixes: 274993071 Test: CtsVoiceInteractionTestCases Test: CtsSoundTriggerTestCases Test: Manual verification Change-Id: Ie170e0a2572aae051acc6ec325b5b39afaf761de --- .../SoundTriggerMiddlewareImplTest.java | 14 -------- .../SoundTriggerModule.java | 33 ++----------------- 2 files changed, 2 insertions(+), 45 deletions(-) diff --git a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java index 5a2451f3b17e7..3d963ed4fe376 100644 --- a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java +++ b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java @@ -225,13 +225,6 @@ public class SoundTriggerMiddlewareImplTest { // Stop the recognition. stopRecognition(module, handle, hwHandle); - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( - RecognitionEventSys.class); - verify(callback).onRecognition(eq(handle), eventCaptor.capture(), eq(101)); - RecognitionEventSys lastEvent = eventCaptor.getValue(); - assertEquals(-1, lastEvent.halEventReceivedMillis); - assertEquals(RecognitionStatus.ABORTED, lastEvent.recognitionEvent.status); - // Unload the model. unloadModel(module, handle, hwHandle); module.detach(); @@ -276,13 +269,6 @@ public class SoundTriggerMiddlewareImplTest { // Stop the recognition. stopRecognition(module, handle, hwHandle); - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( - PhraseRecognitionEventSys.class); - verify(callback).onPhraseRecognition(eq(handle), eventCaptor.capture(), eq(101)); - PhraseRecognitionEventSys lastEvent = eventCaptor.getValue(); - assertEquals(-1, lastEvent.halEventReceivedMillis); - assertEquals(RecognitionStatus.ABORTED, lastEvent.phraseRecognitionEvent.common.status); - // Unload the model. unloadModel(module, handle, hwHandle); module.detach(); diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java index 84cec5592831e..177b4da8fc756 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java @@ -23,7 +23,6 @@ import android.media.soundtrigger.PhraseSoundModel; import android.media.soundtrigger.Properties; import android.media.soundtrigger.RecognitionConfig; import android.media.soundtrigger.SoundModel; -import android.media.soundtrigger.SoundModelType; import android.media.soundtrigger.Status; import android.media.soundtrigger_middleware.ISoundTriggerCallback; import android.media.soundtrigger_middleware.ISoundTriggerModule; @@ -385,7 +384,6 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo private class Model implements ISoundTriggerHal.ModelCallback { public int mHandle; private ModelState mState = ModelState.INIT; - private int mType = SoundModelType.INVALID; private SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession mSession; private @NonNull @@ -402,7 +400,6 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession audioSession) { mSession = audioSession; mHandle = mHalService.loadSoundModel(model, this); - mType = SoundModelType.GENERIC; setState(ModelState.LOADED); mLoadedModels.put(mHandle, this); return mHandle; @@ -412,7 +409,7 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession audioSession) { mSession = audioSession; mHandle = mHalService.loadPhraseSoundModel(model, this); - mType = SoundModelType.KEYPHRASE; + setState(ModelState.LOADED); mLoadedModels.put(mHandle, this); return mHandle; @@ -441,34 +438,9 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo return; } } - // This must be invoked outside the lock. mHalService.stopRecognition(mHandle); - - // No more callbacks for this model after this point. synchronized (SoundTriggerModule.this) { - // Generate an abortion callback to the client if the model is still active. - if (getState() == ModelState.ACTIVE) { - if (mCallback != null) { - try { - switch (mType) { - case SoundModelType.GENERIC: - mCallback.onRecognition(mHandle, AidlUtil.newAbortEvent(), - mSession.mSessionHandle); - break; - case SoundModelType.KEYPHRASE: - mCallback.onPhraseRecognition(mHandle, - AidlUtil.newAbortPhraseEvent(), - mSession.mSessionHandle); - break; - default: - throw new RuntimeException( - "Unexpected model type: " + mType); - } - } catch (RemoteException e) { - } - } - setState(ModelState.LOADED); - } + setState(ModelState.LOADED); } } @@ -559,5 +531,4 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo } } } - } From 70b1371bbc01e18b4a245b94ea998c95adc2237d Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Fri, 21 Apr 2023 18:48:20 -0700 Subject: [PATCH 2/3] Remove SoundTriggerService stopping state Essentially reverts f3e7be00588ea67dfaeb8be3847802d238a9a974 The addition of this state caused several state mismatch issues, and spurious error callbacks. Removing, and solving the underlying race condition in a subsequent commit. Bug: 236826280 Bug: 268217943 Fixes: 275079746 Test: AlwaysOnHotwordDetectorTest# testAbortRecognitionAndOnResourceAvailable_recognitionPausedAndResumed Test: CtsSoundTriggerTestCases Test: CtsVoiceInteractionTestCases Test: Manual verification Change-Id: Icedc330c9fefbe6444dfec886672a97d603098e4 --- .../soundtrigger/SoundTriggerHelper.java | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java index b4066ab1ff392..c485501e45092 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java @@ -468,7 +468,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { } } - if (unloadModel && (modelData.isModelLoaded() || modelData.isStopPending())) { + if (unloadModel && modelData.isModelLoaded()) { Slog.d(TAG, "Unloading previously loaded stale model."); if (mModule == null) { return STATUS_ERROR; @@ -851,7 +851,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { Slog.w(TAG, "Recognition aborted"); MetricsLogger.count(mContext, "sth_recognition_aborted", 1); ModelData modelData = getModelDataForLocked(event.soundModelHandle); - if (modelData != null && (modelData.isModelStarted() || modelData.isStopPending())) { + if (modelData != null && modelData.isModelStarted()) { modelData.setStopped(); try { IRecognitionStatusCallback callback = modelData.getCallback(); @@ -865,7 +865,6 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { .printLog(ALOGW, TAG)); forceStopAndUnloadModelLocked(modelData, e); } - updateRecognitionLocked(modelData, true); } } @@ -936,7 +935,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { private int updateRecognitionLocked(ModelData model, boolean notifyClientOnError) { boolean shouldStartModel = model.isRequested() && isRecognitionAllowedByDeviceState(model); - if (shouldStartModel == model.isModelStarted() || model.isStopPending()) { + if (shouldStartModel == model.isModelStarted()) { // No-op. return STATUS_OK; } @@ -1041,10 +1040,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { if (mModule == null) { return; } - if (modelData.isStopPending()) { - // No need to wait for the stop to be confirmed. - modelData.setStopped(); - } else if (modelData.isModelStarted()) { + if (modelData.isModelStarted()) { Slog.d(TAG, "Stopping previously started dangling model " + modelData.getHandle()); if (mModule.stopRecognition(modelData.getHandle()) == STATUS_OK) { modelData.setStopped(); @@ -1256,7 +1252,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { } } } else { - modelData.setStopPending(); + modelData.setStopped(); MetricsLogger.count(mContext, "sth_stop_recognition_success", 1); // Notify of pause if needed. if (notify) { @@ -1303,9 +1299,6 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { // Started implies model was successfully loaded and start was called. static final int MODEL_STARTED = 2; - // Model stop request has been sent. Waiting for an event to signal model being stopped. - static final int MODEL_STOP_PENDING = 3; - // One of MODEL_NOTLOADED, MODEL_LOADED, MODEL_STARTED (which implies loaded). private int mModelState; private UUID mModelId; @@ -1383,10 +1376,6 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { return mModelState == MODEL_NOTLOADED; } - synchronized boolean isStopPending() { - return mModelState == MODEL_STOP_PENDING; - } - synchronized void setStarted() { mModelState = MODEL_STARTED; } @@ -1395,10 +1384,6 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { mModelState = MODEL_LOADED; } - synchronized void setStopPending() { - mModelState = MODEL_STOP_PENDING; - } - synchronized void setLoaded() { mModelState = MODEL_LOADED; } From 749650c2cb0c1ab68221ec86f86027d5bd48d634 Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Fri, 21 Apr 2023 18:35:40 -0700 Subject: [PATCH 3/3] Add recognition session tokens A recognition event can race with a start/stop/start in quick succession. Attributing the event to the incorrect startReco leads to state mismatch between lower/upper layers. - Associate each startRecognition with an IBinder token at the STModule layer - Invalidate the token field on downwards stop calls, and drop callbacks when no session is active - Add the token as a field to framework recognition event, so async clients can drop stale events - Additionally, add protection against a startRecognition while the lock is dropped in stopRecognition - Call new startReco version in STService, and drop callbacks for stale sessions - Drop wtf on stale callback in the HAL enforcer, since we handle it gracefully at a higher layer Bug: 236826280 Fixes: 275080257 Test: SoundTriggerManagerTest# testStartTriggerStopRecognitionRace_doesNotFail Test: CtsVoiceInteractionTestCases Test: FrameworksVoiceInteractionTests Test: Smoke tests Change-Id: I8a613b5f6821636e545309c09e6dfbb67626ea2b --- .../hardware/soundtrigger/ConversionUtil.java | 6 +- .../hardware/soundtrigger/SoundTrigger.java | 69 ++++++++++++++----- .../soundtrigger/SoundTriggerModule.java | 10 +++ .../voice/AlwaysOnHotwordDetector.java | 3 +- .../ISoundTriggerModule.aidl | 3 +- .../PhraseRecognitionEventSys.aidl | 5 ++ .../RecognitionEventSys.aidl | 5 ++ .../server/soundtrigger/SoundTriggerTest.java | 10 ++- .../soundtrigger/SoundTriggerHelper.java | 34 ++++++++- .../SoundTriggerHalEnforcer.java | 4 +- .../SoundTriggerMiddlewareLogging.java | 9 +-- .../SoundTriggerMiddlewarePermission.java | 4 +- .../SoundTriggerMiddlewareService.java | 5 +- .../SoundTriggerMiddlewareValidation.java | 5 +- .../SoundTriggerModule.java | 25 +++++-- 15 files changed, 157 insertions(+), 40 deletions(-) diff --git a/core/java/android/hardware/soundtrigger/ConversionUtil.java b/core/java/android/hardware/soundtrigger/ConversionUtil.java index 21fe686fa2e36..5c07fa48d4662 100644 --- a/core/java/android/hardware/soundtrigger/ConversionUtil.java +++ b/core/java/android/hardware/soundtrigger/ConversionUtil.java @@ -232,7 +232,8 @@ public class ConversionUtil { recognitionEvent.captureAvailable, captureSession, recognitionEvent.captureDelayMs, recognitionEvent.capturePreambleMs, recognitionEvent.triggerInData, audioFormat, recognitionEvent.data, - recognitionEvent.recognitionStillActive, aidlEvent.halEventReceivedMillis); + recognitionEvent.recognitionStillActive, aidlEvent.halEventReceivedMillis, + aidlEvent.token); } public static SoundTrigger.RecognitionEvent aidl2apiPhraseRecognitionEvent( @@ -254,7 +255,8 @@ public class ConversionUtil { recognitionEvent.common.captureDelayMs, recognitionEvent.common.capturePreambleMs, recognitionEvent.common.triggerInData, audioFormat, - recognitionEvent.common.data, apiExtras, aidlEvent.halEventReceivedMillis); + recognitionEvent.common.data, apiExtras, aidlEvent.halEventReceivedMillis, + aidlEvent.token); } // In case of a null input returns a non-null valid output. diff --git a/core/java/android/hardware/soundtrigger/SoundTrigger.java b/core/java/android/hardware/soundtrigger/SoundTrigger.java index 6d43ddf7fe948..301b412e6ce26 100644 --- a/core/java/android/hardware/soundtrigger/SoundTrigger.java +++ b/core/java/android/hardware/soundtrigger/SoundTrigger.java @@ -63,6 +63,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Locale; +import java.util.Objects; import java.util.UUID; /** @@ -1226,6 +1227,14 @@ public class SoundTrigger { @ElapsedRealtimeLong public final long halEventReceivedMillis; + /** + * Binder token returned by {@link SoundTriggerModule#startRecognitionWithToken( + * int soundModelHandle, SoundTrigger.RecognitionConfig config)} + * @hide + */ + public final IBinder token; + + /** @hide */ @TestApi @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553) @@ -1235,14 +1244,16 @@ public class SoundTrigger { @ElapsedRealtimeLong long halEventReceivedMillis) { this(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, - data, status == RECOGNITION_STATUS_GET_STATE_RESPONSE, halEventReceivedMillis); + data, status == RECOGNITION_STATUS_GET_STATE_RESPONSE, halEventReceivedMillis, + null); } /** @hide */ public RecognitionEvent(int status, int soundModelHandle, boolean captureAvailable, int captureSession, int captureDelayMs, int capturePreambleMs, boolean triggerInData, @NonNull AudioFormat captureFormat, @Nullable byte[] data, - boolean recognitionStillActive, @ElapsedRealtimeLong long halEventReceivedMillis) { + boolean recognitionStillActive, @ElapsedRealtimeLong long halEventReceivedMillis, + IBinder token) { this.status = status; this.soundModelHandle = soundModelHandle; this.captureAvailable = captureAvailable; @@ -1254,6 +1265,7 @@ public class SoundTrigger { this.data = data != null ? data : new byte[0]; this.recognitionStillActive = recognitionStillActive; this.halEventReceivedMillis = halEventReceivedMillis; + this.token = token; } /** @@ -1311,6 +1323,16 @@ public class SoundTrigger { return halEventReceivedMillis; } + /** + * Get token associated with this recognition session returned by + *{@link SoundTriggerModule#startRecognitionWithToken( + * int soundModelHandle, SoundTrigger.RecognitionConfig config)} + * @hide + */ + public IBinder getToken() { + return token; + } + /** @hide */ public static final @android.annotation.NonNull Parcelable.Creator CREATOR = new Parcelable.Creator() { @@ -1346,9 +1368,10 @@ public class SoundTrigger { byte[] data = in.readBlob(); boolean recognitionStillActive = in.readBoolean(); long halEventReceivedMillis = in.readLong(); + IBinder token = in.readStrongBinder(); return new RecognitionEvent(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, data, - recognitionStillActive, halEventReceivedMillis); + recognitionStillActive, halEventReceivedMillis, token); } /** @hide */ @@ -1376,6 +1399,7 @@ public class SoundTrigger { dest.writeBlob(data); dest.writeBoolean(recognitionStillActive); dest.writeLong(halEventReceivedMillis); + dest.writeStrongBinder(token); } @Override public int hashCode() { @@ -1396,6 +1420,7 @@ public class SoundTrigger { result = prime * result + status; result = result + (recognitionStillActive ? 1289 : 1291); result = prime * result + Long.hashCode(halEventReceivedMillis); + result = prime * result + Objects.hashCode(token); return result; } @@ -1425,6 +1450,9 @@ public class SoundTrigger { if (halEventReceivedMillis != other.halEventReceivedMillis) { return false; } + if (!Objects.equals(token, other.token)) { + return false; + } if (status != other.status) return false; if (triggerInData != other.triggerInData) @@ -1462,8 +1490,8 @@ public class SoundTrigger { + ", data=" + (data == null ? 0 : data.length) + ", recognitionStillActive=" + recognitionStillActive + ", halEventReceivedMillis=" + halEventReceivedMillis - + "]"; - } + + ", token=" + token + + "]"; } } /** @@ -1886,10 +1914,12 @@ public class SoundTrigger { int captureSession, int captureDelayMs, int capturePreambleMs, boolean triggerInData, @NonNull AudioFormat captureFormat, @Nullable byte[] data, @Nullable KeyphraseRecognitionExtra[] keyphraseExtras, - @ElapsedRealtimeLong long halEventReceivedMillis) { + @ElapsedRealtimeLong long halEventReceivedMillis, + IBinder token) { this(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, data, keyphraseExtras, - status == RECOGNITION_STATUS_GET_STATE_RESPONSE, halEventReceivedMillis); + status == RECOGNITION_STATUS_GET_STATE_RESPONSE, halEventReceivedMillis, + token); } public KeyphraseRecognitionEvent(int status, int soundModelHandle, @@ -1897,10 +1927,11 @@ public class SoundTrigger { int captureSession, int captureDelayMs, int capturePreambleMs, boolean triggerInData, @NonNull AudioFormat captureFormat, @Nullable byte[] data, @Nullable KeyphraseRecognitionExtra[] keyphraseExtras, - boolean recognitionStillActive, @ElapsedRealtimeLong long halEventReceivedMillis) { + boolean recognitionStillActive, @ElapsedRealtimeLong long halEventReceivedMillis, + IBinder token) { super(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, - data, recognitionStillActive, halEventReceivedMillis); + data, recognitionStillActive, halEventReceivedMillis, token); this.keyphraseExtras = keyphraseExtras != null ? keyphraseExtras : new KeyphraseRecognitionExtra[0]; } @@ -1938,12 +1969,13 @@ public class SoundTrigger { byte[] data = in.readBlob(); boolean recognitionStillActive = in.readBoolean(); long halEventReceivedMillis = in.readLong(); + IBinder token = in.readStrongBinder(); KeyphraseRecognitionExtra[] keyphraseExtras = in.createTypedArray(KeyphraseRecognitionExtra.CREATOR); return new KeyphraseRecognitionEvent(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, data, keyphraseExtras, recognitionStillActive, - halEventReceivedMillis); + halEventReceivedMillis, token); } @Override @@ -1966,6 +1998,7 @@ public class SoundTrigger { dest.writeBlob(data); dest.writeBoolean(recognitionStillActive); dest.writeLong(halEventReceivedMillis); + dest.writeStrongBinder(token); dest.writeTypedArray(keyphraseExtras, flags); } @@ -2015,6 +2048,7 @@ public class SoundTrigger { + ", data=" + (data == null ? 0 : data.length) + ", recognitionStillActive=" + recognitionStillActive + ", halEventReceivedMillis=" + halEventReceivedMillis + + ", token=" + token + "]"; } } @@ -2030,20 +2064,23 @@ public class SoundTrigger { public GenericRecognitionEvent(int status, int soundModelHandle, boolean captureAvailable, int captureSession, int captureDelayMs, int capturePreambleMs, boolean triggerInData, @NonNull AudioFormat captureFormat, @Nullable byte[] data, - @ElapsedRealtimeLong long halEventReceivedMillis) { + @ElapsedRealtimeLong long halEventReceivedMillis, + IBinder token) { this(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, data, - status == RECOGNITION_STATUS_GET_STATE_RESPONSE, halEventReceivedMillis); + status == RECOGNITION_STATUS_GET_STATE_RESPONSE, + halEventReceivedMillis, token); } public GenericRecognitionEvent(int status, int soundModelHandle, boolean captureAvailable, int captureSession, int captureDelayMs, int capturePreambleMs, boolean triggerInData, @NonNull AudioFormat captureFormat, @Nullable byte[] data, - boolean recognitionStillActive, @ElapsedRealtimeLong long halEventReceivedMillis) { + boolean recognitionStillActive, @ElapsedRealtimeLong long halEventReceivedMillis, + IBinder token) { super(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, - data, recognitionStillActive, halEventReceivedMillis); + data, recognitionStillActive, halEventReceivedMillis, token); } public static final @android.annotation.NonNull Parcelable.Creator CREATOR @@ -2062,7 +2099,7 @@ public class SoundTrigger { return new GenericRecognitionEvent(event.status, event.soundModelHandle, event.captureAvailable, event.captureSession, event.captureDelayMs, event.capturePreambleMs, event.triggerInData, event.captureFormat, event.data, - event.recognitionStillActive, event.halEventReceivedMillis); + event.recognitionStillActive, event.halEventReceivedMillis, event.token); } @Override @@ -2092,7 +2129,7 @@ public class SoundTrigger { * * @hide */ - static int handleException(Exception e) { + public static int handleException(Exception e) { Log.w(TAG, "Exception caught", e); if (e instanceof RemoteException) { return STATUS_DEAD_OBJECT; diff --git a/core/java/android/hardware/soundtrigger/SoundTriggerModule.java b/core/java/android/hardware/soundtrigger/SoundTriggerModule.java index 5cdbe233aa3b3..48d4ea40fecdf 100644 --- a/core/java/android/hardware/soundtrigger/SoundTriggerModule.java +++ b/core/java/android/hardware/soundtrigger/SoundTriggerModule.java @@ -246,6 +246,16 @@ public class SoundTriggerModule { } } + /** + * Same as above, but return a binder token associated with the session. + * @hide + */ + public synchronized IBinder startRecognitionWithToken(int soundModelHandle, + SoundTrigger.RecognitionConfig config) throws RemoteException { + return mService.startRecognition(soundModelHandle, + ConversionUtil.api2aidlRecognitionConfig(config)); + } + /** * Stop listening to all key phrases in a {@link SoundTrigger.SoundModel} * @param soundModelHandle The sound model handle to stop listening to diff --git a/core/java/android/service/voice/AlwaysOnHotwordDetector.java b/core/java/android/service/voice/AlwaysOnHotwordDetector.java index 91c350aa9abaa..5fa1a1ebc3151 100644 --- a/core/java/android/service/voice/AlwaysOnHotwordDetector.java +++ b/core/java/android/service/voice/AlwaysOnHotwordDetector.java @@ -970,7 +970,8 @@ public class AlwaysOnHotwordDetector extends AbstractDetector { new KeyphraseRecognitionEvent(status, soundModelHandle, captureAvailable, captureSession, captureDelayMs, capturePreambleMs, triggerInData, captureFormat, data, keyphraseRecognitionExtras.toArray( - new KeyphraseRecognitionExtra[0]), halEventReceivedMillis), + new KeyphraseRecognitionExtra[0]), halEventReceivedMillis, + new Binder()), mInternalCallback); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); diff --git a/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerModule.aidl b/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerModule.aidl index 18688ce7d48db..4bdefd011e48c 100644 --- a/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerModule.aidl +++ b/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerModule.aidl @@ -79,8 +79,9 @@ interface ISoundTriggerModule { * * May throw a ServiceSpecificException with an RESOURCE_CONTENTION status to indicate that * resources required for starting the model are currently consumed by other clients. + * @return - A token delivered along with future recognition events. */ - void startRecognition(int modelHandle, in RecognitionConfig config); + IBinder startRecognition(int modelHandle, in RecognitionConfig config); /** * Stop a recognition of a previously active recognition. Will NOT generate a recognition event. diff --git a/media/aidl/android/media/soundtrigger_middleware/PhraseRecognitionEventSys.aidl b/media/aidl/android/media/soundtrigger_middleware/PhraseRecognitionEventSys.aidl index 6c912ed7e056c..d9d16ec30bff3 100644 --- a/media/aidl/android/media/soundtrigger_middleware/PhraseRecognitionEventSys.aidl +++ b/media/aidl/android/media/soundtrigger_middleware/PhraseRecognitionEventSys.aidl @@ -33,4 +33,9 @@ parcelable PhraseRecognitionEventSys { */ // @ElapsedRealtimeLong long halEventReceivedMillis = -1; + /** + * Token relating this event to a particular recognition session, returned by + * {@link ISoundTriggerModule.startRecognition(int, RecognitionConfig} + */ + IBinder token; } diff --git a/media/aidl/android/media/soundtrigger_middleware/RecognitionEventSys.aidl b/media/aidl/android/media/soundtrigger_middleware/RecognitionEventSys.aidl index 84e327d5df8c6..20ec8c236dcca 100644 --- a/media/aidl/android/media/soundtrigger_middleware/RecognitionEventSys.aidl +++ b/media/aidl/android/media/soundtrigger_middleware/RecognitionEventSys.aidl @@ -33,4 +33,9 @@ parcelable RecognitionEventSys { */ // @ElapsedRealtimeLong long halEventReceivedMillis = -1; + /** + * Token relating this event to a particular recognition session, returned by + * {@link ISoundTriggerModule.startRecognition(int, RecognitionConfig} + */ + IBinder token; } diff --git a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger/SoundTriggerTest.java b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger/SoundTriggerTest.java index e6a1be8f018df..35170b3516b75 100644 --- a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger/SoundTriggerTest.java +++ b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger/SoundTriggerTest.java @@ -28,6 +28,7 @@ import android.os.Parcel; import android.test.InstrumentationTestCase; import android.test.suitebuilder.annotation.LargeTest; import android.test.suitebuilder.annotation.SmallTest; +import android.os.Binder; import java.util.Arrays; import java.util.Locale; @@ -346,7 +347,8 @@ public class SoundTriggerTest extends InstrumentationTestCase { .build(), null /* data */, null /* keyphraseExtras */, - 12345678 /* halEventReceivedMillis */); + 12345678 /* halEventReceivedMillis */, + new Binder() /* token */); // Write to a parcel Parcel parcel = Parcel.obtain(); @@ -379,7 +381,8 @@ public class SoundTriggerTest extends InstrumentationTestCase { .build(), new byte[1] /* data */, kpExtra, - 12345678 /* halEventReceivedMillis */); + 12345678 /* halEventReceivedMillis */, + new Binder() /* token */); // Write to a parcel Parcel parcel = Parcel.obtain(); @@ -428,7 +431,8 @@ public class SoundTriggerTest extends InstrumentationTestCase { .build(), data, kpExtra, - 12345678 /* halEventReceivedMillis */); + 12345678 /* halEventReceivedMillis */, + new Binder() /* token */); // Write to a parcel Parcel parcel = Parcel.obtain(); diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java index c485501e45092..4404ae6b23078 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java @@ -40,6 +40,7 @@ import android.hardware.soundtrigger.SoundTriggerModule; import android.os.Binder; import android.os.DeadObjectException; import android.os.Handler; +import android.os.IBinder; import android.os.Looper; import android.os.Message; import android.os.RemoteException; @@ -769,6 +770,10 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { return; } ModelData model = getModelDataForLocked(event.soundModelHandle); + if (!Objects.equals(event.getToken(), model.getToken())) { + // Stale event, do nothing + return; + } if (model == null || !model.isGenericModel()) { Slog.w(TAG, "Generic recognition event: Model does not exist for handle: " + event.soundModelHandle); @@ -851,6 +856,10 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { Slog.w(TAG, "Recognition aborted"); MetricsLogger.count(mContext, "sth_recognition_aborted", 1); ModelData modelData = getModelDataForLocked(event.soundModelHandle); + if (!Objects.equals(event.getToken(), modelData.getToken())) { + // Stale event, do nothing + return; + } if (modelData != null && modelData.isModelStarted()) { modelData.setStopped(); try { @@ -888,6 +897,10 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { MetricsLogger.count(mContext, "sth_keyphrase_recognition_event", 1); int keyphraseId = getKeyphraseIdFromEvent(event); ModelData modelData = getKeyphraseModelDataLocked(keyphraseId); + if (!Objects.equals(event.getToken(), modelData.getToken())) { + // Stale event, do nothing + return; + } if (modelData == null || !modelData.isKeyphraseModel()) { Slog.e(TAG, "Keyphase model data does not exist for ID:" + keyphraseId); @@ -1184,7 +1197,12 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { if (mModule == null) { return STATUS_ERROR; } - int status = mModule.startRecognition(modelData.getHandle(), config); + int status = STATUS_OK; + try { + modelData.setToken(mModule.startRecognitionWithToken(modelData.getHandle(), config)); + } catch (Exception e) { + status = SoundTrigger.handleException(e); + } if (status != SoundTrigger.STATUS_OK) { Slog.w(TAG, "startRecognition failed with " + status); MetricsLogger.count(mContext, "sth_start_recognition_error", 1); @@ -1339,6 +1357,9 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { // The SoundModel instance, one of KeyphraseSoundModel or GenericSoundModel. private SoundModel mSoundModel = null; + // Token used to disambiguate recognition sessions. + private IBinder mRecognitionToken = null; + private ModelData(UUID modelId, int modelType) { mModelId = modelId; // Private constructor, since we require modelType to be one of TYPE_GENERIC, @@ -1381,6 +1402,9 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { } synchronized void setStopped() { + // If we are moving to the stopped state, we should clear out our + // startRecognition token + mRecognitionToken = null; mModelState = MODEL_LOADED; } @@ -1452,6 +1476,14 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { return mSoundModel; } + synchronized IBinder getToken() { + return mRecognitionToken; + } + + synchronized void setToken(IBinder token) { + mRecognitionToken = token; + } + synchronized int getModelType() { return mModelType; } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerHalEnforcer.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerHalEnforcer.java index bac24669696ca..c3e0a3cd0292a 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerHalEnforcer.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerHalEnforcer.java @@ -256,7 +256,7 @@ public class SoundTriggerHalEnforcer implements ISoundTriggerHal { public void recognitionCallback(int model, RecognitionEventSys event) { synchronized (mModelStates) { ModelState state = mModelStates.get(model); - if (state == null || state == ModelState.INACTIVE) { + if (state == null) { Log.wtfStack(TAG, "Unexpected recognition event for model: " + model); reboot(); return; @@ -282,7 +282,7 @@ public class SoundTriggerHalEnforcer implements ISoundTriggerHal { public void phraseRecognitionCallback(int model, PhraseRecognitionEventSys event) { synchronized (mModelStates) { ModelState state = mModelStates.get(model); - if (state == null || state == ModelState.INACTIVE) { + if (state == null) { Log.wtfStack(TAG, "Unexpected recognition event for model: " + model); reboot(); return; diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java index 2ee4e3cff02c9..ecd65ae9fa2fd 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java @@ -241,13 +241,14 @@ public class SoundTriggerMiddlewareLogging implements ISoundTriggerMiddlewareInt } @Override - public void startRecognition(int modelHandle, RecognitionConfig config) + public IBinder startRecognition(int modelHandle, RecognitionConfig config) throws RemoteException { try { - mDelegate.startRecognition(modelHandle, config); - mEventLogger.enqueue(SessionEvent.createForVoid( - START_RECOGNITION, modelHandle, config) + var result = mDelegate.startRecognition(modelHandle, config); + mEventLogger.enqueue(SessionEvent.createForReturn( + START_RECOGNITION, result, modelHandle, config) .printLog(ALOGI, TAG)); + return result; } catch (Exception e) { mEventLogger.enqueue(SessionEvent.createForException( START_RECOGNITION, e, modelHandle, config) diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java index 00b894e1c6b58..6b724de73488d 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java @@ -241,10 +241,10 @@ public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddleware } @Override - public void startRecognition(int modelHandle, @NonNull RecognitionConfig config) + public IBinder startRecognition(int modelHandle, @NonNull RecognitionConfig config) throws RemoteException { enforcePermissions(); - mDelegate.startRecognition(modelHandle, config); + return mDelegate.startRecognition(modelHandle, config); } @Override diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java index 91e5466969714..1558acf547d1d 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java @@ -34,6 +34,7 @@ import android.media.soundtrigger_middleware.ISoundTriggerInjection; import android.media.soundtrigger_middleware.ISoundTriggerMiddlewareService; import android.media.soundtrigger_middleware.ISoundTriggerModule; import android.media.soundtrigger_middleware.SoundTriggerModuleDescriptor; +import android.os.IBinder; import android.os.RemoteException; import com.android.server.SystemService; @@ -176,10 +177,10 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic } @Override - public void startRecognition(int modelHandle, RecognitionConfig config) + public IBinder startRecognition(int modelHandle, RecognitionConfig config) throws RemoteException { try (SafeCloseable ignored = ClearCallingIdentityContext.create()) { - mDelegate.startRecognition(modelHandle, config); + return mDelegate.startRecognition(modelHandle, config); } } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java index f208c03024b2b..2924c124f22ed 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java @@ -434,7 +434,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware } @Override - public void startRecognition(int modelHandle, @NonNull RecognitionConfig config) { + public IBinder startRecognition(int modelHandle, @NonNull RecognitionConfig config) { // Input validation. ValidationUtil.validateRecognitionConfig(config); @@ -458,9 +458,10 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware // From here on, every exception isn't client's fault. try { - mDelegate.startRecognition(modelHandle, config); + var result = mDelegate.startRecognition(modelHandle, config); modelState.config = config; modelState.activityState = ModelState.Activity.ACTIVE; + return result; } catch (Exception e) { throw handleException(e); } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java index 177b4da8fc756..083211c292830 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java @@ -303,10 +303,10 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo } @Override - public void startRecognition(int modelHandle, @NonNull RecognitionConfig config) { + public IBinder startRecognition(int modelHandle, @NonNull RecognitionConfig config) { synchronized (SoundTriggerModule.this) { checkValid(); - mLoadedModels.get(modelHandle).startRecognition(config); + return mLoadedModels.get(modelHandle).startRecognition(config); } } @@ -385,6 +385,8 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo public int mHandle; private ModelState mState = ModelState.INIT; private SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession mSession; + private IBinder mRecognitionToken = null; + private boolean mIsStopping = false; private @NonNull ModelState getState() { @@ -425,10 +427,15 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo return mSession.mSessionHandle; } - private void startRecognition(@NonNull RecognitionConfig config) { + private IBinder startRecognition(@NonNull RecognitionConfig config) { + if (mIsStopping == true) { + throw new RecoverableException(Status.INTERNAL_ERROR, "Race occurred"); + } mHalService.startRecognition(mHandle, mSession.mDeviceHandle, mSession.mIoHandle, config); + mRecognitionToken = new Binder(); setState(ModelState.ACTIVE); + return mRecognitionToken; } private void stopRecognition() { @@ -437,9 +444,12 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo // This call is idempotent in order to avoid races. return; } + mRecognitionToken = null; + mIsStopping = true; } mHalService.stopRecognition(mHandle); synchronized (SoundTriggerModule.this) { + mIsStopping = false; setState(ModelState.LOADED); } } @@ -474,9 +484,13 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo @NonNull RecognitionEventSys event) { ISoundTriggerCallback callback; synchronized (SoundTriggerModule.this) { + if (mRecognitionToken == null) { + return; + } if (!event.recognitionEvent.recognitionStillActive) { setState(ModelState.LOADED); } + event.token = mRecognitionToken; callback = mCallback; } // The callback must be invoked outside of the lock. @@ -495,12 +509,15 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo @NonNull PhraseRecognitionEventSys event) { ISoundTriggerCallback callback; synchronized (SoundTriggerModule.this) { + if (mRecognitionToken == null) { + return; + } if (!event.phraseRecognitionEvent.common.recognitionStillActive) { setState(ModelState.LOADED); } + event.token = mRecognitionToken; callback = mCallback; } - // The callback must be invoked outside of the lock. try { if (callback != null) {