playback: keep the queue and player in sync across up-next

A context drag that crosses the up-next block moves the block's anchor in the repository but the old player.move used the combined drop position, leaving the two in different orders. Rebuild the window for context moves that happen while a block exists, in both the in-window and window-offset paths.

Also drop the redundant rebuildWindow snapshot parameter, the unused media-item context read-back, and add coverage for shuffled album context, ranged album moves and ranged up-next moves.
This commit is contained in:
austinried
2026-10-19 14:13:22 +00:00
parent 3f1d9b0d8d
commit 43f0721d26
4 changed files with 133 additions and 19 deletions
@@ -235,7 +235,6 @@ class Media3PlayerHandle(
?.takeIf { it.containsKey(EXTRA_DURATION_MS) } ?.takeIf { it.containsKey(EXTRA_DURATION_MS) }
?.getLong(EXTRA_DURATION_MS), ?.getLong(EXTRA_DURATION_MS),
replayGain = mediaItem.replayGain(), replayGain = mediaItem.replayGain(),
albumContext = mediaItem.albumContext(),
) )
} }
@@ -456,7 +456,7 @@ class PlaybackController(
if (snapshot.upNextSize > 0L || updated.upNextSize > 0L || snapshot.shuffled) { if (snapshot.upNextSize > 0L || updated.upNextSize > 0L || snapshot.shuffled) {
queueRepository.setCursor(target) queueRepository.setCursor(target)
if (position != current) { if (position != current) {
rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target, updated) rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target)
refresh(target) refresh(target)
} else { } else {
loadWindow(target, autoplay = playing, startPositionMs = 0) loadWindow(target, autoplay = playing, startPositionMs = 0)
@@ -546,13 +546,13 @@ class PlaybackController(
return@withLock return@withLock
} }
when { when {
fromInWindow && toInWindow -> { fromInWindow && toInWindow && (snapshot.upNextSize == 0L || snapshot.isUpNext(from)) -> {
updating = true updating = true
player.move((from - windowStart).toInt(), (to - windowStart).toInt()) player.move((from - windowStart).toInt(), (to - windowStart).toInt())
updating = false updating = false
} }
!fromInWindow && !toInWindow -> { !fromInWindow && !toInWindow && (snapshot.upNextSize == 0L || snapshot.isUpNext(from)) -> {
// The moved track never enters the window, so the playlist is unchanged; only // The moved track never enters the window, so the playlist is unchanged; only
// the window's positions shift, and a move that spans it shifts them by one. // the window's positions shift, and a move that spans it shifts them by one.
when { when {
@@ -569,7 +569,7 @@ class PlaybackController(
} }
else -> { else -> {
rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target, updated) rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target)
} }
} }
shiftWindowLocked(target) shiftWindowLocked(target)
@@ -768,7 +768,7 @@ class PlaybackController(
if (wasUpNext) { if (wasUpNext) {
loadWindow(target, autoplay = true) loadWindow(target, autoplay = true)
} else { } else {
rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target, updated) rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target)
refresh(target) refresh(target)
} }
} }
@@ -864,7 +864,7 @@ class PlaybackController(
layoutVersion++ layoutVersion++
queueRepository.setCursor(position) queueRepository.setCursor(position)
windowJob?.cancel() windowJob?.cancel()
rebuildWindow(queueRepository.window(updated, position, QUEUE_WINDOW_RADIUS), position, updated) rebuildWindow(queueRepository.window(updated, position, QUEUE_WINDOW_RADIUS), position)
refresh(position) refresh(position)
} }
@@ -913,11 +913,7 @@ class PlaybackController(
if (anchor.changed) layoutVersion++ if (anchor.changed) layoutVersion++
this@PlaybackController.snapshot = finalSnapshot this@PlaybackController.snapshot = finalSnapshot
queueRepository.setCursor(anchor.position) queueRepository.setCursor(anchor.position)
rebuildWindow( rebuildWindow(queueRepository.window(finalSnapshot, anchor.position, QUEUE_WINDOW_RADIUS), anchor.position)
queueRepository.window(finalSnapshot, anchor.position, QUEUE_WINDOW_RADIUS),
anchor.position,
finalSnapshot,
)
endedHandled = false endedHandled = false
refresh(anchor.position) refresh(anchor.position)
} }
@@ -1282,7 +1278,7 @@ class PlaybackController(
(currentId?.let { queueRepository.combinedIndexOf(snapshot, it) } ?: center) (currentId?.let { queueRepository.combinedIndexOf(snapshot, it) } ?: center)
.coerceIn(0, snapshot.size - 1) .coerceIn(0, snapshot.size - 1)
queueRepository.setCursor(position) queueRepository.setCursor(position)
rebuildWindow(queueRepository.window(snapshot, position, QUEUE_WINDOW_RADIUS), position, snapshot) rebuildWindow(queueRepository.window(snapshot, position, QUEUE_WINDOW_RADIUS), position)
refresh(position) refresh(position)
return return
} }
@@ -1318,9 +1314,9 @@ class PlaybackController(
private suspend fun rebuildWindow( private suspend fun rebuildWindow(
window: List<QueueWindowItem>, window: List<QueueWindowItem>,
position: Long, position: Long,
snapshot: QueueSnapshot,
) { ) {
val player = player ?: return val player = player ?: return
val snapshot = this.snapshot ?: return
downloads.awaitLoaded() downloads.awaitLoaded()
val ordered = val ordered =
if (offline) window.filter { downloads.localUri(it.item.song.id) != null } else window if (offline) window.filter { downloads.localUri(it.item.song.id) != null } else window
@@ -1420,7 +1416,7 @@ class PlaybackController(
if (target == current) { if (target == current) {
// Keep the playing item in place and only add/remove its neighbours, so a downloaded // Keep the playing item in place and only add/remove its neighbours, so a downloaded
// track does not re-prepare (and audibly gap) when offline mode is toggled. // track does not re-prepare (and audibly gap) when offline mode is toggled.
rebuildWindow(queueRepository.window(snapshot, target, QUEUE_WINDOW_RADIUS), target, snapshot) rebuildWindow(queueRepository.window(snapshot, target, QUEUE_WINDOW_RADIUS), target)
refresh(target) refresh(target)
} else { } else {
loadWindow(target, autoplay = player.playWhenReady, startPositionMs = 0) loadWindow(target, autoplay = player.playWhenReady, startPositionMs = 0)
@@ -505,6 +505,43 @@ class QueueRepositoryTest {
assertEquals(1L, entry.rangeEnd) assertEquals(1L, entry.rangeEnd)
} }
@Test
fun movingARangedAlbumTrackInUpNextKeepsTheAbsoluteOffsets() =
runTest {
seedLibrary()
db.libraryDao().upsertSongs(
listOf(
song("s6", "al1", track = 4, album = "First Album"),
song("s7", "al1", track = 5, album = "First Album"),
song("x1", "al3", track = 1),
),
)
val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al2")))
repository.addUpNext(snapshot, currentPosition = 0, entry = albumRange(1, 4), playNext = false)
repository.addUpNext(
repository.snapshot(),
currentPosition = 0,
entry = repository.songEntry(1, "x1"),
playNext = false,
)
assertEquals(listOf("s4", "s2", "s3", "s6", "s7", "x1", "s5"), resolveAll(repository.snapshot()))
repository.move(repository.snapshot(), from = 4, to = 5)
var updated = repository.snapshot()
assertEquals(listOf("s4", "s2", "s3", "s6", "x1", "s7", "s5"), resolveAll(updated))
val moved = updated.locate(5)!!.first
assertEquals(QueueKind.Album, moved.kind)
assertEquals(4L, moved.rangeStart)
assertEquals(4L, moved.rangeEnd)
repository.move(repository.snapshot(), from = 5, to = 4)
updated = repository.snapshot()
assertEquals(listOf("s4", "s2", "s3", "s6", "s7", "x1", "s5"), resolveAll(updated))
assertTrue(updated.isAlbumContext(4))
}
@Test @Test
fun movingAPlaylistTrackKeepsItAPinnedSong() = fun movingAPlaylistTrackKeepsItAPinnedSong() =
runTest { runTest {
@@ -897,6 +934,51 @@ class QueueRepositoryTest {
assertEquals("First Album", repository.sourceName(entry.kind, entry.sourceId, entry.refId)) assertEquals("First Album", repository.sourceName(entry.kind, entry.sourceId, entry.refId))
} }
@Test
fun movingWithinARangedAlbumKeepsTheAbsoluteOffsets() =
runTest {
seedLibrary()
db.libraryDao().upsertSongs(
listOf(
song("s6", "al1", track = 4, album = "First Album"),
song("s7", "al1", track = 5, album = "First Album"),
),
)
repository.snapshotAfter(listOf(albumRange(1, 4)))
assertEquals(listOf("s2", "s3", "s6", "s7"), resolveAll(repository.snapshot()))
repository.move(repository.snapshot(), from = 0, to = 2)
val snapshot = repository.snapshot()
assertEquals(listOf("s3", "s6", "s2", "s7"), resolveAll(snapshot))
val (entry, _) = snapshot.locate(2)!!
assertEquals(QueueKind.Album, entry.kind)
assertEquals(1L, entry.rangeStart)
assertEquals(1L, entry.rangeEnd)
}
@Test
fun movingBackwardWithinARangedAlbumKeepsTheAbsoluteOffsets() =
runTest {
seedLibrary()
db.libraryDao().upsertSongs(
listOf(
song("s6", "al1", track = 4, album = "First Album"),
song("s7", "al1", track = 5, album = "First Album"),
),
)
repository.snapshotAfter(listOf(albumRange(1, 4)))
repository.move(repository.snapshot(), from = 3, to = 0)
val snapshot = repository.snapshot()
assertEquals(listOf("s7", "s2", "s3", "s6"), resolveAll(snapshot))
val (entry, _) = snapshot.locate(0)!!
assertEquals(QueueKind.Album, entry.kind)
assertEquals(4L, entry.rangeStart)
assertEquals(4L, entry.rangeEnd)
}
@Test @Test
fun movingAnAlbumTrackBackCompactsTheAlbumEntry() = fun movingAnAlbumTrackBackCompactsTheAlbumEntry() =
runTest { runTest {
@@ -508,18 +508,26 @@ class PlaybackControllerTest {
assertTrue(handle.items.first { it.id == "x1" }.albumContext) assertTrue(handle.items.first { it.id == "x1" }.albumContext)
} }
@Test
fun shuffledAlbumTracksKeepAlbumContext() {
seedShuffledQueue(seed = 5L)
assertTrue(handle.items.isNotEmpty())
assertTrue(handle.items.all { it.albumContext })
}
@Test @Test
fun movingAnAlbumTrackKeepsItsAlbumGainContext() { fun movingAnAlbumTrackKeepsItsAlbumGainContext() {
seedAlbum(4, sourceId = 1) seedAlbum(60, sourceId = 1)
controller.playAlbum(1, "al1", 0) controller.playAlbum(1, "al1", 0)
await { handle.items.any { it.id == "s1" } } await { handle.items.any { it.id == "s1" } }
runBlocking { controller.move(0, 2) } runBlocking { controller.move(0, 40) }
await { controller.state.value.position == 2L } await { controller.state.value.position == 40L }
assertTrue(handle.items.first { it.id == "s1" }.albumContext) assertTrue(handle.items.first { it.id == "s1" }.albumContext)
val (entry, _) = runBlocking { queues.snapshot().locate(2)!! } val (entry, _) = runBlocking { queues.snapshot().locate(40)!! }
assertEquals(QueueKind.Album, entry.kind) assertEquals(QueueKind.Album, entry.kind)
assertEquals("al1", entry.refId) assertEquals("al1", entry.refId)
} }
@@ -540,6 +548,35 @@ class PlaybackControllerTest {
assertTrue(handle.items.first { it.id == "s2" }.albumContext) assertTrue(handle.items.first { it.id == "s2" }.albumContext)
} }
@Test
fun movingAContextTrackAcrossUpNextKeepsPlayerAndQueueInSync() {
seedAlbum(3, sourceId = 1)
seedSong("x1", "al2")
controller.playAlbum(1, "al1", 0)
await {
controller.state.value.item
?.id == "s1"
}
controller.addToQueue(1, QueueKind.Song, "x1")
await { handle.items.any { it.id == "x1" } }
runBlocking { controller.move(0, 3) }
await { controller.state.value.position == 2L }
val expected =
runBlocking {
val snapshot = queues.snapshot()
(0 until snapshot.size).mapNotNull { queues.itemAt(snapshot, it)?.song?.id }
}
assertEquals(expected, handle.items.map { it.id })
assertEquals(
"s1",
controller.state.value.item
?.id,
)
}
@Test @Test
fun tappingAContextTrackWhileAQueuedTrackPlaysKeepsTheQueue() { fun tappingAContextTrackWhileAQueuedTrackPlaysKeepsTheQueue() {
seedAlbum(3, sourceId = 1) seedAlbum(3, sourceId = 1)