Fix issues with multiple languages and multi-users
For multi-user the issue was looking into the user ID of the current
process instead of the active user. The current process was the system
process and the call to UserManager was returning a user handle that
wasn't of any use while trying to map sound models to a user.
For language, the issue was that we were incorrectly just looking up the
model based on the keyphrase id, however we should have also taken the
enrolled model's locale into account.
Explicitly document that for model management the string representation of locales
is a BCP47 language tag.
Remove debug logging.
Bug: 16798166
Bug: 17462570
Bug: 17463511
Change-Id: Ieffb3e218de63f6e7f40af9705dced481a35b0ad
diff --git a/core/java/android/service/voice/AlwaysOnHotwordDetector.java b/core/java/android/service/voice/AlwaysOnHotwordDetector.java
index 8aa2689..ac7d539 100644
--- a/core/java/android/service/voice/AlwaysOnHotwordDetector.java
+++ b/core/java/android/service/voice/AlwaysOnHotwordDetector.java
@@ -170,8 +170,7 @@
= SoundTrigger.RECOGNITION_MODE_USER_IDENTIFICATION;
static final String TAG = "AlwaysOnHotwordDetector";
- // TODO: Set to false.
- static final boolean DBG = true;
+ static final boolean DBG = false;
private static final int STATUS_ERROR = SoundTrigger.STATUS_ERROR;
private static final int STATUS_OK = SoundTrigger.STATUS_OK;
@@ -575,7 +574,7 @@
int code = STATUS_ERROR;
try {
code = mModelManagementService.startRecognition(mVoiceInteractionService,
- mKeyphraseMetadata.id, mInternalCallback,
+ mKeyphraseMetadata.id, mLocale.toLanguageTag(), mInternalCallback,
new RecognitionConfig(captureTriggerAudio, allowMultipleTriggers,
recognitionExtra, null /* additional data */));
} catch (RemoteException e) {
@@ -690,7 +689,7 @@
if (availability == STATE_NOT_READY
|| availability == STATE_KEYPHRASE_UNENROLLED
|| availability == STATE_KEYPHRASE_ENROLLED) {
- enrolled = internalGetIsEnrolled(mKeyphraseMetadata.id);
+ enrolled = internalGetIsEnrolled(mKeyphraseMetadata.id, mLocale);
if (!enrolled) {
availability = STATE_KEYPHRASE_UNENROLLED;
} else {
@@ -741,10 +740,10 @@
/**
* @return The corresponding {@link KeyphraseSoundModel} or null if none is found.
*/
- private boolean internalGetIsEnrolled(int keyphraseId) {
+ private boolean internalGetIsEnrolled(int keyphraseId, Locale locale) {
try {
return mModelManagementService.isEnrolledForKeyphrase(
- mVoiceInteractionService, keyphraseId);
+ mVoiceInteractionService, keyphraseId, locale.toLanguageTag());
} catch (RemoteException e) {
Slog.w(TAG, "RemoteException in listRegisteredKeyphraseSoundModels!", e);
}
diff --git a/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl b/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl
index 22ec4be..5a10524 100644
--- a/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl
+++ b/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl
@@ -33,32 +33,44 @@
void finish(IBinder token);
/**
- * Lists the registered Sound model for keyphrase detection.
- * May be null if no matching sound models exist.
+ * Gets the registered Sound model for keyphrase detection for the current user.
+ * May be null if no matching sound model exists.
+ *
+ * @param keyphraseId The unique identifier for the keyphrase.
+ * @param bcp47Locale The BCP47 language tag for the keyphrase's locale.
*/
- SoundTrigger.KeyphraseSoundModel getKeyphraseSoundModel(int keyphraseId);
+ SoundTrigger.KeyphraseSoundModel getKeyphraseSoundModel(int keyphraseId, in String bcp47Locale);
/**
- * Updates the given keyphrase sound model. Adds the model if it doesn't exist currently.
+ * Add/Update the given keyphrase sound model.
*/
int updateKeyphraseSoundModel(in SoundTrigger.KeyphraseSoundModel model);
/**
- * Deletes the given keyphrase sound model.
+ * Deletes the given keyphrase sound model for the current user.
+ *
+ * @param keyphraseId The unique identifier for the keyphrase.
+ * @param bcp47Locale The BCP47 language tag for the keyphrase's locale.
*/
- int deleteKeyphraseSoundModel(int keyphraseId);
+ int deleteKeyphraseSoundModel(int keyphraseId, in String bcp47Locale);
/**
- * Indicates if there's a keyphrase sound model available for the given keyphrase ID.
- */
- boolean isEnrolledForKeyphrase(IVoiceInteractionService service, int keyphraseId);
- /**
* Gets the properties of the DSP hardware on this device, null if not present.
*/
SoundTrigger.ModuleProperties getDspModuleProperties(in IVoiceInteractionService service);
/**
+ * Indicates if there's a keyphrase sound model available for the given keyphrase ID.
+ * This performs the check for the current user.
+ *
+ * @param service The current VoiceInteractionService.
+ * @param keyphraseId The unique identifier for the keyphrase.
+ * @param bcp47Locale The BCP47 language tag for the keyphrase's locale.
+ */
+ boolean isEnrolledForKeyphrase(IVoiceInteractionService service, int keyphraseId,
+ String bcp47Locale);
+ /**
* Starts a recognition for the given keyphrase.
*/
int startRecognition(in IVoiceInteractionService service, int keyphraseId,
- in IRecognitionStatusCallback callback,
+ in String bcp47Locale, in IRecognitionStatusCallback callback,
in SoundTrigger.RecognitionConfig recognitionConfig);
/**
* Stops a recognition for the given keyphrase.
diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/DatabaseHelper.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/DatabaseHelper.java
index cc0d8df..9c15f2b 100644
--- a/services/voiceinteraction/java/com/android/server/voiceinteraction/DatabaseHelper.java
+++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/DatabaseHelper.java
@@ -24,10 +24,10 @@
import android.hardware.soundtrigger.SoundTrigger;
import android.hardware.soundtrigger.SoundTrigger.Keyphrase;
import android.hardware.soundtrigger.SoundTrigger.KeyphraseSoundModel;
-import android.os.UserManager;
import android.text.TextUtils;
import android.util.Slog;
+import java.util.Locale;
import java.util.UUID;
/**
@@ -37,8 +37,7 @@
*/
public class DatabaseHelper extends SQLiteOpenHelper {
static final String TAG = "SoundModelDBHelper";
- // TODO: Set to false.
- static final boolean DBG = true;
+ static final boolean DBG = false;
private static final String NAME = "sound_model.db";
private static final int VERSION = 4;
@@ -67,11 +66,8 @@
+ SoundModelContract.KEY_HINT_TEXT + " TEXT,"
+ SoundModelContract.KEY_USERS + " TEXT" + ")";
- private final UserManager mUserManager;
-
public DatabaseHelper(Context context) {
super(context, NAME, null, VERSION);
- mUserManager = (UserManager) context.getSystemService(Context.USER_SERVICE);
}
@Override
@@ -122,17 +118,20 @@
/**
* Deletes the sound model and associated keyphrases.
*/
- public boolean deleteKeyphraseSoundModel(UUID modelUuid) {
- if (modelUuid == null) {
- Slog.w(TAG, "Model UUID must be specified for deletion");
- return false;
- }
-
+ public boolean deleteKeyphraseSoundModel(int keyphraseId, int userHandle, String bcp47Locale) {
+ // Sanitize the locale to guard against SQL injection.
+ bcp47Locale = Locale.forLanguageTag(bcp47Locale).toLanguageTag();
synchronized(this) {
- SQLiteDatabase db = getWritableDatabase();
- String soundModelClause = SoundModelContract.KEY_MODEL_UUID + "='"
- + modelUuid.toString() + "'";
+ KeyphraseSoundModel soundModel = getKeyphraseSoundModel(keyphraseId, userHandle,
+ bcp47Locale);
+ if (soundModel == null) {
+ return false;
+ }
+ // Delete all sound models for the given keyphrase and specified user.
+ SQLiteDatabase db = getWritableDatabase();
+ String soundModelClause = SoundModelContract.KEY_MODEL_UUID
+ + "='" + soundModel.uuid.toString() + "'";
try {
return db.delete(SoundModelContract.TABLE, soundModelClause, null) != 0;
} finally {
@@ -147,11 +146,15 @@
*
* TODO: We only support one keyphrase currently.
*/
- public KeyphraseSoundModel getKeyphraseSoundModel(int keyphraseId) {
+ public KeyphraseSoundModel getKeyphraseSoundModel(int keyphraseId, int userHandle,
+ String bcp47Locale) {
+ // Sanitize the locale to guard against SQL injection.
+ bcp47Locale = Locale.forLanguageTag(bcp47Locale).toLanguageTag();
synchronized(this) {
// Find the corresponding sound model ID for the keyphrase.
String selectQuery = "SELECT * FROM " + SoundModelContract.TABLE
- + " WHERE " + SoundModelContract.KEY_KEYPHRASE_ID + " = '" + keyphraseId + "'";
+ + " WHERE " + SoundModelContract.KEY_KEYPHRASE_ID + "= '" + keyphraseId
+ + "' AND " + SoundModelContract.KEY_LOCALE + "='" + bcp47Locale + "'";
SQLiteDatabase db = getReadableDatabase();
Cursor c = db.rawQuery(selectQuery, null);
@@ -160,14 +163,16 @@
do {
int type = c.getInt(c.getColumnIndex(SoundModelContract.KEY_TYPE));
if (type != SoundTrigger.SoundModel.TYPE_KEYPHRASE) {
- Slog.w(TAG, "Ignoring sound model since it's type is incorrect");
+ if (DBG) {
+ Slog.w(TAG, "Ignoring SoundModel since it's type is incorrect");
+ }
continue;
}
String modelUuid = c.getString(
c.getColumnIndex(SoundModelContract.KEY_MODEL_UUID));
if (modelUuid == null) {
- Slog.w(TAG, "Ignoring sound model since it doesn't specify an ID");
+ Slog.w(TAG, "Ignoring SoundModel since it doesn't specify an ID");
continue;
}
@@ -176,7 +181,7 @@
c.getColumnIndex(SoundModelContract.KEY_RECOGNITION_MODES));
int[] users = getArrayForCommaSeparatedString(
c.getString(c.getColumnIndex(SoundModelContract.KEY_USERS)));
- String locale = c.getString(
+ String modelLocale = c.getString(
c.getColumnIndex(SoundModelContract.KEY_LOCALE));
String text = c.getString(
c.getColumnIndex(SoundModelContract.KEY_HINT_TEXT));
@@ -184,28 +189,37 @@
// Only add keyphrases meant for the current user.
if (users == null) {
// No users present in the keyphrase.
- Slog.w(TAG, "Ignoring keyphrase since it doesn't specify users");
+ Slog.w(TAG, "Ignoring SoundModel since it doesn't specify users");
continue;
}
boolean isAvailableForCurrentUser = false;
- int currentUser = mUserManager.getUserHandle();
for (int user : users) {
- if (currentUser == user) {
+ if (userHandle == user) {
isAvailableForCurrentUser = true;
break;
}
}
if (!isAvailableForCurrentUser) {
- Slog.w(TAG, "Ignoring keyphrase since it's not for the current user");
+ if (DBG) {
+ Slog.w(TAG, "Ignoring SoundModel since user handles don't match");
+ }
continue;
+ } else {
+ if (DBG) Slog.d(TAG, "Found a SoundModel for user: " + userHandle);
}
Keyphrase[] keyphrases = new Keyphrase[1];
keyphrases[0] = new Keyphrase(
- keyphraseId, recognitionModes, locale, text, users);
- return new KeyphraseSoundModel(UUID.fromString(modelUuid),
+ keyphraseId, recognitionModes, modelLocale, text, users);
+ KeyphraseSoundModel model = new KeyphraseSoundModel(
+ UUID.fromString(modelUuid),
null /* FIXME use vendor UUID */, data, keyphrases);
+ if (DBG) {
+ Slog.d(TAG, "Found SoundModel for the given keyphrase/locale/user: "
+ + model);
+ }
+ return model;
} while (c.moveToNext());
}
Slog.w(TAG, "No SoundModel available for the given keyphrase");
@@ -218,15 +232,17 @@
}
private static String getCommaSeparatedString(int[] users) {
- if (users == null || users.length == 0) {
+ if (users == null) {
return "";
}
- String csv = "";
- for (int user : users) {
- csv += String.valueOf(user);
- csv += ",";
+ StringBuilder sb = new StringBuilder();
+ for (int i = 0; i < users.length; i++) {
+ if (i != 0) {
+ sb.append(',');
+ }
+ sb.append(users[i]);
}
- return csv.substring(0, csv.length() - 1);
+ return sb.toString();
}
private static int[] getArrayForCommaSeparatedString(String text) {
diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerHelper.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerHelper.java
index ad38b22..8ce7f74 100644
--- a/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerHelper.java
+++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerHelper.java
@@ -50,8 +50,7 @@
*/
public class SoundTriggerHelper implements SoundTrigger.StatusListener {
static final String TAG = "SoundTriggerHelper";
- // TODO: Set to false.
- static final boolean DBG = true;
+ static final boolean DBG = false;
/**
* Return codes for {@link #startRecognition(int, KeyphraseSoundModel,
@@ -166,8 +165,14 @@
}
}
+ // Unload the previous model if the current one isn't invalid
+ // and, it's not the same as the new one, or we are already started
+ // if we are already started, we can get multiple calls to start
+ // if the underlying sound model changes, in which case we should unload and reload.
+ // The model reuse helps only in cases when we trigger and stop internally
+ // without a start recognition call.
if (mCurrentSoundModelHandle != INVALID_VALUE
- && !soundModel.uuid.equals(mCurrentSoundModelUuid)) {
+ && (!soundModel.uuid.equals(mCurrentSoundModelUuid) || mStarted)) {
Slog.w(TAG, "Unloading previous sound model");
int status = mModule.unloadSoundModel(mCurrentSoundModelHandle);
if (status != SoundTrigger.STATUS_OK) {
diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java
index 7c7b732..82b7f8b 100644
--- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java
+++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java
@@ -446,7 +446,7 @@
//----------------- Model management APIs --------------------------------//
@Override
- public KeyphraseSoundModel getKeyphraseSoundModel(int keyphraseId) {
+ public KeyphraseSoundModel getKeyphraseSoundModel(int keyphraseId, String bcp47Locale) {
synchronized (this) {
if (mContext.checkCallingPermission(Manifest.permission.MANAGE_VOICE_KEYPHRASES)
!= PackageManager.PERMISSION_GRANTED) {
@@ -455,9 +455,14 @@
}
}
+ if (bcp47Locale == null) {
+ throw new IllegalArgumentException("Illegal argument(s) in getKeyphraseSoundModel");
+ }
+
+ final int callingUid = UserHandle.getCallingUserId();
final long caller = Binder.clearCallingIdentity();
try {
- return mDbHelper.getKeyphraseSoundModel(keyphraseId);
+ return mDbHelper.getKeyphraseSoundModel(keyphraseId, callingUid, bcp47Locale);
} finally {
Binder.restoreCallingIdentity(caller);
}
@@ -495,7 +500,7 @@
}
@Override
- public int deleteKeyphraseSoundModel(int keyphraseId) {
+ public int deleteKeyphraseSoundModel(int keyphraseId, String bcp47Locale) {
synchronized (this) {
if (mContext.checkCallingPermission(Manifest.permission.MANAGE_VOICE_KEYPHRASES)
!= PackageManager.PERMISSION_GRANTED) {
@@ -504,13 +509,16 @@
}
}
+ if (bcp47Locale == null) {
+ throw new IllegalArgumentException(
+ "Illegal argument(s) in deleteKeyphraseSoundModel");
+ }
+
+ final int callingUid = UserHandle.getCallingUserId();
final long caller = Binder.clearCallingIdentity();
boolean deleted = false;
try {
- KeyphraseSoundModel soundModel = mDbHelper.getKeyphraseSoundModel(keyphraseId);
- if (soundModel != null) {
- deleted = mDbHelper.deleteKeyphraseSoundModel(soundModel.uuid);
- }
+ deleted = mDbHelper.deleteKeyphraseSoundModel(keyphraseId, callingUid, bcp47Locale);
return deleted ? SoundTriggerHelper.STATUS_OK : SoundTriggerHelper.STATUS_ERROR;
} finally {
if (deleted) {
@@ -527,7 +535,8 @@
//----------------- SoundTrigger APIs --------------------------------//
@Override
- public boolean isEnrolledForKeyphrase(IVoiceInteractionService service, int keyphraseId) {
+ public boolean isEnrolledForKeyphrase(IVoiceInteractionService service, int keyphraseId,
+ String bcp47Locale) {
synchronized (this) {
if (mImpl == null || mImpl.mService == null
|| service.asBinder() != mImpl.mService.asBinder()) {
@@ -536,9 +545,15 @@
}
}
+ if (bcp47Locale == null) {
+ throw new IllegalArgumentException("Illegal argument(s) in isEnrolledForKeyphrase");
+ }
+
+ final int callingUid = UserHandle.getCallingUserId();
final long caller = Binder.clearCallingIdentity();
try {
- KeyphraseSoundModel model = mDbHelper.getKeyphraseSoundModel(keyphraseId);
+ KeyphraseSoundModel model =
+ mDbHelper.getKeyphraseSoundModel(keyphraseId, callingUid, bcp47Locale);
return model != null;
} finally {
Binder.restoreCallingIdentity(caller);
@@ -566,7 +581,8 @@
@Override
public int startRecognition(IVoiceInteractionService service, int keyphraseId,
- IRecognitionStatusCallback callback, RecognitionConfig recognitionConfig) {
+ String bcp47Locale, IRecognitionStatusCallback callback,
+ RecognitionConfig recognitionConfig) {
// Allow the call if this is the current voice interaction service.
synchronized (this) {
if (mImpl == null || mImpl.mService == null
@@ -575,14 +591,16 @@
"Caller is not the current voice interaction service");
}
- if (callback == null || recognitionConfig == null) {
+ if (callback == null || recognitionConfig == null || bcp47Locale == null) {
throw new IllegalArgumentException("Illegal argument(s) in startRecognition");
}
}
+ int callingUid = UserHandle.getCallingUserId();
final long caller = Binder.clearCallingIdentity();
try {
- KeyphraseSoundModel soundModel = mDbHelper.getKeyphraseSoundModel(keyphraseId);
+ KeyphraseSoundModel soundModel =
+ mDbHelper.getKeyphraseSoundModel(keyphraseId, callingUid, bcp47Locale);
if (soundModel == null
|| soundModel.uuid == null
|| soundModel.keyphrases == null) {