From 8ae5ba5e4bd801638a0bac5eefdb1a099b0ddcce Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sat, 26 Sep 2026 07:28:10 +0000 Subject: [PATCH] bound the queue view and compact edited queue ranges --- .../subtracks/data/repo/QueueRepository.kt | 87 +++++++++-- .../com/subtracks/ui/playback/QueueScreen.kt | 72 ++++----- .../data/repo/QueueRepositoryTest.kt | 79 ++++++++++ .../ui/playback/QueueViewModelTest.kt | 138 +++++++++++++++++- 4 files changed, 320 insertions(+), 56 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt index 97ff3d42..94aa75e9 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -13,10 +13,21 @@ import com.subtracks.data.model.SongListItem import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicLong const val QUEUE_CHUNK = 60 +private const val LENGTH_CACHE_LIMIT = 512 + +private data class EntryLengthKey( + val sourceId: Long, + val kind: QueueKind, + val refId: String, + val offset: Long, + val count: Long?, +) + data class ResolvedQueueEntry( val entry: QueueEntry, val length: Long, @@ -74,6 +85,8 @@ class QueueRepository( @Volatile private var cachedFlatVersion = -1L + private val lengthCache = ConcurrentHashMap() + private val cursorMutex = Mutex() fun playlistEntry( @@ -134,7 +147,7 @@ class QueueRepository( val loaded = db.useReaderConnection { transactor -> transactor.deferredTransaction { - val entries = dao.entries().map { ResolvedQueueEntry(it, it.resolvedLength()) } + val entries = dao.entries().map { ResolvedQueueEntry(it, lengthOf(it)) } val row = dao.cursor() val shuffled = row?.shuffleEnabled == true val order = if (shuffled) loadShuffleOrder(row.shuffleSeed, entries.sumOf { it.length }) else null @@ -173,6 +186,7 @@ class QueueRepository( fun invalidateLibraryCache() { cachedFlatIds = null + lengthCache.clear() queueVersion.incrementAndGet() } @@ -272,11 +286,11 @@ class QueueRepository( ): QueueSnapshot { val order = snapshot.shuffleOrder if (order == null) { - write(removeEntry(snapshot.entries, position).map { it.entry }) + write(compact(removeEntry(snapshot.entries, position)).map { it.entry }) return snapshot() } val flat = order.getOrNull(position.toInt()) ?: return snapshot - write(removeEntry(snapshot.entries, flat).map { it.entry }) + write(compact(removeEntry(snapshot.entries, flat)).map { it.entry }) val reordered = order.toMutableList().apply { removeAt(position.toInt()) } for (index in reordered.indices) if (reordered[index] > flat) reordered[index] -= 1 setShuffle(true, reordered.toLongArray()) @@ -299,8 +313,8 @@ class QueueRepository( val located = snapshot.locate(from) ?: return false val song = itemAt(snapshot, from) ?: return false val removed = removeEntry(snapshot.entries, from) - val moved = songEntry(located.first.sourceId, song.song.id) - write(insertEntry(removed, to.coerceIn(0, removed.sumOf { it.length }), moved)) + val moved = ResolvedQueueEntry(songEntry(located.first.sourceId, song.song.id), 1L) + write(compact(insertEntry(removed, to.coerceIn(0, removed.sumOf { it.length }), moved)).map { it.entry }) return true } @@ -372,6 +386,24 @@ class QueueRepository( } } + private suspend fun lengthOf(entry: QueueEntry): Long { + val key = entry.lengthKey() + lengthCache[key]?.let { return it } + val length = entry.resolvedLength() + if (lengthCache.size >= LENGTH_CACHE_LIMIT) lengthCache.clear() + lengthCache[key] = length + return length + } + + private fun QueueEntry.lengthKey() = + EntryLengthKey( + sourceId = sourceId, + kind = kind, + refId = refId, + offset = offset, + count = count, + ) + private suspend fun QueueEntry.resolvedLength(): Long { val total = when (kind) { @@ -425,9 +457,9 @@ class QueueRepository( private fun insertEntry( entries: List, index: Long, - newEntry: QueueEntry, - ): List { - val result = mutableListOf() + newEntry: ResolvedQueueEntry, + ): List { + val result = mutableListOf() var start = 0L var placed = false for (resolved in entries) { @@ -435,21 +467,29 @@ class QueueRepository( when { !placed && index <= start -> { result += newEntry - result += resolved.entry + result += resolved placed = true } !placed && index in start..end -> { val cut = index - start val offset = resolved.entry.offset - if (cut > 0) result += resolved.entry.range(offset, offset + cut - 1) + if (cut > 0) { + result += ResolvedQueueEntry(resolved.entry.range(offset, offset + cut - 1), cut) + } result += newEntry - if (cut < resolved.length) result += resolved.entry.range(offset + cut, offset + resolved.length - 1) + if (cut < resolved.length) { + result += + ResolvedQueueEntry( + resolved.entry.range(offset + cut, offset + resolved.length - 1), + resolved.length - cut, + ) + } placed = true } else -> { - result += resolved.entry + result += resolved } } start += resolved.length @@ -457,4 +497,27 @@ class QueueRepository( if (!placed) result += newEntry return result } + + private fun compact(entries: List): List { + val merged = mutableListOf() + for (resolved in entries) { + val previous = merged.lastOrNull() + if (previous != null && previous.contiguousWith(resolved)) { + merged[merged.size - 1] = + ResolvedQueueEntry( + previous.entry.range(previous.entry.offset, resolved.entry.offset + resolved.length - 1), + previous.length + resolved.length, + ) + } else { + merged += resolved + } + } + return merged + } + + private fun ResolvedQueueEntry.contiguousWith(next: ResolvedQueueEntry): Boolean = + entry.kind == next.entry.kind && + entry.sourceId == next.entry.sourceId && + entry.refId == next.entry.refId && + entry.offset + length == next.entry.offset } diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt index 1b25ce89..ac776364 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -69,6 +69,8 @@ private const val LOAD_THRESHOLD = 5 private const val OLDER_LOAD = 30 private const val POSITION_TIMEOUT_MS = 1_000L +internal const val QUEUE_WINDOW_ROWS = QUEUE_CHUNK * 3 + data class QueueRow( val id: Long, val position: Long, @@ -92,9 +94,6 @@ class QueueViewModel( private var nextId = 0L private val mutex = Mutex() - private var size = 0L - private var first = 0L - private var last = -1L private fun newRow(item: QueueWindowItem) = QueueRow(nextId++, item.position, item.item) @@ -114,21 +113,16 @@ class QueueViewModel( viewModelScope.launch { mutex.withLock { val snapshot = queueRepository.snapshot() - size = snapshot.size val reused = reusedIds() rows.clear() - first = 0 - last = -1 - if (size > 0L) { + if (snapshot.size > 0L) { val cursor = withTimeoutOrNull(POSITION_TIMEOUT_MS) { playbackController.state.first { it.position != null }.position - }?.coerceIn(0, size - 1) ?: 0L + }?.coerceIn(0, snapshot.size - 1) ?: 0L val start = (cursor - OLDER_LOAD).coerceAtLeast(0) - val end = (start + QUEUE_CHUNK - 1).coerceAtMost(size - 1) + val end = (start + QUEUE_CHUNK - 1).coerceAtMost(snapshot.size - 1) rows.addAll(queueRepository.range(snapshot, start, end).map { rowFor(it, reused) }) - first = rows.firstOrNull()?.position ?: 0 - last = rows.lastOrNull()?.position ?: -1 initialIndex = rows.indexOfFirst { it.position == cursor }.coerceAtLeast(0) } ready = true @@ -138,34 +132,33 @@ class QueueViewModel( } fun loadOlder() { - if (first <= 0L) return + val top = rows.firstOrNull()?.position ?: return + if (top <= 0L) return viewModelScope.launch { mutex.withLock { - if (first <= 0L || rows.isEmpty()) return@withLock + val first = rows.firstOrNull()?.position ?: return@withLock + if (first <= 0L) return@withLock val snapshot = queueRepository.snapshot() - size = snapshot.size - val from = (first - QUEUE_CHUNK).coerceAtLeast(0) - val loaded = queueRepository.range(snapshot, from, first - 1) + val loaded = queueRepository.range(snapshot, (first - QUEUE_CHUNK).coerceAtLeast(0), first - 1) if (loaded.isEmpty()) return@withLock rows.addAll(0, loaded.map(::newRow)) - first = rows.first().position + while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(rows.size - 1) } } } fun loadNewer() { - if (rows.isEmpty() || last >= size - 1) return + if (rows.lastOrNull() == null) return viewModelScope.launch { mutex.withLock { - if (rows.isEmpty() || last >= size - 1) return@withLock + val last = rows.lastOrNull()?.position ?: return@withLock val snapshot = queueRepository.snapshot() - size = snapshot.size - if (last >= size - 1) return@withLock - val to = (last + QUEUE_CHUNK).coerceAtMost(size - 1) + if (last >= snapshot.size - 1) return@withLock + val to = (last + QUEUE_CHUNK).coerceAtMost(snapshot.size - 1) val loaded = queueRepository.range(snapshot, last + 1, to) if (loaded.isEmpty()) return@withLock rows.addAll(loaded.map(::newRow)) - last = rows.last().position + while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(0) } } } @@ -191,12 +184,15 @@ class QueueViewModel( viewModelScope.launch { playbackController.move(from, to) mutex.withLock { + val base = rows.firstOrNull()?.position ?: return@withLock // The loaded order already reflects the move, so only the positions need updating. - for (i in rows.indices) { - val row = rows[i] - val position = first + i - if (row.position != position) rows[i] = row.copy(position = position) - } + val updated = + rows.mapIndexed { index, row -> + val position = base + index + if (row.position == position) row else row.copy(position = position) + } + rows.clear() + rows.addAll(updated) } } } @@ -214,18 +210,15 @@ class QueueViewModel( } playbackController.removeAt(position) val snapshot = queueRepository.snapshot() - size = snapshot.size if (rows.isEmpty()) { reload() return@withLock } - first = rows.first().position - last = rows.last().position - val next = last + 1 - if (next <= size - 1) { + val next = rows.last().position + 1 + if (next <= snapshot.size - 1) { queueRepository.range(snapshot, next, next).firstOrNull()?.let { rows.add(newRow(it)) } - last = rows.last().position } + while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(0) } } } @@ -239,22 +232,17 @@ class QueueViewModel( private suspend fun reload() { val snapshot = queueRepository.snapshot() - size = snapshot.size - if (size == 0L) { + if (snapshot.size == 0L) { rows.clear() - first = 0 - last = -1 return } - val start = (rows.firstOrNull()?.position ?: 0L).coerceIn(0, size - 1) - val end = (start + rows.size.coerceAtLeast(QUEUE_CHUNK) - 1).coerceAtMost(size - 1) + val start = (rows.firstOrNull()?.position ?: 0L).coerceIn(0, snapshot.size - 1) + val end = (start + rows.size.coerceAtLeast(QUEUE_CHUNK) - 1).coerceAtMost(snapshot.size - 1) val loaded = queueRepository.range(snapshot, start, end) val reused = reusedIds() val updated = loaded.map { rowFor(it, reused) } rows.clear() rows.addAll(updated) - first = rows.firstOrNull()?.position ?: 0 - last = rows.lastOrNull()?.position ?: -1 } } 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 216924c0..d32c17be 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt @@ -299,6 +299,84 @@ class QueueRepositoryTest { assertEquals(listOf("s2", "s3", "s1"), resolveAll(repository.snapshot())) } + @Test + fun removingASongBetweenContiguousRangesMergesThem() = + runTest { + seedLibrary() + repository.replace( + listOf( + albumRange(0, 0), + repository.songEntry(1, "s4"), + albumRange(1, 2), + ), + ) + assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot())) + + repository.removeAt(repository.snapshot(), 1) + + val compacted = repository.snapshot() + assertEquals(1, compacted.entries.size) + assertEquals(listOf("s1", "s2", "s3"), resolveAll(compacted)) + } + + @Test + fun rangesSeparatedByAGapAreNotMerged() = + runTest { + seedLibrary() + repository.replace( + listOf( + albumRange(0, 0), + repository.songEntry(1, "s4"), + albumRange(2, 2), + ), + ) + assertEquals(listOf("s1", "s4", "s3"), resolveAll(repository.snapshot())) + + repository.removeAt(repository.snapshot(), 1) + + val kept = repository.snapshot() + assertEquals(2, kept.entries.size) + assertEquals(listOf("s1", "s3"), resolveAll(kept)) + } + + @Test + fun movingATrackOutAndBackCompactsTheQueue() = + runTest { + seedLibrary() + repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"), repository.songEntry(1, "s4"))) + + repository.move(repository.snapshot(), from = 3, to = 1) + assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot())) + + repository.move(repository.snapshot(), from = 1, to = 3) + + val compacted = repository.snapshot() + assertEquals(listOf("s1", "s2", "s3", "s4"), resolveAll(compacted)) + assertEquals(2, compacted.entries.size) + } + + @Test + fun resolvedLengthsSurviveQueueEditsUntilTheLibraryIsInvalidated() = + runTest { + seedLibrary() + repository.replace(listOf(repository.albumEntry(1, "al1"), repository.songEntry(1, "s5"))) + assertEquals(4, repository.snapshot().size) + + repository.removeAt(repository.snapshot(), 3) + assertEquals(3, repository.snapshot().size) + + db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album"))) + assertEquals(3, repository.snapshot().size) + + repository.invalidateLibraryCache() + assertEquals(4, repository.snapshot().size) + } + + private fun albumRange( + start: Long, + end: Long, + ) = QueueEntry(position = 0, sourceId = 1, kind = QueueKind.Album, refId = "al1", rangeStart = start, rangeEnd = end) + private suspend fun QueueRepository.snapshotAfter(entries: List): QueueSnapshot { replace(entries) return snapshot() @@ -316,6 +394,7 @@ class QueueRepositoryTest { assertTrue(repository.snapshot().shuffled) db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album"))) + repository.invalidateLibraryCache() assertFalse(repository.snapshot().shuffled) } diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt index 316a0095..e06f774d 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -140,6 +140,138 @@ class QueueViewModelTest { assertEquals(listOf(71L, 72L, 70L), positions.take(3)) } + @Test + fun scrollingNewerEvictsOlderRowsToBoundTheWindow() { + runBlocking { seedSongs(400) } + controller.playAlbum(1, "al1", 100) + await { controller.state.value.position == 100L } + + viewModel.open() + await { viewModel.ready } + assertEquals((70L..129L).toList(), viewModel.rows.map { it.position }) + + extendToEnd() + + assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size) + assertEquals((220L..399L).toList(), viewModel.rows.map { it.position }) + } + + @Test + fun scrollingOlderAfterEvictionDropsNewerRows() { + runBlocking { seedSongs(400) } + controller.playAlbum(1, "al1", 100) + await { controller.state.value.position == 100L } + + viewModel.open() + await { viewModel.ready } + extendToEnd() + + viewModel.loadOlder() + awaitWindow(first = 160, last = 339) + + assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size) + } + + @Test + fun movingARowAfterEvictionKeepsPositionsContiguous() { + runBlocking { seedSongs(400) } + controller.playAlbum(1, "al1", 100) + await { controller.state.value.position == 100L } + + viewModel.open() + await { viewModel.ready } + extendToEnd() + + val movedSongId = + viewModel.rows[10] + .song.song.id + val from = viewModel.rows[10].position + val to = viewModel.rows[13].position + viewModel.reorder(10, 13) + viewModel.move(from, to) + await { + viewModel.rows.size == QUEUE_WINDOW_ROWS && + viewModel.rows.getOrNull(13)?.position == 233L + } + + assertEquals( + movedSongId, + viewModel.rows[13] + .song.song.id, + ) + assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size) + assertContiguous() + } + + @Test + fun removeAndUndoKeepTheBoundedWindowConsistent() { + runBlocking { seedSongs(400) } + controller.playAlbum(1, "al1", 100) + await { controller.state.value.position == 100L } + + viewModel.open() + await { viewModel.ready } + val removedSongId = + viewModel.rows[30] + .song.song.id + + viewModel.remove(viewModel.rows[30].position) + await { + viewModel.rows.size == 60 && + viewModel.rows.firstOrNull()?.position == 70L && + viewModel.rows + .getOrNull(30) + ?.song + ?.song + ?.id != removedSongId + } + assertEquals((70L..129L).toList(), viewModel.rows.map { it.position }) + + viewModel.undo() + await { + viewModel.rows.size == 60 && + viewModel.rows.firstOrNull()?.position == 70L && + viewModel.rows + .getOrNull(30) + ?.song + ?.song + ?.id == removedSongId + } + assertContiguous() + } + + private fun assertContiguous() = assertTrue(isContiguous()) + + private fun isContiguous(): Boolean { + val positions = viewModel.rows.map { it.position } + return positions.isNotEmpty() && positions == (positions.first()..positions.last()).toList() + } + + private fun awaitWindow( + first: Long, + last: Long, + ) = await { + viewModel.rows.size == (last - first + 1).toInt() && + viewModel.rows.firstOrNull()?.position == first && + viewModel.rows.lastOrNull()?.position == last + } + + private fun extendTo( + position: Long, + first: Long, + ) { + viewModel.loadNewer() + awaitWindow(first = first, last = position) + } + + private fun extendToEnd() { + extendTo(189, first = 70) + extendTo(249, first = 70) + extendTo(309, first = 130) + extendTo(369, first = 190) + extendTo(399, first = 220) + } + private fun row(position: Int) = QueueRow( position.toLong(), @@ -189,8 +321,10 @@ class QueueViewModelTest { } private fun await(predicate: () -> Boolean) { - val deadline = System.nanoTime() + 5_000_000_000L - while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) + repeat(500) { + if (predicate()) return + Thread.sleep(10) + } assertTrue("Timed out waiting for the expected state", predicate()) } }