[vlc-devel] [PATCH] MediaCodec: simplify jni thread attach/detach

[Rémi Denis-Courmont]

On Tue, 25 Feb 2014 17:02:13 +0800, Zhang Rui <bbcallen at gmail.com> wrote:
> Avoid repeatly AttachCurrentThread()/DetachCurrentThread()
> ---
>  modules/codec/omxil/android_mediacodec.c | 57
>  +++++++++++++++++++++++++-------
>  1 file changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/modules/codec/omxil/android_mediacodec.c
> b/modules/codec/omxil/android_mediacodec.c
> index 8fa68fa..1cb711c 100644
> --- a/modules/codec/omxil/android_mediacodec.c
> +++ b/modules/codec/omxil/android_mediacodec.c
> @@ -210,6 +210,47 @@ static int jstrcmp(JNIEnv* env, jobject str, const
> char* str2)
>      return ret;
>  }
>  
> +static pthread_key_t g_thread_key;
> +static pthread_once_t g_key_once = PTHREAD_ONCE_INIT;
> +
> +static void vlcjni_thread_destroy(void* value)
> +{
> +    JNIEnv *env = (JNIEnv*) value;
> +    if (env != NULL) {

Tautology, I believe.

> +        (*myVm)->DetachCurrentThread(myVm);
> +        pthread_setspecific(g_thread_key, NULL);
> +    }
> +}
> +
> +static void vlcjni_make_thread_key()

Missing void.

> +{
> +    pthread_key_create(&g_thread_key, vlcjni_thread_destroy);
> +}

This may be theoretical so far. But technically that is a leak and a
potentially dangling clean-up handler callback.

> +
> +static jint vlcjni_setup_thread_env(JNIEnv **p_env)
> +{
> +    JavaVM *jvm = myVm;
> +    if (!jvm)
> +        return -1;
> +
> +    pthread_once(&g_key_once, vlcjni_make_thread_key);
> +
> +    JNIEnv *env = (JNIEnv*) pthread_getspecific(g_thread_key);
> +    if (env) {
> +        *p_env = env;
> +        return 0;
> +    }
> +
> +    static JavaVMAttachArgs vlc_thr_args = {JNI_VERSION_1_2,
> "MediaCodec", NULL};

Not reentrant.

> +    if ((*jvm)->AttachCurrentThread(jvm, &env, &vlc_thr_args) ==
JNI_OK) {
> +        pthread_setspecific(g_thread_key, env);
> +        *p_env = env;
> +        return 0;
> +    }
> +
> +    return -1;
> +}
> +
> 
/*****************************************************************************
>   * OpenDecoder: Create the decoder instance
>  
*****************************************************************************/

-- 
Rémi Denis-Courmont
Sent from my collocated server