From fb95a462fba724554086f64d94e87fd50392919d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 16:42:19 -0400 Subject: [PATCH] fix(player): align audio + UI on track change, prewarm covers + palette MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four related fixes to the player flow that together remove the audio↔UI lag on track change: 1. **Prefetcher pre-warms covers + palette for the next N tracks.** The existing prefetcher pinned audio files only. When auto-advance landed on the next track, the cover bytes were cold → mediaItem broadcast with artUri=null → now-playing screen stalled in _scheduleSwap awaiting precacheImage of a file that didn't exist yet. Each upcoming queue item now also fires AlbumCoverCache.getOrFetch (writes bytes to disk so _toMediaItem's peekCached returns the path) and AlbumColorCache.getOrExtract (memoizes the dominant color). Both fire-and-forget; idempotent if already cached. 2. **AlbumColorCache.peekColor sync getter** so the now-playing fast-path can read the memoized color without awaiting a Future. 3. **_scheduleSwap fast path** when cover bytes + palette are already cached (the common in-queue auto-advance case): commit _displayedMedia / _displayedDominant synchronously in setState without awaiting. The async preload remains as the slow-path fallback for genuine cold cache. This is what closes the gap the user reported: "art is loading and metadata hasn't updated but the new song is playing." 4. **setQueueFromTracks: build source before broadcasting queue / mediaItem.** Previously we broadcast immediately for snappy UI; if _buildAudioSource threw, the UI showed the new track while the player held the old source. Now: build first, broadcast only on success. Source build is sub-100ms on warm cache so the tap response cost is imperceptible. _suppressIndexUpdates is set around setAudioSources + broadcast so a transient currentIndex emission can't cross-broadcast the OLD queue's entry at the NEW index. 5. **playbackEventStream error handler skips past failing tracks.** Previously errors only logged. The player would go silent on a 404 / decoder failure but mediaItem stayed on the failed track — user saw "now playing X" with no audio. Now seekToNext on error; if at queue tail, pause cleanly so PlaybackState reflects idle. --- flutter_client/lib/cache/prefetcher.dart | 49 ++++++++-- .../lib/player/album_color_extractor.dart | 9 ++ flutter_client/lib/player/audio_handler.dart | 93 +++++++++++++++---- .../lib/player/now_playing_screen.dart | 31 ++++++- 4 files changed, 156 insertions(+), 26 deletions(-) diff --git a/flutter_client/lib/cache/prefetcher.dart b/flutter_client/lib/cache/prefetcher.dart index 0321ab87..785d05f6 100644 --- a/flutter_client/lib/cache/prefetcher.dart +++ b/flutter_client/lib/cache/prefetcher.dart @@ -1,6 +1,9 @@ +import 'dart:async'; + import 'package:audio_service/audio_service.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import '../player/album_color_extractor.dart'; import '../player/player_provider.dart'; import 'audio_cache_manager.dart'; import 'cache_settings_provider.dart'; @@ -39,14 +42,48 @@ class Prefetcher { (currentIdx + settings.prefetchWindow).clamp(0, queue.length - 1); final mgr = _ref.read(audioCacheManagerProvider); + final coverCache = _ref.read(albumCoverCacheProvider); + final colorCache = _ref.read(albumColorCacheProvider); + // Walk the window. For each upcoming track we want THREE things + // ready when the player transitions into it: + // + // 1. Audio file on disk (otherwise playback stalls on stream + // load — the original prefetcher concern). + // 2. Cover bytes on disk under AlbumCoverCache (otherwise + // _toMediaItem's peekCached returns null, mediaItem + // broadcasts with artUri=null, and the now-playing screen + // stalls in _scheduleSwap awaiting precacheImage of a file + // that doesn't exist yet). + // 3. Palette color extracted and memoized in AlbumColorCache + // (otherwise the gradient backdrop has to wait for + // PaletteGenerator to run after the cover lands — + // visible as the cover snapping in before the gradient). + // + // Each call is idempotent (cache-aware): if already cached, it's + // a no-op. Everything fire-and-forget so the reconcile completes + // quickly even on a fresh queue. for (var i = currentIdx; i <= endIdx; i++) { - final trackId = queue[i].id; - if (await mgr.isCached(trackId)) continue; - // Fire-and-forget; downloads happen in the background. Errors - // inside pin() are caught + cleaned up internally. - // ignore: unawaited_futures - mgr.pin(trackId, source: CacheSource.autoPrefetch); + final media = queue[i]; + final trackId = media.id; + final albumId = media.extras?['album_id'] as String?; + + if (!await mgr.isCached(trackId)) { + // ignore: unawaited_futures + mgr.pin(trackId, source: CacheSource.autoPrefetch); + } + + if (albumId != null && albumId.isNotEmpty) { + // Cover bytes: getOrFetch returns the file path; the side + // effect (writing to disk) is what we care about. + // ignore: unawaited_futures + coverCache.getOrFetch(albumId); + // Palette: getOrExtract chains off coverCache.getOrFetch so + // it'll wait for the cover before sampling — safe to call + // in parallel here. + // ignore: unawaited_futures + colorCache.getOrExtract(albumId); + } } // Eviction pass after pinning new files. diff --git a/flutter_client/lib/player/album_color_extractor.dart b/flutter_client/lib/player/album_color_extractor.dart index d1a134c8..32db2ab1 100644 --- a/flutter_client/lib/player/album_color_extractor.dart +++ b/flutter_client/lib/player/album_color_extractor.dart @@ -39,6 +39,15 @@ class AlbumColorCache { return color; } + /// Synchronous peek. Returns the previously-extracted color if this + /// album has been resolved this process; otherwise null. Used by + /// the now-playing fast-path swap so a warm cache transitions the + /// gradient in lockstep with the audio, instead of awaiting the + /// async [getOrExtract] future. A null return means "either no + /// extraction yet or extraction returned null" — caller falls back + /// to the async path. + Color? peekColor(String albumId) => _cache[albumId]; + Future _extract(String albumId) async { try { final coverCache = _ref.read(albumCoverCacheProvider); diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 1518d7c9..6217ae7d 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -15,11 +15,16 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl _player.playbackEventStream.listen( _broadcastState, // ExoPlayer surfaces stream errors (404, premature EOS, decoder - // failure, network drop) here. Without an error sink, the - // player just goes quiet — exactly the "starts then stops" - // symptom we hit. + // failure, network drop) here. Logging alone leaves the UI + // claiming "now playing X" while audio is silent — the user- + // observable mismatch between visual and audio state. Skip + // forward so the queue advances past the failed track and + // mediaItem updates to whatever's actually playing. If the + // failure is at the queue tail, just_audio will go idle on its + // own and _broadcastState reflects that. onError: (Object e, StackTrace st) { debugPrint('audio_handler: playbackEventStream error: $e\n$st'); + unawaited(_handlePlaybackError()); }, ); _player.currentIndexStream.listen(_onCurrentIndexChanged); @@ -113,9 +118,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // check and stop calling player mutations — important so a stale // fill doesn't append old-playlist tracks into the new queue. final myGen = ++_queueGeneration; - // Reset suppress flag in case a prior backward-fill bailed on - // gen check before reaching its `finally`. - _suppressIndexUpdates = false; _lastTracks = tracks; // Pause the old source immediately so the previous track stops @@ -127,25 +129,49 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl await _player.pause(); } - // Populate the visible queue + current mediaItem immediately so - // the player UI reflects the user's tap before any source has - // been built. Source list at the just_audio layer fills in - // asynchronously below. + // Build MediaItems up front (pure — no side effects); we'll + // broadcast queue/mediaItem only AFTER setAudioSources resolves + // so the audio engine and the UI flip together. If the build + // throws, the UI stays on the previous track (correct: audio + // also stays on the previous track since setAudioSources never + // ran). final items = tracks.map(_toMediaItem).toList(); - queue.add(items); - mediaItem.add(items[clampedInitial]); - // Fast path: build only the initial source so the player can - // start. Remaining sources stream in via _fillRemainingSources() - // in the background — addAudioSource for next/auto-advance - // tracks, insertAudioSource for skipPrev tracks. - final initial = await _buildAudioSource(tracks[clampedInitial]); + // Build only the initial source for fast start. Remaining + // sources stream in via _fillRemainingSources() — addAudioSource + // for next/auto-advance tracks, insertAudioSource for skipPrev. + final AudioSource initial; + try { + initial = await _buildAudioSource(tracks[clampedInitial]); + } catch (e, st) { + // Source build failed (bad URL, missing baseUrl, etc.). Don't + // broadcast the new state — leaving queue/mediaItem on the + // previous track keeps UI in sync with what the player is + // actually doing (which is "still on the previous track, + // paused"). + debugPrint('audio_handler: _buildAudioSource failed: $e\n$st'); + return; + } if (myGen != _queueGeneration) { debugPrint('audio_handler: superseded before setAudioSources (gen=$myGen)'); return; } - await _player.setAudioSources([initial], initialIndex: 0); + // Suppress _onCurrentIndexChanged side effects while the source + // list is being swapped — without this, a transient currentIndex + // emission during setAudioSources could broadcast the OLD queue's + // entry at the NEW index. Re-enabled after the broadcasts land. + _suppressIndexUpdates = true; + try { + await _player.setAudioSources([initial], initialIndex: 0); + // Broadcast in this order: queue first (so any consumer that + // reacts to mediaItem and reads queue.value sees the consistent + // pair), then mediaItem. + queue.add(items); + mediaItem.add(items[clampedInitial]); + } finally { + _suppressIndexUpdates = false; + } unawaited(_loadArtForCurrentItem()); unawaited(_fillRemainingSources(tracks, clampedInitial, myGen)); @@ -365,6 +391,37 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl await mgr.registerStreamCache(trackId, path, size); } + /// Called when playbackEventStream emits an error. Skips past the + /// failing track so the UI and audio re-converge — without this, + /// _player goes silent on a 404 / decoder failure / EOS but + /// mediaItem stays on the failed track and the user sees a "now + /// playing" header for something that isn't. + /// + /// If we're at the last track, seekToNext is a no-op; the state + /// drops to idle and _broadcastState reflects that. + Future _handlePlaybackError() async { + final currentIdx = _player.currentIndex; + final queueLen = queue.value.length; + if (currentIdx == null || currentIdx + 1 >= queueLen) { + // Last track or no queue context — just pause; _broadcastState + // already reflects the idle/error processingState. + try { + await _player.pause(); + } catch (_) {} + return; + } + try { + await _player.seekToNext(); + } catch (_) { + // If seekToNext fails too (next source not built yet, etc.), + // there's nothing safe to do — pause and let the user recover + // manually. + try { + await _player.pause(); + } catch (_) {} + } + } + void _onCurrentIndexChanged(int? idx) { if (idx == null) return; if (_suppressIndexUpdates) return; diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart index 818d3ea7..5f529aea 100644 --- a/flutter_client/lib/player/now_playing_screen.dart +++ b/flutter_client/lib/player/now_playing_screen.dart @@ -106,11 +106,39 @@ class _NowPlayingScreenState extends ConsumerState { Future _scheduleSwap(MediaItem newMedia) async { _pendingPreloadId = newMedia.id; + // Fast path: when Prefetcher has already warmed both the cover + // bytes (file:// artUri populated via _toMediaItem's peekCached) + // AND the palette color (AlbumColorCache.peekColor returns + // non-null), we can commit synchronously. The slow async path + // exists for genuine cold-cache moments (first play of an album + // that sync hasn't seen yet); the fast path is what makes the + // common in-queue auto-advance flip the visual in lockstep with + // the audio transition instead of lagging a tick behind. + final artUri = newMedia.artUri; + final albumId = newMedia.extras?['album_id'] as String?; + if (artUri != null && + artUri.isScheme('file') && + albumId != null && + albumId.isNotEmpty) { + final cachedColor = ref.read(albumColorCacheProvider).peekColor(albumId); + if (cachedColor != null) { + if (!mounted) return; + if (_pendingPreloadId != newMedia.id) return; + setState(() { + _displayedMedia = newMedia; + _displayedDominant = cachedColor; + }); + return; + } + } + + // Slow path: cover and/or color are not yet cached. Hold the + // current displayed state, preload, then atomic-commit. + // // 1. Precache the cover image bytes so when _AlbumArt mounts with // the new media, FileImage paints synchronously. Non-file // artUris fall through to ServerImage which handles its own // network load + 120ms fade — they won't snap. - final artUri = newMedia.artUri; if (artUri != null && artUri.isScheme('file') && context.mounted) { try { await precacheImage(FileImage(File.fromUri(artUri)), context); @@ -125,7 +153,6 @@ class _NowPlayingScreenState extends ConsumerState { // same FileImage, typically resolving within ~50ms once the // decode completes. Color? newDominant; - final albumId = newMedia.extras?['album_id'] as String?; if (albumId != null && albumId.isNotEmpty) { try { newDominant = await ref.read(albumColorProvider(albumId).future);