From 5228b0030ba142ff7bc398db01bdad3b9de8f59b Mon Sep 17 00:00:00 2001 From: zarzet Date: Thu, 30 Jul 2026 13:48:05 +0700 Subject: [PATCH] fix(player): restore lyrics after cold start --- lib/screens/now_playing_screen.dart | 68 ++++++++++++++-------- lib/services/music_player_service.dart | 44 ++++++++++++++ test/music_player_media_metadata_test.dart | 34 +++++++++++ 3 files changed, 122 insertions(+), 24 deletions(-) diff --git a/lib/screens/now_playing_screen.dart b/lib/screens/now_playing_screen.dart index 2e05d8a1..d682e95a 100644 --- a/lib/screens/now_playing_screen.dart +++ b/lib/screens/now_playing_screen.dart @@ -8,7 +8,6 @@ import 'package:spotiflac_android/l10n/l10n.dart'; import 'package:spotiflac_android/providers/music_player_provider.dart'; import 'package:spotiflac_android/services/library_database.dart'; import 'package:spotiflac_android/services/music_player_service.dart'; -import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/utils/file_access.dart'; import 'package:spotiflac_android/utils/int_utils.dart'; import 'package:spotiflac_android/utils/lyrics_parser.dart'; @@ -173,6 +172,7 @@ class _NowPlayingScreenState extends ConsumerState { ProviderSubscription>? _mediaItemSub; String? _loadedSource; String? _loadedResolvedSource; + String? _loadedMetadataPath; Map? _metadata; ParsedLyrics _lyrics = ParsedLyrics.empty; bool _loadingMeta = false; @@ -201,7 +201,10 @@ class _NowPlayingScreenState extends ConsumerState { super.dispose(); } - void _loadMetadataForItem(MediaItem? item) { + void _loadMetadataForItem( + MediaItem? item, { + bool inspectUnresolvedContentUri = false, + }) { if (item == null) return; final source = item.extras?['source']?.toString() ?? ''; if (source.isEmpty) return; @@ -211,6 +214,7 @@ class _NowPlayingScreenState extends ConsumerState { source, resolvedSource: resolvedSource, fallbackMetadata: playbackAudioMetadataFromMediaItem(item), + inspectUnresolvedContentUri: inspectUnresolvedContentUri, ), ); } @@ -219,41 +223,51 @@ class _NowPlayingScreenState extends ConsumerState { String source, { String? resolvedSource, Map fallbackMetadata = const {}, + bool inspectUnresolvedContentUri = false, }) async { final effectiveResolvedSource = resolvedSource?.trim(); - if (source == _loadedSource && - effectiveResolvedSource == _loadedResolvedSource) { + final path = + (effectiveResolvedSource != null && effectiveResolvedSource.isNotEmpty) + ? effectiveResolvedSource + : source; + final unresolvedContentUri = + path == source && source.startsWith('content://'); + final sameItem = + source == _loadedSource && + effectiveResolvedSource == _loadedResolvedSource; + + if (sameItem) { if (_metadata == null && fallbackMetadata.isNotEmpty) { setState(() => _metadata = fallbackMetadata); } - return; + if (_loadingMeta || _loadedMetadataPath == path) return; + if (unresolvedContentUri && !inspectUnresolvedContentUri) return; + setState(() => _loadingMeta = true); + } else { + _loadedSource = source; + _loadedResolvedSource = effectiveResolvedSource; + _loadedMetadataPath = null; + setState(() { + _loadingMeta = !unresolvedContentUri || inspectUnresolvedContentUri; + _metadata = fallbackMetadata.isEmpty ? null : fallbackMetadata; + _lyrics = ParsedLyrics.empty; + }); } - _loadedSource = source; - _loadedResolvedSource = effectiveResolvedSource; - setState(() { - _loadingMeta = true; - _metadata = fallbackMetadata.isEmpty ? null : fallbackMetadata; - _lyrics = ParsedLyrics.empty; - }); + + // Avoid copying a restored SAF file merely because the player shell became + // visible. If the user opens Lyrics before playback resolves a local temp + // source, inspect the content URI on demand instead. + if (unresolvedContentUri && !inspectUnresolvedContentUri) return; + try { - final path = - (effectiveResolvedSource != null && - effectiveResolvedSource.isNotEmpty) - ? effectiveResolvedSource - : source; - if (path == source && source.startsWith('content://')) { - if (mounted && _loadedSource == source) { - setState(() => _loadingMeta = false); - } - return; - } - final meta = await PlatformBridge.readFileMetadata(path); + final meta = await readPlaybackFileMetadataWithRetry(path); if (!mounted || _loadedSource != source || _loadedResolvedSource != effectiveResolvedSource) { return; } setState(() { + _loadedMetadataPath = path; _metadata = mergePlaybackFileMetadata(fallbackMetadata, meta); _lyrics = LyricsParser.parse((meta['lyrics'] ?? '').toString()); _loadingMeta = false; @@ -378,6 +392,12 @@ class _NowPlayingScreenState extends ConsumerState { if (_currentPage != page) { setState(() => _currentPage = page); } + if (page == 1) { + _loadMetadataForItem( + ref.read(currentMediaItemProvider).value, + inspectUnresolvedContentUri: true, + ); + } }, children: [ _playerPage(mediaItem, controller, colorScheme), diff --git a/lib/services/music_player_service.dart b/lib/services/music_player_service.dart index 9de3b6d4..830bf5c0 100644 --- a/lib/services/music_player_service.dart +++ b/lib/services/music_player_service.dart @@ -176,6 +176,50 @@ Map mergePlaybackFileMetadata( return merged; } +typedef PlaybackMetadataReader = + Future> Function(String path); + +/// Reads playback metadata with a small bounded retry window for transient +/// cold-start/native bridge failures. +/// +/// The native bridge reports some read failures as an `error` field instead +/// of throwing. Treat both forms identically so Now Playing does not cache an +/// empty Lyrics view until the route is reopened. A successful response with +/// no lyrics is still final and is never retried. +Future> readPlaybackFileMetadataWithRetry( + String path, { + PlaybackMetadataReader? reader, + List retryDelays = const [ + Duration.zero, + Duration(milliseconds: 250), + Duration(milliseconds: 750), + ], +}) async { + final read = reader ?? PlatformBridge.readFileMetadata; + final delays = retryDelays.isEmpty ? const [Duration.zero] : retryDelays; + Object? lastError; + var lastStack = StackTrace.current; + + for (final delay in delays) { + if (delay > Duration.zero) await Future.delayed(delay); + try { + final metadata = await read(path); + final reportedError = metadata['error']?.toString().trim() ?? ''; + if (reportedError.isEmpty) return metadata; + lastError = StateError(reportedError); + lastStack = StackTrace.current; + } catch (error, stack) { + lastError = error; + lastStack = stack; + } + } + + Error.throwWithStackTrace( + lastError ?? StateError('Metadata reader returned no result'), + lastStack, + ); +} + /// Returns a safe source-start position for restored playback. A completed /// snapshot starts over instead of immediately completing again on resume. Duration normalizedPlaybackResumePosition( diff --git a/test/music_player_media_metadata_test.dart b/test/music_player_media_metadata_test.dart index 8fcef92d..47a5bc17 100644 --- a/test/music_player_media_metadata_test.dart +++ b/test/music_player_media_metadata_test.dart @@ -67,4 +67,38 @@ void main() { Duration.zero, ); }); + + test('cold-start metadata read retries thrown and reported errors', () async { + var calls = 0; + + final metadata = await readPlaybackFileMetadataWithRetry( + '/music/track.flac', + retryDelays: const [Duration.zero, Duration.zero, Duration.zero], + reader: (path) async { + calls++; + if (calls == 1) throw StateError('backend not ready'); + if (calls == 2) return {'error': 'file temporarily unavailable'}; + return {'lyrics': '[00:01.00]Ready'}; + }, + ); + + expect(calls, 3); + expect(metadata['lyrics'], '[00:01.00]Ready'); + }); + + test('successful metadata without lyrics is not retried', () async { + var calls = 0; + + final metadata = await readPlaybackFileMetadataWithRetry( + '/music/instrumental.flac', + retryDelays: const [Duration.zero, Duration.zero, Duration.zero], + reader: (path) async { + calls++; + return {'title': 'Instrumental'}; + }, + ); + + expect(calls, 1); + expect(metadata['title'], 'Instrumental'); + }); }