Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change external capture state notification mechanism

Browse Source 
Previously, external capture state notifications were delivered from
the audio policy manager to the sound trigger service as normal
"forward calls". However this creates a cyclic dependency between
sound trigger and APM, which is especially problematic during startup,
when the audio server is assumed to start prior to the system server
and thus cannot block on the sound trigger service becoming available.

With this change, the sound trigger service would register a callback
with APM for these notifications.

A small refactoring was needed in order to keep the internal interface
of sound trigger classes having the setExternalCapture() methods,
while removing them from the external interface.

Test: Manual verification, checking the logs, validating behavior in
      response to killing the audioserver process.
Bug: 146157104
Merged-In: I4b4467fbac3607ee170394dc1b3309e7e3d422d8
Change-Id: Ibf68a5e5681afca9c5b1b821bb8ac18a2eebb4ca
This commit is contained in:
Ytai Ben-Tsvi
2020-03-26 13:09:38 -07:00
parent 27480cf8c0
commit 48f90c22e5
9 changed files with 219 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
media/java/android/media/soundtrigger_middleware/ISoundTriggerMiddlewareService.aidl
View File

@@ -39,10 +39,4 @@ interface ISoundTriggerMiddlewareService {
* one of the handles from the returned list.
*/
ISoundTriggerModule attach(int handle, ISoundTriggerCallback callback);
/**
* Notify the service that external input capture is taking place. This may cause some of the
* active recognitions to be aborted.
*/
void setExternalCaptureState(boolean active);
}

1
services/core/Android.bp
View File

@@ -126,6 +126,7 @@ java_library_static {
"android.hardware.rebootescrow-java",
"android.hardware.soundtrigger-V2.3-java",
"android.hidl.manager-V1.2-java",
"capture_state_listener-aidl-java",
"dnsresolver_aidl_interface-V2-java",
"netd_event_listener_interface-java",
"overlayable_policy_aidl-java",

157
services/core/java/com/android/server/soundtrigger_middleware/ExternalCaptureStateTracker.java Normal file
View File

@@ -0,0 +1,157 @@
/*
* Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.soundtrigger_middleware;
import android.media.ICaptureStateListener;
import android.os.IBinder;
import android.os.Parcel;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.util.Log;
import java.util.concurrent.Semaphore;
import java.util.function.Consumer;
/**
* This is a never-give-up listener for sound trigger external capture state notifications, as
* published by the audio policy service.
*
* This class will constantly try to connect to the service over a background thread and tolerate
* its death. The client will be notified by a single provided function that is called in a
* synchronized manner.
* For simplicity, there is currently no way to stop the tracker. This is possible to add if the
* need ever arises.
*/
public class ExternalCaptureStateTracker {
private static final String TAG = "CaptureStateTracker";
/** Our client's listener. */
private final Consumer<Boolean> mListener;
/** A lock used to ensure synchronized access to mListener. */
private final Object mListenerLock = new Object();
/**
* The binder listener that we're providing to the audio policy service. Ensures synchronized
* access to mListener.
*/
private final Listener mSyncListener = new Listener();
/** The name of the audio policy service. */
private final String mAudioPolicyServiceName;
/** This semaphore will get a permit every time we need to reconnect. */
private final Semaphore mNeedToConnect = new Semaphore(1);
/**
* We must hold a reference to the APM service, even though we're not actually using it after
* installing the callback. Otherwise, binder silently un-links our death listener.
*/
private IBinder mService;
/**
* Constructor. Will start a background thread to do the work.
*
* @param audioPolicyServiceName The name of the audio policy service to connect to.
* @param listener A client provided listener that will be called on state
* changes. May be
* called multiple consecutive times with the same value. Never
* called
* concurrently.
*/
public ExternalCaptureStateTracker(String audioPolicyServiceName,
Consumer<Boolean> listener) {
mAudioPolicyServiceName = audioPolicyServiceName;
mListener = listener;
new Thread(this::run).start();
}
/**
* Routine for the background thread. Keeps trying to reconnect.
*/
private void run() {
while (true) {
mNeedToConnect.acquireUninterruptibly();
connectWithRetry();
}
}
/**
* Try to connect. Retry in case of RemoteException.
*/
private void connectWithRetry() {
while (true) {
try {
connect();
return;
} catch (RemoteException e) {
Log.w(TAG, "Exception caught trying to connect", e);
} catch (ServiceManager.ServiceNotFoundException e) {
Log.w(TAG, "Service not yet available, waiting", e);
try {
Thread.sleep(1000);
} catch (InterruptedException ex) {
}
} catch (Exception e) {
Log.e(TAG, "Unexpected exception caught trying to connect", e);
throw e;
}
}
}
/**
* Connect to the service, install listener and death notifier.
*
* @throws RemoteException In case of a binder issue.
*/
private void connect() throws RemoteException, ServiceManager.ServiceNotFoundException {
Log.d(TAG, "Connecting to audio policy service: " + mAudioPolicyServiceName);
mService = ServiceManager.getServiceOrThrow(mAudioPolicyServiceName);
synchronized (mListenerLock) {
boolean active = registerSoundTriggerCaptureStateListener(mService, mSyncListener);
mListener.accept(active);
}
mService.linkToDeath(() -> {
Log.w(TAG, "Audio policy service died");
mNeedToConnect.release();
}, 0);
}
/**
* Since the audio policy service does not have an AIDL interface, this method does the
* necessary manual marshalling.
*
* @param service The service binder object.
* @param listener The listener binder object to register.
* @return The active state at the time of registration.
*/
private boolean registerSoundTriggerCaptureStateListener(IBinder service,
ICaptureStateListener listener) throws RemoteException {
Parcel request = Parcel.obtain();
Parcel response = Parcel.obtain();
request.writeInterfaceToken("android.media.IAudioPolicyService");
request.writeStrongBinder(listener.asBinder());
service.transact(82 /* REGISTER_SOUNDTRIGGER_CAPTURE_STATE_LISTENER */, request, response,
0);
return response.readBoolean();
}
private class Listener extends ICaptureStateListener.Stub {
@Override
public void setCaptureState(boolean active) {
synchronized (mListenerLock) {
mListener.accept(active);
}
}
}
}

27
services/core/java/com/android/server/soundtrigger_middleware/ISoundTriggerMiddlewareInternal.java Normal file
View File

@@ -0,0 +1,27 @@
/*
* Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.soundtrigger_middleware;
import android.media.ICaptureStateListener;
import android.media.soundtrigger_middleware.ISoundTriggerMiddlewareService;
/**
* This interface unifies ISoundTriggerMiddlewareService with ICaptureStateListener.
*/
public interface ISoundTriggerMiddlewareInternal extends ISoundTriggerMiddlewareService,
ICaptureStateListener {
}

4
services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImpl.java
View File

@@ -50,7 +50,7 @@ import java.util.List;
*
* @hide
*/
public class SoundTriggerMiddlewareImpl implements ISoundTriggerMiddlewareService {
public class SoundTriggerMiddlewareImpl implements ISoundTriggerMiddlewareInternal {
static private final String TAG = "SoundTriggerMiddlewareImpl";
private final SoundTriggerModule[] mModules;
@@ -124,7 +124,7 @@ public class SoundTriggerMiddlewareImpl implements ISoundTriggerMiddlewareServic
}
@Override
public void setExternalCaptureState(boolean active) {
public void setCaptureState(boolean active) {
for (SoundTriggerModule module : mModules) {
module.setExternalCaptureState(active);
}

14
services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java
View File

@@ -62,11 +62,11 @@ import java.util.LinkedList;
* String, Object, Object[])}, {@link #logVoidReturnWithObject(Object, String, Object[])} and {@link
* #logExceptionWithObject(Object, String, Exception, Object[])}.
*/
public class SoundTriggerMiddlewareLogging implements ISoundTriggerMiddlewareService, Dumpable {
public class SoundTriggerMiddlewareLogging implements ISoundTriggerMiddlewareInternal, Dumpable {
private static final String TAG = "SoundTriggerMiddlewareLogging";
private final @NonNull ISoundTriggerMiddlewareService mDelegate;
private final @NonNull ISoundTriggerMiddlewareInternal mDelegate;
public SoundTriggerMiddlewareLogging(@NonNull ISoundTriggerMiddlewareService delegate) {
public SoundTriggerMiddlewareLogging(@NonNull ISoundTriggerMiddlewareInternal delegate) {
mDelegate = delegate;
}
@@ -96,12 +96,12 @@ public class SoundTriggerMiddlewareLogging implements ISoundTriggerMiddlewareSer
}
@Override
public void setExternalCaptureState(boolean active) throws RemoteException {
public void setCaptureState(boolean active) throws RemoteException {
try {
mDelegate.setExternalCaptureState(active);
logVoidReturn("setExternalCaptureState", active);
mDelegate.setCaptureState(active);
logVoidReturn("setCaptureState", active);
} catch (Exception e) {
logException("setExternalCaptureState", e, active);
logException("setCaptureState", e, active);
throw e;
}
}

16
services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java
View File

@@ -63,14 +63,21 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic
static private final String TAG = "SoundTriggerMiddlewareService";
@NonNull
private final ISoundTriggerMiddlewareService mDelegate;
private final ISoundTriggerMiddlewareInternal mDelegate;
/**
* Constructor for internal use only. Could be exposed for testing purposes in the future.
* Users should access this class via {@link Lifecycle}.
*/
private SoundTriggerMiddlewareService(@NonNull ISoundTriggerMiddlewareService delegate) {
private SoundTriggerMiddlewareService(@NonNull ISoundTriggerMiddlewareInternal delegate) {
mDelegate = Objects.requireNonNull(delegate);
new ExternalCaptureStateTracker("media.audio_policy", active -> {
try {
mDelegate.setCaptureState(active);
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
}
});
}
@Override
@@ -86,11 +93,6 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic
return new ModuleService(mDelegate.attach(handle, callback));
}
@Override
public void setExternalCaptureState(boolean active) throws RemoteException {
mDelegate.setExternalCaptureState(active);
}
@Override protected void dump(FileDescriptor fd, PrintWriter fout, String[] args) {
if (mDelegate instanceof Dumpable) {
((Dumpable) mDelegate).dump(fout);

32
services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java
View File

@@ -105,7 +105,7 @@ import java.util.Set;
*
* {@hide}
*/
public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddlewareService, Dumpable {
public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddlewareInternal, Dumpable {
private static final String TAG = "SoundTriggerMiddlewareValidation";
private enum ModuleState {
@@ -114,12 +114,12 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
DEAD
};
private final @NonNull ISoundTriggerMiddlewareService mDelegate;
private final @NonNull ISoundTriggerMiddlewareInternal mDelegate;
private final @NonNull Context mContext;
private Map<Integer, Set<ModuleService>> mModules;
public SoundTriggerMiddlewareValidation(
@NonNull ISoundTriggerMiddlewareService delegate, @NonNull Context context) {
@NonNull ISoundTriggerMiddlewareInternal delegate, @NonNull Context context) {
mDelegate = delegate;
mContext = context;
}
@@ -213,21 +213,15 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
}
@Override
public void setExternalCaptureState(boolean active) {
// Permission check.
checkPreemptPermissions();
// Input validation (always valid).
// State validation (always valid).
public void setCaptureState(boolean active) {
// This is an internal call. No permissions needed.
//
// Normally, we would acquire a lock here. However, we do not access any state here so it
// is safe to not lock. This call is typically done from a different context than all the
// other calls and may result in a deadlock if we lock here (between the audio server and
// the system server).
// From here on, every exception isn't client's fault.
try {
mDelegate.setExternalCaptureState(active);
mDelegate.setCaptureState(active);
} catch (Exception e) {
throw handleException(e);
}
@@ -249,16 +243,6 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
enforcePermission(Manifest.permission.CAPTURE_AUDIO_HOTWORD);
}
/**
* Throws a {@link SecurityException} if caller permanently doesn't have the given permission,
* or a {@link ServiceSpecificException} with a {@link Status#TEMPORARY_PERMISSION_DENIED} if
* caller temporarily doesn't have the right permissions to preempt active sound trigger
* sessions.
*/
void checkPreemptPermissions() {
enforcePermission(Manifest.permission.PREEMPT_SOUND_TRIGGER);
}
/**
* Throws a {@link SecurityException} if caller permanently doesn't have the given permission,
* or a {@link ServiceSpecificException} with a {@link Status#TEMPORARY_PERMISSION_DENIED} if
@@ -806,4 +790,4 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware
}
}
}
}
}

16
services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java
View File

@@ -1106,7 +1106,7 @@ public class SoundTriggerMiddlewareImplTest {
public void testAbortRecognition() throws Exception {
// Make sure the HAL doesn't support concurrent capture.
initService(false);
mService.setExternalCaptureState(false);
mService.setCaptureState(false);
ISoundTriggerCallback callback = createCallbackMock();
ISoundTriggerModule module = mService.attach(0, callback);
@@ -1120,7 +1120,7 @@ public class SoundTriggerMiddlewareImplTest {
startRecognition(module, handle, hwHandle);
// Abort.
mService.setExternalCaptureState(true);
mService.setCaptureState(true);
ArgumentCaptor<RecognitionEvent> eventCaptor = ArgumentCaptor.forClass(
RecognitionEvent.class);
@@ -1142,7 +1142,7 @@ public class SoundTriggerMiddlewareImplTest {
verifyNotStartRecognition();
// Now enable it and make sure we are notified.
mService.setExternalCaptureState(false);
mService.setCaptureState(false);
verify(callback).onRecognitionAvailabilityChange(true);
// Unload the model.
@@ -1154,7 +1154,7 @@ public class SoundTriggerMiddlewareImplTest {
public void testAbortPhraseRecognition() throws Exception {
// Make sure the HAL doesn't support concurrent capture.
initService(false);
mService.setExternalCaptureState(false);
mService.setCaptureState(false);
ISoundTriggerCallback callback = createCallbackMock();
ISoundTriggerModule module = mService.attach(0, callback);
@@ -1168,7 +1168,7 @@ public class SoundTriggerMiddlewareImplTest {
startRecognition(module, handle, hwHandle);
// Abort.
mService.setExternalCaptureState(true);
mService.setCaptureState(true);
ArgumentCaptor<PhraseRecognitionEvent> eventCaptor = ArgumentCaptor.forClass(
PhraseRecognitionEvent.class);
@@ -1190,7 +1190,7 @@ public class SoundTriggerMiddlewareImplTest {
verifyNotStartRecognition();
// Now enable it and make sure we are notified.
mService.setExternalCaptureState(false);
mService.setCaptureState(false);
verify(callback).onRecognitionAvailabilityChange(true);
// Unload the model.
@@ -1216,7 +1216,7 @@ public class SoundTriggerMiddlewareImplTest {
startRecognition(module, handle, hwHandle);
// Signal concurrent capture. Shouldn't abort.
mService.setExternalCaptureState(true);
mService.setCaptureState(true);
verify(callback, never()).onRecognition(anyInt(), any());
verify(callback, never()).onRecognitionAvailabilityChange(anyBoolean());
@@ -1252,7 +1252,7 @@ public class SoundTriggerMiddlewareImplTest {
startRecognition(module, handle, hwHandle);
// Signal concurrent capture. Shouldn't abort.
mService.setExternalCaptureState(true);
mService.setCaptureState(true);
verify(callback, never()).onPhraseRecognition(anyInt(), any());
verify(callback, never()).onRecognitionAvailabilityChange(anyBoolean());