diff mbox series

[FFmpeg-devel,1/2] avcodec/mediacodec_wrapper: clean up ff_AMediaCodecList_getCodecNameByType a bit

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

Checks

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

Commit Message

sfan5 Feb. 17, 2021, 3:50 p.m. UTC 
Hi,

while looking into mediacodec for unrelated reasons I saw some room for 
improvement.

Therefore, here's a series with two small patches.
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(-)

Comments

Matthieu Bouron March 8, 2021, 9:11 a.m. UTC | #1 
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,
Matthieu Bouron March 10, 2021, 1:24 p.m. UTC | #2 
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 mbox series

Patch

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: