[FFmpeg-devel] [PATCH 1/2] lavc: add JNI support

[Matthieu Bouron]

On Tue, Mar 01, 2016 at 09:06:35PM +0100, wm4 wrote:
> On Tue, 1 Mar 2016 20:57:12 +0100
> Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> 
> > On Tue, Mar 01, 2016 at 08:55:23PM +0100, Matthieu Bouron wrote:
> > > On Tue, Mar 01, 2016 at 08:38:04PM +0100, wm4 wrote:  
> > > > On Tue, 1 Mar 2016 20:33:14 +0100
> > > > Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> > > >   
> > > > > On Tue, Mar 01, 2016 at 08:24:27PM +0100, Matthieu Bouron wrote:  
> > > > > > On Tue, Mar 01, 2016 at 08:13:48PM +0100, wm4 wrote:    
> > > > > > > On Tue, 1 Mar 2016 20:10:50 +0100
> > > > > > > Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> > > > > > >     
> > > > > > > > On Tue, Mar 01, 2016 at 07:56:30PM +0100, wm4 wrote:    
> > > > > > > > > On Tue, 1 Mar 2016 19:52:08 +0100
> > > > > > > > > Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> > > > > > > > >       
> > > > > > > > > > On Fri, Feb 26, 2016 at 05:36:40PM +0100, Matthieu Bouron wrote:      
> > > > > > > > > > > On Fri, Feb 26, 2016 at 4:41 PM, Matthieu Bouron <matthieu.bouron at gmail.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > >         
> > > > > > > > > > > > On Mon, Feb 22, 2016 at 12:20:35PM +0100, Matthieu Bouron wrote:        
> > > > > > > > > > > > > From: Matthieu Bouron <matthieu.bouron at stupeflix.com>
> > > > > > > > > > > > >        
> > > > > > > > > > > >        
> > > > > > > > > > > 
> > > > > > > > > > > [...]
> > > > > > > > > > > 
> > > > > > > > > > >         
> > > > > > > > > > > >
> > > > > > > > > > > > Patch updated with the following differences:
> > > > > > > > > > > >   * fix of switch/case code style
> > > > > > > > > > > >   * removal of a miss declaration of FFJNIField enum as a global variable
> > > > > > > > > > > >
> > > > > > > > > > > >        
> > > > > > > > > > > Patch updated with the following differences:
> > > > > > > > > > >   * fixes a few typo in comments
> > > > > > > > > > >   * fixes a if statement in ff_jni_init_jfields        
> > > > > > > > > > 
> > > > > > > > > > Patch updated with the following differences:
> > > > > > > > > >   * fixes a few typo in comments and message logs
> > > > > > > > > >   * add missing .so at end of library names when trying to find
> > > > > > > > > >   JNI_GetCreatedVMs symbol (also add libart.so)
> > > > > > > > > >   * reintroduce public functions that allow the user to set the Java
> > > > > > > > > >   virtual machine (av_jni_(set,get)_java_vm) as the private C++
> > > > > > > > > >   JniInvocation wrapper is not available on all devices (android >= 4.4)      
> > > > > > > > > 
> > > > > > > > > I'm wondering if the VM should be stored per AVCodecContext. (Since it
> > > > > > > > > has to be set explicitly again by the user.)      
> > > > > > > > 
> > > > > > > > I think it is fine to store one VM for all the AVCodecContext as it will
> > > > > > > > be the same during all the lifetime of the application. I should also
> > > > > > > > enforce that the VM cannot be changed afterwards.
> > > > > > > > 
> > > > > > > > av_jni_set_java_vm stores the Java VM pointer locally in jni.c and not in
> > > > > > > > ffjni.c and it is retrieved in jni.c using av_jni_get_java_vm. I've done
> > > > > > > > it that way because if at some point we are to include ffjni.c from
> > > > > > > > libavformat it won't work (it will only set the java vm in the libavcodec
> > > > > > > > memory space).    
> > > > > > > 
> > > > > > > The problem is that this is not library-safe. What if two libs within
> > > > > > > the process (which both are using libavcodec) are setting a different
> > > > > > > VM?    
> > > > > > 
> > > > > > In the Android application context, you only have, AFAIK, one VM running
> > > > > > during the lifetime of the application. The code does handle the general    
> > > > > 
> > > > > does *not* (sorry)
> > > > >   
> > > > > > JNI case (even outside Android) but do we really want to do that anyway ?    
> > > > 
> > > > If there's guaranteed to be exactly one, I don't get why we can't get
> > > > it into existence ourselves?
> > > >
> > > > Where exactly would an application get the VM handle from?  
> > > 
> > > You get the Java VM pointer when JNI_onLoad(JavaVM *vm, void *reserved) is
> > > executed at library load. This is where you can pass the pointer to lavc.
> > > 
> > > IMHO, av_jni_set_java_vm should set the VM once and then warn (or return an error)
> > > if the user is providing a different Java VM pointer.
> > > 
> > > In theory, on some version of Android, you can spawn your own VM using their
> > > private C++ JniInvocation wrapper.
> > > On a regular desktop, you can create Java VMs (using JNI_createJavaVM).  
> > 
> > Also note that the JniInvocation wrapper is still used if the user hasn't
> > provided a JavaVM using av_jni_set_java_vm but that will only work on
> > android >= 4.4.2
> 
> I still don't quite understand the implications, so no resistance from
> me anymore, I guess.

New patch attached with the following differences:
  * only one Java VM can be set using av_jni_set_java_vm. If the user try
  to set another Java VM at some point the function will error out and log
  an error message.
  * fixes some typo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavc-add-JNI-support.patch
Type: text/x-diff
Size: 29089 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160302/336332b6/attachment.patch>