From 8199e4dbba8287831d7ef4fbd5d7a2d94b467f2e Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:27:26 +0700 Subject: [PATCH] fix(player): keep shuffle order stable and restore the original queue --- lib/services/music_player_automix.dart | 4 +- lib/services/music_player_service.dart | 180 +++++++++++++-------- test/music_player_automix_test.dart | 137 ++++++++++++++++ test/music_player_media_metadata_test.dart | 59 +++++-- 4 files changed, 296 insertions(+), 84 deletions(-) diff --git a/lib/services/music_player_automix.dart b/lib/services/music_player_automix.dart index ba76c350..c1bcf56b 100644 --- a/lib/services/music_player_automix.dart +++ b/lib/services/music_player_automix.dart @@ -84,9 +84,7 @@ class _MusicAutoMix { _playGeneration = handler._playRequestGeneration; _queueRevision = handler._sessionQueueRevision; final index = handler._index; - final nextIndex = handler._shuffle - ? handler._pickNextShuffle() - : index + 1 < handler._media.length + final nextIndex = index + 1 < handler._media.length ? index + 1 : handler._repeatMode == AudioServiceRepeatMode.all ? 0 diff --git a/lib/services/music_player_service.dart b/lib/services/music_player_service.dart index e1206e95..cfe7c9a0 100644 --- a/lib/services/music_player_service.dart +++ b/lib/services/music_player_service.dart @@ -102,22 +102,19 @@ void refreshPlaybackNormalization(String source) { if (handler != null) unawaited(handler._refreshNormalizationSource(source)); } -List buildShuffleCandidatePool({ +List buildShuffledQueueOrder({ required int mediaCount, required int currentIndex, - required Iterable recentIndices, + required Random random, }) { - final recent = recentIndices.toSet(); - final pool = []; - for (var index = 0; index < mediaCount; index++) { - if (index != currentIndex && !recent.contains(index)) pool.add(index); - } - if (pool.isEmpty) { - for (var index = 0; index < mediaCount; index++) { - if (index != currentIndex) pool.add(index); - } - } - return pool; + final rest = [ + for (var i = 0; i < mediaCount; i++) + if (i != currentIndex) i, + ]..shuffle(random); + return [ + if (currentIndex >= 0 && currentIndex < mediaCount) currentIndex, + ...rest, + ]; } final AudioContext _musicAudioContext = AudioContext( @@ -369,9 +366,10 @@ class MusicPlayerHandler extends BaseAudioHandler DateTime? _sleepTimerEndsAt; bool _shuffle = false; + List? _originalQueueOrder; + int _shuffleRequestGeneration = 0; AudioServiceRepeatMode _repeatMode = AudioServiceRepeatMode.none; final Random _random = Random(); - final List _recent = []; final List _playHistory = []; // True when playback was paused because another app took audio focus. @@ -905,6 +903,62 @@ class MusicPlayerHandler extends BaseAudioHandler _sessionQueueRevision++; } + List> _sessionMedia() { + final original = _originalQueueOrder; + final ranks = Map.identity(); + if (original != null) { + for (var i = 0; i < original.length; i++) { + ranks[original[i]] = i; + } + } + return [ + for (var i = 0; i < _media.length; i++) + { + ..._media[i].toJson(), + 'queueOriginalIndex': ranks[_queueItems[i]] ?? i, + }, + ]; + } + + void _applyQueueOrder(List order) { + final current = _index >= 0 && _index < _queueItems.length + ? _queueItems[_index] + : null; + final media = Map.identity(); + for (var i = 0; i < _media.length; i++) { + media[_queueItems[i]] = _media[i]; + } + _queueItems + ..clear() + ..addAll(order); + _media + ..clear() + ..addAll(order.map((item) => media[item]!)); + _index = current == null + ? -1 + : order.indexWhere((item) => identical(item, current)); + _playHistory.clear(); + if (_index >= 0) _playHistory.add(_index); + } + + void _shuffleQueue() { + _originalQueueOrder ??= List.of(_queueItems); + final order = buildShuffledQueueOrder( + mediaCount: _media.length, + currentIndex: _index, + random: _random, + ); + _applyQueueOrder(order.map((index) => _queueItems[index]).toList()); + } + + void _rememberEnqueued(List items, {required bool playNext}) { + final original = _originalQueueOrder; + if (original == null) return; + final current = _queueItems[_index]; + final at = original.indexWhere((item) => identical(item, current)); + original.insertAll(playNext && at >= 0 ? at + 1 : original.length, items); + } + /// Persists the queue only when it changed. Periodic position updates write /// fixed-size scalar columns, avoiding full queue JSON serialization every /// ten seconds for large playback sessions. @@ -924,7 +978,7 @@ class MusicPlayerHandler extends BaseAudioHandler final repeatMode = _repeatMode.name; if (_scheduledSessionQueueRevision != queueRevision) { - final media = _media.map((item) => item.toJson()).toList(growable: false); + final media = _sessionMedia(); _scheduledSessionQueueRevision = queueRevision; return _enqueueSessionWrite(() async { try { @@ -964,7 +1018,7 @@ class MusicPlayerHandler extends BaseAudioHandler // intentionally the only state-only path that serializes the queue. await AppStateDatabase.instance.savePlaybackSession({ 'version': 2, - 'media': _media.map((item) => item.toJson()).toList(growable: false), + 'media': _sessionMedia(), 'index': index, 'positionMs': positionMs, 'shuffle': shuffle, @@ -1004,6 +1058,7 @@ class MusicPlayerHandler extends BaseAudioHandler required int index, required Duration position, required bool shuffle, + List? originalOrder, bool queueNeedsRewrite = false, AudioServiceRepeatMode repeatMode = AudioServiceRepeatMode.none, }) async { @@ -1020,6 +1075,13 @@ class MusicPlayerHandler extends BaseAudioHandler ..addAll(items.map((m) => m.toMediaItem())); _index = index.clamp(0, items.length - 1); _shuffle = shuffle; + if (shuffle) { + final order = [for (var i = 0; i < items.length; i++) i]; + if (originalOrder?.length == items.length) { + order.sort((a, b) => originalOrder![a].compareTo(originalOrder[b])); + } + _originalQueueOrder = order.map((i) => _queueItems[i]).toList(); + } _repeatMode = repeatMode; _pendingRestorePosition = position > Duration.zero ? position : null; _sourceReady = false; @@ -1069,11 +1131,13 @@ class MusicPlayerHandler extends BaseAudioHandler _queueItems ..clear() ..addAll(items.map((m) => m.toMediaItem())); + _originalQueueOrder = null; + _index = initialIndex.clamp(0, items.length - 1); + if (_shuffle) _shuffleQueue(); _markSessionQueueChanged(); - _recent.clear(); _playHistory.clear(); queue.add(List.unmodifiable(_queueItems)); - await _playIndex(initialIndex.clamp(0, items.length - 1)); + await _playIndex(_index); } Future enqueue(PlayableMedia item, {bool playNext = false}) async { @@ -1085,12 +1149,11 @@ class MusicPlayerHandler extends BaseAudioHandler ? (_index + 1).clamp(0, _media.length) : _media.length; _media.insert(insertAt, item); - _queueItems.insert(insertAt, item.toMediaItem()); + final queueItem = item.toMediaItem(); + _queueItems.insert(insertAt, queueItem); + _rememberEnqueued([queueItem], playNext: playNext); _markSessionQueueChanged(); - for (var i = 0; i < _recent.length; i++) { - if (_recent[i] >= insertAt) _recent[i]++; - } for (var i = 0; i < _playHistory.length; i++) { if (_playHistory[i] >= insertAt) _playHistory[i]++; } @@ -1110,17 +1173,18 @@ class MusicPlayerHandler extends BaseAudioHandler return; } var at = playNext ? (_index + 1).clamp(0, _media.length) : _media.length; + final queued = []; for (final item in items) { _media.insert(at, item); - _queueItems.insert(at, item.toMediaItem()); - for (var i = 0; i < _recent.length; i++) { - if (_recent[i] >= at) _recent[i]++; - } + final queueItem = item.toMediaItem(); + _queueItems.insert(at, queueItem); + queued.add(queueItem); for (var i = 0; i < _playHistory.length; i++) { if (_playHistory[i] >= at) _playHistory[i]++; } at++; } + _rememberEnqueued(queued, playNext: playNext); _markSessionQueueChanged(); queue.add(List.unmodifiable(_queueItems)); _broadcastState(); @@ -1151,7 +1215,6 @@ class MusicPlayerHandler extends BaseAudioHandler } } - _recent.clear(); _playHistory.clear(); queue.add(List.unmodifiable(_queueItems)); @@ -1411,27 +1474,9 @@ class MusicPlayerHandler extends BaseAudioHandler } } - int _pickNextShuffle() { - if (_media.length <= 1) return _index; - final pool = buildShuffleCandidatePool( - mediaCount: _media.length, - currentIndex: _index, - recentIndices: _recent, - ); - return pool[_random.nextInt(pool.length)]; - } - void _recordPlayHistory(int index) { _playHistory.add(index); if (_playHistory.length > 200) _playHistory.removeAt(0); - _recent.add(index); - final maxRecent = ((_media.length - 1) * 0.6).floor().clamp( - 1, - _media.length > 1 ? _media.length - 1 : 1, - ); - while (_recent.length > maxRecent) { - _recent.removeAt(0); - } } Future _onComplete() async { @@ -1441,17 +1486,6 @@ class MusicPlayerHandler extends BaseAudioHandler await _playIndex(_index, recordHistory: false); return; } - if (_shuffle) { - if (_media.length > 1) { - await _playIndex(_pickNextShuffle()); - } else if (_repeatMode == AudioServiceRepeatMode.all && - _media.isNotEmpty) { - await _playIndex(_index, recordHistory: false); - } else { - _broadcastState(playerState: PlayerState.completed); - } - return; - } if (_index >= 0 && _index < _media.length - 1) { await _playIndex(_index + 1); } else if (_repeatMode == AudioServiceRepeatMode.all && _media.isNotEmpty) { @@ -1557,8 +1591,24 @@ class MusicPlayerHandler extends BaseAudioHandler @override Future setShuffleMode(AudioServiceShuffleMode shuffleMode) async { + final generation = ++_shuffleRequestGeneration; + final enabled = shuffleMode == AudioServiceShuffleMode.all; await _autoMix.cancel(); - _shuffle = shuffleMode == AudioServiceShuffleMode.all; + if (generation != _shuffleRequestGeneration || + _disposed || + enabled == _shuffle) { + return; + } + _shuffle = enabled; + if (enabled) { + _shuffleQueue(); + } else { + final original = _originalQueueOrder; + if (original != null) _applyQueueOrder(List.of(original)); + _originalQueueOrder = null; + } + _markSessionQueueChanged(); + queue.add(List.unmodifiable(_queueItems)); _broadcastState(); if (_media.isNotEmpty && _index >= 0) { unawaited(_persistSession(position: playbackState.value.position)); @@ -1600,7 +1650,6 @@ class MusicPlayerHandler extends BaseAudioHandler _pausedByInterruption = false; _interruptionActive = false; _userPaused = false; - _recent.clear(); _playHistory.clear(); _pendingRestorePosition = null; _scheduledSessionQueueRevision = -1; @@ -1616,10 +1665,6 @@ class MusicPlayerHandler extends BaseAudioHandler @override Future skipToNext() async { - if (_shuffle) { - if (_media.length > 1) await _playIndex(_pickNextShuffle()); - return; - } if (_index < _media.length - 1) await _playIndex(_index + 1); } @@ -1700,12 +1745,14 @@ class MusicPlayerHandler extends BaseAudioHandler var removedBeforeCurrent = 0; final kept = []; + final keptQueue = []; for (var i = 0; i < _media.length; i++) { if (_media[i].source == target) { if (i < _index) removedBeforeCurrent++; continue; } kept.add(_media[i]); + keptQueue.add(_queueItems[i]); } if (kept.length == _media.length) return; @@ -1715,9 +1762,10 @@ class MusicPlayerHandler extends BaseAudioHandler ..addAll(kept); _queueItems ..clear() - ..addAll(kept.map((m) => m.toMediaItem())); + ..addAll(keptQueue); + final keptSet = Set.identity()..addAll(keptQueue); + _originalQueueOrder?.removeWhere((item) => !keptSet.contains(item)); _markSessionQueueChanged(); - _recent.clear(); _playHistory.clear(); queue.add(List.unmodifiable(_queueItems)); @@ -1904,6 +1952,10 @@ Future _restorePersistedPlaybackSession() async { index: index, position: position, shuffle: session['shuffle'] == true, + originalOrder: [ + for (final i in keptOriginalIndices) + ((rawMedia[i] as Map)['queueOriginalIndex'] as num?)?.toInt() ?? i, + ], queueNeedsRewrite: items.length != rawMedia.length || artworkRelocated, repeatMode: AudioServiceRepeatMode.values.firstWhere( (mode) => mode.name == session['repeat'], diff --git a/test/music_player_automix_test.dart b/test/music_player_automix_test.dart index 9fe2af4a..f6d6ef7f 100644 --- a/test/music_player_automix_test.dart +++ b/test/music_player_automix_test.dart @@ -269,6 +269,128 @@ void main() { ); }); + test( + 'shuffle plays the published queue and off restores the original order', + () async { + final tracks = [ + for (var i = 0; i < 8; i++) + PlayableMedia( + id: '$i', + source: '/$i.flac', + title: '$i', + artist: 'Artist', + ), + ]; + await handler.setQueueAndPlay(tracks, initialIndex: 3); + await handler.setShuffleMode(AudioServiceShuffleMode.all); + final planned = handler.queue.value.map((item) => item.id).toList(); + expect(planned.first, '3'); + expect(planned.toSet(), tracks.map((item) => item.id).toSet()); + for (var i = 1; i < planned.length; i++) { + await handler.skipToNext(); + expect(handler.mediaItem.value?.id, planned[i]); + expect(native.sources['music-player'], '/${planned[i]}.flac'); + expect(handler.queue.value.map((item) => item.id), planned); + } + final last = handler.mediaItem.value; + final resumes = native.resumedSources.length; + await handler.setShuffleMode(AudioServiceShuffleMode.none); + expect( + handler.queue.value.map((item) => item.id), + tracks.map((item) => item.id), + ); + expect(handler.mediaItem.value, last); + expect(handler.playbackState.value.queueIndex, int.parse(planned.last)); + expect(native.resumedSources.length, resumes); + await handler.setShuffleMode(AudioServiceShuffleMode.all); + expect(handler.queue.value.first.id, planned.last); + expect(handler.mediaItem.value, last); + }, + ); + + test( + 'automatic completion follows shuffle order and respects repeat off', + () async { + await handler.setQueueAndPlay(_tracks); + await handler.setShuffleMode(AudioServiceShuffleMode.all); + final planned = handler.queue.value.map((item) => item.id).toList(); + for (var i = 1; i < planned.length; i++) { + await native.event('music-player', 'audio.onComplete'); + await _until( + () => + handler.mediaItem.value?.id == planned[i] && + handler.playbackState.value.processingState == + AudioProcessingState.ready, + ); + } + await native.event('music-player', 'audio.onComplete'); + await _until( + () => + handler.playbackState.value.processingState == + AudioProcessingState.completed, + ); + expect(handler.mediaItem.value?.id, planned.last); + expect(handler.queue.value.map((item) => item.id), planned); + }, + ); + + test( + 'shuffle restoration retains duplicate entries and explicit queue edits', + () async { + await handler.setQueueAndPlay([ + _tracks[0], + _tracks[0], + _tracks[1], + _tracks[2], + ]); + await handler.setShuffleMode(AudioServiceShuffleMode.all); + await handler.enqueue( + const PlayableMedia( + id: 'next', + source: '/next.flac', + title: 'Next', + artist: '', + ), + playNext: true, + ); + await handler.enqueueAll([ + const PlayableMedia( + id: 'last', + source: '/last.flac', + title: 'Last', + artist: '', + ), + ]); + expect(handler.queue.value[1].id, 'next'); + await handler.onSourceDeleted('/two.flac'); + await handler.setShuffleMode(AudioServiceShuffleMode.none); + expect(handler.queue.value.map((item) => item.id), [ + 'one', + 'next', + 'one', + 'three', + 'last', + ]); + expect(handler.playbackState.value.queueIndex, 0); + }, + ); + + test('restored shuffle can return to the saved original order', () async { + await handler.restoreSession( + items: [_tracks[1], _tracks[2], _tracks[0]], + index: 1, + position: const Duration(seconds: 12), + shuffle: true, + originalOrder: [1, 2, 0], + ); + await handler.setShuffleMode(AudioServiceShuffleMode.none); + expect(handler.queue.value.map((item) => item.id), ['one', 'two', 'three']); + expect(handler.mediaItem.value?.id, 'three'); + expect(handler.playbackState.value.queueIndex, 2); + expect(handler.playbackState.value.position.inSeconds, 12); + expect(native.resumedSources, isEmpty); + }); + test( 'notification favorite retains clicked track and ignores double taps', () async { @@ -311,6 +433,21 @@ void main() { return incoming; } + test( + 'AutoMix prepares and plays the next entry in the shuffled queue', + () async { + await handler.setShuffleMode(AudioServiceShuffleMode.all); + await prepare(); + final planned = handler.queue.value.map((item) => item.id).toList(); + final incoming = native.prepared; + expect(native.sources[incoming], '/${planned[1]}.flac'); + native.positions['music-player'] = 55000; + await _until(() => handler.mediaItem.value?.id == planned[1]); + expect(handler.queue.value.map((item) => item.id), planned); + expect(handler.playbackState.value.queueIndex, 1); + }, + ); + test( 'disabled AutoMix uses only the ordinary player and no analysis', () async { diff --git a/test/music_player_media_metadata_test.dart b/test/music_player_media_metadata_test.dart index 71bfa905..2a8d459a 100644 --- a/test/music_player_media_metadata_test.dart +++ b/test/music_player_media_metadata_test.dart @@ -1,3 +1,5 @@ +import 'dart:math'; + import 'package:flutter_test/flutter_test.dart'; import 'package:spotiflac_android/services/music_player_service.dart'; @@ -164,25 +166,48 @@ void main() { expect(metadata['title'], 'Instrumental'); }); - test('shuffle candidate selection excludes recent tracks', () { + test('shuffle plans the whole queue once with the current song first', () { + final order = buildShuffledQueueOrder( + mediaCount: 12, + currentIndex: 4, + random: Random(42), + ); + expect(order.first, 4); + expect(order.toSet(), {for (var i = 0; i < 12; i++) i}); + expect(order.length, 12); expect( - buildShuffleCandidatePool( - mediaCount: 6, - currentIndex: 2, - recentIndices: const [0, 1, 3], - ), - [4, 5], + order.skip(1), + isNot(orderedEquals([0, 1, 2, 3, 5, 6, 7, 8, 9, 10, 11])), ); }); - test('shuffle candidate selection resets after exhausting the pool', () { - expect( - buildShuffleCandidatePool( - mediaCount: 4, - currentIndex: 2, - recentIndices: const [0, 1, 3], - ), - [0, 1, 3], - ); - }); + test( + 'new shuffle draws a new permutation and handles empty/single queues', + () { + final random = Random(42); + final first = buildShuffledQueueOrder( + mediaCount: 12, + currentIndex: 4, + random: random, + ); + final second = buildShuffledQueueOrder( + mediaCount: 12, + currentIndex: 4, + random: random, + ); + expect(first, isNot(orderedEquals(second))); + expect( + buildShuffledQueueOrder( + mediaCount: 0, + currentIndex: -1, + random: random, + ), + isEmpty, + ); + expect( + buildShuffledQueueOrder(mediaCount: 1, currentIndex: 0, random: random), + [0], + ); + }, + ); }