diff --git a/lib/services/music_player_service.dart b/lib/services/music_player_service.dart index 8cb5a027..e1206e95 100644 --- a/lib/services/music_player_service.dart +++ b/lib/services/music_player_service.dart @@ -364,6 +364,7 @@ class MusicPlayerHandler extends BaseAudioHandler bool _initialized = false; bool _sourceReady = false; Future? _activePlayOperation; + Future _sourceChangeTail = Future.value(); Timer? _sleepTimer; DateTime? _sleepTimerEndsAt; @@ -442,7 +443,7 @@ class MusicPlayerHandler extends BaseAudioHandler if (identical(player, _player)) _handlePositionChanged(position); }), player.onDurationChanged.listen((duration) { - if (!identical(player, _player)) return; + if (!identical(player, _player) || !_sourceReady) return; final current = mediaItem.value; if (current != null && duration > Duration.zero) { mediaItem.add(current.copyWith(duration: duration)); @@ -693,6 +694,7 @@ class MusicPlayerHandler extends BaseAudioHandler } void _handlePositionChanged(Duration position) { + if (!_sourceReady || _switchingGeneration != 0) return; _broadcastPosition(position); _autoMix.onPosition(position); if (_restoringSession || @@ -1047,7 +1049,8 @@ class MusicPlayerHandler extends BaseAudioHandler } bool _isCurrentPlayRequest(int generation, PlayableMedia media) { - return generation == _playRequestGeneration && + return !_disposed && + generation == _playRequestGeneration && _index >= 0 && _index < _media.length && _media[_index].id == media.id && @@ -1161,7 +1164,7 @@ class MusicPlayerHandler extends BaseAudioHandler bool recordHistory = true, Duration startPosition = Duration.zero, }) async { - if (index < 0 || index >= _media.length) return; + if (_disposed || index < 0 || index >= _media.length) return; // A normal track change supersedes a pending restore. A restored start // keeps it until the source is actually ready so a transient failure can // be retried from the same position. @@ -1170,16 +1173,71 @@ class MusicPlayerHandler extends BaseAudioHandler } final generation = ++_playRequestGeneration; _sourceReady = false; - await _autoMix.cancel(); - if (generation != _playRequestGeneration || _disposed) return; + // Advance the requested index synchronously so consecutive Next taps do + // not all target the same song while AutoMix/source preparation awaits. _index = index; + final media = _media[index]; + _switchingGeneration = generation; + await _serializeSourceChange(() async { + try { + if (!_isCurrentPlayRequest(generation, media)) return; + await _autoMix.cancel(); + if (!_isCurrentPlayRequest(generation, media)) return; + // Retire the audible source before publishing another song's title. + await _player.stop(); + if (!_isCurrentPlayRequest(generation, media)) return; + await _loadIndex( + index, + generation, + media, + recordHistory: recordHistory, + startPosition: startPosition, + ); + } finally { + if (_switchingGeneration == generation) _switchingGeneration = 0; + } + }); + } + + Future _serializeSourceChange(Future Function() action) { + final operation = _sourceChangeTail.then((_) => action()); + // A failed operation must not poison later transport requests. + _sourceChangeTail = operation.catchError((Object _) {}); + return operation; + } + + Future _startResolvedSource( + String path, + int generation, + PlayableMedia media, + Duration? position, + ) async { + // AudioPlayer.play combines prepare/seek/resume without a cancellation + // boundary. A late prepare from an old request could resume the wrong + // source. Serialize preparation and recheck before making it audible. + await _player.setSource(DeviceFileSource(path)); + if (!_isCurrentPlayRequest(generation, media)) return; + if (position != null) { + await _player.seek(position); + if (!_isCurrentPlayRequest(generation, media)) return; + } + await _player.resume(); + if (!_isCurrentPlayRequest(generation, media)) await _player.pause(); + } + + Future _loadIndex( + int index, + int generation, + PlayableMedia media, { + required bool recordHistory, + required Duration startPosition, + }) async { _pausedByInterruption = false; _interruptionActive = false; _userPaused = false; if (recordHistory) _recordPlayHistory(index); - final media = _media[index]; final effectiveStartPosition = normalizedPlaybackResumePosition( startPosition, duration: media.duration, @@ -1257,7 +1315,7 @@ class MusicPlayerHandler extends BaseAudioHandler ? effectiveStartPosition : null; try { - await _player.play(DeviceFileSource(resolved), position: startAt); + await _startResolvedSource(resolved, generation, media, startAt); if (playbackLease != null) { await PlatformBridge.closeContentUriPlaybackLease( playbackLease.token, @@ -1285,15 +1343,18 @@ class MusicPlayerHandler extends BaseAudioHandler fallback, cacheKey: media.source, ); + if (!_isCurrentPlayRequest(generation, media)) return; await _player.setVolume(normalizationVolume); + if (!_isCurrentPlayRequest(generation, media)) return; _normalizationVolume = normalizationVolume; - await _player.play(DeviceFileSource(fallback), position: startAt); + await _startResolvedSource(fallback, generation, media, startAt); } if (!_isCurrentPlayRequest(generation, media)) return; _sourceReady = true; _pendingRestorePosition = null; _activeResolvedPath = usingLocalSafCopy ? resolved : null; await _cleanupPendingResolvedPaths(); + if (!_isCurrentPlayRequest(generation, media)) return; // Plain file paths were already published before loading. Re-publishing // them with an identical resolved path made Now Playing clear and probe // the same metadata twice on every Next. SAF needs this second event so @@ -1473,14 +1534,18 @@ class MusicPlayerHandler extends BaseAudioHandler @override Future pause() async { - _playRequestGeneration++; + final generation = ++_playRequestGeneration; _switchingGeneration = 0; _userPaused = true; _pausedByInterruption = false; await _autoMix.cancel(); - await _player.pause(); - _broadcastState(playerState: PlayerState.paused); - await _persistSession(position: await _currentPositionForPersist()); + await _serializeSourceChange(() async { + if (generation != _playRequestGeneration || _disposed) return; + await _player.pause(); + if (generation != _playRequestGeneration || _disposed) return; + _broadcastState(playerState: PlayerState.paused); + await _persistSession(position: await _currentPositionForPersist()); + }); } @override @@ -1516,14 +1581,21 @@ class MusicPlayerHandler extends BaseAudioHandler @override Future stop() async { cancelSleepTimer(); - _playRequestGeneration++; + final generation = ++_playRequestGeneration; _switchingGeneration = 0; _userPaused = true; await _autoMix.cancel(); + await _serializeSourceChange(() => _stopSession(generation)); + } + + Future _stopSession(int generation) async { + if (generation != _playRequestGeneration || _disposed) return; await _player.stop(); + if (generation != _playRequestGeneration || _disposed) return; _sourceReady = false; _activeResolvedPath = null; await _cleanupPendingResolvedPaths(); + if (generation != _playRequestGeneration || _disposed) return; _index = -1; _pausedByInterruption = false; _interruptionActive = false; @@ -1535,6 +1607,7 @@ class MusicPlayerHandler extends BaseAudioHandler _persistedSessionQueueRevision = -1; // An explicit stop ends the session for good; nothing to restore later. await _enqueueSessionWrite(AppStateDatabase.instance.clearPlaybackSession); + if (generation != _playRequestGeneration || _disposed) return; // A stopped session has no current item; this also hides the mini player. mediaItem.add(null); _broadcastState(playerState: PlayerState.stopped); @@ -1670,6 +1743,7 @@ class MusicPlayerHandler extends BaseAudioHandler } cancelSleepTimer(); _playRequestGeneration++; + await _sourceChangeTail; await _autoMix.dispose(); for (final sub in _subscriptions) { await sub.cancel(); diff --git a/test/music_player_automix_test.dart b/test/music_player_automix_test.dart index d6bb9721..9fe2af4a 100644 --- a/test/music_player_automix_test.dart +++ b/test/music_player_automix_test.dart @@ -38,6 +38,9 @@ class _AudioNative { final positions = {}; final live = {}; final playing = {}; + final sources = {}; + final sourceGates = >{}; + final resumedSources = []; void install() { for (final name in [ @@ -64,6 +67,9 @@ class _AudioNative { (_) async => null, ); case 'setSourceUrl': + final source = args['url']! as String; + await sourceGates[source]?.future; + sources[id] = source; unawaited(event(id, 'audio.onPrepared', true)); unawaited(event(id, 'audio.onDuration', 60000)); case 'seek': @@ -71,6 +77,7 @@ class _AudioNative { unawaited(event(id, 'audio.onSeekComplete')); case 'resume': playing.add(id); + resumedSources.add(sources[id]!); case 'pause' || 'stop': playing.remove(id); case 'dispose': @@ -153,6 +160,56 @@ void main() { expect(native.playing, isEmpty); }); + test( + 'rapid Next keeps the final audible source and displayed title together', + () async { + final gate = Completer(); + native.sourceGates['/one.flac'] = gate; + final first = handler.setQueueAndPlay(_tracks); + await _until(() => native.calls.any((c) => c.$2 == 'setSourceUrl')); + final second = handler.skipToNext(); + final third = handler.skipToNext(); + await Future.delayed(const Duration(milliseconds: 30)); + gate.complete(); + await Future.wait([first, second, third]); + expect(handler.mediaItem.value?.id, 'three'); + expect(native.sources['music-player'], '/three.flac'); + expect(native.resumedSources, ['/three.flac']); + }, + ); + + test('replacing a queue during preparation drops the old request', () async { + final gate = Completer(); + native.sourceGates['/one.flac'] = gate; + final first = handler.setQueueAndPlay(_tracks); + await _until(() => native.calls.any((c) => c.$2 == 'setSourceUrl')); + final replacement = handler.setQueueAndPlay([_tracks[1]]); + gate.complete(); + await Future.wait([first, replacement]); + expect(handler.mediaItem.value?.id, 'two'); + expect(native.sources['music-player'], '/two.flac'); + expect(native.resumedSources, ['/two.flac']); + }); + + for (final stop in [false, true]) { + test( + 'a pending prepare cannot resume after ${stop ? 'stop' : 'pause'}', + () async { + final gate = Completer(); + native.sourceGates['/one.flac'] = gate; + final first = handler.setQueueAndPlay(_tracks); + await _until(() => native.calls.any((c) => c.$2 == 'setSourceUrl')); + final cancel = stop ? handler.stop() : handler.pause(); + gate.complete(); + await Future.wait([first, cancel]); + expect(native.resumedSources, isEmpty); + expect(native.playing, isEmpty); + expect(handler.playbackState.value.playing, isFalse); + if (stop) expect(handler.mediaItem.value, isNull); + }, + ); + } + test('Mornye notification follows theme and keeps transport state', () async { await handler.restoreSession( items: _tracks,