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

Commit 602b3286 authored by Eric Laurent's avatar Eric Laurent
Browse files

Fix issue 3509396: AudioEffect.getParameter JAVA. 

Removed dead/buggy code in AudioEffect.getParameter() and
AudioEffect.command() that was meant to return the actual length of
meaningful data in the returned parameter or command reply.

This is replaced by the method return status indicating this length when
positive (negative return codes still indicate the same errors as before).

Modified automated AudioEffect tests accordingly.

Change-Id: Ie89617f912766b8dee73b81f92af9c48027c982d
parent 7a4b8bd5

media/java/android/media/audiofx/AudioEffect.java

+63 −53
Original line number Diff line number Diff line
@@ -573,27 +573,16 @@ public class AudioEffect { 
     *

     * @param param the identifier of the parameter to set

     * @param value the new value for the specified parameter

     * @return {@link #SUCCESS} in case of success, {@link #ERROR_BAD_VALUE},

     *         {@link #ERROR_NO_MEMORY}, {@link #ERROR_INVALID_OPERATION} or

     *         {@link #ERROR_DEAD_OBJECT} in case of failure When called, value.length

     *         indicates the maximum size of the returned parameters value. When

     *         returning, value.length is updated with the actual size of the

     *         returned value.

     * @return the number of meaningful bytes in value array in case of success or

     *  {@link #ERROR_BAD_VALUE}, {@link #ERROR_NO_MEMORY}, {@link #ERROR_INVALID_OPERATION}

     *  or {@link #ERROR_DEAD_OBJECT} in case of failure.

     * @throws IllegalStateException

     * @hide

     */

    public int getParameter(byte[] param, byte[] value)

            throws IllegalStateException {

        checkState("getParameter()");

        int[] vSize = new int[1];

        vSize[0] = value.length;

        int status = native_getParameter(param.length, param, vSize, value);

        if (value.length > vSize[0]) {

            byte[] resizedValue = new byte[vSize[0]];

            System.arraycopy(value, 0, resizedValue, 0, vSize[0]);

            value = resizedValue;

        }

        return status;

        return native_getParameter(param.length, param, value.length, value);

    }



    /**
@@ -615,6 +604,7 @@ public class AudioEffect { 
     * array of 1 or 2 integers

     *

     * @see #getParameter(byte[], byte[])

     * In case of success, returns the number of meaningful integers in value array.

     * @hide

     */

    public int getParameter(int param, int[] value)
@@ -628,10 +618,15 @@ public class AudioEffect { 


        int status = getParameter(p, v);



        if (status == 4 || status == 8) {

            value[0] = byteArrayToInt(v);

        if (v.length > 4) {

            if (status == 8) {

                value[1] = byteArrayToInt(v, 4);

            }

            status /= 4;

        } else {

            status = ERROR;

        }

        return status;

    }
@@ -640,6 +635,7 @@ public class AudioEffect { 
     * array of 1 or 2 short integers

     *

     * @see #getParameter(byte[], byte[])

     * In case of success, returns the number of meaningful short integers in value array.

     * @hide

     */

    public int getParameter(int param, short[] value)
@@ -653,10 +649,15 @@ public class AudioEffect { 


        int status = getParameter(p, v);



        if (status == 2 || status == 4) {

            value[0] = byteArrayToShort(v);

        if (v.length > 2) {

            if (status == 4) {

                value[1] = byteArrayToShort(v, 2);

            }

            status /= 2;

        } else {

            status = ERROR;

        }

        return status;

    }
@@ -665,6 +666,7 @@ public class AudioEffect { 
     * the value is also an array of 1 or 2 integers

     *

     * @see #getParameter(byte[], byte[])

     * In case of success, the returns the number of meaningful integers in value array.

     * @hide

     */

    public int getParameter(int[] param, int[] value)
@@ -681,10 +683,15 @@ public class AudioEffect { 


        int status = getParameter(p, v);



        if (status == 4 || status == 8) {

            value[0] = byteArrayToInt(v);

        if (v.length > 4) {

            if (status == 8) {

                value[1] = byteArrayToInt(v, 4);

            }

            status /= 4;

        } else {

            status = ERROR;

        }

        return status;

    }
@@ -693,6 +700,7 @@ public class AudioEffect { 
     * the value is an array of 1 or 2 short integers

     *

     * @see #getParameter(byte[], byte[])

     * In case of success, returns the number of meaningful short integers in value array.

     * @hide

     */

    public int getParameter(int[] param, short[] value)
@@ -709,10 +717,15 @@ public class AudioEffect { 


        int status = getParameter(p, v);



        if (status == 2 || status == 4) {

            value[0] = byteArrayToShort(v);

        if (v.length > 2) {

            if (status == 4) {

                value[1] = byteArrayToShort(v, 2);

            }

            status /= 2;

        } else {

            status = ERROR;

        }

        return status;

    }
@@ -740,24 +753,14 @@ public class AudioEffect { 
    /**

     * Send a command to the effect engine. This method is intended to send

     * proprietary commands to a particular effect implementation.

     *

     * In case of success, returns the number of meaningful bytes in reply array.

     * In case of failure, the returned value is negative and implementation specific.

     * @hide

     */

    public int command(int cmdCode, byte[] command, byte[] reply)

            throws IllegalStateException {

        checkState("command()");

        int[] replySize = new int[1];

        replySize[0] = reply.length;



        int status = native_command(cmdCode, command.length, command,

                replySize, reply);



        if (reply.length > replySize[0]) {

            byte[] resizedReply = new byte[replySize[0]];

            System.arraycopy(reply, 0, resizedReply, 0, replySize[0]);

            reply = resizedReply;

        }

        return status;

        return native_command(cmdCode, command.length, command, reply.length, reply);

    }



    // --------------------------------------------------------------------------
@@ -1145,10 +1148,10 @@ public class AudioEffect { 
            int vsize, byte[] value);



    private native final int native_getParameter(int psize, byte[] param,

            int[] vsize, byte[] value);

            int vsize, byte[] value);



    private native final int native_command(int cmdCode, int cmdSize,

            byte[] cmdData, int[] repSize, byte[] repData);

            byte[] cmdData, int repSize, byte[] repData);



    private static native Object[] native_query_effects();
@@ -1172,9 +1175,8 @@ public class AudioEffect { 
     * @hide

     */

    public void checkStatus(int status) {

        if (isError(status)) {

            switch (status) {

        case AudioEffect.SUCCESS:

            break;

            case AudioEffect.ERROR_BAD_VALUE:

                throw (new IllegalArgumentException(

                        "AudioEffect: bad parameter value"));
@@ -1185,6 +1187,14 @@ public class AudioEffect { 
                throw (new RuntimeException("AudioEffect: set/get parameter error"));

            }

        }

    }



    /**

     * @hide

     */

    public static boolean isError(int status) {

        return (status < 0);

    }



    /**

     * @hide

media/jni/audioeffect/android_media_AudioEffect.cpp

+18 −37
Original line number Diff line number Diff line
@@ -570,12 +570,11 @@ setParameter_Exit: 


static jint

android_media_AudioEffect_native_getParameter(JNIEnv *env,

        jobject thiz, int psize, jbyteArray pJavaParam,

        jintArray pJavaValueSize, jbyteArray pJavaValue) {

        jobject thiz, jint psize, jbyteArray pJavaParam,

        jint vsize, jbyteArray pJavaValue) {

    // retrieve the AudioEffect object

    jbyte* lpParam = NULL;

    jbyte* lpValue = NULL;

    jbyte* lpValueSize = NULL;

    jint lStatus = AUDIOEFFECT_ERROR_BAD_VALUE;

    effect_param_t *p;

    int voffset;
@@ -589,7 +588,7 @@ android_media_AudioEffect_native_getParameter(JNIEnv *env, 
        return AUDIOEFFECT_ERROR_NO_INIT;

    }



    if (psize == 0 || pJavaValueSize == NULL || pJavaParam == NULL || pJavaValue == NULL) {

    if (psize == 0 || vsize == 0 || pJavaParam == NULL || pJavaValue == NULL) {

        return AUDIOEFFECT_ERROR_BAD_VALUE;

    }
@@ -607,26 +606,18 @@ android_media_AudioEffect_native_getParameter(JNIEnv *env, 
        goto getParameter_Exit;

    }



    // get the pointer for the value size from the java array

    lpValueSize = (jbyte *) env->GetPrimitiveArrayCritical(pJavaValueSize, NULL);

    if (lpValueSize == NULL) {

        LOGE("getParameter: Error retrieving value size pointer");

        goto getParameter_Exit;

    }



    voffset = ((psize - 1) / sizeof(int) + 1) * sizeof(int);

    p = (effect_param_t *) malloc(sizeof(effect_param_t) + voffset

            + lpValueSize[0]);

    p = (effect_param_t *) malloc(sizeof(effect_param_t) + voffset + vsize);

    memcpy(p->data, lpParam, psize);

    p->psize = psize;

    p->vsize = lpValueSize[0];

    p->vsize = vsize;



    lStatus = lpAudioEffect->getParameter(p);

    if (lStatus == NO_ERROR) {

        lStatus = p->status;

        if (lStatus == NO_ERROR) {

            memcpy(lpValue, p->data + voffset, p->vsize);

            lpValueSize[0] = p->vsize;

            vsize = p->vsize;

        }

    }
@@ -640,19 +631,18 @@ getParameter_Exit: 
    if (lpValue != NULL) {

        env->ReleasePrimitiveArrayCritical(pJavaValue, lpValue, 0);

    }

    if (lpValueSize != NULL) {

        env->ReleasePrimitiveArrayCritical(pJavaValueSize, lpValueSize, 0);

    }



    if (lStatus == NO_ERROR) {

        return vsize;

    }

    return translateError(lStatus);

}



static jint android_media_AudioEffect_native_command(JNIEnv *env, jobject thiz,

        jint cmdCode, jint cmdSize, jbyteArray jCmdData, jintArray jReplySize,

        jint cmdCode, jint cmdSize, jbyteArray jCmdData, jint replySize,

        jbyteArray jReplyData) {

    jbyte* pCmdData = NULL;

    jbyte* pReplyData = NULL;

    jint* pReplySize = NULL;

    jint lStatus = AUDIOEFFECT_ERROR_BAD_VALUE;



    // retrieve the AudioEffect object
@@ -665,7 +655,7 @@ static jint android_media_AudioEffect_native_command(JNIEnv *env, jobject thiz, 
        return AUDIOEFFECT_ERROR_NO_INIT;

    }



    if ((cmdSize != 0 && jCmdData == NULL) || (jReplySize != NULL && jReplyData == NULL)) {

    if ((cmdSize != 0 && jCmdData == NULL) || (replySize != 0 && jReplyData == NULL)) {

        return AUDIOEFFECT_ERROR_BAD_VALUE;

    }
@@ -678,17 +668,8 @@ static jint android_media_AudioEffect_native_command(JNIEnv *env, jobject thiz, 
        }

    }



    // get the pointer for the reply size from the java array

    if (jReplySize != NULL) {

        pReplySize = (jint *) env->GetPrimitiveArrayCritical(jReplySize, NULL);

        if (pReplySize == NULL) {

            LOGE("setParameter: Error retrieving reply pointer");

            goto command_Exit;

        }

    }



    // get the pointer for the reply from the java array

    if (pReplySize != NULL && pReplySize[0] != 0 && jReplyData != NULL) {

    if (replySize != 0 && jReplyData != NULL) {

        pReplyData = (jbyte *) env->GetPrimitiveArrayCritical(jReplyData, NULL);

        if (pReplyData == NULL) {

            LOGE("setParameter: Error retrieving reply pointer");
@@ -699,7 +680,7 @@ static jint android_media_AudioEffect_native_command(JNIEnv *env, jobject thiz, 
    lStatus = translateError(lpAudioEffect->command((uint32_t)cmdCode,

                                                    (uint32_t)cmdSize,

                                                    pCmdData,

                                                    (uint32_t *)pReplySize,

                                                    (uint32_t *)&replySize,

                                                    pReplyData));



command_Exit:
@@ -710,10 +691,10 @@ command_Exit: 
    if (pReplyData != NULL) {

        env->ReleasePrimitiveArrayCritical(jReplyData, pReplyData, 0);

    }

    if (pReplySize != NULL) {

        env->ReleasePrimitiveArrayCritical(jReplySize, pReplySize, 0);

    }



    if (lStatus == NO_ERROR) {

        return replySize;

    }

    return lStatus;

}
@@ -803,8 +784,8 @@ static JNINativeMethod gMethods[] = { 
    {"native_getEnabled",    "()Z",      (void *)android_media_AudioEffect_native_getEnabled},

    {"native_hasControl",    "()Z",      (void *)android_media_AudioEffect_native_hasControl},

    {"native_setParameter",  "(I[BI[B)I",  (void *)android_media_AudioEffect_native_setParameter},

    {"native_getParameter",  "(I[B[I[B)I",  (void *)android_media_AudioEffect_native_getParameter},

    {"native_command",       "(II[B[I[B)I", (void *)android_media_AudioEffect_native_command},

    {"native_getParameter",  "(I[BI[B)I",  (void *)android_media_AudioEffect_native_getParameter},

    {"native_command",       "(II[BI[B)I", (void *)android_media_AudioEffect_native_command},

    {"native_query_effects", "()[Ljava/lang/Object;", (void *)android_media_AudioEffect_native_queryEffects},

};

media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/functional/MediaAudioEffectTest.java

+11 −16
Original line number Diff line number Diff line
@@ -744,7 +744,7 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
            assertNotNull(msg + ": could not create AudioEffect", effect);

            byte[] param = intToByteArray(Equalizer.PARAM_CURRENT_PRESET);

            byte[] value = new byte[2];

            if (effect.getParameter(param, value) == AudioEffect.SUCCESS) {

            if (!AudioEffect.isError(effect.getParameter(param, value))) {

                result = true;

            }

        } catch (IllegalArgumentException e) {
@@ -777,8 +777,8 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
                                    0);

            assertNotNull(msg + ": could not create AudioEffect", effect);

            int[] value = new int[1];

            if (effect.getParameter(EnvironmentalReverb.PARAM_DECAY_TIME, value)

                    == AudioEffect.SUCCESS) {

            if (!AudioEffect.isError(

                    effect.getParameter(EnvironmentalReverb.PARAM_DECAY_TIME, value))) {

                result = true;

            }

        } catch (IllegalArgumentException e) {
@@ -811,8 +811,7 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
                                    0);

            assertNotNull(msg + ": could not create AudioEffect", effect);

            short[] value = new short[1];

            if (effect.getParameter(Equalizer.PARAM_CURRENT_PRESET, value)

                    == AudioEffect.SUCCESS) {

            if (!AudioEffect.isError(effect.getParameter(Equalizer.PARAM_CURRENT_PRESET, value))) {

                result = true;

            }

        } catch (IllegalArgumentException e) {
@@ -845,8 +844,7 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
                                    0);

            assertNotNull(msg + ": could not create AudioEffect", effect);

            byte[] value = new byte[2];

            if (effect.getParameter(Equalizer.PARAM_CURRENT_PRESET, value)

                    == AudioEffect.SUCCESS) {

            if (!AudioEffect.isError(effect.getParameter(Equalizer.PARAM_CURRENT_PRESET, value))) {

                result = true;

            }

        } catch (IllegalArgumentException e) {
@@ -881,8 +879,7 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
            int[] param = new int[1];

            int[] value = new int[1];

            param[0] = EnvironmentalReverb.PARAM_DECAY_TIME;

            if (effect.getParameter(param, value)

                    == AudioEffect.SUCCESS) {

            if (!AudioEffect.isError(effect.getParameter(param, value))) {

                result = true;

            }

        } catch (IllegalArgumentException e) {
@@ -917,8 +914,7 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
            int[] param = new int[1];

            short[] value = new short[1];

            param[0] = Equalizer.PARAM_CURRENT_PRESET;

            if (effect.getParameter(param, value)

                    == AudioEffect.SUCCESS) {

            if (!AudioEffect.isError(effect.getParameter(param, value))) {

                result = true;

            }

        } catch (IllegalArgumentException e) {
@@ -953,8 +949,7 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
            int[] param = new int[1];

            byte[] value = new byte[2];

            param[0] = Equalizer.PARAM_CURRENT_PRESET;

            if (effect.getParameter(param, value)

                    == AudioEffect.SUCCESS) {

            if (!AudioEffect.isError(effect.getParameter(param, value))) {

                result = true;

            }

        } catch (IllegalArgumentException e) {
@@ -1082,8 +1077,8 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 


            short[] value = new short[1];

            status = effect2.getParameter(Equalizer.PARAM_CURRENT_PRESET, value);

            assertEquals(msg + ": Effect2 getParameter failed",

                    AudioEffect.SUCCESS, status);

            assertFalse(msg + ": Effect2 getParameter failed",

                    AudioEffect.isError(status));

            assertEquals(msg + ": Effect1 changed parameter",

                    (short)0, value[0]);
@@ -1278,7 +1273,7 @@ public class MediaAudioEffectTest extends ActivityInstrumentationTestCase2<Media 
                byte[] cmd = new byte[0];

                byte[] reply = new byte[4];

                int status = effect.command(3, cmd, reply);

                assertEquals(msg + ": command failed", AudioEffect.SUCCESS, status);

                assertFalse(msg + ": command failed", AudioEffect.isError(status));

                assertTrue(msg + ": effect not enabled", effect.getEnabled());

                result = true;

            } catch (IllegalStateException e) {