Re: [FFmpeg-devel] [PATCH 2/4] lavc/mediacodec_wrapper: Refactor ff_AMediaCodecList_getCodecNameByType()

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

> 
> A lot shorter, nested variables and keeps going in case of exceptions.
> Makes use of C99 declarations.
> 
> This patch has been released by Epic Games' legal department.

This patch triggered multiple warning:

warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]

C99 allows declaration-after-statement, but it’s not allowed by our
coding style. -Wdeclaration-after-statement is enabled explicitly in
configure.

> ---
>  libavcodec/mediacodec_wrapper.c | 162 +++++++++-----------------------
>  1 file changed, 45 insertions(+), 117 deletions(-)
> 
> diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c
> index 34ec2134aa..82eead2833 100644
> --- a/libavcodec/mediacodec_wrapper.c
> +++ b/libavcodec/mediacodec_wrapper.c
> @@ -2,6 +2,7 @@
>   * Android MediaCodec Wrapper
>   *
>   * Copyright (c) 2015-2016 Matthieu Bouron <matthieu.bouron stupeflix.com>
> + * Modifications by Epic Games, Inc., 2023.
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -365,26 +366,13 @@ int ff_AMediaCodecProfile_getProfileFromAVCodecContext(AVCodecContext *avctx)
>  char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int encoder, void *log_ctx)
>  {
>      int ret;
> -    int i;
> -    int codec_count;
>      int found_codec = 0;
>      char *name = NULL;
> -    char *supported_type = NULL;
>  
>      JNIEnv *env = NULL;
>      struct JNIAMediaCodecListFields jfields = { 0 };
>      struct JNIAMediaFormatFields mediaformat_jfields = { 0 };
>  
> -    jobject codec_name = NULL;
> -
> -    jobject info = NULL;
> -    jobject type = NULL;
> -    jobjectArray types = NULL;
> -
> -    jobject capabilities = NULL;
> -    jobject profile_level = NULL;
> -    jobjectArray profile_levels = NULL;
> -
>      JNI_GET_ENV_OR_RETURN(env, log_ctx, NULL);
>  
>      if ((ret = ff_jni_init_jfields(env, &jfields, jni_amediacodeclist_mapping, 0, log_ctx)) < 0) {
> @@ -395,29 +383,26 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e
>          goto done;
>      }
>  
> -    codec_count = (*env)->CallStaticIntMethod(env, jfields.mediacodec_list_class, jfields.get_codec_count_id);
> +    int codec_count = (*env)->CallStaticIntMethod(env, jfields.mediacodec_list_class, jfields.get_codec_count_id);
>      if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
>          goto done;
>      }
>  
> -    for(i = 0; i < codec_count; i++) {
> -        int j;
> -        int type_count;
> -        int is_encoder;
> -
> -        info = (*env)->CallStaticObjectMethod(env, jfields.mediacodec_list_class, jfields.get_codec_info_at_id, i);
> +    for (int i = 0; i < codec_count && !found_codec; i++) {
> +        jobject info = (*env)->CallStaticObjectMethod(env, jfields.mediacodec_list_class, jfields.get_codec_info_at_id, i);
>          if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -            goto done;
> +            continue;
>          }
>  
> -        types = (*env)->CallObjectMethod(env, info, jfields.get_supported_types_id);
> +        jobject codec_name = NULL;
> +        jobjectArray types = (*env)->CallObjectMethod(env, info, jfields.get_supported_types_id);
>          if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -            goto done;
> +            goto done_with_info;
>          }
>  
> -        is_encoder = (*env)->CallBooleanMethod(env, info, jfields.is_encoder_id);
> +        int is_encoder = (*env)->CallBooleanMethod(env, info, jfields.is_encoder_id);
>          if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -            goto done;
> +            goto done_with_info;
>          }
>  
>          if (is_encoder != encoder) {
> @@ -427,7 +412,7 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e
>          if (jfields.is_software_only_id) {
>              int is_software_only = (*env)->CallBooleanMethod(env, info, jfields.is_software_only_id);
>              if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -                goto done;
> +                goto done_with_info;
>              }
>  
>              if (is_software_only) {
> @@ -437,17 +422,12 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e
>  
>          codec_name = (*env)->CallObjectMethod(env, info, jfields.get_name_id);
>          if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -            goto done;
> +            goto done_with_info;
>          }
>  
>          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;
> +            goto done_with_info;
>          }
>  
>          /* Skip software decoders */
> @@ -459,141 +439,89 @@ char *ff_AMediaCodecList_getCodecNameByType(const char *mime, int profile, int e
>              goto done_with_info;
>          }
>  
> -        type_count = (*env)->GetArrayLength(env, types);
> -        for (j = 0; j < type_count; j++) {
> -            int k;
> -            int profile_count;
> -
> -            type = (*env)->GetObjectArrayElement(env, types, j);
> +        int type_count = (*env)->GetArrayLength(env, types);
> +        for (int j = 0; j < type_count && !found_codec; j++) {
> +            jobject type = (*env)->GetObjectArrayElement(env, types, j);
>              if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -                goto done;
> +                continue;
>              }
>  
> -            supported_type = ff_jni_jstring_to_utf_chars(env, type, log_ctx);
> +            jobject capabilities = NULL;
> +            jobjectArray profile_levels = NULL;
> +            char *supported_type = ff_jni_jstring_to_utf_chars(env, type, log_ctx);
>              if (!supported_type) {
> -                goto done;
> +                goto done_with_type;
>              }
>  
> -            if (av_strcasecmp(supported_type, mime)) {
> +            int strcasecmp_ret = av_strcasecmp(supported_type, mime);
> +            av_freep(&supported_type);
> +            if (strcasecmp_ret) {
>                  goto done_with_type;
>              }
>  
>              capabilities = (*env)->CallObjectMethod(env, info, jfields.get_codec_capabilities_id, type);
>              if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -                goto done;
> +                goto done_with_type;
>              }
>  
>              profile_levels = (*env)->GetObjectField(env, capabilities, jfields.profile_levels_id);
>              if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> -                goto done;
> +                goto done_with_type;
>              }
>  
> -            profile_count = (*env)->GetArrayLength(env, profile_levels);
> -            if (!profile_count) {
> +            // match profile if desired by user (profile >= 0) and codec has non-empty profileLevels
> +            int profile_count = (*env)->GetArrayLength(env, profile_levels);
> +            if (!profile_count || profile < 0) {
>                  found_codec = 1;
> -            }
> -            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;
> +            } else {
> +                for (int k = 0; k < profile_count && !found_codec; k++) {
> +                    jobject profile_level = (*env)->GetObjectArrayElement(env, profile_levels, k);
> +                    if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
> +                        continue;
> +                    }
> +
> +                    int supported_profile = (*env)->GetIntField(env, profile_level, jfields.profile_id);
> +                    if (ff_jni_exception_check(env, 1, log_ctx) >= 0) {
> +                        found_codec = profile == supported_profile;
> +                    }
>  
> -                if (profile_level) {
>                      (*env)->DeleteLocalRef(env, profile_level);
> -                    profile_level = NULL;
> -                }
> -
> -                if (found_codec) {
> -                    break;
>                  }
>              }
>  
>  done_with_type:
>              if (profile_levels) {
>                  (*env)->DeleteLocalRef(env, profile_levels);
> -                profile_levels = NULL;
>              }
>  
>              if (capabilities) {
>                  (*env)->DeleteLocalRef(env, capabilities);
> -                capabilities = NULL;
>              }
>  
>              if (type) {
>                  (*env)->DeleteLocalRef(env, type);
> -                type = NULL;
> -            }
> -
> -            av_freep(&supported_type);
> -
> -            if (found_codec) {
> -                break;
>              }
>          }
>  
>  done_with_info:
>          if (info) {
>              (*env)->DeleteLocalRef(env, info);
> -            info = NULL;
>          }
>  
>          if (types) {
>              (*env)->DeleteLocalRef(env, types);
> -            types = NULL;
>          }
>  
> -        if (found_codec) {
> -            break;
> +        if (codec_name) {
> +            (*env)->DeleteLocalRef(env, codec_name);
>          }
>  
> -        av_freep(&name);
> +        if (!found_codec) {
> +            av_freep(&name);
> +        }
>      }
>  
>  done:
> -    if (codec_name) {
> -        (*env)->DeleteLocalRef(env, codec_name);
> -    }
> -
> -    if (info) {
> -        (*env)->DeleteLocalRef(env, info);
> -    }
> -
> -    if (type) {
> -        (*env)->DeleteLocalRef(env, type);
> -    }
> -
> -    if (types) {
> -        (*env)->DeleteLocalRef(env, types);
> -    }
> -
> -    if (capabilities) {
> -        (*env)->DeleteLocalRef(env, capabilities);
> -    }
> -
> -    if (profile_level) {
> -        (*env)->DeleteLocalRef(env, profile_level);
> -    }
> -
> -    if (profile_levels) {
> -        (*env)->DeleteLocalRef(env, profile_levels);
> -    }
> -
> -    av_freep(&supported_type);
> -
>      ff_jni_reset_jfields(env, &jfields, jni_amediacodeclist_mapping, 0, log_ctx);
>      ff_jni_reset_jfields(env, &mediaformat_jfields, jni_amediaformat_mapping, 0, log_ctx);
>  
> -- 
> 2.30.2
> 
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".