From 1b45588d3be254fac98f1252339b8198f314ae5e Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:12:06 +0200 Subject: [PATCH] fix(player): keep MediaCodec surface alive through MPV teardown Setting vo=null during disposal could reinitialize MediaCodec and deadlock vendor decoders while the surface was already being released. Terminate libmpv synchronously with its surface references intact, then release the Android surfaces, and cover repeated hardware-decoded teardown on devices. --- .../assets/ffmpeg/mediacodec_teardown.mp4 | Bin 0 -> 2769 bytes .../plezy/mpv/MpvLifecycleDeviceTest.kt | 149 ++++++++++++++++++ android/app/src/debug/AndroidManifest.xml | 6 + .../plezy/mpv/MpvLifecycleTestActivity.kt | 18 +++ .../com/edde746/plezy/mpv/MpvPlayerCore.kt | 25 ++- .../com/edde746/plezy/mpv/MpvPlayerPlugin.kt | 16 +- android/libmpv/src/main/cpp/main.cpp | 31 +--- 7 files changed, 195 insertions(+), 50 deletions(-) create mode 100644 android/app/src/androidTest/assets/ffmpeg/mediacodec_teardown.mp4 create mode 100644 android/app/src/androidTest/kotlin/com/edde746/plezy/mpv/MpvLifecycleDeviceTest.kt create mode 100644 android/app/src/debug/kotlin/com/edde746/plezy/mpv/MpvLifecycleTestActivity.kt diff --git a/android/app/src/androidTest/assets/ffmpeg/mediacodec_teardown.mp4 b/android/app/src/androidTest/assets/ffmpeg/mediacodec_teardown.mp4 new file mode 100644 index 0000000000000000000000000000000000000000..5e7a7b3bc337ad4ae51fb86183b029a74153ce46 GIT binary patch literal 2769 zcmZQzU{FXasVvAW&d+6FU}6B#nZ@}=iDk)#xdkSM3=Aw%x%v5J3=9l8xn&tC3@Cv4 z1p@>71qMb25JJc>BA8$n8s7mdh?8JqU|@DFDN4*{U|@(T$p+iWjHDWB8v6w#F$fbX z1K}{BIf{V+WG^F#*aB9elA2Od%)r1PlbTap0u}-5fSL<7i)n9eN@gM`jB-=J?tm%- zQ8o+=3>6HaM?h=_-Heo+A~1`AfuSrjB^Aa7(P5b>srep>c_}%mAT>-GxtV!s3=9k+ zWw~HO!B&A3GB7Zxq(J0kQi@VRYCysud7jdu90iD%m^6w@l5!Xr81@vG6vH9}q-G5$ z(m)y@>Yywr#puAWg~5Y?0gORnj3PdXWyxm7dWL$228IebnMoB!W+osR7?$||pMik^ zloXs9oes$Tf5;%2?sPzInIJ1$T@8x^0|PTCNcowMcDE!jGO%TwaN^f#U|?VnC`c?W zfVu(XE|H{?B4_|h$v_R4C@uj9K8OoaV*^Tj3=9ks#U;g{NCAm6LCuf{MN)A|aWY5{ zDhAa9N_UJb#U;g6P&p7~!@$67&cMLHjR*5EFfic5`1InFJA}thkUo5Ng5*GI$iX7T zCCT|9&qKn8#fyP~A+a>21eACqb5p@INLmp&q2{J!79|!GfYK634vcj&A-O?iWl28B zK$(<8ux3zzGx#KyrNL4R0|Ns`T2X2$0|Q%sZc1Va0|Qeb8|(l7xp!P&?tOOCWBW9n z8wxj--@OMVD+OJJ^npI2<9kery5m~3mHP?VZxYiOpBlA4s0pPX%LXk}n!ppclC zm{VDtYHLtoXk}1gXl$sEn`&E{o1su#nv|PrYiOWQP+Vzi2qNQ)Qfv+N3=FIc^b8CX zax*JZQ{us9D&(fd7bWJUr`j5sC?sbT<>w~GgY+tt6s6|mWER^RDOglkq$HQv8Ym58% zC?w^S7A023C+FuDB!XR(pI2N`l$e>9ni8LxS5lM+Qj=Dcn44N`YoJh`nwg$aQebPK zkeynYnO9eP zFtwQeu=hZi)(EC2!+yrc5Yb2nI5V>W&a6BDVRAUm>S)kEPysTS!*SM(2?+Lz1qk+z z4G8v$0|@qw3kdd$2MG3$4^TF{<7@`;2Q~>{x6BqmFclC?0|e6n!3;n!6A;V-1hWCb zoPc01KrlBTm + runPlaybackCycle(instrumentation, activity, fixture, cycle) + } + } finally { + instrumentation.runOnMainSync(activity::finish) + instrumentation.waitForIdleSync() + fixture.delete() + } + } + + private fun runPlaybackCycle( + instrumentation: Instrumentation, + activity: MpvLifecycleTestActivity, + fixture: File, + cycle: Int + ) { + val initialized = CountDownLatch(1) + val initializationResult = AtomicReference() + val events = RecordingDelegate() + val core = AtomicReference() + + instrumentation.runOnMainSync { + core.set(MpvPlayerCore(activity, hardwareDecoding = true).also { playerCore -> + playerCore.delegate = events + playerCore.initialize { success -> + initializationResult.set(success) + initialized.countDown() + } + }) + } + + assertCompletes(initialized, "MPV initialization", cycle) + assertTrue("MPV initialization failed in cycle $cycle", initializationResult.get()) + setProperty(instrumentation, core.get(), "hwdec", "mediacodec", cycle) + setProperty(instrumentation, core.get(), "aid", "no", cycle) + + val commandCompleted = CountDownLatch(1) + val commandResult = AtomicReference() + instrumentation.runOnMainSync { + core.get().command(arrayOf("loadfile", fixture.absolutePath, "replace")) { success -> + commandResult.set(success) + commandCompleted.countDown() + } + } + assertCompletes(commandCompleted, "loadfile command", cycle) + assertTrue("loadfile command failed in cycle $cycle", commandResult.get()) + assertCompletes(events.fileLoaded, "file-loaded event", cycle) + assertCompletes(events.playbackRestart, "playback-restart event", cycle) + + assertEquals("mediacodec", core.get().getProperty("current-vo")) + assertTrue( + "Expected MediaCodec hardware decoding in cycle $cycle", + core.get().getProperty("hwdec-current")?.startsWith("mediacodec") == true + ) + + val disposed = CountDownLatch(1) + instrumentation.runOnMainSync { core.get().dispose { disposed.countDown() } } + assertCompletes(disposed, "terminal teardown", cycle, DISPOSE_TIMEOUT_SECONDS) + instrumentation.runOnMainSync { + val content = activity.findViewById(android.R.id.content) + assertEquals("Player surface container leaked in cycle $cycle", 1, content.childCount) + } + } + + private fun setProperty( + instrumentation: Instrumentation, + core: MpvPlayerCore, + name: String, + value: String, + cycle: Int + ) { + val completed = CountDownLatch(1) + val result = AtomicReference>() + instrumentation.runOnMainSync { + core.setProperty(name, value) { outcome -> + result.set(outcome) + completed.countDown() + } + } + assertCompletes(completed, "$name property write", cycle) + assertTrue("$name property write failed in cycle $cycle", result.get().isSuccess) + } + + private fun assertCompletes( + latch: CountDownLatch, + operation: String, + cycle: Int, + timeoutSeconds: Long = OPERATION_TIMEOUT_SECONDS + ) { + assertTrue( + "$operation timed out in cycle $cycle after ${timeoutSeconds}s", + latch.await(timeoutSeconds, TimeUnit.SECONDS) + ) + } + + private fun copyFixture(bytes: ByteArray, cacheDir: File): File = + File.createTempFile("mpv-lifecycle-", ".mp4", cacheDir).apply { writeBytes(bytes) } + + private class RecordingDelegate : PlayerDelegate { + val fileLoaded = CountDownLatch(1) + val playbackRestart = CountDownLatch(1) + + override fun onPropertyChange(name: String, value: Any?) = Unit + + override fun onEvent(name: String, data: Map?) { + when (name) { + "file-loaded" -> fileLoaded.countDown() + "playback-restart" -> playbackRestart.countDown() + } + } + } + + private companion object { + const val CYCLE_COUNT = 8 + const val OPERATION_TIMEOUT_SECONDS = 10L + const val DISPOSE_TIMEOUT_SECONDS = 15L + } +} diff --git a/android/app/src/debug/AndroidManifest.xml b/android/app/src/debug/AndroidManifest.xml index 399f6981d..f0336a72c 100644 --- a/android/app/src/debug/AndroidManifest.xml +++ b/android/app/src/debug/AndroidManifest.xml @@ -4,4 +4,10 @@ to allow setting breakpoints, to provide hot reload, etc. --> + + + diff --git a/android/app/src/debug/kotlin/com/edde746/plezy/mpv/MpvLifecycleTestActivity.kt b/android/app/src/debug/kotlin/com/edde746/plezy/mpv/MpvLifecycleTestActivity.kt new file mode 100644 index 000000000..af0b75a5c --- /dev/null +++ b/android/app/src/debug/kotlin/com/edde746/plezy/mpv/MpvLifecycleTestActivity.kt @@ -0,0 +1,18 @@ +package com.edde746.plezy.mpv + +import android.app.Activity +import android.os.Bundle +import android.view.WindowManager +import android.widget.FrameLayout + +class MpvLifecycleTestActivity : Activity() { + override fun onCreate(savedInstanceState: Bundle?) { + window.addFlags( + WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD or + WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON or + WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON + ) + super.onCreate(savedInstanceState) + setContentView(FrameLayout(this)) + } +} diff --git a/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerCore.kt b/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerCore.kt index b3db277b6..79b457eb7 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerCore.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerCore.kt @@ -1847,6 +1847,8 @@ class MpvPlayerCore private constructor( val osdSv = osdSurfaceView val container = surfaceContainer val contentView = if (audioOnly) null else activity.findViewById(android.R.id.content) + val retiringPlaceholderSurface = placeholderSurface + val retiringPlaceholderImageReader = placeholderImageReader surfaceContainer = null surfaceView = null @@ -1861,9 +1863,7 @@ class MpvPlayerCore private constructor( overlayLayoutListener = null pendingSurface = null - placeholderSurface?.release() placeholderSurface = null - placeholderImageReader?.close() placeholderImageReader = null pausedForSurfaceLoss = false pausedForAudioFocusLoss = false @@ -1879,27 +1879,18 @@ class MpvPlayerCore private constructor( pendingVideoOutputDisableJob = null isInitialized = false - // Detach surface and close player on background thread, then remove views + // Close the player on a background thread, then release surfaces and remove views. if (p != null) { Thread { try { - // Detach surface BEFORE close to prevent GPU mutex contention with - // view removal (audio-only never attached one) - if (!audioOnly) { - try { - runBlocking { - p.setProperty("force-window", "no") - p.setProperty("vo", "null") - } - p.detachSurface() - } catch (e: Exception) { - Log.w(TAG, "Failed to detach surface during dispose", e) - } - } + // Native close blocks through decoder and VO teardown. Keep both the + // SurfaceView surfaces and any attached placeholder alive until it returns. p.close() } catch (e: Exception) { Log.w(TAG, "MPV close failed", e) } + retiringPlaceholderSurface?.release() + retiringPlaceholderImageReader?.close() player = null Log.d(TAG, "Disposed (native)") Handler(Looper.getMainLooper()).post { @@ -1913,6 +1904,8 @@ class MpvPlayerCore private constructor( }.start() } else { // No player — safe to remove views immediately + retiringPlaceholderSurface?.release() + retiringPlaceholderImageReader?.close() Handler(Looper.getMainLooper()).postAtFrontOfQueue { sv?.holder?.removeCallback(this) osdSv?.holder?.removeCallback(osdSurfaceCallback) diff --git a/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerPlugin.kt b/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerPlugin.kt index b66db158f..10c2b74e0 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerPlugin.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/mpv/MpvPlayerPlugin.kt @@ -69,10 +69,10 @@ open class MpvPlayerPlugin( // down that successor's core; it is acknowledged without touching anything. private var coreInstanceId: Long? = null - // How long a Dart `dispose` waits for the native teardown before being - // answered anyway. Generous against slow-but-healthy teardowns (a 4K HDR - // session's surface/audio release); small against the alternative, which - // is wedging every subsequent playback session behind a hung teardown. + // How long a Dart `dispose` waits for native teardown before being + // acknowledged. Native lifecycle operations remain serialized after the + // watchdog fires, so a successor cannot overlap a stuck decoder and exhaust + // the device's codec instances. private val disposeWatchdogMs = 6_000L /** Same semantics as Activity.runOnUiThread, without needing an Activity. */ @@ -356,10 +356,10 @@ open class MpvPlayerPlugin( result.success(null) return@runOnMain } - // A hung native teardown must not wedge the Dart-side release chain: - // answer after the watchdog even if the teardown thread is stuck, so - // the next session can start on a fresh core. The stuck core leaks its - // resources until the process ends — recoverable, unlike the wedge. + // A hung native teardown must not wedge the Dart-side release chain. + // Native create/destroy remains serialized behind that teardown, so a + // successor cannot accumulate another MediaCodec instance while the old + // one still owns its resources. val completed = AtomicBoolean(false) fun completeOnce(reason: String) { if (completed.compareAndSet(false, true)) { diff --git a/android/libmpv/src/main/cpp/main.cpp b/android/libmpv/src/main/cpp/main.cpp index 8ae03f69c..5206fcf6c 100644 --- a/android/libmpv/src/main/cpp/main.cpp +++ b/android/libmpv/src/main/cpp/main.cpp @@ -23,25 +23,6 @@ extern "C" { void render_cleanup(JNIEnv* env); -static void* destroy_mpv_thread(void* arg) { - mpv_handle* handle = (mpv_handle*)arg; - mpv_terminate_destroy(handle); - return NULL; -} - -// Fire-and-forget mpv_terminate_destroy on a detached thread. -// Safe because the event thread has been joined and g_mpv cleared — -// no other code references this handle. -static void async_destroy(mpv_handle* handle) { - pthread_t tid; - if (pthread_create(&tid, NULL, destroy_mpv_thread, handle) == 0) { - pthread_detach(tid); - } else { - // Fallback: destroy synchronously if thread creation fails - mpv_terminate_destroy(handle); - } -} - extern "C" { jni_func(void, nativeCreate, jobject appctx); jni_func(void, nativeInit); @@ -80,20 +61,17 @@ jni_func(void, nativeCreate, jobject appctx) { mpv_wakeup(leaked_mpv); pthread_join(event_thread_id, NULL); g_mpv = NULL; + mpv_terminate_destroy(leaked_mpv); render_cleanup(env); } g_mpv = mpv_create(); if (!g_mpv) { die("context init failed"); - if (leaked_mpv) mpv_terminate_destroy(leaked_mpv); return; } mpv_request_log_messages(g_mpv, "v"); - - // Async teardown of leaked handle — doesn't block caller - if (leaked_mpv) async_destroy(leaked_mpv); } jni_func(void, nativeInit) { @@ -128,10 +106,11 @@ jni_func(void, nativeDestroy) { pthread_join(event_thread_id, NULL); g_mpv = NULL; - render_cleanup(env); - // Async teardown — nativeDestroy returns immediately - async_destroy(local_mpv); + // The MediaCodec VO can retain the Surface until final decoder teardown. + // Keep its JNI refs alive for the entire blocking termination. + mpv_terminate_destroy(local_mpv); + render_cleanup(env); } jni_func(void, nativeCommand, jobjectArray jarray) {