On Fri, Dec 15, 2017 at 04:13:22PM +0100, Matthieu Bouron wrote: > On Thu, Dec 14, 2017 at 06:38:48PM +0100, wm4 wrote: > > On Thu, 14 Dec 2017 18:06:11 +0100 > > Matthieu Bouron <matthieu.bou...@gmail.com> wrote: > > > > > On Thu, Dec 14, 2017 at 01:02:49PM +0100, wm4 wrote: > > > > On Thu, 14 Dec 2017 12:53:38 +0100 > > > > Matthieu Bouron <matthieu.bou...@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.bou...@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. > > > > > > Patch updated. I did not use offsetof as it felt a bit weird to pass an > > > offset of a struct to a function to call the right method. > > > > > > > Looks much nicer! > > I will push the patchset in a few hours if there is no objection. >
Patch applied. -- Matthieu B. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel