From 0f016e4e130304db1cb3250c0ef9b750b7bdec70 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:46:51 +0200 Subject: [PATCH] fix(player): start the playback fetch before player construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The playback data resolve — the one network request that must land before open() — was kicked off only after the SharedPreferences load, the Windows display-mode sync, the music-session teardown in claimVideo(), Player construction, and (Android/Exo) a native setLogLevel, so none of that setup overlapped the round trip. Move the kickoff to immediately after the settings reads; the resolve depends only on settings and provider lookups, so display sync, player construction, and the whole mpv property chain now hide behind the network latency instead of preceding it. Also remove redundant work elsewhere on the start path: memoize PlayerAndroid.getHeapSize (asked again on every open for an immutable device value), skip the ten subtitle-style settings reads on mpv backends where setSubtitleStyle is a no-op, and stop navigateToVideoPlayer awaiting SettingsService.getInstance twice per launch — the external-player read now sits behind the supportsExternalPlayers guard. --- lib/mpv/player/platform/player_android.dart | 15 ++++ .../video_player/parts/playback_open.dart | 16 ++-- lib/screens/video_player_screen.dart | 69 +++++++++-------- lib/utils/video_player_navigation.dart | 64 ++++++++------- test/mpv/player_android_heap_size_test.dart | 77 +++++++++++++++++++ 5 files changed, 176 insertions(+), 65 deletions(-) create mode 100644 test/mpv/player_android_heap_size_test.dart 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); + }); +}