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!
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to