playback: serialize transitions and preserve detached playback

Run transition bookkeeping under the queue mutation lock, retain detached playback when its final survivor is removed, and restore offline repeat wrapping without a forward survivor. Centralize detached survivor rebuilding and add regressions for each defect.
This commit is contained in:
austinried
2026-11-14 12:05:28 +00:00
parent 018f30a3d8
commit 767c3db1ae
3 changed files with 169 additions and 55 deletions
@@ -408,6 +408,24 @@ class PlaybackController(
if (snapshot.shuffled && !snapshot.isUpNext(position)) return@withMutationLock if (snapshot.shuffled && !snapshot.isUpNext(position)) return@withMutationLock
if (snapshot.size == 1L) { if (snapshot.size == 1L) {
if (!removeAt(snapshot, position)) return@withMutationLock if (!removeAt(snapshot, position)) return@withMutationLock
if (detachedWindow && currentPosition() == null) {
val updated = readSnapshot()
this@PlaybackController.snapshot = updated
windowJob?.cancel()
rebuildDetachedSurvivors(updated, 0)
queueRepository.setCursor(0)
lastEdit =
ReloadUndo(
entries,
cursor,
snapshot.shuffleSeed,
upNext,
snapshot.upNextAnchor,
updated.version,
)
refresh(null)
return@withMutationLock
}
stopLocked() stopLocked()
lastEdit = lastEdit =
ReloadUndo( ReloadUndo(
@@ -736,11 +754,14 @@ class PlaybackController(
val snapshot = readSnapshot() val snapshot = readSnapshot()
this@PlaybackController.snapshot = snapshot this@PlaybackController.snapshot = snapshot
if (snapshot.size == 0L) return@withMutationLock if (snapshot.size == 0L) return@withMutationLock
val current = currentQueuePosition() ?: return@withMutationLock
if (detachedWindow) { if (detachedWindow) {
detachedPosition?.let { jumpToLocked(it, null) } val target =
detachedPosition
?: if (offline && repeatMode != RepeatMode.Off) downloadedAtOrAfter(snapshot, 0) else null
target?.let { jumpToLocked(it, null) }
return@withMutationLock return@withMutationLock
} }
val current = currentQueuePosition() ?: return@withMutationLock
if (snapshot.isUpNext(current)) { if (snapshot.isUpNext(current)) {
consumeUpNextLocked(current) consumeUpNextLocked(current)
return@withMutationLock return@withMutationLock
@@ -777,14 +798,6 @@ class PlaybackController(
else -> null else -> null
} }
private suspend fun consumeUpNext(
position: Long,
requestedVersion: Long,
) = queueRepository.withMutationLock {
if (this@PlaybackController.snapshot?.resetVersion != requestedVersion) return@withMutationLock
consumeUpNextLocked(position)
}
private suspend fun QueueMutationScope.consumeUpNextLocked(position: Long) { private suspend fun QueueMutationScope.consumeUpNextLocked(position: Long) {
val snapshot = this@PlaybackController.snapshot ?: return val snapshot = this@PlaybackController.snapshot ?: return
if (!snapshot.isUpNext(position)) return if (!snapshot.isUpNext(position)) return
@@ -1007,19 +1020,24 @@ class PlaybackController(
isOffline = offline isOffline = offline
} }
val snapshot = current ?: return null val snapshot = current ?: return null
val at = position ?: return null
val detachedNext = detachedWindow && currentPosition() == null val detachedNext = detachedWindow && currentPosition() == null
if (!isOffline) { if (!isOffline) {
val at = position ?: return null
if (detachedNext) return queueItemAt(snapshot, at) if (detachedNext) return queueItemAt(snapshot, at)
return nextPosition(snapshot, at) return nextPosition(snapshot, at)
?.takeIf { it != at } ?.takeIf { it != at }
?.let { queueItemAt(snapshot, it) } ?.let { queueItemAt(snapshot, it) }
} }
downloads.awaitLoaded() downloads.awaitLoaded()
val from = if (detachedNext) at else at + 1 val at = position
val target = val target =
downloadedAtOrAfter(snapshot, from) if (at == null) {
?: if (repeatMode != RepeatMode.Off) downloadedAtOrAfter(snapshot, 0L) else null if (detachedNext && repeatMode != RepeatMode.Off) downloadedAtOrAfter(snapshot, 0L) else null
} else {
val from = if (detachedNext) at else at + 1
downloadedAtOrAfter(snapshot, from)
?: if (repeatMode != RepeatMode.Off) downloadedAtOrAfter(snapshot, 0L) else null
}
return target?.takeIf { it != at || detachedNext }?.let { queueItemAt(snapshot, it) } return target?.takeIf { it != at || detachedNext }?.let { queueItemAt(snapshot, it) }
} }
@@ -1327,21 +1345,23 @@ class PlaybackController(
} }
private suspend fun onPositionChanged() { private suspend fun onPositionChanged() {
val player = player ?: return queueRepository.withMutationLock {
pruneDetached(player) val player = player ?: return@withMutationLock
val snapshot = snapshot ?: return pruneDetached(player)
if (snapshot.size == 0L) return val snapshot = snapshot ?: return@withMutationLock
val position = (currentQueuePosition() ?: return).coerceIn(0, snapshot.size - 1) if (snapshot.size == 0L) return@withMutationLock
val previous = lastPosition val position = (currentQueuePosition() ?: return@withMutationLock).coerceIn(0, snapshot.size - 1)
lastPosition = position val previous = lastPosition
if (previous != null && previous != position && snapshot.isUpNext(previous)) { lastPosition = position
consumeUpNext(previous, snapshot.resetVersion) if (previous != null && previous != position && snapshot.isUpNext(previous)) {
return consumeUpNextLocked(previous)
return@withMutationLock
}
lastSavedPositionMs = 0
queueRepository.setCursor(position)
refresh(position)
scheduleWindowShift(position)
} }
lastSavedPositionMs = 0
queueRepository.setCursor(position)
refresh(position)
scheduleWindowShift(position)
} }
private fun scheduleWindowShift(center: Long) { private fun scheduleWindowShift(center: Long) {
@@ -1466,30 +1486,10 @@ class PlaybackController(
} }
return return
} }
trimToCurrent(player) val nearest = (current?.position ?: lastPosition ?: 0L).coerceIn(0, (updated.size - 1).coerceAtLeast(0))
if (updated.size == 0L) { rebuildDetachedSurvivors(updated, nearest)
markDetached(emptyList(), null) queueRepository.setCursor(lastPosition ?: 0L)
lastPosition = null refresh(lastPosition)
queueRepository.setCursor(0)
refresh(null)
return
}
val nearest = (current?.position ?: lastPosition ?: 0L).coerceIn(0, updated.size - 1)
val window =
queueRepository
.range(updated, nearest, (nearest + QUEUE_WINDOW_RADIUS).coerceAtMost(updated.size - 1))
.filter { !offline || downloads.localUri(it.item.song.id) != null }
updating = true
try {
window.forEachIndexed { index, entry -> player.insertAt(index + 1, entry.playerItem(updated)) }
} finally {
updating = false
}
val next = window.firstOrNull()?.position
markDetached(window, next)
lastPosition = next
queueRepository.setCursor(next ?: 0L)
refresh(next)
} }
private fun trimToCurrent(player: PlayerHandle) { private fun trimToCurrent(player: PlayerHandle) {
@@ -1585,7 +1585,7 @@ class PlaybackController(
): Boolean { ): Boolean {
if (!detachedWindow || currentPosition() != null) return false if (!detachedWindow || currentPosition() != null) return false
queueRepository.setCursor(position) queueRepository.setCursor(position)
rebuildWindow(queueRepository.window(snapshot, position, QUEUE_WINDOW_RADIUS), position) rebuildDetachedSurvivors(snapshot, position)
refresh(position) refresh(position)
return true return true
} }
@@ -1752,12 +1752,17 @@ class PlaybackController(
downloads.awaitLoaded() downloads.awaitLoaded()
queueRepository.withMutationLock { queueRepository.withMutationLock {
if (!endJobIsCurrent(endedSnapshot, endedPosition)) return@withMutationLock if (!endJobIsCurrent(endedSnapshot, endedPosition)) return@withMutationLock
val snapshot = this@PlaybackController.snapshot ?: return@withMutationLock
if (detachedWindow && detachedPosition == null) { if (detachedWindow && detachedPosition == null) {
stopLocked() val target = if (repeatMode == RepeatMode.All) downloadedAtOrAfter(snapshot, 0L) else null
if (target != null) {
jumpToLocked(target, requestedVersion)
} else {
stopLocked()
}
return@withMutationLock return@withMutationLock
} }
val current = endedPosition ?: return@withMutationLock val current = endedPosition ?: return@withMutationLock
val snapshot = this@PlaybackController.snapshot ?: return@withMutationLock
val from = if (detachedWindow) current else current + 1 val from = if (detachedWindow) current else current + 1
val target = val target =
downloadedAtOrAfter(snapshot, from) downloadedAtOrAfter(snapshot, from)
@@ -45,6 +45,9 @@ class FakePlayerHandle : PlayerHandle {
@Volatile @Volatile
var expectedThread: Thread? = null var expectedThread: Thread? = null
@Volatile
var beforeRemoveFirst: (() -> Unit)? = null
private fun checkThread() { private fun checkThread() {
val expected = expectedThread ?: return val expected = expectedThread ?: return
check(Thread.currentThread() == expected) { "Expected $expected, was ${Thread.currentThread()}" } check(Thread.currentThread() == expected) { "Expected $expected, was ${Thread.currentThread()}" }
@@ -131,6 +134,7 @@ class FakePlayerHandle : PlayerHandle {
} }
override fun removeFirst() { override fun removeFirst() {
beforeRemoveFirst?.invoke()
operations += "removeFirst" operations += "removeFirst"
items.removeAt(0) items.removeAt(0)
index-- index--
@@ -3931,6 +3931,96 @@ class PlaybackControllerTest {
assertEquals("s4", handle.currentItem?.id) assertEquals("s4", handle.currentItem?.id)
} }
@Test
fun aTransitionWaitsForARemovalToRebuildThePlayerWindow() {
seedAlbum(2, sourceId = 1)
seedSong("a1", "al2")
seedSong("b1", "al2")
controller.playAlbum(1, "al1", 0)
await { handle.currentItem?.id == "s1" && controller.state.value.isPlaying }
controller.addToQueue(1, QueueKind.Song, "a1")
controller.addToQueue(1, QueueKind.Song, "b1")
await { runBlocking { queues.snapshot().upNextSize } == 2L }
controller.next()
await { handle.currentItem?.id == "a1" && controller.state.value.position == 1L }
val dispatcherBlocked = CountDownLatch(1)
val releaseDispatcher = CountDownLatch(1)
controllerScope.launch {
dispatcherBlocked.countDown()
releaseDispatcher.await()
}
assertTrue(dispatcherBlocked.await(TEST_TIMEOUT_MS, TimeUnit.MILLISECONDS))
handle.advanceTo(handle.currentIndex + 1)
val rebuildStarted = CountDownLatch(1)
val releaseRebuild = CountDownLatch(1)
handle.beforeRemoveFirst = {
handle.beforeRemoveFirst = null
rebuildStarted.countDown()
releaseRebuild.await(TEST_TIMEOUT_MS, TimeUnit.MILLISECONDS)
}
val removal = repoScope.async { controller.removeAt(1) }
try {
assertTrue(rebuildStarted.await(TEST_TIMEOUT_MS, TimeUnit.MILLISECONDS))
releaseDispatcher.countDown()
Thread.sleep(100)
} finally {
releaseDispatcher.countDown()
releaseRebuild.countDown()
}
runBlocking { withTimeout(TEST_TIMEOUT_MS) { removal.await() } }
Thread.sleep(200)
assertEquals(listOf("s1", "b1", "s2"), resolvedIds())
assertEquals("b1", handle.currentItem?.id)
assertEquals(1L, controller.state.value.position)
}
@Test
fun removingTheLastSurvivorKeepsDetachedPlaybackAndUndo() {
seedPlaylist(2)
controller.playPlaylist(1, "p1", 0)
await { handle.currentItem?.id == "s1" && controller.state.value.isPlaying }
val point = runBlocking { queues.captureResetPoint() }
setPlaylist("s2")
runBlocking { queues.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p1")), point) }
await { handle.items.map { it.id } == listOf("s1", "s2") }
runBlocking { controller.removeAt(0) }
assertEquals(listOf("s1"), handle.items.map { it.id })
assertEquals("s1", handle.currentItem?.id)
assertNull(controller.state.value.position)
assertEquals(0L, runBlocking { queues.cursor() })
runBlocking { controller.undo() }
assertEquals(listOf("s1", "s2"), handle.items.map { it.id })
assertEquals("s1", handle.currentItem?.id)
assertEquals(0L, controller.state.value.position)
}
@Test
fun offlineDetachedPlaybackWithoutAForwardSurvivorWrapsOnManualNext() {
detachOfflineTrackWithoutAForwardSurvivor()
val upcoming = runBlocking { controller.upcomingItem()?.id }
controller.next()
Thread.sleep(200)
assertEquals("s1", handle.currentItem?.id)
assertEquals("s1", upcoming)
assertEquals(0L, controller.state.value.position)
}
@Test
fun offlineDetachedPlaybackWithoutAForwardSurvivorWrapsWhenEnded() {
detachOfflineTrackWithoutAForwardSurvivor()
handle.finish()
await { handle.currentItem?.id == "s1" && controller.state.value.isPlaying }
assertEquals(0L, controller.state.value.position)
}
@Test @Test
fun aResetRemapsAContextTrackThatMovedToUpNext() { fun aResetRemapsAContextTrackThatMovedToUpNext() {
seedPlaylist(5) seedPlaylist(5)
@@ -4113,6 +4203,21 @@ class PlaybackControllerTest {
assertEquals("s0", runBlocking { queues.itemAt(queues.snapshot(), 0) }?.song?.id) assertEquals("s0", runBlocking { queues.itemAt(queues.snapshot(), 0) }?.song?.id)
} }
private fun detachOfflineTrackWithoutAForwardSurvivor() {
seedPlaylist(3)
markDownloaded(1, "s1")
markDownloaded(1, "s2")
setOffline()
controller.playPlaylist(1, "p1", 1)
await { handle.currentItem?.id == "s2" && controller.state.value.isPlaying }
val point = runBlocking { queues.captureResetPoint() }
setPlaylist("s1", "s3")
runBlocking { queues.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p1")), point) }
await { handle.items.map { it.id } == listOf("s2") && controller.state.value.position == null }
controller.cycleRepeat()
assertEquals(RepeatMode.All, controller.state.value.repeat)
}
private fun markDownloaded( private fun markDownloaded(
sourceId: Long, sourceId: Long,
songId: String, songId: String,