From 044ca5adfa0301c96bf8c721c39490a83d630706 Mon Sep 17 00:00:00 2001 From: lpeter Date: Thu, 29 Apr 2021 19:07:00 +0800 Subject: [PATCH] Close audio source after getting callback from HotwordDetectionService Due to the file descriptor of audio source was closed before the attempt to parcel the file descriptor when passing the file descriptor to the HotwordDetectionService. It will cause the exception below: java.lang.RuntimeException: Bad file descriptor It would be better to close audio source when getting callback from HotwordDetectionService Bug: 185569471 Test: atest CtsVoiceInteractionTestCases Test: atest CtsVoiceInteractionTestCases --instant Change-Id: I10774fd29d559b5d2bedc21b0ebb2ef849a1b7c6 --- .../HotwordDetectionConnection.java | 89 +++++-------------- .../VoiceInteractionManagerServiceImpl.java | 5 ++ 2 files changed, 27 insertions(+), 67 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java index 3fbd40f8a060f..d1bd159ee7ff5 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java @@ -270,14 +270,9 @@ final class HotwordDetectionConnection { if (DEBUG) { Slog.d(TAG, "startListeningFromExternalSource"); } - - ParcelFileDescriptor.AutoCloseInputStream audioReader = - new ParcelFileDescriptor.AutoCloseInputStream(audioStream); - - handleSoftwareHotwordDetection( + handleExternalSourceHotwordDetection( + audioStream, audioFormat, - AudioReader.createFromInputStream(audioReader), - AUDIO_SOURCE_EXTERNAL, options, callback); } @@ -556,14 +551,18 @@ final class HotwordDetectionConnection { } } - private void handleSoftwareHotwordDetection( + private void handleExternalSourceHotwordDetection( + ParcelFileDescriptor audioStream, AudioFormat audioFormat, - AudioReader audioSource, - int audioSourceValue, @Nullable PersistableBundle options, IMicrophoneHotwordDetectionVoiceInteractionCallback callback) { - Pair clientPipe = createPipe(); + if (DEBUG) { + Slog.d(TAG, "#handleExternalSourceHotwordDetection"); + } + AudioReader audioSource = AudioReader.createFromInputStream( + new ParcelFileDescriptor.AutoCloseInputStream(audioStream)); + Pair clientPipe = createPipe(); if (clientPipe == null) { // TODO: Need to propagate as unknown error or something? return; @@ -579,8 +578,8 @@ final class HotwordDetectionConnection { try (AudioReader source = audioSource; OutputStream fos = new ParcelFileDescriptor.AutoCloseOutputStream(serviceAudioSink)) { - byte[] buffer = new byte[1024]; + byte[] buffer = new byte[1024]; while (true) { int bytesRead = source.read(buffer, 0, 1024); @@ -608,76 +607,32 @@ final class HotwordDetectionConnection { service -> service.detectFromMicrophoneSource( serviceAudioSource, // TODO: consider making a proxy callback + copy of audio format - audioSourceValue, audioFormat, options, + AUDIO_SOURCE_EXTERNAL, + audioFormat, + options, new IDspHotwordDetectionCallback.Stub() { @Override public void onRejected(@Nullable HotwordRejectedResult result) throws RemoteException { + bestEffortClose(serviceAudioSink); + bestEffortClose(serviceAudioSource); + bestEffortClose(audioSource); + // TODO: Propagate the HotwordRejectedResult. } @Override public void onDetected(@Nullable HotwordDetectedResult triggerResult) throws RemoteException { - // Stop bestEffortClose(serviceAudioSink); - - if (audioSourceValue == AUDIO_SOURCE_EXTERNAL) { - callback.onDetected(triggerResult, null, null); - // TODO: Add a delay before closing. - bestEffortClose(audioSource); - } - - Pair clientPipe = - createPipe(); - - if (clientPipe == null) { - // Error. - // Need to propagate as unknown error or something? - return; - } - ParcelFileDescriptor callbackAudioSink = clientPipe.second; - ParcelFileDescriptor callbackClientRead = clientPipe.first; - - mAudioCopyExecutor.execute(() -> { - try (AudioReader source = audioSource; - OutputStream fos = - new ParcelFileDescriptor.AutoCloseOutputStream( - callbackAudioSink)) { - - // TODO: get ring buffer suffix here - // fos.write(lastSeveralSeconds); - - byte[] buffer = new byte[1024]; - while (true) { - int bytesRead = source.read(buffer, 0, 1024); - - if (bytesRead < 0) { - break; - } - - fos.write(buffer, 0, bytesRead); - } - } catch (IOException e) { - Slog.w(TAG, "Failed supplying audio data to validator", e); - } finally { - synchronized (mLock) { - mCurrentAudioSink = null; - } - } - }); - + bestEffortClose(serviceAudioSource); // TODO: noteOp here. - // Remove hotword offset from trigger result - // TODO: consider propagating only capture session. - callback.onDetected(triggerResult, null, callbackClientRead); + callback.onDetected(triggerResult, null /* audioFormat */, + null /* audioStream */); // TODO: Add a delay before closing. - bestEffortClose(callbackClientRead); + bestEffortClose(audioSource); } })); - - // TODO: verify this is the right thing to do here. - bestEffortClose(serviceAudioSource); } private static void bestEffortClose(Closeable closeable) { diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java index 0552841574579..9be1ac42d8850 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java @@ -477,6 +477,11 @@ class VoiceInteractionManagerServiceImpl implements VoiceInteractionSessionConne return; } + if (audioStream == null) { + Slog.w(TAG, "External source is null for hotword detector"); + throw new IllegalStateException("External source is null for hotword detector"); + } + mHotwordDetectionConnection .startListeningFromExternalSource(audioStream, audioFormat, options, callback); }