diff --git a/lib/mpv/player/platform/player_android.dart b/lib/mpv/player/platform/player_android.dart index fcd30caae..267ce44a6 100644 --- a/lib/mpv/player/platform/player_android.dart +++ b/lib/mpv/player/platform/player_android.dart @@ -1,4 +1,5 @@ import 'package:collection/collection.dart'; +import 'package:flutter/foundation.dart' show visibleForTesting; import 'package:flutter/services.dart'; import '../../../services/device_performance.dart'; @@ -471,16 +472,30 @@ class PlayerAndroid extends PlayerBase { } } + /// First successful (positive) [getHeapSize] result; the device heap is + /// immutable per process, so one channel round trip serves every caller. + static int? _cachedHeapSizeMB; + /// Returns the device's large heap size in MB, or 0 if unavailable (Android only). + /// + /// Successful positive results are memoized — the playback-open hot path asks + /// again on every open. Failures keep returning 0 without being cached, so a + /// transient channel error can't pin the fallback for the process lifetime. static Future getHeapSize() async { + final cached = _cachedHeapSizeMB; + if (cached != null) return cached; try { final result = await _methodChannel.invokeMethod('getHeapSize'); + if (result != null && result > 0) _cachedHeapSizeMB = result; return result ?? 0; } catch (e) { return 0; } } + @visibleForTesting + static void debugResetHeapSizeCache() => _cachedHeapSizeMB = null; + @override Future runtimePlayerType() async { if (disposed) return 'unknown'; diff --git a/lib/screens/video_player/parts/playback_open.dart b/lib/screens/video_player/parts/playback_open.dart index 8b58938c5..91a30b374 100644 --- a/lib/screens/video_player/parts/playback_open.dart +++ b/lib/screens/video_player/parts/playback_open.dart @@ -439,11 +439,17 @@ extension _VideoPlayerOpenMethods on VideoPlayerScreenState { } } - /// Push the user's subtitle style to the native rendering layer (no-op on - /// mpv backends, which style via `sub-*` properties). Must run after - /// open() since that's when ExoPlayer initializes its subtitle views. - Future _applyNativeSubtitleStyle(Player player, SettingsService settingsService) { - return player.setSubtitleStyle( + /// Push the user's subtitle style to the native rendering layer. Must run + /// after open() since that's when ExoPlayer initializes its subtitle views. + /// Only the ExoPlayer backend consumes it — [Player.setSubtitleStyle] is a + /// no-op on every mpv backend, which styles via `sub-*` properties — so the + /// style settings reads are skipped there. Gated on the same + /// configured-backend signal as track_controls/video_settings_sheet; the + /// Android mpv fallback keeps playerType 'exoplayer' and still receives the + /// call, exactly as before. + Future _applyNativeSubtitleStyle(Player player, SettingsService settingsService) async { + if (player.playerType != 'exoplayer') return; + await player.setSubtitleStyle( fontSize: settingsService.read(SettingsService.subtitleFontSize).toDouble(), textColor: settingsService.read(SettingsService.subtitleTextColor), borderSize: settingsService.read(SettingsService.subtitleBorderSize).toDouble(), diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index fbc5efde4..5566a3b28 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -1261,7 +1261,9 @@ class VideoPlayerScreenState extends State with WidgetsBindin initPhase = 'loading settings'; final settingsService = await SettingsService.getInstance(); - if (!_isPlayerInitializationCurrent(generation)) return; + // Literal `mounted` check: the kickoff block below reads `context`, and + // the lint cannot see the mounted check inside the helper. + if (!mounted || !_isPlayerInitializationCurrent(generation)) return; _autoPipEnabled = settingsService.read(SettingsService.autoPip); _exitFullscreenOnPlayerClose = settingsService.read(SettingsService.exitFullscreenOnPlayerClose); _rewindOnResume = settingsService.read(SettingsService.rewindOnResume); @@ -1271,37 +1273,15 @@ class VideoPlayerScreenState extends State with WidgetsBindin final debugLoggingEnabled = settingsService.read(SettingsService.enableDebugLogging); final useExoPlayer = settingsService.read(SettingsService.useExoPlayer); - if (Platform.isWindows) { - initPhase = 'syncing display mode'; - _displayModeService = DisplayModeService(settingsService, FullscreenStateManager()); - await _displayModeService!.syncWithNative(); - if (!_isPlayerInitializationCurrent(generation)) return; - if (!_fullscreenListenerAttached) { - FullscreenStateManager().addListener(_onFullscreenChanged); - _fullscreenListenerAttached = true; - } - } - - // One-native-instance rule: a live music session owns the only audio - // core — stop it and wait for its dispose before constructing the - // video core (see PlaybackCoordinator). - initPhase = 'claiming playback session'; - await PlaybackCoordinator.instance.claimVideo(); - if (!mounted || generation != _playerInitializationGeneration) return; - - initPhase = 'creating player'; - final currentPlayer = Player(useExoPlayer: useExoPlayer); - attemptPlayer = currentPlayer; - if (!mounted || generation != _playerInitializationGeneration) return; - if (Platform.isAndroid && useExoPlayer) { - await currentPlayer.setLogLevel(debugLoggingEnabled ? 'v' : 'warn'); - if (!mounted || generation != _playerInitializationGeneration) return; - } - - // Kick off getPlaybackData() in parallel with the rest of MPV setup. + // Kick off getPlaybackData() before the Windows display-mode sync, the + // music-session teardown in claimVideo(), player construction, and the + // whole mpv property chain below: the resolve depends only on settings + + // provider lookups, so starting it first hides all of that setup behind + // the network round trip(s). // The network/DB work has no dependency on the player — it just needs - // the context (providers), which is still safe to touch here because - // no async gaps invalidate it before the calls below read it. + // the context (providers), which is still safe to touch here because no + // async gaps invalidate it between the guard after the settings await + // and the reads below. // Skipped for live TV (has its own tune path) and offline (its own // branch in _startPlayback). if (!widget.isLive && !_offlineLibraryMode) { @@ -1349,6 +1329,33 @@ class VideoPlayerScreenState extends State with WidgetsBindin _playbackDataFuture!.ignore(); } + if (Platform.isWindows) { + initPhase = 'syncing display mode'; + _displayModeService = DisplayModeService(settingsService, FullscreenStateManager()); + await _displayModeService!.syncWithNative(); + if (!_isPlayerInitializationCurrent(generation)) return; + if (!_fullscreenListenerAttached) { + FullscreenStateManager().addListener(_onFullscreenChanged); + _fullscreenListenerAttached = true; + } + } + + // One-native-instance rule: a live music session owns the only audio + // core — stop it and wait for its dispose before constructing the + // video core (see PlaybackCoordinator). + initPhase = 'claiming playback session'; + await PlaybackCoordinator.instance.claimVideo(); + if (!mounted || generation != _playerInitializationGeneration) return; + + initPhase = 'creating player'; + final currentPlayer = Player(useExoPlayer: useExoPlayer); + attemptPlayer = currentPlayer; + if (!mounted || generation != _playerInitializationGeneration) return; + if (Platform.isAndroid && useExoPlayer) { + await currentPlayer.setLogLevel(debugLoggingEnabled ? 'v' : 'warn'); + if (!mounted || generation != _playerInitializationGeneration) return; + } + if (!_isPlayerInitializationCurrent(generation)) return; initPhase = 'configuring player'; await currentPlayer.configureSubtitleFonts(); diff --git a/lib/utils/video_player_navigation.dart b/lib/utils/video_player_navigation.dart index 72ce38f10..109705efc 100644 --- a/lib/utils/video_player_navigation.dart +++ b/lib/utils/video_player_navigation.dart @@ -306,24 +306,39 @@ Future navigateToVideoPlayer( } try { - // Check if external player is enabled + // Check if external player is enabled. The platform guard comes first so + // platforms that can never launch an external player skip the settings + // lookup, and the singleton the saved-version resolve above already + // initialized is reused — getInstance() is awaited at most once per + // launch on this path. try { - final settingsService = await SettingsService.getInstance(); - if (PlatformDetector.supportsExternalPlayers() && settingsService.read(SettingsService.useExternalPlayer)) { - bool launched = false; + if (PlatformDetector.supportsExternalPlayers()) { + final settingsService = SettingsService.instanceOrNull ?? await SettingsService.getInstance(); + if (settingsService.read(SettingsService.useExternalPlayer)) { + bool launched = false; - if (isOffline) { - final globalKey = metadata.globalKey; - final videoPath = await downloadProvider.getVideoFilePath( - globalKey, - mediaIndex: mediaIndex, - mediaSourceId: mediaSourceId, - ); - if (videoPath != null && context.mounted) { - final videoUrl = videoPath.contains('://') ? videoPath : 'file://$videoPath'; + if (isOffline) { + final globalKey = metadata.globalKey; + final videoPath = await downloadProvider.getVideoFilePath( + globalKey, + mediaIndex: mediaIndex, + mediaSourceId: mediaSourceId, + ); + if (videoPath != null && context.mounted) { + final videoUrl = videoPath.contains('://') ? videoPath : 'file://$videoPath'; + launched = await ExternalPlayerService.launch( + context: context, + videoUrl: videoUrl, + metadata: metadata, + client: mediaClient, + offlineWatchService: offlineWatchService, + mediaIndex: mediaIndex, + mediaSourceId: mediaSourceId, + ); + } + } else if (context.mounted) { launched = await ExternalPlayerService.launch( context: context, - videoUrl: videoUrl, metadata: metadata, client: mediaClient, offlineWatchService: offlineWatchService, @@ -331,22 +346,13 @@ Future navigateToVideoPlayer( mediaSourceId: mediaSourceId, ); } - } else if (context.mounted) { - launched = await ExternalPlayerService.launch( - context: context, - metadata: metadata, - client: mediaClient, - offlineWatchService: offlineWatchService, - mediaIndex: mediaIndex, - mediaSourceId: mediaSourceId, - ); - } - if (launched) { - // External playback never reaches the in-player session commit, so - // record the local last-played history here. - if (!isOffline) unawaited(LocalPlaybackHistory.recordPlayback(metadata)); - return null; + if (launched) { + // External playback never reaches the in-player session commit, so + // record the local last-played history here. + if (!isOffline) unawaited(LocalPlaybackHistory.recordPlayback(metadata)); + return null; + } } } } catch (e) { diff --git a/test/mpv/player_android_heap_size_test.dart b/test/mpv/player_android_heap_size_test.dart new file mode 100644 index 000000000..96bf027ff --- /dev/null +++ b/test/mpv/player_android_heap_size_test.dart @@ -0,0 +1,77 @@ +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/mpv/player/platform/player_android.dart'; + +import '../test_helpers/mock_player_channels.dart'; + +/// The device heap is immutable per process, so [PlayerAndroid.getHeapSize] +/// memoizes the first successful result: the second read on the playback-open +/// hot path (`_applyNetworkStreamTuning`) must not pay a channel round trip. +/// Failures keep the documented `0` sentinel and must never be pinned. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUp(PlayerAndroid.debugResetHeapSizeCache); + + Future withHeapSizeHandler({ + required Future Function(MethodCall call) methodHandler, + required Future Function() testBody, + }) { + return withMockPlayerChannels( + methodChannelName: 'com.plezy/exo_player', + eventChannelName: 'com.plezy/exo_player/events', + methodHandler: methodHandler, + testBody: testBody, + ); + } + + test('a successful heap read is served from cache on subsequent calls', () async { + var channelCalls = 0; + await withHeapSizeHandler( + methodHandler: (call) async { + if (call.method != 'getHeapSize') return null; + channelCalls++; + return 512; + }, + testBody: () async { + expect(await PlayerAndroid.getHeapSize(), 512); + expect(await PlayerAndroid.getHeapSize(), 512); + }, + ); + expect(channelCalls, 1); + }); + + test('a failed heap read returns 0 and is not cached', () async { + await withHeapSizeHandler( + methodHandler: (call) async => throw PlatformException(code: 'unavailable'), + testBody: () async { + expect(await PlayerAndroid.getHeapSize(), 0); + }, + ); + // The channel recovers: the earlier failure must not have pinned the 0 + // sentinel, or Auto buffer sizing would stay degraded for the whole run. + await withHeapSizeHandler( + methodHandler: (call) async => call.method == 'getHeapSize' ? 256 : null, + testBody: () async { + expect(await PlayerAndroid.getHeapSize(), 256); + }, + ); + }); + + test('a null heap report keeps the 0 sentinel without caching it', () async { + var channelCalls = 0; + await withHeapSizeHandler( + methodHandler: (call) async { + if (call.method != 'getHeapSize') return null; + channelCalls++; + return null; + }, + testBody: () async { + expect(await PlayerAndroid.getHeapSize(), 0); + expect(await PlayerAndroid.getHeapSize(), 0); + }, + ); + // Unavailable is re-asked, never memoized. + expect(channelCalls, 2); + }); +}