Message ID | DB6PR0901MB1495E7046EBABBA3046E793BEC869@DB6PR0901MB1495.eurprd09.prod.outlook.com |
---|---|
State | Accepted |
Commit | 1a033008e8f53af075d11974e0e7de23d11b34e2 |
Headers | show |
Series | [FFmpeg-devel,1/2] avcodec/mediacodec_wrapper: clean up ff_AMediaCodecList_getCodecNameByType a bit | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
On Wed, Feb 17, 2021 at 04:50:00PM +0100, sfan5 wrote: > Hi, > > while looking into mediacodec for unrelated reasons I saw some room for > improvement. > > Therefore, here's a series with two small patches. > > From cadd2b2d4a5ffbb4dcc34faf2d3e139e1d4d608b Mon Sep 17 00:00:00 2001 > From: sfan5 <sfan5@live.de> > Date: Thu, 11 Feb 2021 19:23:26 +0100 > Subject: [PATCH 1/2] avcodec/mediacodec_wrapper: clean up > ff_AMediaCodecList_getCodecNameByType a bit > > We can skip software decoders before even looking at the mime types. > --- > libavcodec/mediacodec_wrapper.c | 113 ++++++++++++++++---------------- > 1 file changed, 57 insertions(+), 56 deletions(-) > > diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c > index 79abc8b6aa..f1945bcfc0 100644 > --- a/libavcodec/mediacodec_wrapper.c > +++ b/libavcodec/mediacodec_wrapper.c > @@ -441,6 +441,30 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e > goto done_with_info; > } > > + codec_name = (*env)->CallObjectMethod(env, info, jfields.get_name_id); > + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > + goto done; > + } > + > + name = ff_jni_jstring_to_utf_chars(env, codec_name, log_ctx); > + if (!name) { > + goto done; > + } > + > + if (codec_name) { > + (*env)->DeleteLocalRef(env, codec_name); > + codec_name = NULL; > + } > + > + /* Skip software decoders */ > + if ( > + strstr(name, "OMX.google") || > + strstr(name, "OMX.ffmpeg") || > + (strstr(name, "OMX.SEC") && strstr(name, ".sw.")) || > + !strcmp(name, "OMX.qcom.video.decoder.hevcswvdec")) { > + goto done_with_info; > + } > + > type_count = (*env)->GetArrayLength(env, types); > for (j = 0; j < type_count; j++) { > int k; > @@ -456,74 +480,51 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e > goto done; > } > > - if (!av_strcasecmp(supported_type, mime)) { > - codec_name = (*env)->CallObjectMethod(env, info, jfields.get_name_id); > - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > - goto done; > - } > + if (av_strcasecmp(supported_type, mime)) { > + goto done_with_type; > + } > > - name = ff_jni_jstring_to_utf_chars(env, codec_name, log_ctx); > - if (!name) { > - goto done; > - } > + capabilities = (*env)->CallObjectMethod(env, info, jfields.get_codec_capabilities_id, type); > + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > + goto done; > + } > > - if (codec_name) { > - (*env)->DeleteLocalRef(env, codec_name); > - codec_name = NULL; > - } > + profile_levels = (*env)->GetObjectField(env, capabilities, jfields.profile_levels_id); > + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > + goto done; > + } > > - /* Skip software decoders */ > - if ( > - strstr(name, "OMX.google") || > - strstr(name, "OMX.ffmpeg") || > - (strstr(name, "OMX.SEC") && strstr(name, ".sw.")) || > - !strcmp(name, "OMX.qcom.video.decoder.hevcswvdec")) { > - av_freep(&name); > - goto done_with_type; > + profile_count = (*env)->GetArrayLength(env, profile_levels); > + if (!profile_count) { > + found_codec = 1; > + } > + for (k = 0; k < profile_count; k++) { > + int supported_profile = 0; > + > + if (profile < 0) { > + found_codec = 1; > + break; > } > > - capabilities = (*env)->CallObjectMethod(env, info, jfields.get_codec_capabilities_id, type); > + profile_level = (*env)->GetObjectArrayElement(env, profile_levels, k); > if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > goto done; > } > > - profile_levels = (*env)->GetObjectField(env, capabilities, jfields.profile_levels_id); > + supported_profile = (*env)->GetIntField(env, profile_level, jfields.profile_id); > if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > goto done; > } > > - profile_count = (*env)->GetArrayLength(env, profile_levels); > - if (!profile_count) { > - found_codec = 1; > + found_codec = profile == supported_profile; > + > + if (profile_level) { > + (*env)->DeleteLocalRef(env, profile_level); > + profile_level = NULL; > } > - for (k = 0; k < profile_count; k++) { > - int supported_profile = 0; > - > - if (profile < 0) { > - found_codec = 1; > - break; > - } > - > - profile_level = (*env)->GetObjectArrayElement(env, profile_levels, k); > - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > - goto done; > - } > - > - supported_profile = (*env)->GetIntField(env, profile_level, jfields.profile_id); > - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > - goto done; > - } > - > - found_codec = profile == supported_profile; > - > - if (profile_level) { > - (*env)->DeleteLocalRef(env, profile_level); > - profile_level = NULL; > - } > - > - if (found_codec) { > - break; > - } > + > + if (found_codec) { > + break; > } > } > > @@ -548,8 +549,6 @@ done_with_type: > if (found_codec) { > break; > } > - > - av_freep(&name); > } > > done_with_info: > @@ -566,6 +565,8 @@ done_with_info: > if (found_codec) { > break; > } > + > + av_freep(&name); > } > > done: > -- > 2.30.1 > LGTM, I'll push the patch in two days if there is no objection. Thanks,
On Mon, Mar 08, 2021 at 10:11:58AM +0100, Matthieu Bouron wrote: > On Wed, Feb 17, 2021 at 04:50:00PM +0100, sfan5 wrote: > > Hi, > > > > while looking into mediacodec for unrelated reasons I saw some room for > > improvement. > > > > Therefore, here's a series with two small patches. > > > > > From cadd2b2d4a5ffbb4dcc34faf2d3e139e1d4d608b Mon Sep 17 00:00:00 2001 > > From: sfan5 <sfan5@live.de> > > Date: Thu, 11 Feb 2021 19:23:26 +0100 > > Subject: [PATCH 1/2] avcodec/mediacodec_wrapper: clean up > > ff_AMediaCodecList_getCodecNameByType a bit > > > > We can skip software decoders before even looking at the mime types. > > --- > > libavcodec/mediacodec_wrapper.c | 113 ++++++++++++++++---------------- > > 1 file changed, 57 insertions(+), 56 deletions(-) > > > > diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c > > index 79abc8b6aa..f1945bcfc0 100644 > > --- a/libavcodec/mediacodec_wrapper.c > > +++ b/libavcodec/mediacodec_wrapper.c > > @@ -441,6 +441,30 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e > > goto done_with_info; > > } > > > > + codec_name = (*env)->CallObjectMethod(env, info, jfields.get_name_id); > > + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > + goto done; > > + } > > + > > + name = ff_jni_jstring_to_utf_chars(env, codec_name, log_ctx); > > + if (!name) { > > + goto done; > > + } > > + > > + if (codec_name) { > > + (*env)->DeleteLocalRef(env, codec_name); > > + codec_name = NULL; > > + } > > + > > + /* Skip software decoders */ > > + if ( > > + strstr(name, "OMX.google") || > > + strstr(name, "OMX.ffmpeg") || > > + (strstr(name, "OMX.SEC") && strstr(name, ".sw.")) || > > + !strcmp(name, "OMX.qcom.video.decoder.hevcswvdec")) { > > + goto done_with_info; > > + } > > + > > type_count = (*env)->GetArrayLength(env, types); > > for (j = 0; j < type_count; j++) { > > int k; > > @@ -456,74 +480,51 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e > > goto done; > > } > > > > - if (!av_strcasecmp(supported_type, mime)) { > > - codec_name = (*env)->CallObjectMethod(env, info, jfields.get_name_id); > > - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > - goto done; > > - } > > + if (av_strcasecmp(supported_type, mime)) { > > + goto done_with_type; > > + } > > > > - name = ff_jni_jstring_to_utf_chars(env, codec_name, log_ctx); > > - if (!name) { > > - goto done; > > - } > > + capabilities = (*env)->CallObjectMethod(env, info, jfields.get_codec_capabilities_id, type); > > + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > + goto done; > > + } > > > > - if (codec_name) { > > - (*env)->DeleteLocalRef(env, codec_name); > > - codec_name = NULL; > > - } > > + profile_levels = (*env)->GetObjectField(env, capabilities, jfields.profile_levels_id); > > + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > + goto done; > > + } > > > > - /* Skip software decoders */ > > - if ( > > - strstr(name, "OMX.google") || > > - strstr(name, "OMX.ffmpeg") || > > - (strstr(name, "OMX.SEC") && strstr(name, ".sw.")) || > > - !strcmp(name, "OMX.qcom.video.decoder.hevcswvdec")) { > > - av_freep(&name); > > - goto done_with_type; > > + profile_count = (*env)->GetArrayLength(env, profile_levels); > > + if (!profile_count) { > > + found_codec = 1; > > + } > > + for (k = 0; k < profile_count; k++) { > > + int supported_profile = 0; > > + > > + if (profile < 0) { > > + found_codec = 1; > > + break; > > } > > > > - capabilities = (*env)->CallObjectMethod(env, info, jfields.get_codec_capabilities_id, type); > > + profile_level = (*env)->GetObjectArrayElement(env, profile_levels, k); > > if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > goto done; > > } > > > > - profile_levels = (*env)->GetObjectField(env, capabilities, jfields.profile_levels_id); > > + supported_profile = (*env)->GetIntField(env, profile_level, jfields.profile_id); > > if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > goto done; > > } > > > > - profile_count = (*env)->GetArrayLength(env, profile_levels); > > - if (!profile_count) { > > - found_codec = 1; > > + found_codec = profile == supported_profile; > > + > > + if (profile_level) { > > + (*env)->DeleteLocalRef(env, profile_level); > > + profile_level = NULL; > > } > > - for (k = 0; k < profile_count; k++) { > > - int supported_profile = 0; > > - > > - if (profile < 0) { > > - found_codec = 1; > > - break; > > - } > > - > > - profile_level = (*env)->GetObjectArrayElement(env, profile_levels, k); > > - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > - goto done; > > - } > > - > > - supported_profile = (*env)->GetIntField(env, profile_level, jfields.profile_id); > > - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { > > - goto done; > > - } > > - > > - found_codec = profile == supported_profile; > > - > > - if (profile_level) { > > - (*env)->DeleteLocalRef(env, profile_level); > > - profile_level = NULL; > > - } > > - > > - if (found_codec) { > > - break; > > - } > > + > > + if (found_codec) { > > + break; > > } > > } > > > > @@ -548,8 +549,6 @@ done_with_type: > > if (found_codec) { > > break; > > } > > - > > - av_freep(&name); > > } > > > > done_with_info: > > @@ -566,6 +565,8 @@ done_with_info: > > if (found_codec) { > > break; > > } > > + > > + av_freep(&name); > > } > > > > done: > > -- > > 2.30.1 > > > > LGTM, I'll push the patch in two days if there is no objection. > Thanks, Reworded to "avcodec/mediacodec_wrapper: check if codec is software earlier" and applied as 1a033008e8f53af075d11974e0e7de23d11b34e2. Thanks.
diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c index 79abc8b6aa..f1945bcfc0 100644 --- a/libavcodec/mediacodec_wrapper.c +++ b/libavcodec/mediacodec_wrapper.c @@ -441,6 +441,30 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e goto done_with_info; } + codec_name = (*env)->CallObjectMethod(env, info, jfields.get_name_id); + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { + goto done; + } + + name = ff_jni_jstring_to_utf_chars(env, codec_name, log_ctx); + if (!name) { + goto done; + } + + if (codec_name) { + (*env)->DeleteLocalRef(env, codec_name); + codec_name = NULL; + } + + /* Skip software decoders */ + if ( + strstr(name, "OMX.google") || + strstr(name, "OMX.ffmpeg") || + (strstr(name, "OMX.SEC") && strstr(name, ".sw.")) || + !strcmp(name, "OMX.qcom.video.decoder.hevcswvdec")) { + goto done_with_info; + } + type_count = (*env)->GetArrayLength(env, types); for (j = 0; j < type_count; j++) { int k; @@ -456,74 +480,51 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e goto done; } - if (!av_strcasecmp(supported_type, mime)) { - codec_name = (*env)->CallObjectMethod(env, info, jfields.get_name_id); - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { - goto done; - } + if (av_strcasecmp(supported_type, mime)) { + goto done_with_type; + } - name = ff_jni_jstring_to_utf_chars(env, codec_name, log_ctx); - if (!name) { - goto done; - } + capabilities = (*env)->CallObjectMethod(env, info, jfields.get_codec_capabilities_id, type); + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { + goto done; + } - if (codec_name) { - (*env)->DeleteLocalRef(env, codec_name); - codec_name = NULL; - } + profile_levels = (*env)->GetObjectField(env, capabilities, jfields.profile_levels_id); + if (ff_jni_exception_check(env, 1, log_ctx) < 0) { + goto done; + } - /* Skip software decoders */ - if ( - strstr(name, "OMX.google") || - strstr(name, "OMX.ffmpeg") || - (strstr(name, "OMX.SEC") && strstr(name, ".sw.")) || - !strcmp(name, "OMX.qcom.video.decoder.hevcswvdec")) { - av_freep(&name); - goto done_with_type; + profile_count = (*env)->GetArrayLength(env, profile_levels); + if (!profile_count) { + found_codec = 1; + } + for (k = 0; k < profile_count; k++) { + int supported_profile = 0; + + if (profile < 0) { + found_codec = 1; + break; } - capabilities = (*env)->CallObjectMethod(env, info, jfields.get_codec_capabilities_id, type); + profile_level = (*env)->GetObjectArrayElement(env, profile_levels, k); if (ff_jni_exception_check(env, 1, log_ctx) < 0) { goto done; } - profile_levels = (*env)->GetObjectField(env, capabilities, jfields.profile_levels_id); + supported_profile = (*env)->GetIntField(env, profile_level, jfields.profile_id); if (ff_jni_exception_check(env, 1, log_ctx) < 0) { goto done; } - profile_count = (*env)->GetArrayLength(env, profile_levels); - if (!profile_count) { - found_codec = 1; + found_codec = profile == supported_profile; + + if (profile_level) { + (*env)->DeleteLocalRef(env, profile_level); + profile_level = NULL; } - for (k = 0; k < profile_count; k++) { - int supported_profile = 0; - - if (profile < 0) { - found_codec = 1; - break; - } - - profile_level = (*env)->GetObjectArrayElement(env, profile_levels, k); - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { - goto done; - } - - supported_profile = (*env)->GetIntField(env, profile_level, jfields.profile_id); - if (ff_jni_exception_check(env, 1, log_ctx) < 0) { - goto done; - } - - found_codec = profile == supported_profile; - - if (profile_level) { - (*env)->DeleteLocalRef(env, profile_level); - profile_level = NULL; - } - - if (found_codec) { - break; - } + + if (found_codec) { + break; } } @@ -548,8 +549,6 @@ done_with_type: if (found_codec) { break; } - - av_freep(&name); } done_with_info: @@ -566,6 +565,8 @@ done_with_info: if (found_codec) { break; } + + av_freep(&name); } done: