diff --git a/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt b/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt index 8c2a20b8..ce33c723 100644 --- a/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt +++ b/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt @@ -235,7 +235,6 @@ class Media3PlayerHandle( ?.takeIf { it.containsKey(EXTRA_DURATION_MS) } ?.getLong(EXTRA_DURATION_MS), replayGain = mediaItem.replayGain(), - albumContext = mediaItem.albumContext(), ) } diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 9449b2a5..1e6a0603 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -456,7 +456,7 @@ class PlaybackController( if (snapshot.upNextSize > 0L || updated.upNextSize > 0L || snapshot.shuffled) { queueRepository.setCursor(target) if (position != current) { - rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target, updated) + rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target) refresh(target) } else { loadWindow(target, autoplay = playing, startPositionMs = 0) @@ -546,13 +546,13 @@ class PlaybackController( return@withLock } when { - fromInWindow && toInWindow -> { + fromInWindow && toInWindow && (snapshot.upNextSize == 0L || snapshot.isUpNext(from)) -> { updating = true player.move((from - windowStart).toInt(), (to - windowStart).toInt()) 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 window's positions shift, and a move that spans it shifts them by one. when { @@ -569,7 +569,7 @@ class PlaybackController( } else -> { - rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target, updated) + rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target) } } shiftWindowLocked(target) @@ -768,7 +768,7 @@ class PlaybackController( if (wasUpNext) { loadWindow(target, autoplay = true) } else { - rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target, updated) + rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target) refresh(target) } } @@ -864,7 +864,7 @@ class PlaybackController( layoutVersion++ queueRepository.setCursor(position) windowJob?.cancel() - rebuildWindow(queueRepository.window(updated, position, QUEUE_WINDOW_RADIUS), position, updated) + rebuildWindow(queueRepository.window(updated, position, QUEUE_WINDOW_RADIUS), position) refresh(position) } @@ -913,11 +913,7 @@ class PlaybackController( if (anchor.changed) layoutVersion++ this@PlaybackController.snapshot = finalSnapshot queueRepository.setCursor(anchor.position) - rebuildWindow( - queueRepository.window(finalSnapshot, anchor.position, QUEUE_WINDOW_RADIUS), - anchor.position, - finalSnapshot, - ) + rebuildWindow(queueRepository.window(finalSnapshot, anchor.position, QUEUE_WINDOW_RADIUS), anchor.position) endedHandled = false refresh(anchor.position) } @@ -1282,7 +1278,7 @@ class PlaybackController( (currentId?.let { queueRepository.combinedIndexOf(snapshot, it) } ?: center) .coerceIn(0, snapshot.size - 1) queueRepository.setCursor(position) - rebuildWindow(queueRepository.window(snapshot, position, QUEUE_WINDOW_RADIUS), position, snapshot) + rebuildWindow(queueRepository.window(snapshot, position, QUEUE_WINDOW_RADIUS), position) refresh(position) return } @@ -1318,9 +1314,9 @@ class PlaybackController( private suspend fun rebuildWindow( window: List, position: Long, - snapshot: QueueSnapshot, ) { val player = player ?: return + val snapshot = this.snapshot ?: return downloads.awaitLoaded() val ordered = if (offline) window.filter { downloads.localUri(it.item.song.id) != null } else window @@ -1420,7 +1416,7 @@ class PlaybackController( if (target == current) { // 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. - rebuildWindow(queueRepository.window(snapshot, target, QUEUE_WINDOW_RADIUS), target, snapshot) + rebuildWindow(queueRepository.window(snapshot, target, QUEUE_WINDOW_RADIUS), target) refresh(target) } else { loadWindow(target, autoplay = player.playWhenReady, startPositionMs = 0) diff --git a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt index 605052dc..775b6eee 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt @@ -505,6 +505,43 @@ class QueueRepositoryTest { 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 fun movingAPlaylistTrackKeepsItAPinnedSong() = runTest { @@ -897,6 +934,51 @@ class QueueRepositoryTest { 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 fun movingAnAlbumTrackBackCompactsTheAlbumEntry() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 5f42d339..1796f4bc 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -508,18 +508,26 @@ class PlaybackControllerTest { 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 fun movingAnAlbumTrackKeepsItsAlbumGainContext() { - seedAlbum(4, sourceId = 1) + seedAlbum(60, sourceId = 1) controller.playAlbum(1, "al1", 0) await { handle.items.any { it.id == "s1" } } - runBlocking { controller.move(0, 2) } - await { controller.state.value.position == 2L } + runBlocking { controller.move(0, 40) } + await { controller.state.value.position == 40L } 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("al1", entry.refId) } @@ -540,6 +548,35 @@ class PlaybackControllerTest { 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 fun tappingAContextTrackWhileAQueuedTrackPlaysKeepsTheQueue() { seedAlbum(3, sourceId = 1)