Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8dc40270 authored by Ytai Ben-Tsvi's avatar Ytai Ben-Tsvi Committed by Ytai Ben-tsvi
Browse files

Explicitly close sound model file descriptors 

This is a workaround for the fact that currently, relying on the GC to
release those resources is unreliable and on the other hand having an
explicit close() that isn't tightly coupled with the internal details
of the parcelable is not possible (filed a bug).

Test: Manual verification of soundtrigger operation.
Test: atest com.android.server.soundtrigger_middleware.SoundHw2CompatTest
Fixes: 219411458
Change-Id: I43bd43ca80eadf97f0254678b0e817fc6f8a06b2
Merged-In: I43bd43ca80eadf97f0254678b0e817fc6f8a06b2
(cherry picked from commit 578eadd1)
parent 9492d508

core/java/android/hardware/soundtrigger/SoundTriggerModule.java

+30 −2
Original line number Diff line number Diff line
@@ -37,6 +37,8 @@ import android.os.Message; 
import android.os.RemoteException;

import android.util.Log;



import java.io.IOException;



/**

 * The SoundTriggerModule provides APIs to control sound models and sound detection

 * on a given sound trigger hardware module.
@@ -137,13 +139,39 @@ public class SoundTriggerModule { 
            if (model instanceof SoundTrigger.GenericSoundModel) {

                SoundModel aidlModel = ConversionUtil.api2aidlGenericSoundModel(

                        (SoundTrigger.GenericSoundModel) model);

                try {

                    soundModelHandle[0] = mService.loadModel(aidlModel);

                } finally {

                    // TODO(b/219825762): We should be able to use the entire object in a

                    //  try-with-resources

                    //   clause, instead of having to explicitly close internal fields.

                    if (aidlModel.data != null) {

                        try {

                            aidlModel.data.close();

                        } catch (IOException e) {

                            Log.e(TAG, "Failed to close file", e);

                        }

                    }

                }

                return SoundTrigger.STATUS_OK;

            }

            if (model instanceof SoundTrigger.KeyphraseSoundModel) {

                PhraseSoundModel aidlModel = ConversionUtil.api2aidlPhraseSoundModel(

                        (SoundTrigger.KeyphraseSoundModel) model);

                try {

                    soundModelHandle[0] = mService.loadPhraseModel(aidlModel);

                } finally {

                    // TODO(b/219825762): We should be able to use the entire object in a

                    //  try-with-resources

                    //   clause, instead of having to explicitly close internal fields.

                    if (aidlModel.common.data != null) {

                        try {

                            aidlModel.common.data.close();

                        } catch (IOException e) {

                            Log.e(TAG, "Failed to close file", e);

                        }

                    }

                }

                return SoundTrigger.STATUS_OK;

            }

            return SoundTrigger.STATUS_BAD_VALUE;

services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Compat.java

+24 −0
Original line number Diff line number Diff line
@@ -29,7 +29,9 @@ import android.os.IBinder; 
import android.os.IHwBinder;

import android.os.RemoteException;

import android.system.OsConstants;

import android.util.Log;



import java.io.IOException;

import java.util.HashMap;

import java.util.Map;

import java.util.Objects;
@@ -54,6 +56,8 @@ import java.util.concurrent.atomic.AtomicReference; 
 * </ul>

 */

final class SoundTriggerHw2Compat implements ISoundTriggerHal {

    private static final String TAG = "SoundTriggerHw2Compat";



    private final @NonNull Runnable mRebootRunnable;

    private final @NonNull IHwBinder mBinder;

    private @NonNull android.hardware.soundtrigger.V2_0.ISoundTriggerHw mUnderlying_2_0;
@@ -226,6 +230,16 @@ final class SoundTriggerHw2Compat implements ISoundTriggerHal { 
            return handle.get();

        } catch (RemoteException e) {

            throw e.rethrowAsRuntimeException();

        } finally {

            // TODO(b/219825762): We should be able to use the entire object in a try-with-resources

            //   clause, instead of having to explicitly close internal fields.

            if (hidlModel.data != null) {

                try {

                    hidlModel.data.close();

                } catch (IOException e) {

                    Log.e(TAG, "Failed to close file", e);

                }

            }

        }

    }
@@ -252,6 +266,16 @@ final class SoundTriggerHw2Compat implements ISoundTriggerHal { 
            return handle.get();

        } catch (RemoteException e) {

            throw e.rethrowAsRuntimeException();

        } finally {

            // TODO(b/219825762): We should be able to use the entire object in a try-with-resources

            //   clause, instead of having to explicitly close internal fields.

            if (hidlModel.common.data != null) {

                try {

                    hidlModel.common.data.close();

                } catch (IOException e) {

                    Log.e(TAG, "Failed to close file", e);

                }

            }

        }

    }

services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java

+15 −14
Original line number Diff line number Diff line
@@ -31,7 +31,6 @@ import static org.mockito.Mockito.atMost; 
import static org.mockito.Mockito.clearInvocations;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.spy;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -49,7 +48,6 @@ import android.os.IBinder; 
import android.os.IHwBinder;

import android.os.IHwInterface;

import android.os.RemoteException;

import android.system.OsConstants;



import org.junit.After;

import org.junit.Before;
@@ -60,6 +58,7 @@ import org.mockito.ArgumentCaptor; 


import java.util.LinkedList;

import java.util.List;

import java.util.concurrent.atomic.AtomicReference;



@RunWith(Parameterized.class)

public class SoundHw2CompatTest {
@@ -240,14 +239,15 @@ public class SoundHw2CompatTest { 
                (android.hardware.soundtrigger.V2_1.ISoundTriggerHw) mHalDriver;



        final int handle = 29;

        ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel> modelCaptor =

                ArgumentCaptor.forClass(

                        android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel.class);

        AtomicReference<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel> model =

                new AtomicReference<>();

        ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback> callbackCaptor =

                ArgumentCaptor.forClass(

                        android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback.class);



        doAnswer(invocation -> {

            // We need to dup the model, as it gets invalidated after the call returns.

            model.set(TestUtil.dupModel_2_1(invocation.getArgument(0)));

            android.hardware.soundtrigger.V2_1.ISoundTriggerHw.loadSoundModel_2_1Callback

                    resultCallback = invocation.getArgument(3);
@@ -259,10 +259,9 @@ public class SoundHw2CompatTest { 
        assertEquals(handle,

                mCanonical.loadSoundModel(TestUtil.createGenericSoundModel(), canonicalCallback));



        verify(driver_2_1).loadSoundModel_2_1(modelCaptor.capture(), callbackCaptor.capture(),

                anyInt(), any());

        verify(driver_2_1).loadSoundModel_2_1(any(), callbackCaptor.capture(), anyInt(), any());



        TestUtil.validateGenericSoundModel_2_1(modelCaptor.getValue());

        TestUtil.validateGenericSoundModel_2_1(model.get());

        validateCallback_2_1(callbackCaptor.getValue(), canonicalCallback);

        return handle;

    }
@@ -355,14 +354,16 @@ public class SoundHw2CompatTest { 
                (android.hardware.soundtrigger.V2_1.ISoundTriggerHw) mHalDriver;



        final int handle = 29;

        ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel>

                modelCaptor = ArgumentCaptor.forClass(

                android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel.class);

        AtomicReference<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel> model =

                new AtomicReference<>();

        ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback> callbackCaptor =

                ArgumentCaptor.forClass(

                        android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback.class);



        doAnswer(invocation -> {

            // We need to dup the model, as it gets invalidated after the call returns.

            model.set(TestUtil.dupPhraseModel_2_1(invocation.getArgument(0)));



            android.hardware.soundtrigger.V2_1.ISoundTriggerHw.loadPhraseSoundModel_2_1Callback

                    resultCallback = invocation.getArgument(3);
@@ -374,10 +375,10 @@ public class SoundHw2CompatTest { 
        assertEquals(handle, mCanonical.loadPhraseSoundModel(TestUtil.createPhraseSoundModel(),

                canonicalCallback));



        verify(driver_2_1).loadPhraseSoundModel_2_1(modelCaptor.capture(), callbackCaptor.capture(),

                anyInt(), any());

        verify(driver_2_1).loadPhraseSoundModel_2_1(any(), callbackCaptor.capture(), anyInt(),

                any());



        TestUtil.validatePhraseSoundModel_2_1(modelCaptor.getValue());

        TestUtil.validatePhraseSoundModel_2_1(model.get());

        validateCallback_2_1(callbackCaptor.getValue(), canonicalCallback);

        return handle;

    }

services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java

+42 −27
Original line number Diff line number Diff line
@@ -47,6 +47,7 @@ import android.os.HidlMemoryUtil; 
import android.os.ParcelFileDescriptor;

import android.os.SharedMemory;



import java.io.IOException;

import java.nio.ByteBuffer;

import java.util.List;
@@ -82,6 +83,19 @@ class TestUtil { 
                HidlMemoryUtil.hidlMemoryToByteArray(model.data));

    }



    static android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel dupModel_2_1(

            android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel model) {

        android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel dup =

                new android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel();

        dup.header = model.header;

        try {

            dup.data = model.data.dup();

        } catch (IOException e) {

            throw new RuntimeException(e);

        }

        return dup;

    }



    private static void validateSoundModel_2_0(

            android.hardware.soundtrigger.V2_0.ISoundTriggerHw.SoundModel model, int type) {

        assertEquals(type, model.type);
@@ -121,6 +135,15 @@ class TestUtil { 
        validatePhrases_2_0(model.phrases);

    }



    static android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel dupPhraseModel_2_1(

            android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel model) {

        android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel dup =

                new android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel();

        dup.common = dupModel_2_1(model.common);

        dup.phrases = model.phrases;

        return dup;

    }



    static void validatePhraseSoundModel_2_0(

            android.hardware.soundtrigger.V2_0.ISoundTriggerHw.PhraseSoundModel model) {

        validateSoundModel_2_0(model.common,
@@ -190,31 +213,27 @@ class TestUtil { 
        properties.maxKeyPhrases = 567;

        properties.maxUsers = 678;

        properties.recognitionModes =

                RecognitionMode.VOICE_TRIGGER

                        | RecognitionMode.USER_IDENTIFICATION

                        | RecognitionMode.USER_AUTHENTICATION

                        | RecognitionMode.GENERIC_TRIGGER;

                RecognitionMode.VOICE_TRIGGER | RecognitionMode.USER_IDENTIFICATION

                        | RecognitionMode.USER_AUTHENTICATION | RecognitionMode.GENERIC_TRIGGER;

        properties.captureTransition = true;

        properties.maxBufferMs = 321;

        properties.concurrentCapture = supportConcurrentCapture;

        properties.triggerInEvent = true;

        properties.powerConsumptionMw = 432;

        properties.supportedModelArch = "supportedModelArch";

        properties.audioCapabilities = AudioCapabilities.ECHO_CANCELLATION

                | AudioCapabilities.NOISE_SUPPRESSION;

        properties.audioCapabilities =

                AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION;

        return properties;

    }



    static void validateDefaultProperties(Properties properties,

            boolean supportConcurrentCapture) {

    static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture) {

        validateDefaultProperties(properties, supportConcurrentCapture,

                AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION,

                "supportedModelArch");

    }



    static void validateDefaultProperties(Properties properties,

            boolean supportConcurrentCapture, @AudioCapabilities int audioCapabilities,

            @NonNull String supportedModelArch) {

    static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture,

            @AudioCapabilities int audioCapabilities, @NonNull String supportedModelArch) {

        assertEquals("implementor", properties.implementor);

        assertEquals("description", properties.description);

        assertEquals(123, properties.version);
@@ -222,10 +241,9 @@ class TestUtil { 
        assertEquals(456, properties.maxSoundModels);

        assertEquals(567, properties.maxKeyPhrases);

        assertEquals(678, properties.maxUsers);

        assertEquals(RecognitionMode.GENERIC_TRIGGER

                | RecognitionMode.USER_AUTHENTICATION

                | RecognitionMode.USER_IDENTIFICATION

                | RecognitionMode.VOICE_TRIGGER, properties.recognitionModes);

        assertEquals(RecognitionMode.GENERIC_TRIGGER | RecognitionMode.USER_AUTHENTICATION

                        | RecognitionMode.USER_IDENTIFICATION | RecognitionMode.VOICE_TRIGGER,

                properties.recognitionModes);

        assertTrue(properties.captureTransition);

        assertEquals(321, properties.maxBufferMs);

        assertEquals(supportConcurrentCapture, properties.concurrentCapture);
@@ -246,8 +264,8 @@ class TestUtil { 
        config.phraseRecognitionExtras[0].levels[0].userId = 234;

        config.phraseRecognitionExtras[0].levels[0].levelPercent = 34;

        config.data = new byte[]{5, 4, 3, 2, 1};

        config.audioCapabilities = AudioCapabilities.ECHO_CANCELLATION

                | AudioCapabilities.NOISE_SUPPRESSION;

        config.audioCapabilities =

                AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION;

        return config;

    }
@@ -295,13 +313,12 @@ class TestUtil { 
            int captureHandle) {

        validateRecognitionConfig_2_1(config.base, captureDevice, captureHandle);



        assertEquals(AudioCapabilities.ECHO_CANCELLATION

                | AudioCapabilities.NOISE_SUPPRESSION, config.audioCapabilities);

        assertEquals(AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION,

                config.audioCapabilities);

    }



    static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_0(

            int hwHandle,

            int status) {

            int hwHandle, int status) {

        android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.RecognitionEvent halEvent =

                new android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.RecognitionEvent();

        halEvent.status = status;
@@ -351,8 +368,7 @@ class TestUtil { 
        return event;

    }



    static ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_1(

            int hwHandle,

    static ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_1(int hwHandle,

            int status) {

        ISoundTriggerHwCallback.RecognitionEvent halEvent =

                new ISoundTriggerHwCallback.RecognitionEvent();
@@ -386,8 +402,7 @@ class TestUtil { 
        PhraseRecognitionExtra extra = new PhraseRecognitionExtra();

        extra.id = 123;

        extra.confidenceLevel = 52;

        extra.recognitionModes = RecognitionMode.VOICE_TRIGGER

                | RecognitionMode.GENERIC_TRIGGER;

        extra.recognitionModes = RecognitionMode.VOICE_TRIGGER | RecognitionMode.GENERIC_TRIGGER;

        ConfidenceLevel level = new ConfidenceLevel();

        level.userId = 31;

        level.levelPercent = 43;
@@ -396,8 +411,8 @@ class TestUtil { 
        return event;

    }



    static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent

    createPhraseRecognitionEvent_2_0(int hwHandle, int status) {

    static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent createPhraseRecognitionEvent_2_0(

            int hwHandle, int status) {

        android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent halEvent =

                new android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent();

        halEvent.common = createRecognitionEvent_2_0(hwHandle, status);