From 1b406dcc6b61b70f17fef6221caffe523225eb09 Mon Sep 17 00:00:00 2001 From: Artyom Zavrin Date: Fri, 22 Sep 2023 04:42:31 +0300 Subject: [PATCH 1/2] Fix Segfault if SAF is used with concat demuxer --- .../src/main/cpp/ffmpegkit.c | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c b/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c index dddcf93e..78ebbdde 100644 --- a/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c +++ b/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c @@ -564,21 +564,58 @@ void *callbackThreadFunction() { } /** - * Used by saf protocol; is expected to be called from a Java thread, therefore we don't need attach/detach + * Used by saf protocol; If it is called from a Java thread, we don't need attach/detach. + * However it can be called from other threads as well (as it happens for concat demuxer), + * in that case we perform attach & detach. + * Returns file descriptor created for this SAF id or 0 if an error occurs. */ int saf_open(int safId) { JNIEnv *env = NULL; - (*globalVm)->GetEnv(globalVm, (void**) &env, JNI_VERSION_1_6); - return (*env)->CallStaticIntMethod(env, configClass, safOpenMethod, safId); + bool attached = false; + jint getEnvRc = (*globalVm)->GetEnv(globalVm, (void**) &env, JNI_VERSION_1_6); + if (getEnvRc != JNI_OK) { + if (getEnvRc != JNI_EDETACHED) { + LOGE("Callback thread failed to GetEnv for class %s with rc %d.\n", configClassName, getEnvRc); + return 0; + } + if ((*globalVm)->AttachCurrentThread(globalVm, &env, NULL) != 0) { + LOGE("Callback thread failed to AttachCurrentThread for class %s.\n", configClassName); + return 0; + } else { + attached = true; + } + } + int result = (*env)->CallStaticIntMethod(env, configClass, safOpenMethod, safId); + if (attached) (*globalVm)->DetachCurrentThread(globalVm); + return result; } /** - * Used by saf protocol; is expected to be called from a Java thread, therefore we don't need attach/detach + * Used by saf protocol; If it is called from a Java thread, we don't need attach/detach. + * However it can be called from other threads as well (as it happens for concat demuxer), + * in that case we perform attach & detach. + * Returns 1 if the given file descriptor is closed successfully, 0 if an error occurs. */ int saf_close(int fd) { JNIEnv *env = NULL; - (*globalVm)->GetEnv(globalVm, (void**) &env, JNI_VERSION_1_6); - return (*env)->CallStaticIntMethod(env, configClass, safCloseMethod, fd); + bool attached = false; + jint getEnvRc = (*globalVm)->GetEnv(globalVm, (void**) &env, JNI_VERSION_1_6); + if (getEnvRc != JNI_OK) { + if (getEnvRc != JNI_EDETACHED) { + LOGE("Callback thread failed to GetEnv for class %s with rc %d.\n", configClassName, getEnvRc); + return 0; + } + + if ((*globalVm)->AttachCurrentThread(globalVm, &env, NULL) != 0) { + LOGE("Callback thread failed to AttachCurrentThread for class %s.\n", configClassName); + return 0; + } else { + attached = true; + } + } + int result = (*env)->CallStaticIntMethod(env, configClass, safCloseMethod, fd); + if (attached) (*globalVm)->DetachCurrentThread(globalVm); + return result; } /** From e53cad64045d98ca59c779b5d39ff1ef6b19cbd8 Mon Sep 17 00:00:00 2001 From: Artyom Zavrin Date: Thu, 28 Sep 2023 07:38:19 +0300 Subject: [PATCH 2/2] Fix error texts in saf_open/saf_close --- android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c b/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c index 78ebbdde..d7e7023c 100644 --- a/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c +++ b/android/ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c @@ -575,11 +575,11 @@ int saf_open(int safId) { jint getEnvRc = (*globalVm)->GetEnv(globalVm, (void**) &env, JNI_VERSION_1_6); if (getEnvRc != JNI_OK) { if (getEnvRc != JNI_EDETACHED) { - LOGE("Callback thread failed to GetEnv for class %s with rc %d.\n", configClassName, getEnvRc); + LOGE("saf_open failed to GetEnv for class %s with rc %d.\n", configClassName, getEnvRc); return 0; } if ((*globalVm)->AttachCurrentThread(globalVm, &env, NULL) != 0) { - LOGE("Callback thread failed to AttachCurrentThread for class %s.\n", configClassName); + LOGE("saf_open failed to AttachCurrentThread for class %s.\n", configClassName); return 0; } else { attached = true; @@ -602,12 +602,12 @@ int saf_close(int fd) { jint getEnvRc = (*globalVm)->GetEnv(globalVm, (void**) &env, JNI_VERSION_1_6); if (getEnvRc != JNI_OK) { if (getEnvRc != JNI_EDETACHED) { - LOGE("Callback thread failed to GetEnv for class %s with rc %d.\n", configClassName, getEnvRc); + LOGE("saf_close failed to GetEnv for class %s with rc %d.\n", configClassName, getEnvRc); return 0; } if ((*globalVm)->AttachCurrentThread(globalVm, &env, NULL) != 0) { - LOGE("Callback thread failed to AttachCurrentThread for class %s.\n", configClassName); + LOGE("saf_close failed to AttachCurrentThread for class %s.\n", configClassName); return 0; } else { attached = true;