Bug 927 - Multithreading (MT) issues libav/ffmpeg

FFMPEG Natives:

  - Move 'mutex_avcodec_openclose' to local static and initialize at initSymbols0

  - setStream0:
    - Add another locked mutex block around:
       - [ sp_avformat_open_input .. sp_avformat_find_stream_info ]

  This solves the issue of:

    [NULL @ 0x89d20c60] insufficient thread locking around avcodec_open/close()

6 files changed, 83 insertions, 78 deletions

diff --git a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGNatives.java b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGNatives.java
index c3fc2898f..b4b887bc9 100644
--- a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGNatives.java
+++ b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGNatives.java
@@ -31,20 +31,19 @@ import com.jogamp.opengl.util.texture.TextureSequence.TextureFrame;
 
 /* pp */ abstract class FFMPEGNatives {
 
-    private static final Object mutex_avcodec_openclose = new Object();
+    private static final Object mutex_avcodec_openclose_jni = new Object();
 
-    abstract boolean initSymbols0(long[] symbols, int count);
+    final boolean initSymbols0(long[] symbols, int count) {
+        return initSymbols0(mutex_avcodec_openclose_jni, symbols, count);
+    }
+    abstract boolean initSymbols0(Object mutex_avcodec_openclose, long[] symbols, int count);
     abstract int getAvUtilMajorVersionCC0();
     abstract int getAvFormatMajorVersionCC0();
     abstract int getAvCodecMajorVersionCC0();
     abstract int getAvResampleMajorVersionCC0();
     abstract int getSwResampleMajorVersionCC0();
 
-    final long createInstance0(FFMPEGMediaPlayer upstream, boolean verbose) {
-        return createInstance0(mutex_avcodec_openclose, upstream, verbose);
-    }
-    abstract long createInstance0(Object mutex_avcodec_openclose, FFMPEGMediaPlayer upstream, boolean verbose);
-
+    abstract long createInstance0(FFMPEGMediaPlayer upstream, boolean verbose);
     abstract void destroyInstance0(long moviePtr);
 
     /**
diff --git a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv08Natives.java b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv08Natives.java
index 6ca0ea311..6bab23f25 100644
--- a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv08Natives.java
+++ b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv08Natives.java
@@ -29,7 +29,7 @@ package jogamp.opengl.util.av.impl;
 
 class FFMPEGv08Natives extends FFMPEGNatives {
     @Override
-    native boolean initSymbols0(long[] symbols, int count);
+    native boolean initSymbols0(Object mutex_avcodec_openclose, long[] symbols, int count);
 
     @Override
     native int getAvUtilMajorVersionCC0();
@@ -47,7 +47,7 @@ class FFMPEGv08Natives extends FFMPEGNatives {
     native int getSwResampleMajorVersionCC0();
 
     @Override
-    native long createInstance0(Object mutex_avcodec_openclose, FFMPEGMediaPlayer upstream, boolean verbose);
+    native long createInstance0(FFMPEGMediaPlayer upstream, boolean verbose);
 
     @Override
     native void destroyInstance0(long moviePtr);
diff --git a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv09Natives.java b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv09Natives.java
index 1d9f7f5fa..a48b5f21f 100644
--- a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv09Natives.java
+++ b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv09Natives.java
@@ -29,7 +29,7 @@ package jogamp.opengl.util.av.impl;
 
 class FFMPEGv09Natives extends FFMPEGNatives {
     @Override
-    native boolean initSymbols0(long[] symbols, int count);
+    native boolean initSymbols0(Object mutex_avcodec_openclose, long[] symbols, int count);
 
     @Override
     native int getAvUtilMajorVersionCC0();
@@ -47,7 +47,7 @@ class FFMPEGv09Natives extends FFMPEGNatives {
     native int getSwResampleMajorVersionCC0();
 
     @Override
-    native long createInstance0(Object mutex_avcodec_openclose, FFMPEGMediaPlayer upstream, boolean verbose);
+    native long createInstance0(FFMPEGMediaPlayer upstream, boolean verbose);
 
     @Override
     native void destroyInstance0(long moviePtr);
diff --git a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv10Natives.java b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv10Natives.java
index 19bc10f5f..f35fb29dc 100644
--- a/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv10Natives.java
+++ b/src/jogl/classes/jogamp/opengl/util/av/impl/FFMPEGv10Natives.java
@@ -29,7 +29,7 @@ package jogamp.opengl.util.av.impl;
 
 class FFMPEGv10Natives extends FFMPEGNatives {
     @Override
-    native boolean initSymbols0(long[] symbols, int count);
+    native boolean initSymbols0(Object mutex_avcodec_openclose, long[] symbols, int count);
 
     @Override
     native int getAvUtilMajorVersionCC0();
@@ -47,7 +47,7 @@ class FFMPEGv10Natives extends FFMPEGNatives {
     native int getSwResampleMajorVersionCC0();
 
     @Override
-    native long createInstance0(Object mutex_avcodec_openclose, FFMPEGMediaPlayer upstream, boolean verbose);
+    native long createInstance0(FFMPEGMediaPlayer upstream, boolean verbose);
 
     @Override
     native void destroyInstance0(long moviePtr);
diff --git a/src/jogl/native/libav/ffmpeg_impl_template.c b/src/jogl/native/libav/ffmpeg_impl_template.c
index 274eb37a5..a7e9f4857 100644
--- a/src/jogl/native/libav/ffmpeg_impl_template.c
+++ b/src/jogl/native/libav/ffmpeg_impl_template.c
@@ -200,11 +200,36 @@ static SWR_FREE sp_swr_free;
 static SWR_CONVERT sp_swr_convert;
 // count: 65
 
+// We use JNI Monitor Locking, since this removes the need 
+// to statically link-in pthreads on window ..
+// #define USE_PTHREAD_LOCKING 1
+//
+#define USE_JNI_LOCKING 1
+
+#if defined (USE_PTHREAD_LOCKING)
+    #include <pthread.h>
+    #warning USE LOCKING PTHREAD
+    static pthread_mutex_t mutex_avcodec_openclose;
+    #define MY_MUTEX_LOCK(e,s) pthread_mutex_lock(&(s))
+    #define MY_MUTEX_UNLOCK(e,s) pthread_mutex_unlock(&(s))
+#elif defined (USE_JNI_LOCKING)
+    static jobject mutex_avcodec_openclose;
+    #define MY_MUTEX_LOCK(e,s) (*e)->MonitorEnter(e, s)
+    #define MY_MUTEX_UNLOCK(e,s) (*e)->MonitorExit(e, s)
+#else
+    #warning USE LOCKING NONE
+    #define MY_MUTEX_LOCK(e,s)
+    #define MY_MUTEX_UNLOCK(e,s)
+#endif
+
 #define SYMBOL_COUNT 65
 
 JNIEXPORT jboolean JNICALL FF_FUNC(initSymbols0)
-  (JNIEnv *env, jobject instance, jobject jSymbols, jint count)
+  (JNIEnv *env, jobject instance, jobject jmutex_avcodec_openclose, jobject jSymbols, jint count)
 {
+    #ifdef USE_PTHREAD_LOCKING
+        pthread_mutexattr_t renderLockAttr;
+    #endif
     int64_t* symbols; // jlong -> int64_t -> intptr_t -> FUNC_PTR
     int i;
 
@@ -315,6 +340,23 @@ JNIEXPORT jboolean JNICALL FF_FUNC(initSymbols0)
         }
     #endif
 
+    #if defined (USE_PTHREAD_LOCKING)
+        pthread_mutexattr_init(&renderLockAttr);
+        pthread_mutexattr_settype(&renderLockAttr, PTHREAD_MUTEX_RECURSIVE);
+        pthread_mutex_init(&mutex_avcodec_openclose, &renderLockAttr); // recursive
+    #elif defined (USE_JNI_LOCKING)
+        mutex_avcodec_openclose = (*env)->NewGlobalRef(env, jmutex_avcodec_openclose);
+    #endif
+
+    /** At static destroy: Never
+    #if defined (USE_PTHREAD_LOCKING)
+        pthread_mutex_unlock(&mutex_avcodec_openclose);
+        pthread_mutex_destroy(&mutex_avcodec_openclose);
+    #elif defined (USE_JNI_LOCKING)
+        (*env)->DeleteGlobalRef(env, mutex_avcodec_openclose);
+    #endif
+    */
+
     return JNI_TRUE;
 }
 
@@ -344,14 +386,6 @@ static void _setIsGLOriented(JNIEnv *env, FFMPEGToolBasicAV_t* pAV) {
     }
 }
 
-#ifdef USE_PTHREAD_LOCKING
-  #define MY_MUTEX_LOCK(e,s) pthread_mutex_lock(&(s))
-  #define MY_MUTEX_UNLOCK(e,s) pthread_mutex_unlock(&(s))
-#else
-  #define MY_MUTEX_LOCK(e,s) (*e)->MonitorEnter(e, s)
-  #define MY_MUTEX_UNLOCK(e,s) (*e)->MonitorExit(e, s)
-#endif
-
 static void freeInstance(JNIEnv *env, FFMPEGToolBasicAV_t* pAV) {
     int i;
     if(NULL != pAV) {
@@ -369,7 +403,7 @@ static void freeInstance(JNIEnv *env, FFMPEGToolBasicAV_t* pAV) {
             pAV->aResampleBuffer = NULL;
         }
 
-        MY_MUTEX_LOCK(env, pAV->mutex_avcodec_openclose);
+        MY_MUTEX_LOCK(env, mutex_avcodec_openclose);
         {
             // Close the V codec
             if(NULL != pAV->pVCodecCtx) {
@@ -385,7 +419,7 @@ static void freeInstance(JNIEnv *env, FFMPEGToolBasicAV_t* pAV) {
             }
             pAV->pACodec=NULL;
         }
-        MY_MUTEX_UNLOCK(env, pAV->mutex_avcodec_openclose);
+        MY_MUTEX_UNLOCK(env, mutex_avcodec_openclose);
 
         // Close the frames
         if(NULL != pAV->pVFrame) {
@@ -434,13 +468,6 @@ static void freeInstance(JNIEnv *env, FFMPEGToolBasicAV_t* pAV) {
             pAV->ffmpegMediaPlayer = NULL;
         }
 
-        #ifdef USE_PTHREAD_LOCKING
-            pthread_mutex_unlock(&pAV->mutex_avcodec_openclose);
-            pthread_mutex_destroy(&pAV->mutex_avcodec_openclose);
-        #else
-            (*env)->DeleteGlobalRef(env, pAV->mutex_avcodec_openclose);
-        #endif
-
         free(pAV);
     }
 }
@@ -508,11 +535,8 @@ JNIEXPORT jint JNICALL FF_FUNC(getSwResampleMajorVersionCC0)
 }
 
 JNIEXPORT jlong JNICALL FF_FUNC(createInstance0)
-  (JNIEnv *env, jobject instance, jobject mutex_avcodec_openclose, jobject ffmpegMediaPlayer, jboolean verbose)
+  (JNIEnv *env, jobject instance, jobject ffmpegMediaPlayer, jboolean verbose)
 {
-    #ifdef USE_PTHREAD_LOCKING
-        pthread_mutexattr_t renderLockAttr;
-    #endif
     FFMPEGToolBasicAV_t * pAV = calloc(1, sizeof(FFMPEGToolBasicAV_t));
     if(NULL==pAV) {
         JoglCommon_throwNewRuntimeException(env, "Couldn't alloc instance");
@@ -547,14 +571,6 @@ JNIEXPORT jlong JNICALL FF_FUNC(createInstance0)
     pAV->vid=AV_STREAM_ID_AUTO;
     pAV->aid=AV_STREAM_ID_AUTO;
 
-    #ifdef USE_PTHREAD_LOCKING
-        pthread_mutexattr_init(&renderLockAttr);
-        pthread_mutexattr_settype(&renderLockAttr, PTHREAD_MUTEX_RECURSIVE);
-        pthread_mutex_init(&pAV->mutex_avcodec_openclose, &renderLockAttr); // recursive
-    #else
-        pAV->mutex_avcodec_openclose = (*env)->NewGlobalRef(env, mutex_avcodec_openclose);
-    #endif
-
     if(pAV->verbose) {
         fprintf(stderr, "Info: Use avresample %d, swresample %d, device %d, refCount %d\n", 
             AV_HAS_API_AVRESAMPLE(pAV), AV_HAS_API_SWRESAMPLE(pAV), HAS_FUNC(sp_avdevice_register_all), pAV->useRefCountedFrames);
@@ -740,22 +756,29 @@ JNIEXPORT void JNICALL FF_FUNC(setStream0)
             sp_av_dict_set(&inOpts, "framerate", buffer, 0);
         }
     }
-    res = sp_avformat_open_input(&pAV->pFormatCtx, filename, inFmt, NULL != inOpts ? &inOpts : NULL);
-    if( NULL != inOpts ) {
-        sp_av_dict_free(&inOpts);
-    }
-    if(res != 0) {
-        JoglCommon_throwNewRuntimeException(env, "Couldn't open URI: %s [%dx%d @ %d hz], err %d", filename, vWidth, vHeight, vRate, res);
-        (*env)->ReleaseStringChars(env, jURL, (const jchar *)urlPath);
-        return;
-    }
 
-    // Retrieve detailed stream information
-    if(sp_avformat_find_stream_info(pAV->pFormatCtx, NULL)<0) {
-        (*env)->ReleaseStringChars(env, jURL, (const jchar *)urlPath);
-        JoglCommon_throwNewRuntimeException(env, "Couldn't find stream information");
-        return;
+    MY_MUTEX_LOCK(env, mutex_avcodec_openclose);
+    {
+        res = sp_avformat_open_input(&pAV->pFormatCtx, filename, inFmt, NULL != inOpts ? &inOpts : NULL);
+        if( NULL != inOpts ) {
+            sp_av_dict_free(&inOpts);
+        }
+        if(res != 0) {
+            MY_MUTEX_UNLOCK(env, mutex_avcodec_openclose);
+            JoglCommon_throwNewRuntimeException(env, "Couldn't open URI: %s [%dx%d @ %d hz], err %d", filename, vWidth, vHeight, vRate, res);
+            (*env)->ReleaseStringChars(env, jURL, (const jchar *)urlPath);
+            return;
+        }
+
+        // Retrieve detailed stream information
+        if(sp_avformat_find_stream_info(pAV->pFormatCtx, NULL)<0) {
+            MY_MUTEX_UNLOCK(env, mutex_avcodec_openclose);
+            (*env)->ReleaseStringChars(env, jURL, (const jchar *)urlPath);
+            JoglCommon_throwNewRuntimeException(env, "Couldn't find stream information");
+            return;
+        }
     }
+    MY_MUTEX_UNLOCK(env, mutex_avcodec_openclose);
 
     if(pAV->verbose) {
         // Dump information about file onto standard error
@@ -763,7 +786,6 @@ JNIEXPORT void JNICALL FF_FUNC(setStream0)
     }
     (*env)->ReleaseStringChars(env, jURL, (const jchar *)urlPath);
 
-
     // FIXME: Libav Binary compatibility! JAU01
     if (pAV->pFormatCtx->duration != AV_NOPTS_VALUE) {
         pAV->duration = pAV->pFormatCtx->duration / AV_TIME_BASE_MSEC;
@@ -855,14 +877,14 @@ JNIEXPORT void JNICALL FF_FUNC(setStream0)
         }
 
         // Open codec
-        MY_MUTEX_LOCK(env, pAV->mutex_avcodec_openclose);
+        MY_MUTEX_LOCK(env, mutex_avcodec_openclose);
         {
             #if LIBAVCODEC_VERSION_MAJOR >= 55
                 pAV->pACodecCtx->refcounted_frames = pAV->useRefCountedFrames;
             #endif
             res = sp_avcodec_open2(pAV->pACodecCtx, pAV->pACodec, NULL);
         }
-        MY_MUTEX_UNLOCK(env, pAV->mutex_avcodec_openclose);
+        MY_MUTEX_UNLOCK(env, mutex_avcodec_openclose);
         if(res<0) {
             JoglCommon_throwNewRuntimeException(env, "Couldn't open audio codec %d, %s", pAV->pACodecCtx->codec_id, pAV->acodec);
             return;
@@ -1017,14 +1039,14 @@ JNIEXPORT void JNICALL FF_FUNC(setStream0)
         }
 
         // Open codec
-        MY_MUTEX_LOCK(env, pAV->mutex_avcodec_openclose);
+        MY_MUTEX_LOCK(env, mutex_avcodec_openclose);
         {
             #if LIBAVCODEC_VERSION_MAJOR >= 55
                 pAV->pVCodecCtx->refcounted_frames = pAV->useRefCountedFrames;
             #endif
             res = sp_avcodec_open2(pAV->pVCodecCtx, pAV->pVCodec, NULL);
         }
-        MY_MUTEX_UNLOCK(env, pAV->mutex_avcodec_openclose);
+        MY_MUTEX_UNLOCK(env, mutex_avcodec_openclose);
         if(res<0) {
             JoglCommon_throwNewRuntimeException(env, "Couldn't open video codec %d, %s", pAV->pVCodecCtx->codec_id, pAV->vcodec);
             return;
diff --git a/src/jogl/native/libav/ffmpeg_tool.h b/src/jogl/native/libav/ffmpeg_tool.h
index 66c3519e6..136be2ecc 100644
--- a/src/jogl/native/libav/ffmpeg_tool.h
+++ b/src/jogl/native/libav/ffmpeg_tool.h
@@ -64,15 +64,6 @@ typedef struct SwrContext SwrContext;
 #include <stdio.h>
 #include <stdlib.h>
 
-// We use JNI Monitor Locking, since this removes the need 
-// to statically link-in pthreads on window ..
-//   #define USE_PTHREAD_LOCKING 1
-//
-#ifdef USE_PTHREAD_LOCKING
-    #include <pthread.h>
-    #error PTHREAD path not tested yet
-#endif
-
 #include <GL/gl.h>
 
 typedef void (APIENTRYP PFNGLTEXSUBIMAGE2DPROC) (GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLenum type, const GLvoid *pixels);
@@ -139,7 +130,6 @@ typedef struct {
     int64_t dtsLast;  // DTS of the last frame
 } PTSStats;
 
-
 typedef struct {
     jobject          ffmpegMediaPlayer;
     int32_t          verbose;
@@ -157,12 +147,6 @@ typedef struct {
     PFNGLFLUSH procAddrGLFlush;
     PFNGLFINISH procAddrGLFinish;
 
-    #ifdef USE_PTHREAD_LOCKING
-        pthread_mutex_t  mutex_avcodec_openclose;
-    #else
-        jobject mutex_avcodec_openclose;
-    #endif
-
     AVFormatContext* pFormatCtx;
     int32_t          vid;
     AVStream*        pVStream;