<div dir="ltr"><div><div><div>LGTM, but I agree with Edward regarding naming.</div></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-02-10 18:41 GMT+01:00 <span dir="ltr"><<a href="mailto:edward.c.wang@compdigitec.com" target="_blank">edward.c.wang@compdigitec.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
<br>
Zhang Rui <<a href="mailto:bbcallen@gmail.com">bbcallen@gmail.com</a>> a écrit :<br>
<div><div class="h5">>Avoid repeatly AttachCurrentThread()/DetachCurrentThread()<br>
>---<br>
>modules/codec/omxil/android_mediacodec.c | 57<br>
>+++++++++++++++++++++++++-------<br>
> 1 file changed, 45 insertions(+), 12 deletions(-)<br>
><br>
>diff --git a/modules/codec/omxil/android_mediacodec.c<br>
>b/modules/codec/omxil/android_mediacodec.c<br>
>index bf53e76..59846cc 100644<br>
>--- a/modules/codec/omxil/android_mediacodec.c<br>
>+++ b/modules/codec/omxil/android_mediacodec.c<br>
>@@ -201,6 +201,47 @@ static int jstrcmp(JNIEnv* env, jobject str, const<br>
>char* str2)<br>
> return ret;<br>
> }<br>
><br>
>+static pthread_key_t g_thread_key;<br>
>+static pthread_once_t g_key_once = PTHREAD_ONCE_INIT;<br>
>+static JavaVMAttachArgs g_vlc_thr_args = {JNI_VERSION_1_2,<br>
>"MediaCodec", NULL};<br>
>+<br>
>+static void VlcJniThreadDestroy(void* value)<br>
>+{<br>
>+ JNIEnv *env = (JNIEnv*) value;<br>
>+ if (env != NULL) {<br>
>+ (*myVm)->DetachCurrentThread(myVm);<br>
>+ pthread_setspecific(g_thread_key, NULL);<br>
>+ }<br>
>+}<br>
>+<br>
>+static void VlcMakeThreadKey()<br>
>+{<br>
>+ pthread_key_create(&g_thread_key, VlcJniThreadDestroy);<br>
>+}<br>
>+<br>
>+jint VlcSetupThreadEnv(JNIEnv **p_env)<br>
>+{<br>
>+ JavaVM *jvm = myVm;<br>
>+ if (!jvm)<br>
>+ return -1;<br>
>+<br>
>+ pthread_once(&g_key_once, VlcMakeThreadKey);<br>
>+<br>
>+ JNIEnv *env = (JNIEnv*) pthread_getspecific(g_thread_key);<br>
>+ if (env) {<br>
>+ *p_env = env;<br>
>+ return 0;<br>
>+ }<br>
>+<br>
>+ if ((*jvm)->AttachCurrentThread(jvm, &env, &g_vlc_thr_args) ==<br>
>JNI_OK) {<br>
>+ pthread_setspecific(g_thread_key, env);<br>
>+ *p_env = env;<br>
>+ return 0;<br>
>+ }<br>
>+<br>
>+ return -1;<br>
>+}<br>
>+<br>
>/*****************************************************************************<br>
> * OpenDecoder: Create the decoder instance<br>
>*****************************************************************************/<br>
>@@ -246,7 +287,7 @@ static int OpenDecoder(vlc_object_t *p_this)<br>
> p_dec->b_need_packetized = true;<br>
><br>
> JNIEnv* env = NULL;<br>
>- (*myVm)->AttachCurrentThread(myVm, &env, NULL);<br>
>+ VlcSetupThreadEnv(&env);<br>
><br>
> for (int i = 0; classes[i].name; i++) {<br>
> *(jclass*)((uint8_t*)p_sys + classes[i].offset) =<br>
>@@ -413,11 +454,9 @@ static int OpenDecoder(vlc_object_t *p_this)<br>
> goto error;<br>
> (*env)->DeleteLocalRef(env, format);<br>
><br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
> return VLC_SUCCESS;<br>
><br>
> error:<br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
> CloseDecoder(p_this);<br>
> return VLC_EGENERIC;<br>
> }<br>
>@@ -435,7 +474,7 @@ static void CloseDecoder(vlc_object_t *p_this)<br>
> * to prevent the vout from using destroyed output buffers. */<br>
> if (p_sys->direct_rendering)<br>
> InvalidateAllPictures(p_dec);<br>
>- (*myVm)->AttachCurrentThread(myVm, &env, NULL);<br>
>+ VlcSetupThreadEnv(&env);<br>
> if (p_sys->input_buffers)<br>
> (*env)->DeleteGlobalRef(env, p_sys->input_buffers);<br>
> if (p_sys->output_buffers)<br>
>@@ -448,7 +487,6 @@ static void CloseDecoder(vlc_object_t *p_this)<br>
> }<br>
> if (p_sys->buffer_info)<br>
> (*env)->DeleteGlobalRef(env, p_sys->buffer_info);<br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
><br>
> free(p_sys->name);<br>
>ArchitectureSpecificCopyHooksDestroy(p_sys->pixel_format,<br>
>&p_sys->architecture_specific_data);<br>
>@@ -481,14 +519,13 @@ static void DisplayBuffer(picture_sys_t*<br>
>p_picsys, bool b_render)<br>
><br>
> /* Release the MediaCodec buffer. */<br>
> JNIEnv *env = NULL;<br>
>- (*myVm)->AttachCurrentThread(myVm, &env, NULL);<br>
>+ VlcSetupThreadEnv(&env);<br>
>(*env)->CallVoidMethod(env, p_sys->codec, p_sys->release_output_buffer,<br>
>i_index, b_render);<br>
> if ((*env)->ExceptionOccurred(env)) {<br>
>msg_Err(p_dec, "Exception in MediaCodec.releaseOutputBuffer<br>
>(DisplayBuffer)");<br>
> (*env)->ExceptionClear(env);<br>
> }<br>
><br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
> p_picsys->b_valid = false;<br>
><br>
> vlc_mutex_unlock(get_android_opaque_mutex());<br>
>@@ -686,7 +723,7 @@ static picture_t *DecodeVideo(decoder_t *p_dec,<br>
>block_t **pp_block)<br>
><br>
> block_t *p_block = *pp_block;<br>
><br>
>- (*myVm)->AttachCurrentThread(myVm, &env, NULL);<br>
>+ VlcSetupThreadEnv(&env);<br>
><br>
>if (p_block->i_flags & (BLOCK_FLAG_DISCONTINUITY|BLOCK_FLAG_CORRUPTED))<br>
>{<br>
> block_Release(p_block);<br>
>@@ -704,7 +741,6 @@ static picture_t *DecodeVideo(decoder_t *p_dec,<br>
>block_t **pp_block)<br>
> }<br>
> }<br>
> p_sys->decoded = false;<br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
> return NULL;<br>
> }<br>
><br>
>@@ -730,7 +766,6 @@ static picture_t *DecodeVideo(decoder_t *p_dec,<br>
>block_t **pp_block)<br>
> * without assigning NULL to *pp_block. The next call<br>
> * to DecodeVideo will try to send the input packet again.<br>
> */<br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
> return p_pic;<br>
> }<br>
> timeout = 30*1000;<br>
>@@ -756,7 +791,6 @@ static picture_t *DecodeVideo(decoder_t *p_dec,<br>
>block_t **pp_block)<br>
> block_Release(p_block);<br>
> *pp_block = NULL;<br>
> }<br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
> return invalid_picture;<br>
> }<br>
> continue;<br>
>@@ -780,7 +814,6 @@ static picture_t *DecodeVideo(decoder_t *p_dec,<br>
>block_t **pp_block)<br>
> }<br>
> if (!p_pic)<br>
> GetOutput(p_dec, env, &p_pic);<br>
>- (*myVm)->DetachCurrentThread(myVm);<br>
><br>
> block_Release(p_block);<br>
> *pp_block = NULL;<br>
<br>
<br>
</div></div>I would prefer the style vlcjni_thread_destroy over VlcJniThreadDestroy, but otherwise it looks OK.<br>
<br>
<br>
<br>
Regards,<br>
Edward Wang<br>
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
Android mailing list<br>
<a href="mailto:Android@videolan.org">Android@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/android" target="_blank">https://mailman.videolan.org/listinfo/android</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Félix Abecassis<div><a href="http://felix.abecassis.me" target="_blank">http://felix.abecassis.me</a></div>
</div>