diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt index cb947b8e..fd5b930d 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt @@ -27,7 +27,6 @@ class ExoPlayerPlugin : private const val TAG = "ExoPlayerPlugin" private const val METHOD_CHANNEL = "com.plezy/exo_player" private const val EVENT_CHANNEL = "com.plezy/exo_player/events" - private const val MPV_FALLBACK_SWITCH_TIMEOUT_MS = 15_000L } private lateinit var methodChannel: MethodChannel @@ -37,7 +36,6 @@ class ExoPlayerPlugin : private var mpvCore: MpvPlayerCore? = null // MPV fallback player private var usingMpvFallback: Boolean = false private var fallbackInProgress: Boolean = false - private var pendingMpvFallbackSwitchGeneration: Int? = null private var activity: Activity? = null private var activityBinding: ActivityPluginBinding? = null @@ -60,6 +58,7 @@ class ExoPlayerPlugin : // fallback is already active) so one playback's properties never leak into // the next session's fallback. private val pendingMpvProperties = LinkedHashMap() + private var currentExternalSubtitles: List>? = null // FlutterPlugin @@ -95,7 +94,7 @@ class ExoPlayerPlugin : mpvCore = null usingMpvFallback = false fallbackInProgress = false - clearPendingMpvFallbackSwitch() + currentExternalSubtitles = null pendingMpvProperties.clear() activity = null activityBinding = null @@ -111,7 +110,6 @@ class ExoPlayerPlugin : override fun onDetachedFromActivityForConfigChanges() { sessionGeneration++ fallbackInProgress = false - clearPendingMpvFallbackSwitch() activity = null activityBinding = null Log.d(TAG, "Detached from activity for config changes") @@ -216,7 +214,6 @@ class ExoPlayerPlugin : mpvCore = null usingMpvFallback = false fallbackInProgress = false - clearPendingMpvFallbackSwitch() } try { @@ -254,7 +251,7 @@ class ExoPlayerPlugin : mpvCore = null usingMpvFallback = false fallbackInProgress = false - clearPendingMpvFallbackSwitch() + currentExternalSubtitles = null pendingMpvProperties.clear() Log.d(TAG, "Disposed") result.success(null) @@ -275,13 +272,14 @@ class ExoPlayerPlugin : result.error("INVALID_ARGS", "Missing 'uri'", null) return } + val externalSubtitleSnapshot = externalSubtitles?.map { it.toMap() } + currentExternalSubtitles = externalSubtitleSnapshot // Only clear pending MPV state when MPV is the active backend. A same-core // MPV reload must not inherit a fallback switch that was armed for the // previous load. When ExoPlayer is active, keep queued properties for a // potential ExoPlayer→MPV fallback. if (usingMpvFallback) { - clearPendingMpvFallbackSwitch() pendingMpvProperties.clear() } @@ -294,6 +292,7 @@ class ExoPlayerPlugin : if (!autoPlay) options.add("pause=yes") options.add("sid=no") options.add("secondary-sid=no") + appendExternalSubtitleOptions(options, externalSubtitleSnapshot) headers?.forEach { (key, value) -> options.add("http-header-fields-append=$key: $value") } @@ -306,7 +305,7 @@ class ExoPlayerPlugin : } } } else { - playerCore?.open(uri, headers, startPositionMs, autoPlay, isLive, externalSubtitles) + playerCore?.open(uri, headers, startPositionMs, autoPlay, isLive, externalSubtitleSnapshot) } result.success(null) } ?: result.error("NO_ACTIVITY", "Activity not available", null) @@ -751,34 +750,12 @@ class ExoPlayerPlugin : } override fun onEvent(name: String, data: Map?) { - val pendingFallbackSwitch = - name == "file-loaded" && pendingMpvFallbackSwitchGeneration == sessionGeneration && usingMpvFallback - if (pendingFallbackSwitch) { - clearPendingMpvFallbackSwitch() - } - val event = eventPayload(name, data) - mainHandler.post { - eventSink?.success(event) - if (pendingFallbackSwitch) { - eventSink?.success(eventPayload("backend-switched")) - } - } + mainHandler.post { eventSink?.success(event) } } - private fun armPendingMpvFallbackSwitch(generation: Int) { - pendingMpvFallbackSwitchGeneration = generation - mainHandler.postDelayed({ - if (pendingMpvFallbackSwitchGeneration == generation && sessionGeneration == generation && usingMpvFallback) { - Log.w(TAG, "Timed out waiting for MPV fallback file-loaded before backend-switched") - clearPendingMpvFallbackSwitch() - onEvent("backend-switched", null) - } - }, MPV_FALLBACK_SWITCH_TIMEOUT_MS) - } - - private fun clearPendingMpvFallbackSwitch() { - pendingMpvFallbackSwitchGeneration = null + private fun notifyBackendSwitched() { + mainHandler.post { eventSink?.success(eventPayload("backend-switched")) } } /** @@ -803,6 +780,26 @@ class ExoPlayerPlugin : } } + private fun appendExternalSubtitleOptions( + options: MutableList, + externalSubtitles: List>? + ) { + val escapedUris = externalSubtitles.orEmpty() + .mapNotNull { it["uri"] as? String } + .filter { it.isNotEmpty() } + .map(::escapeMpvPathListEntry) + .toList() + + if (escapedUris.isEmpty()) return + + val pathList = escapedUris.joinToString(":") + options.add("sub-files=%${pathList.toByteArray(Charsets.UTF_8).size}%$pathList") + } + + private fun escapeMpvPathListEntry(value: String): String { + return value.replace("\\", "\\\\").replace(":", "\\:") + } + /** * Configure a freshly initialized MPV fallback core: replay the properties * and observers Dart registered against the ExoPlayer session, then resume @@ -814,7 +811,8 @@ class ExoPlayerPlugin : act: Activity, uri: String, headers: Map?, - positionMs: Long + positionMs: Long, + externalSubtitles: List>? ) { // Snapshot Dart-registered state on main thread before clearing val pendingProps = pendingMpvProperties.toList() @@ -831,7 +829,6 @@ class ExoPlayerPlugin : if (mpvCore !== core) { core.dispose() fallbackInProgress = false - clearPendingMpvFallbackSwitch() return } // Configure basic MPV properties for Plex playback @@ -862,11 +859,14 @@ class ExoPlayerPlugin : val startSeconds = positionMs / 1000.0 val options = mutableListOf() options.add(if (positionMs > 0L) "start=$startSeconds" else "start=none") + options.add("sid=no") + options.add("secondary-sid=no") + appendExternalSubtitleOptions(options, externalSubtitles) headers?.forEach { (key, value) -> options.add("http-header-fields-append=$key: $value") } val optionsStr = options.joinToString(",") - armPendingMpvFallbackSwitch(sessionGeneration) + notifyBackendSwitched() core.command(arrayOf("loadfile", mpvUri, "replace", "-1", optionsStr)) // On GPUs without compute shaders, MPV can't do dynamic peak detection @@ -900,6 +900,7 @@ class ExoPlayerPlugin : } val currentActivity = activity ?: return false + val fallbackExternalSubtitles = currentExternalSubtitles?.map { it.toMap() } fallbackInProgress = true Log.i(TAG, "ExoPlayer error, switching to MPV fallback at ${positionMs}ms: $errorMessage") @@ -922,20 +923,17 @@ class ExoPlayerPlugin : mpvCore?.dispose() mpvCore = null usingMpvFallback = false // Clear before handoff - clearPendingMpvFallbackSwitch() val generation = sessionGeneration Handler(Looper.getMainLooper()).post { if (generation != sessionGeneration) { fallbackInProgress = false - clearPendingMpvFallbackSwitch() return@post } val act = activity if (act == null) { fallbackInProgress = false - clearPendingMpvFallbackSwitch() return@post } @@ -952,7 +950,6 @@ class ExoPlayerPlugin : mpvCore = null } fallbackInProgress = false - clearPendingMpvFallbackSwitch() return@initialize } if (!success) { @@ -961,7 +958,6 @@ class ExoPlayerPlugin : mpvCore = null } fallbackInProgress = false - clearPendingMpvFallbackSwitch() Log.e(TAG, "Failed to initialize MPV fallback") onEvent("end-file", mapOf("reason" to "error", "message" to "Fallback failed: $errorMessage")) return@initialize @@ -970,18 +966,16 @@ class ExoPlayerPlugin : usingMpvFallback = true fallbackInProgress = false - setupMpvFallback(core, act, uri, headers, positionMs) + setupMpvFallback(core, act, uri, headers, positionMs, fallbackExternalSubtitles) } } catch (e: Exception) { fallbackInProgress = false - clearPendingMpvFallbackSwitch() Log.e(TAG, "Failed to switch to MPV fallback", e) onEvent("end-file", mapOf("reason" to "error", "message" to "Fallback failed: ${e.message}")) } } } catch (e: Exception) { fallbackInProgress = false - clearPendingMpvFallbackSwitch() Log.e(TAG, "Failed to switch to MPV fallback", e) onEvent("end-file", mapOf("reason" to "error", "message" to "Fallback failed: ${e.message}")) } diff --git a/lib/mpv/player/platform/player_android.dart b/lib/mpv/player/platform/player_android.dart index edcf4b59..a1eb947c 100644 --- a/lib/mpv/player/platform/player_android.dart +++ b/lib/mpv/player/platform/player_android.dart @@ -39,11 +39,11 @@ class PlayerAndroid extends PlayerBase { @override bool get supportsSecondarySubtitles => false; - // Under the mpv fallback the native open path drops the externalSubtitles - // argument, so subsequent opens must use the post-open sub-add dance - // (handleAddSubtitleTrack routes to mpv natively). + // ExoPlayer attaches external subtitles to the MediaItem before prepare; + // the Android mpv fallback mirrors PlayerNative by passing sub-files through + // loadfile options. @override - bool get attachesExternalSubtitlesAtOpen => !_usingMpvFallback; + bool get attachesExternalSubtitlesAtOpen => true; // The fallback runs mpv over MediaCodec — the same display-switch decoder // constraint as PlayerNative on Android. The whole startup-gate chain @@ -67,8 +67,11 @@ class PlayerAndroid extends PlayerBase { // Native player switched from ExoPlayer to MPV due to unsupported format. // Clear stale ExoPlayer tracks so applyTrackSelectionWhenReady waits for // mpv's track-list instead of immediately applying with ExoPlayer IDs. + final wasUsingMpvFallback = _usingMpvFallback; _usingMpvFallback = true; - clearTracks(); + if (!wasUsingMpvFallback) { + clearTracks(); + } backendSwitchedController.add(null); return; } @@ -132,6 +135,7 @@ class PlayerAndroid extends PlayerBase { // again on Android ExoPlayer or seeks/progress jump to roughly 2x (#1221). configureTimeline(offset: Duration.zero, duration: timelineDuration); clearTracks(); + setExternalSubtitleMetadata(externalSubtitles); setSeekable(false); // Show the video layer diff --git a/lib/mpv/player/player_base.dart b/lib/mpv/player/player_base.dart index 23ea3aca..be2ef9a7 100644 --- a/lib/mpv/player/player_base.dart +++ b/lib/mpv/player/player_base.dart @@ -56,6 +56,7 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { Duration? _timelineDuration; int _nextPropId = 0; final Map _propIdToName = {}; + Map _externalSubtitleMetadataByUri = const {}; @protected bool initialized = false; @@ -447,16 +448,18 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { } else if (type == 'sub') { if (selected) selectedSubtitleId = id; final codec = track['codec'] as String?; + final externalFilename = track['external-filename'] as String?; + final externalMetadata = externalFilename == null ? null : _externalSubtitleMetadataByUri[externalFilename]; subtitleTracks.add( SubtitleTrack( id: id, - title: cleanSubtitleTitle(track['title'] as String?, codec: codec), - language: cleanTrackMetadataValue(track['lang'] as String?), - codec: codec, - isDefault: track['default'] as bool? ?? false, - isForced: track['forced'] as bool? ?? false, + title: externalMetadata?.title ?? cleanSubtitleTitle(track['title'] as String?, codec: codec), + language: externalMetadata?.language ?? cleanTrackMetadataValue(track['lang'] as String?), + codec: externalMetadata?.codec ?? codec, + isDefault: externalMetadata?.isDefault ?? (track['default'] as bool? ?? false), + isForced: externalMetadata?.isForced ?? (track['forced'] as bool? ?? false), isExternal: track['external'] as bool? ?? false, - uri: track['external-filename'] as String?, + uri: externalFilename, ), ); } @@ -514,6 +517,18 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { tracksController.add(empty); } + @protected + void setExternalSubtitleMetadata(List? externalSubtitles) { + final metadataByUri = {}; + for (final subtitle in externalSubtitles ?? const []) { + final uri = subtitle.uri; + if (uri != null && uri.isNotEmpty) { + metadataByUri[uri] = subtitle; + } + } + _externalSubtitleMetadataByUri = metadataByUri; + } + @protected void setVolumeState(double volume) { if (_state.volume == volume) return; diff --git a/lib/mpv/player/player_native.dart b/lib/mpv/player/player_native.dart index b479d54d..f950133c 100644 --- a/lib/mpv/player/player_native.dart +++ b/lib/mpv/player/player_native.dart @@ -1,3 +1,4 @@ +import 'dart:convert'; import 'dart:io' show Platform; import 'package:flutter/services.dart'; @@ -35,6 +36,9 @@ class PlayerNative extends PlayerBase { @override bool get providesNativeStats => Platform.isAndroid; + @override + bool get attachesExternalSubtitlesAtOpen => true; + /// Node properties are returned as structured maps on macOS/iOS/Linux, /// but as JSON strings on Android/Windows. static final String _nodeFormat = (Platform.isAndroid || Platform.isWindows) ? 'string' : 'node'; @@ -55,6 +59,27 @@ class PlayerNative extends PlayerBase { }; } + static String _fixedLengthQuote(String value) { + return '%${utf8.encode(value).length}%$value'; + } + + static String _escapePathListEntry(String value, String separator) { + return value.replaceAll(r'\', r'\\').replaceAll(separator, '\\$separator'); + } + + static String? _externalSubtitlesLoadfileOption(List? externalSubtitles) { + final separator = Platform.isWindows ? ';' : ':'; + final escapedUris = externalSubtitles + ?.map((subtitle) => subtitle.uri) + .whereType() + .where((uri) => uri.isNotEmpty) + .map((uri) => _escapePathListEntry(uri, separator)) + .toList(); + if (escapedUris == null || escapedUris.isEmpty) return null; + + return 'sub-files=${_fixedLengthQuote(escapedUris.join(separator))}'; + } + MediaDisplayCriteria? _effectiveDisplayCriteria(MediaDisplayCriteria? criteria) { if (criteria == null || (criteria.doviProfile ?? 0) != 7) return criteria; @@ -151,6 +176,7 @@ class PlayerNative extends PlayerBase { final startPosition = media.start ?? Duration.zero; configureTimeline(offset: timelineOffset, duration: timelineDuration); clearTracks(); + setExternalSubtitleMetadata(externalSubtitles); resetPlaybackProgress(startPosition); setSeekable(false); @@ -187,7 +213,12 @@ class PlayerNative extends PlayerBase { } } - await command(['loadfile', uri, 'replace']); + final loadfileArgs = ['loadfile', uri, 'replace']; + final loadfileOption = _externalSubtitlesLoadfileOption(externalSubtitles); + if (loadfileOption != null) { + loadfileArgs.addAll(['-1', loadfileOption]); + } + await command(loadfileArgs); // mpv's pause property survives loadfile; in-place reloads pause the old // file before resolving, so explicitly unpause for the replacement. Set diff --git a/lib/screens/video_player/parts/episode_navigation.dart b/lib/screens/video_player/parts/episode_navigation.dart index 12a729c5..bd94953c 100644 --- a/lib/screens/video_player/parts/episode_navigation.dart +++ b/lib/screens/video_player/parts/episode_navigation.dart @@ -446,7 +446,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { externalSubtitlePlan: externalSubtitlePlan, // Same guard as the start path: don't resume a player a newer flow // owns, and let a pending startup gate (or Watch Together's group - // start) own the resume instead. Android mpv external-subtitle opens + // start) own the resume instead. Post-open external-subtitle paths // resume once here so the startup refresh gate can observe a frame. shouldResumeAfterSubtitleLoad: () => (!frameRatePlan.holdPlaybackStart || resumeForStartupFrame) && diff --git a/lib/screens/video_player/parts/playback_open.dart b/lib/screens/video_player/parts/playback_open.dart index 8f99e9c5..7d806824 100644 --- a/lib/screens/video_player/parts/playback_open.dart +++ b/lib/screens/video_player/parts/playback_open.dart @@ -405,9 +405,10 @@ extension _VideoPlayerOpenMethods on VideoPlayerScreenState { ); } - /// Apply track selection for a freshly opened source: mpv backends get - /// external subtitles via the post-open sub-add dance (opened paused to - /// avoid the issue #226 race), others arm selection directly. + /// Apply track selection for a freshly opened source: backends that cannot + /// attach external subtitles during open use the post-open sub-add dance + /// (opened paused to avoid the issue #226 race), others arm selection + /// directly. /// [shouldResumeAfterSubtitleLoad] lets a startup gate own the resume. /// [applySelectionWhenResumeSkipped] is for flows that legitimately stay /// paused (e.g. a transcode restart while paused): selection is still diff --git a/lib/screens/video_player/parts/playback_start.dart b/lib/screens/video_player/parts/playback_start.dart index 4c3a045c..d4cce2c2 100644 --- a/lib/screens/video_player/parts/playback_start.dart +++ b/lib/screens/video_player/parts/playback_start.dart @@ -222,9 +222,9 @@ extension _VideoPlayerPlaybackStartMethods on VideoPlayerScreenState { final shouldAutoPlay = !shouldHoldPlaybackStart && !wtOwnsStart && externalSubtitlePlan.canStartBeforeTrackSetup; - // ExoPlayer: attach external subs at open time so it discovers - // them in a single prepare() — no media reload needed for selection. - // MPV (all platforms including Android): external subs added after open via sub-add. + // Backends that support at-open sidecars receive them with open() + // so tracks are discovered in a single prepare/loadfile cycle. Any + // backend that cannot do that still uses the post-open sub-add path. final openTiming = _playbackOpenTiming( backend: _currentMetadata.backend, isTranscoding: result.isTranscoding, @@ -326,9 +326,10 @@ extension _VideoPlayerPlaybackStartMethods on VideoPlayerScreenState { trackManager: _trackManager!, externalSubtitlePlan: externalSubtitlePlan, // When a startup gate below owns the resume, skip this one to - // avoid a double-play. Android mpv external-subtitle opens are the - // exception: after sub-add we must resume once so mpv can produce the - // startup frame that the decoder-refresh gate is waiting for. + // avoid a double-play. Post-open external-subtitle paths are the + // exception: after they attach we must resume once so mpv can + // produce the startup frame that the decoder-refresh gate is waiting + // for. // Watch Together stays paused for the group start, so selection is // armed through the resume-skipped branch. shouldResumeAfterSubtitleLoad: () => diff --git a/lib/services/track_manager.dart b/lib/services/track_manager.dart index 27956ac0..e2a444a4 100644 --- a/lib/services/track_manager.dart +++ b/lib/services/track_manager.dart @@ -93,12 +93,11 @@ class TrackManager { _lastExternalSubtitles = externalSubtitles; } - /// Add external subtitle tracks to the player in parallel. + /// Add external subtitle tracks to the player in metadata order. /// - /// Each sub-add does its own HTTP fetch of the sidecar file, so sequential - /// adds dominate startup (~170ms × N). Firing them in parallel lets - /// libavformat's network IO overlap and stops Dart → method channel → native - /// round-trips from stacking. + /// MPV assigns subtitle track IDs in completion order, so parallel sub-adds + /// make the track list nondeterministic. Keep this ordered for the fallback + /// paths that cannot attach sidecars through loadfile. Future addExternalSubtitles(List externalSubtitles, {Future? waitUntilReady}) async { if (externalSubtitles.isEmpty) return; @@ -115,21 +114,19 @@ class TrackManager { appLogger.d('Adding ${externalSubtitles.length} external subtitle(s) to player'); - await Future.wait( - externalSubtitles.where((s) => s.uri != null).map((subtitleTrack) async { - try { - await player.addSubtitleTrack( - uri: subtitleTrack.uri!, - title: subtitleTrack.title, - language: subtitleTrack.language, - select: subtitleTrack.isDefault, - ); - appLogger.d('Added external subtitle: ${subtitleTrack.title ?? subtitleTrack.uri}'); - } catch (e) { - appLogger.w('Failed to add external subtitle: ${subtitleTrack.title ?? subtitleTrack.uri}', error: e); - } - }), - ); + for (final subtitleTrack in externalSubtitles.where((s) => s.uri != null)) { + try { + await player.addSubtitleTrack( + uri: subtitleTrack.uri!, + title: subtitleTrack.title, + language: subtitleTrack.language, + select: subtitleTrack.isDefault, + ); + appLogger.d('Added external subtitle: ${subtitleTrack.title ?? subtitleTrack.uri}'); + } catch (e) { + appLogger.w('Failed to add external subtitle: ${subtitleTrack.title ?? subtitleTrack.uri}', error: e); + } + } } finally { _externalSubtitleAddsInFlight = false; } @@ -261,7 +258,7 @@ class TrackManager { Future onBackendSwitched() async { appLogger.i('Player backend switched from ExoPlayer to MPV (native fallback)'); - if (_lastExternalSubtitles.isNotEmpty) { + if (_lastExternalSubtitles.isNotEmpty && !player.attachesExternalSubtitlesAtOpen) { try { await addExternalSubtitles(_lastExternalSubtitles); } catch (e) { diff --git a/test/mpv/player_open_test.dart b/test/mpv/player_open_test.dart index d3f5ec7a..082816b3 100644 --- a/test/mpv/player_open_test.dart +++ b/test/mpv/player_open_test.dart @@ -1,4 +1,6 @@ import 'dart:async'; +import 'dart:convert'; +import 'dart:io' show Platform; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -86,6 +88,42 @@ void main() { ); }); + test('ExoPlayer backend switch clears stale tracks before fallback tracks arrive', () async { + await _withMockChannels( + methodChannelName: 'com.plezy/exo_player', + eventChannelName: 'com.plezy/exo_player/events', + testBody: () async { + final player = PlayerAndroid(); + try { + _seedTracks(player); + expect(player.state.tracks.audio, isNotEmpty); + expect(player.needsDecoderRefreshAfterDisplaySwitch, isFalse); + + player.handlePlayerEvent('backend-switched', null); + + expect(player.needsDecoderRefreshAfterDisplaySwitch, isTrue); + expect(player.state.tracks.audio, isEmpty); + expect(player.state.tracks.subtitle, isEmpty); + + player.handlePropertyChange('track-list', const [ + {'type': 'audio', 'id': '1', 'title': 'Fallback Audio', 'lang': 'eng'}, + {'type': 'sub', 'id': '2', 'title': 'Fallback Subtitle', 'lang': 'eng'}, + ]); + + expect(player.state.tracks.audio.single.id, '1'); + expect(player.state.tracks.subtitle.single.id, '2'); + + player.handlePlayerEvent('backend-switched', null); + + expect(player.state.tracks.audio.single.id, '1'); + expect(player.state.tracks.subtitle.single.id, '2'); + } finally { + await player.dispose(); + } + }, + ); + }); + test('ExoPlayer applies DV conversion mode changed during in-flight initialization', () async { final initialize = Completer(); final calls = []; @@ -323,6 +361,93 @@ void main() { ); }); + test('MPV passes external subtitles through loadfile options', () async { + final calls = []; + + await _withMockChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) { + calls.add(call); + switch (call.method) { + case 'initialize': + return Future.value(true); + default: + return Future.value(null); + } + }, + testBody: () async { + final player = PlayerNative(); + try { + expect(player.attachesExternalSubtitlesAtOpen, isTrue); + const english = 'https://example.test/library/parts/1/subtitle.srt?token=a,b:c'; + const french = 'https://example.test/subtitles/fr forced.ass'; + + await player.open( + Media('https://example.test/movie.mkv'), + externalSubtitles: const [ + SubtitleTrack(id: 'external-en', uri: english, title: 'English', language: 'eng', codec: 'srt'), + SubtitleTrack(id: 'external-fr', uri: french, title: 'French Forced', language: 'fra', codec: 'ass'), + ], + ); + + expect(_loadfileArgs(calls), [ + 'loadfile', + 'https://example.test/movie.mkv', + 'replace', + '-1', + 'sub-files=${_fixedLengthPathList([english, french])}', + ]); + expect(_commandCalls(calls, 'sub-add'), isEmpty); + } finally { + await player.dispose(); + } + }, + ); + }); + + test('MPV preserves external subtitle metadata for loadfile sidecars', () async { + await _withMockChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + testBody: () async { + final player = PlayerNative(); + try { + const subtitleUri = 'https://example.test/subtitles/en-forced.srt'; + await player.open( + Media('https://example.test/movie.mkv'), + externalSubtitles: const [ + SubtitleTrack( + id: 'server-subtitle', + uri: subtitleUri, + title: 'English Forced', + language: 'eng', + codec: 'srt', + isDefault: true, + isForced: true, + isExternal: true, + ), + ], + ); + + player.handlePropertyChange('track-list', const [ + {'type': 'sub', 'id': '1', 'codec': 'subrip', 'external': true, 'external-filename': subtitleUri}, + ]); + + final subtitle = player.state.tracks.subtitle.single; + expect(subtitle.title, 'English Forced'); + expect(subtitle.language, 'eng'); + expect(subtitle.codec, 'srt'); + expect(subtitle.isDefault, isTrue); + expect(subtitle.isForced, isTrue); + expect(subtitle.uri, subtitleUri); + } finally { + await player.dispose(); + } + }, + ); + }); + test('MPV open(play: true) unpauses after loadfile even when previously paused', () async { final calls = []; @@ -613,3 +738,27 @@ int _loadfileCallIndex(List calls) { return args.isNotEmpty && args.first == 'loadfile'; }); } + +List _loadfileArgs(List calls) { + final loadIndex = _loadfileCallIndex(calls); + expect(loadIndex, greaterThanOrEqualTo(0)); + return Map.from(calls[loadIndex].arguments as Map)['args'] as List; +} + +Iterable _commandCalls(List calls, String command) { + return calls.where((call) { + if (call.method != 'command') return false; + final args = Map.from(call.arguments as Map)['args'] as List; + return args.isNotEmpty && args.first == command; + }); +} + +String _fixedLengthPathList(List values) { + final separator = Platform.isWindows ? ';' : ':'; + final escaped = values.map((value) => _escapePathListEntry(value, separator)).join(separator); + return '%${utf8.encode(escaped).length}%$escaped'; +} + +String _escapePathListEntry(String value, String separator) { + return value.replaceAll(r'\', r'\\').replaceAll(separator, '\\$separator'); +} diff --git a/test/services/track_manager_test.dart b/test/services/track_manager_test.dart index b502900d..e2669ae6 100644 --- a/test/services/track_manager_test.dart +++ b/test/services/track_manager_test.dart @@ -21,8 +21,8 @@ import '../test_helpers/prefs.dart'; // - Constructor wiring (mutable fields are settable, default values). // - `cacheExternalSubtitles` / `lastExternalSubtitles` round-trip. // - `addExternalSubtitles` invokes the player's addSubtitleTrack for each -// entry with a non-null URI, and silently swallows errors thrown by the -// player (the Future.wait branch wraps each item in try/catch). +// entry with a non-null URI, preserves order, and silently swallows errors +// thrown by the player. // - `cycleSubtitleTrack` / `cycleAudioTrack` are no-ops when the player has // fewer than 2 real tracks (early-return paths). // - `applyTrackSelectionWhenReady` waits for subtitle tracks when server @@ -47,7 +47,7 @@ MediaItem _meta({String id = 'rk1'}) => MediaItem(id: id, backend: MediaBackend. /// Player that records calls and can be configured per-test. class _FakePlayer with PlayerStreamControllersMixin implements Player { PlayerState _state; - _FakePlayer({Tracks tracks = const Tracks(), TrackSelection track = const TrackSelection()}) + _FakePlayer({Tracks tracks = const Tracks(), TrackSelection track = const TrackSelection(), this.attachesExternalSubtitlesAtOpen = false}) : _state = PlayerState(tracks: tracks, track: track); @override @@ -58,6 +58,9 @@ class _FakePlayer with PlayerStreamControllersMixin implements Player { @override PlayerStreams get streams => _streams; + @override + final bool attachesExternalSubtitlesAtOpen; + @override bool get disposed => false; @@ -77,6 +80,7 @@ class _FakePlayer with PlayerStreamControllersMixin implements Player { /// If non-null and >0, fail this many addSubtitleTrack calls before succeeding. int failAddSubtitleTimes = 0; + Future Function(String uri)? onAddSubtitleTrack; @override Future addSubtitleTrack({required String uri, String? title, String? language, bool select = false}) async { @@ -85,6 +89,7 @@ class _FakePlayer with PlayerStreamControllersMixin implements Player { throw StateError('simulated addSubtitleTrack failure'); } addSubtitleCalls.add((uri: uri, title: title, language: language, select: select)); + await onAddSubtitleTrack?.call(uri); } @override @@ -233,7 +238,7 @@ void main() { expect(player.addSubtitleCalls, isEmpty); }); - test('forwards each subtitle with a URI to the player in parallel', () async { + test('forwards each subtitle with a URI to the player in metadata order', () async { final player = _FakePlayer(); final mgr = _make(player: player); addTearDown(mgr.dispose); @@ -245,13 +250,39 @@ void main() { await mgr.addExternalSubtitles(subs); expect(player.addSubtitleCalls, hasLength(2)); - // Order is non-deterministic (Future.wait in parallel) — assert by URI set. - final uris = player.addSubtitleCalls.map((c) => c.uri).toSet(); - expect(uris, {'https://example/a.srt', 'https://example/b.srt'}); + expect(player.addSubtitleCalls.map((c) => c.uri), ['https://example/a.srt', 'https://example/b.srt']); // None should be auto-selected — manager picks afterwards. expect(player.addSubtitleCalls.every((c) => c.select == false), isTrue); }); + test('does not start the next add until the previous subtitle completes', () async { + final player = _FakePlayer(); + final mgr = _make(player: player); + addTearDown(mgr.dispose); + final firstCompletes = Completer(); + addTearDown(() { + if (!firstCompletes.isCompleted) firstCompletes.complete(); + }); + player.onAddSubtitleTrack = (uri) async { + if (uri == 'https://example/a.srt') { + await firstCompletes.future; + } + }; + + final addFuture = mgr.addExternalSubtitles([ + SubtitleTrack.uri('https://example/a.srt', title: 'EN'), + SubtitleTrack.uri('https://example/b.srt', title: 'FR'), + ]); + await _drainAsync(); + + expect(player.addSubtitleCalls.map((c) => c.uri), ['https://example/a.srt']); + + firstCompletes.complete(); + await addFuture; + + expect(player.addSubtitleCalls.map((c) => c.uri), ['https://example/a.srt', 'https://example/b.srt']); + }); + test('skips subtitle entries with null URI', () async { final player = _FakePlayer(); final mgr = _make(player: player); @@ -454,6 +485,36 @@ void main() { }); }); + group('onBackendSwitched', () { + test('re-adds cached external subtitles for post-open fallback backends', () async { + final player = _FakePlayer(); + final mgr = _make(player: player); + addTearDown(mgr.dispose); + + mgr.cacheExternalSubtitles([ + SubtitleTrack.uri('https://example/fallback.srt', title: 'EN'), + ]); + + await mgr.onBackendSwitched(); + + expect(player.addSubtitleCalls.map((c) => c.uri), ['https://example/fallback.srt']); + }); + + test('does not duplicate subtitles when fallback attached them at open', () async { + final player = _FakePlayer(attachesExternalSubtitlesAtOpen: true); + final mgr = _make(player: player); + addTearDown(mgr.dispose); + + mgr.cacheExternalSubtitles([ + SubtitleTrack.uri('https://example/fallback.srt', title: 'EN'), + ]); + + await mgr.onBackendSwitched(); + + expect(player.addSubtitleCalls, isEmpty); + }); + }); + group('onSecondarySubtitleTrackChanged', () { test('is a documented no-op', () { final mgr = _make(player: _FakePlayer());