| Message ID | 20171214100914.10743-1-matthieu.bouron@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, 14 Dec 2017 11:09:13 +0100 Matthieu Bouron <matthieu.bouron@gmail.com> wrote: > --- > libavcodec/mediacodec_wrapper.c | 262 +++++++++++----------------------------- > 1 file changed, 70 insertions(+), 192 deletions(-) > > diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c > index f34450a6d8..4660e895ca 100644 > --- a/libavcodec/mediacodec_wrapper.c > +++ b/libavcodec/mediacodec_wrapper.c > @@ -1132,200 +1132,78 @@ fail: > +#define DECLARE_FF_AMEDIACODEC_CREATE_FUNC(func, jfunc) \ > +FFAMediaCodec* ff_AMediaCodec_##func(const char *arg) \ > +{ \ > + int ret = -1; \ > + JNIEnv *env = NULL; \ > + FFAMediaCodec *codec = NULL; \ > + jstring jarg = NULL; \ > + jobject object = NULL; \ > + \ > + codec = av_mallocz(sizeof(FFAMediaCodec)); \ > + if (!codec) { \ > + return NULL; \ > + } \ > + codec->class = &amediacodec_class; \ > + \ > + env = ff_jni_get_env(codec); \ > + if (!env) { \ > + av_freep(&codec); \ > + return NULL; \ > + } \ > + \ > + if (ff_jni_init_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec) < 0) { \ > + goto fail; \ > + } \ > + \ > + jarg = ff_jni_utf_chars_to_jstring(env, arg, codec); \ > + if (!jarg) { \ > + goto fail; \ > + } \ > + \ > + object = (*env)->CallStaticObjectMethod(env, \ > + codec->jfields.mediacodec_class, \ > + codec->jfields.jfunc, \ > + jarg); \ > + if (ff_jni_exception_check(env, 1, codec) < 0) { \ > + goto fail; \ > + } \ > + \ > + codec->object = (*env)->NewGlobalRef(env, object); \ > + if (!codec->object) { \ > + goto fail; \ > + } \ > + \ > + if (codec_init_static_fields(codec) < 0) { \ > + goto fail; \ > + } \ > + \ > + if (codec->jfields.get_input_buffer_id && codec->jfields.get_output_buffer_id) { \ > + codec->has_get_i_o_buffer = 1; \ > + } \ > + \ > + ret = 0; \ > +fail: \ > + if (jarg) { \ > + (*env)->DeleteLocalRef(env, jarg); \ > + } \ > + \ > + if (object) { \ > + (*env)->DeleteLocalRef(env, object); \ > + } \ > + \ > + if (ret < 0) { \ > + ff_jni_reset_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec); \ > + av_freep(&codec); \ > + } \ > + \ > + return codec; \ > } > Such long macros are very ugly. And the only macro argument that is used in the body is jfunc. Would it be possible to move most of the macro into a proper function, with jfunc as pointer argument or so?
On Thu, Dec 14, 2017 at 12:21:28PM +0100, wm4 wrote: > On Thu, 14 Dec 2017 11:09:13 +0100 > Matthieu Bouron <matthieu.bouron@gmail.com> wrote: > > > --- > > libavcodec/mediacodec_wrapper.c | 262 +++++++++++----------------------------- > > 1 file changed, 70 insertions(+), 192 deletions(-) > > > > diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c > > index f34450a6d8..4660e895ca 100644 > > --- a/libavcodec/mediacodec_wrapper.c > > +++ b/libavcodec/mediacodec_wrapper.c > > @@ -1132,200 +1132,78 @@ fail: > > > > +#define DECLARE_FF_AMEDIACODEC_CREATE_FUNC(func, jfunc) \ > > +FFAMediaCodec* ff_AMediaCodec_##func(const char *arg) \ > > +{ \ > > + int ret = -1; \ > > + JNIEnv *env = NULL; \ > > + FFAMediaCodec *codec = NULL; \ > > + jstring jarg = NULL; \ > > + jobject object = NULL; \ > > + \ > > + codec = av_mallocz(sizeof(FFAMediaCodec)); \ > > + if (!codec) { \ > > + return NULL; \ > > + } \ > > + codec->class = &amediacodec_class; \ > > + \ > > + env = ff_jni_get_env(codec); \ > > + if (!env) { \ > > + av_freep(&codec); \ > > + return NULL; \ > > + } \ > > + \ > > + if (ff_jni_init_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec) < 0) { \ > > + goto fail; \ > > + } \ > > + \ > > + jarg = ff_jni_utf_chars_to_jstring(env, arg, codec); \ > > + if (!jarg) { \ > > + goto fail; \ > > + } \ > > + \ > > + object = (*env)->CallStaticObjectMethod(env, \ > > + codec->jfields.mediacodec_class, \ > > + codec->jfields.jfunc, \ > > + jarg); \ > > + if (ff_jni_exception_check(env, 1, codec) < 0) { \ > > + goto fail; \ > > + } \ > > + \ > > + codec->object = (*env)->NewGlobalRef(env, object); \ > > + if (!codec->object) { \ > > + goto fail; \ > > + } \ > > + \ > > + if (codec_init_static_fields(codec) < 0) { \ > > + goto fail; \ > > + } \ > > + \ > > + if (codec->jfields.get_input_buffer_id && codec->jfields.get_output_buffer_id) { \ > > + codec->has_get_i_o_buffer = 1; \ > > + } \ > > + \ > > + ret = 0; \ > > +fail: \ > > + if (jarg) { \ > > + (*env)->DeleteLocalRef(env, jarg); \ > > + } \ > > + \ > > + if (object) { \ > > + (*env)->DeleteLocalRef(env, object); \ > > + } \ > > + \ > > + if (ret < 0) { \ > > + ff_jni_reset_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec); \ > > + av_freep(&codec); \ > > + } \ > > + \ > > + return codec; \ > > } > > > > Such long macros are very ugly. And the only macro argument that is > used in the body is jfunc. Would it be possible to move most of the > macro into a proper function, with jfunc as pointer argument or so? I agree with you that such macros are ugly. I used this approach as the jfunc value cannot be known before ff_jni_init_jfields is called. I will try another approach and send a new patch. Thanks,
On Thu, 14 Dec 2017 12:53:38 +0100 Matthieu Bouron <matthieu.bouron@gmail.com> wrote: > On Thu, Dec 14, 2017 at 12:21:28PM +0100, wm4 wrote: > > On Thu, 14 Dec 2017 11:09:13 +0100 > > Matthieu Bouron <matthieu.bouron@gmail.com> wrote: > > > > > --- > > > libavcodec/mediacodec_wrapper.c | 262 +++++++++++----------------------------- > > > 1 file changed, 70 insertions(+), 192 deletions(-) > > > > > > diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c > > > index f34450a6d8..4660e895ca 100644 > > > --- a/libavcodec/mediacodec_wrapper.c > > > +++ b/libavcodec/mediacodec_wrapper.c > > > @@ -1132,200 +1132,78 @@ fail: > > > > > > > +#define DECLARE_FF_AMEDIACODEC_CREATE_FUNC(func, jfunc) \ > > > +FFAMediaCodec* ff_AMediaCodec_##func(const char *arg) \ > > > +{ \ > > > + int ret = -1; \ > > > + JNIEnv *env = NULL; \ > > > + FFAMediaCodec *codec = NULL; \ > > > + jstring jarg = NULL; \ > > > + jobject object = NULL; \ > > > + \ > > > + codec = av_mallocz(sizeof(FFAMediaCodec)); \ > > > + if (!codec) { \ > > > + return NULL; \ > > > + } \ > > > + codec->class = &amediacodec_class; \ > > > + \ > > > + env = ff_jni_get_env(codec); \ > > > + if (!env) { \ > > > + av_freep(&codec); \ > > > + return NULL; \ > > > + } \ > > > + \ > > > + if (ff_jni_init_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec) < 0) { \ > > > + goto fail; \ > > > + } \ > > > + \ > > > + jarg = ff_jni_utf_chars_to_jstring(env, arg, codec); \ > > > + if (!jarg) { \ > > > + goto fail; \ > > > + } \ > > > + \ > > > + object = (*env)->CallStaticObjectMethod(env, \ > > > + codec->jfields.mediacodec_class, \ > > > + codec->jfields.jfunc, \ > > > + jarg); \ > > > + if (ff_jni_exception_check(env, 1, codec) < 0) { \ > > > + goto fail; \ > > > + } \ > > > + \ > > > + codec->object = (*env)->NewGlobalRef(env, object); \ > > > + if (!codec->object) { \ > > > + goto fail; \ > > > + } \ > > > + \ > > > + if (codec_init_static_fields(codec) < 0) { \ > > > + goto fail; \ > > > + } \ > > > + \ > > > + if (codec->jfields.get_input_buffer_id && codec->jfields.get_output_buffer_id) { \ > > > + codec->has_get_i_o_buffer = 1; \ > > > + } \ > > > + \ > > > + ret = 0; \ > > > +fail: \ > > > + if (jarg) { \ > > > + (*env)->DeleteLocalRef(env, jarg); \ > > > + } \ > > > + \ > > > + if (object) { \ > > > + (*env)->DeleteLocalRef(env, object); \ > > > + } \ > > > + \ > > > + if (ret < 0) { \ > > > + ff_jni_reset_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec); \ > > > + av_freep(&codec); \ > > > + } \ > > > + \ > > > + return codec; \ > > > } > > > > > > > Such long macros are very ugly. And the only macro argument that is > > used in the body is jfunc. Would it be possible to move most of the > > macro into a proper function, with jfunc as pointer argument or so? > > I agree with you that such macros are ugly. > > I used this approach as the jfunc value cannot be known before > ff_jni_init_jfields is called. > > I will try another approach and send a new patch. Right, it seems it's not as simple as passing a pointer. Maybe offsetof() could help.
diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c index f34450a6d8..4660e895ca 100644 --- a/libavcodec/mediacodec_wrapper.c +++ b/libavcodec/mediacodec_wrapper.c @@ -1132,200 +1132,78 @@ fail: return ret; } -FFAMediaCodec* ff_AMediaCodec_createCodecByName(const char *name) -{ - int ret = -1; - JNIEnv *env = NULL; - FFAMediaCodec *codec = NULL; - jstring codec_name = NULL; - jobject object = NULL; - - codec = av_mallocz(sizeof(FFAMediaCodec)); - if (!codec) { - return NULL; - } - codec->class = &amediacodec_class; - - env = ff_jni_get_env(codec); - if (!env) { - av_freep(&codec); - return NULL; - } - - if (ff_jni_init_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec) < 0) { - goto fail; - } - - codec_name = ff_jni_utf_chars_to_jstring(env, name, codec); - if (!codec_name) { - goto fail; - } - - object = (*env)->CallStaticObjectMethod(env, codec->jfields.mediacodec_class, codec->jfields.create_by_codec_name_id, codec_name); - if (ff_jni_exception_check(env, 1, codec) < 0) { - goto fail; - } - - codec->object = (*env)->NewGlobalRef(env, object); - if (!codec->object) { - goto fail; - } - - if (codec_init_static_fields(codec) < 0) { - goto fail; - } - - if (codec->jfields.get_input_buffer_id && codec->jfields.get_output_buffer_id) { - codec->has_get_i_o_buffer = 1; - } - - ret = 0; -fail: - if (codec_name) { - (*env)->DeleteLocalRef(env, codec_name); - } - - if (object) { - (*env)->DeleteLocalRef(env, object); - } - - if (ret < 0) { - ff_jni_reset_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec); - av_freep(&codec); - } - - return codec; +#define DECLARE_FF_AMEDIACODEC_CREATE_FUNC(func, jfunc) \ +FFAMediaCodec* ff_AMediaCodec_##func(const char *arg) \ +{ \ + int ret = -1; \ + JNIEnv *env = NULL; \ + FFAMediaCodec *codec = NULL; \ + jstring jarg = NULL; \ + jobject object = NULL; \ + \ + codec = av_mallocz(sizeof(FFAMediaCodec)); \ + if (!codec) { \ + return NULL; \ + } \ + codec->class = &amediacodec_class; \ + \ + env = ff_jni_get_env(codec); \ + if (!env) { \ + av_freep(&codec); \ + return NULL; \ + } \ + \ + if (ff_jni_init_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec) < 0) { \ + goto fail; \ + } \ + \ + jarg = ff_jni_utf_chars_to_jstring(env, arg, codec); \ + if (!jarg) { \ + goto fail; \ + } \ + \ + object = (*env)->CallStaticObjectMethod(env, \ + codec->jfields.mediacodec_class, \ + codec->jfields.jfunc, \ + jarg); \ + if (ff_jni_exception_check(env, 1, codec) < 0) { \ + goto fail; \ + } \ + \ + codec->object = (*env)->NewGlobalRef(env, object); \ + if (!codec->object) { \ + goto fail; \ + } \ + \ + if (codec_init_static_fields(codec) < 0) { \ + goto fail; \ + } \ + \ + if (codec->jfields.get_input_buffer_id && codec->jfields.get_output_buffer_id) { \ + codec->has_get_i_o_buffer = 1; \ + } \ + \ + ret = 0; \ +fail: \ + if (jarg) { \ + (*env)->DeleteLocalRef(env, jarg); \ + } \ + \ + if (object) { \ + (*env)->DeleteLocalRef(env, object); \ + } \ + \ + if (ret < 0) { \ + ff_jni_reset_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec); \ + av_freep(&codec); \ + } \ + \ + return codec; \ } -FFAMediaCodec* ff_AMediaCodec_createDecoderByType(const char *mime) -{ - int ret = -1; - JNIEnv *env = NULL; - FFAMediaCodec *codec = NULL; - jstring mime_type = NULL; - jobject object = NULL; - - codec = av_mallocz(sizeof(FFAMediaCodec)); - if (!codec) { - return NULL; - } - codec->class = &amediacodec_class; - - env = ff_jni_get_env(codec); - if (!env) { - av_freep(&codec); - return NULL; - } - - if (ff_jni_init_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec) < 0) { - goto fail; - } - - mime_type = ff_jni_utf_chars_to_jstring(env, mime, codec); - if (!mime_type) { - goto fail; - } - - object = (*env)->CallStaticObjectMethod(env, codec->jfields.mediacodec_class, codec->jfields.create_decoder_by_type_id, mime_type); - if (ff_jni_exception_check(env, 1, codec) < 0) { - goto fail; - } - - codec->object = (*env)->NewGlobalRef(env, object); - if (!codec->object) { - goto fail; - } - - if (codec_init_static_fields(codec) < 0) { - goto fail; - } - - if (codec->jfields.get_input_buffer_id && codec->jfields.get_output_buffer_id) { - codec->has_get_i_o_buffer = 1; - } - - ret = 0; -fail: - if (mime_type) { - (*env)->DeleteLocalRef(env, mime_type); - } - - if (object) { - (*env)->DeleteLocalRef(env, object); - } - - if (ret < 0) { - ff_jni_reset_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec); - av_freep(&codec); - } - - return codec; -} - -FFAMediaCodec* ff_AMediaCodec_createEncoderByType(const char *mime) -{ - int ret = -1; - JNIEnv *env = NULL; - FFAMediaCodec *codec = NULL; - jstring mime_type = NULL; - jobject object = NULL; - - codec = av_mallocz(sizeof(FFAMediaCodec)); - if (!codec) { - return NULL; - } - codec->class = &amediacodec_class; - - env = ff_jni_get_env(codec); - if (!env) { - av_freep(&codec); - return NULL; - } - - if (ff_jni_init_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec) < 0) { - goto fail; - } - - mime_type = ff_jni_utf_chars_to_jstring(env, mime, codec); - if (!mime_type) { - goto fail; - } - - object = (*env)->CallStaticObjectMethod(env, codec->jfields.mediacodec_class, codec->jfields.create_encoder_by_type_id, mime_type); - if (ff_jni_exception_check(env, 1, codec) < 0) { - goto fail; - } - - codec->object = (*env)->NewGlobalRef(env, object); - if (!codec->object) { - goto fail; - } - - if (codec_init_static_fields(codec) < 0) { - goto fail; - } - - if (codec->jfields.get_input_buffer_id && codec->jfields.get_output_buffer_id) { - codec->has_get_i_o_buffer = 1; - } - - ret = 0; -fail: - if (mime_type) { - (*env)->DeleteLocalRef(env, mime_type); - } - - if (object) { - (*env)->DeleteLocalRef(env, object); - } - - if (ret < 0) { - ff_jni_reset_jfields(env, &codec->jfields, jni_amediacodec_mapping, 1, codec); - av_freep(&codec); - } - - return codec; -} +DECLARE_FF_AMEDIACODEC_CREATE_FUNC(createCodecByName, create_by_codec_name_id) +DECLARE_FF_AMEDIACODEC_CREATE_FUNC(createDecoderByType, create_decoder_by_type_id) +DECLARE_FF_AMEDIACODEC_CREATE_FUNC(createEncoderByType, create_encoder_by_type_id) int ff_AMediaCodec_delete(FFAMediaCodec* codec) {