From c5867861eb9cc111f98391eb7e779c3e3655153b Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Tue, 28 Apr 2026 04:13:16 +0200 Subject: [PATCH] fix(playback): coalesce concurrent player init to fix MPV hang --- .../com/edde746/plezy/mpv/MpvPlayerPlugin.kt | 85 +++++++++++++++---- lib/mpv/player/platform/player_android.dart | 20 ++++- lib/mpv/player/player_native.dart | 24 +++++- 3 files changed, 105 insertions(+), 24 deletions(-) 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 d06403f4..97c3e91d 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 @@ -28,6 +28,13 @@ class MpvPlayerPlugin : FlutterPlugin, MethodChannel.MethodCallHandler, private val nameToId = mutableMapOf() private var sessionGeneration = 0 + // Pending `MethodChannel.Result`s for an init that is currently in flight. + // Concurrent `invoke('initialize')` calls share the same outcome instead + // of each tearing down the in-flight core and starting their own — which + // was the root cause of #930. + private val pendingInitResults = mutableListOf() + @Volatile private var isInitializing = false + // FlutterPlugin override fun onAttachedToEngine(binding: FlutterPlugin.FlutterPluginBinding) { @@ -58,6 +65,9 @@ class MpvPlayerPlugin : FlutterPlugin, MethodChannel.MethodCallHandler, ++sessionGeneration playerCore?.dispose() playerCore = null + // Any in-flight init callback would never fire (its scope is cancelled + // by dispose), so close out queued callers explicitly. + completePendingInits(success = false) activity = null activityBinding = null Log.d(TAG, "Detached from activity") @@ -123,33 +133,70 @@ class MpvPlayerPlugin : FlutterPlugin, MethodChannel.MethodCallHandler, return } - currentActivity.runOnUiThread { - try { - // Dispose stale core idempotently - playerCore?.dispose() - playerCore = null + // Coalesce concurrent inits: the second caller waits for the first + // call's outcome instead of disposing the in-flight core. The Dart + // side memoizes too, but this is defense in depth for any direct + // `invoke('initialize')` that bypasses _ensureInitialized. + synchronized(pendingInitResults) { + pendingInitResults += result + if (isInitializing) { + Log.d(TAG, "Init already in flight, queuing caller") + return + } + isInitializing = true + } - val gen = ++sessionGeneration - val core = MpvPlayerCore(currentActivity).apply { + currentActivity.runOnUiThread { + val gen: Int + val core: MpvPlayerCore + try { + // Caller invariant: dispose() was already called explicitly, + // OR `playerCore?.isInitialized == true` and we early-exited + // above. We never tear down a core that's mid-initialization. + if (playerCore != null && playerCore?.isInitialized != true) { + Log.w(TAG, "Discarding stale uninitialized core before re-init") + playerCore?.dispose() + playerCore = null + } + + gen = ++sessionGeneration + core = MpvPlayerCore(currentActivity).apply { delegate = this@MpvPlayerPlugin } playerCore = core + } catch (e: Exception) { + Log.e(TAG, "Failed to initialize: ${e.message}", e) + completePendingInits(success = false, errorMessage = e.message) + return@runOnUiThread + } - core.initialize { success -> - if (gen != sessionGeneration || playerCore !== core) { - Log.d(TAG, "Stale init callback (gen=$gen, current=$sessionGeneration)") - result.success(false) - return@initialize - } + core.initialize { success -> + val stale = gen != sessionGeneration || playerCore !== core + if (stale) { + Log.d(TAG, "Stale init callback (gen=$gen, current=$sessionGeneration)") + } else { // Start hidden - now safe because setVisible operates on the container, // not the SurfaceView directly (matching ExoPlayer's approach) core.setVisible(false) Log.d(TAG, "Initialized: $success") - result.success(success) } - } catch (e: Exception) { - Log.e(TAG, "Failed to initialize: ${e.message}", e) - result.error("INIT_FAILED", e.message, null) + completePendingInits(success = !stale && success) + } + } + } + + private fun completePendingInits(success: Boolean, errorMessage: String? = null) { + val pending = synchronized(pendingInitResults) { + isInitializing = false + val copy = pendingInitResults.toList() + pendingInitResults.clear() + copy + } + for (r in pending) { + if (errorMessage != null) { + r.error("INIT_FAILED", errorMessage, null) + } else { + r.success(success) } } } @@ -160,6 +207,10 @@ class MpvPlayerPlugin : FlutterPlugin, MethodChannel.MethodCallHandler, ++sessionGeneration playerCore = null + // Any in-flight init callback is cancelled with the scope, so + // close out queued callers here instead of leaking them. + completePendingInits(success = false) + core?.dispose { Log.d(TAG, "Disposed") result.success(null) diff --git a/lib/mpv/player/platform/player_android.dart b/lib/mpv/player/platform/player_android.dart index b166b60b..9aeb8fff 100644 --- a/lib/mpv/player/platform/player_android.dart +++ b/lib/mpv/player/platform/player_android.dart @@ -54,20 +54,31 @@ class PlayerAndroid extends PlayerBase { // Initialization // ============================================ + // Memoizes the in-flight init Future so concurrent callers share one + // `invoke('initialize')`. ExoPlayer's native handleInitialize is + // synchronous and would mask a Dart-side race anyway, but we mirror the + // pattern from PlayerNative for consistency and to avoid a partial-init + // hole if any observeProperty call throws. + Future? _initFuture; + Future _ensureInitialized() async { if (initialized) return; + return _initFuture ??= _doInitialize(); + } + Future _doInitialize() async { try { final result = await invoke('initialize', { 'bufferSizeBytes': _bufferSizeBytes, 'tunnelingEnabled': _tunnelingEnabled, }); - initialized = result == true; - if (!initialized) { + if (result != true) { throw Exception('Failed to initialize ExoPlayer'); } - // Register property observers so the plugin knows propId mappings + // Register property observers before flipping `initialized` so partial + // failures don't leave us in a half-initialized state that the memoized + // future would falsely treat as ready. await observeProperty('time-pos', 'double'); await observeProperty('duration', 'double'); await observeProperty('seekable', 'flag'); @@ -80,7 +91,10 @@ class PlayerAndroid extends PlayerBase { await observeProperty('aid', 'string'); await observeProperty('sid', 'string'); await observeProperty('demuxer-cache-time', 'double'); + + initialized = true; } catch (e) { + _initFuture = null; errorController.add(PlayerError('Initialization failed: $e')); rethrow; } diff --git a/lib/mpv/player/player_native.dart b/lib/mpv/player/player_native.dart index 9d040bce..4b10f97c 100644 --- a/lib/mpv/player/player_native.dart +++ b/lib/mpv/player/player_native.dart @@ -37,23 +37,36 @@ class PlayerNative extends PlayerBase { // Initialization // ============================================ + // Memoizes the in-flight init Future so concurrent callers (e.g. the + // parallel `requestAudioFocus()` and `setProperty()` paths kicked off in + // VideoPlayerScreen._initializePlayer) share one `invoke('initialize')`. + // Two concurrent invokes on Android caused MpvPlayerPlugin.handleInitialize + // to dispose-and-recreate the in-flight core, hanging playback (#930). + Future? _initFuture; + Future _ensureInitialized() async { if (initialized) return; + return _initFuture ??= _doInitialize(); + } + Future _doInitialize() async { try { final result = await invoke('initialize'); + final bool ok; if (result is int) { // Linux: initialize returns the texture ID _textureIdValue = result; - initialized = true; + ok = true; } else { - initialized = result == true; + ok = result == true; } - if (!initialized) { + if (!ok) { throw Exception('Failed to initialize player'); } - // Subscribe to MPV properties + // Subscribe to MPV properties before flipping `initialized` so partial + // failures don't leave us in a half-initialized state that the memoized + // future would falsely treat as ready. await observeProperty('time-pos', 'double'); await observeProperty('duration', 'double'); await observeProperty('seekable', 'flag'); @@ -69,7 +82,10 @@ class PlayerNative extends PlayerBase { await observeProperty('demuxer-cache-state', _nodeFormat); await observeProperty('audio-device-list', _nodeFormat); await observeProperty('audio-device', 'string'); + + initialized = true; } catch (e) { + _initFuture = null; errorController.add(PlayerError('Initialization failed: $e')); rethrow; }