From aa956668b1890c875eac96a8fb1709de134acc99 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 28 Sep 2026 12:38:57 +0000 Subject: [PATCH] skip non-downloaded tracks in offline shuffle and the queue view --- .../main/kotlin/com/subtracks/di/AppModule.kt | 2 +- .../subtracks/playback/PlaybackController.kt | 19 +++++-- .../com/subtracks/ui/playback/QueueScreen.kt | 56 ++++++++++--------- .../playback/PlaybackControllerTest.kt | 31 ++++++++++ .../ui/playback/QueueViewModelTest.kt | 54 ++++++++++++++++-- 5 files changed, 127 insertions(+), 35 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/di/AppModule.kt b/app/src/main/kotlin/com/subtracks/di/AppModule.kt index 31b45388..4fd9762d 100644 --- a/app/src/main/kotlin/com/subtracks/di/AppModule.kt +++ b/app/src/main/kotlin/com/subtracks/di/AppModule.kt @@ -90,7 +90,7 @@ fun appModule( viewModel { params -> AlbumDetailViewModel(get(), get(), get(), get(), params.get()) } viewModel { params -> ArtistDetailViewModel(get(), get(), get(), get(), params.get()) } viewModel { params -> PlaylistDetailViewModel(get(), get(), get(), get(), params.get()) } - viewModel { QueueViewModel(get(), get()) } + viewModel { QueueViewModel(get(), get(), get(), get()) } } private fun downloadsRoot(context: Context): File = diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 5546436c..3e309bee 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -1003,15 +1003,24 @@ class PlaybackController( val player = player ?: return val snapshot = snapshot ?: return downloads.awaitLoaded() - val raw = queueRepository.window(snapshot, position, QUEUE_WINDOW_RADIUS) + val requested = + if (offline) { + downloadedAtOrAfter(snapshot, position) ?: downloadedAtOrBefore(snapshot, position - 1) ?: run { + stopLocked() + return + } + } else { + position + } + val raw = queueRepository.window(snapshot, requested, QUEUE_WINDOW_RADIUS) val window = if (offline) raw.filter { downloads.localUri(it.item.song.id) != null } else raw if (window.isEmpty()) { stopLocked() return } - var startIndex = window.indexOfFirst { it.position == position } + var startIndex = window.indexOfFirst { it.position == requested } if (startIndex < 0) { - startIndex = window.indexOfFirst { it.position > position } + startIndex = window.indexOfFirst { it.position > requested } if (startIndex < 0) startIndex = window.lastIndex } val actual = window[startIndex].position @@ -1020,7 +1029,7 @@ class PlaybackController( windowStart = window.first().position windowEnd = window.last().position lastPosition = actual - if (actual != position) queueRepository.setCursor(actual) + if (actual != requested) queueRepository.setCursor(actual) updating = true player.setWindow(window.map { it.item.toQueueItem() }, startIndex, startPositionMs) updating = false @@ -1045,7 +1054,7 @@ class PlaybackController( val snapshot = snapshot ?: return@withLock if (snapshot.size == 0L || player.isEnded) return@withLock downloads.awaitLoaded() - val position = (windowStart + player.currentIndex).coerceIn(0, snapshot.size - 1) + val position = (currentPosition() ?: return@withLock).coerceIn(0, snapshot.size - 1) val window = queueRepository.window(snapshot, position, QUEUE_WINDOW_RADIUS) val uris = player.itemUris() if (window.size != player.itemCount || uris.size != window.size) { 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 fedb3ef3..aadd1b06 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -48,6 +48,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewModelScope import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.SongItem +import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.QUEUE_CHUNK import com.subtracks.data.repo.QueueRepository import com.subtracks.data.repo.QueueWindowItem @@ -90,6 +91,8 @@ internal fun QueueRow.isPlaying( class QueueViewModel( private val queueRepository: QueueRepository, private val playbackController: PlaybackController, + private val sourceRepository: SourceRepository, + private val downloads: DownloadRepository, ) : ViewModel() { val rows = mutableStateListOf() @@ -107,6 +110,8 @@ class QueueViewModel( private fun newRow(item: QueueWindowItem) = QueueRow(nextId++, item.position, item.item, item.upNext) + private fun visible(item: QueueWindowItem): Boolean = !sourceRepository.offline.value || downloads.localUri(item.item.song.id) != null + private fun reusedIds(): Map> = rows.groupBy { it.song.song.id }.mapValues { entry -> entry.value.map { it.id }.toMutableList() } @@ -132,7 +137,7 @@ class QueueViewModel( }?.coerceIn(0, snapshot.size - 1) ?: 0L val start = (cursor - OLDER_LOAD).coerceAtLeast(0) val end = (start + QUEUE_CHUNK - 1).coerceAtMost(snapshot.size - 1) - rows.addAll(queueRepository.range(snapshot, start, end).map { rowFor(it, reused) }) + rows.addAll(queueRepository.range(snapshot, start, end).filter(::visible).map { rowFor(it, reused) }) initialIndex = rows.indexOfFirst { it.position == cursor }.coerceAtLeast(0) } ready = true @@ -152,9 +157,9 @@ class QueueViewModel( rows.clear() return } - 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 start = (rows.minOfOrNull { it.position } ?: 0L).coerceIn(0, snapshot.size - 1) + val end = (rows.maxOfOrNull { it.position } ?: (start + QUEUE_CHUNK - 1)).coerceIn(start, snapshot.size - 1) + val loaded = queueRepository.range(snapshot, start, end).filter(::visible) val reused = reusedIds() val updated = loaded.map { rowFor(it, reused) } if (updated.size == rows.size) { @@ -173,9 +178,15 @@ class QueueViewModel( val first = rows.minOfOrNull { it.position } ?: return@withLock if (first <= 0L) return@withLock val snapshot = queueRepository.snapshot() - val loaded = queueRepository.range(snapshot, (first - QUEUE_CHUNK).coerceAtLeast(0), first - 1) - if (loaded.isEmpty()) return@withLock - rows.addAll(0, loaded.map(::newRow)) + val collected = mutableListOf() + var end = first - 1 + while (end >= 0L && collected.size < QUEUE_CHUNK) { + val start = (end - QUEUE_CHUNK + 1).coerceAtLeast(0) + collected.addAll(0, queueRepository.range(snapshot, start, end).filter(::visible)) + end = start - 1 + } + if (collected.isEmpty()) return@withLock + rows.addAll(0, collected.map(::newRow)) while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(rows.size - 1) } } @@ -188,10 +199,15 @@ class QueueViewModel( val last = rows.maxOfOrNull { it.position } ?: return@withLock val snapshot = queueRepository.snapshot() 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)) + val collected = mutableListOf() + var start = last + 1 + while (start < snapshot.size && collected.size < QUEUE_CHUNK) { + val to = (start + QUEUE_CHUNK - 1).coerceAtMost(snapshot.size - 1) + collected.addAll(queueRepository.range(snapshot, start, to).filter(::visible)) + start = to + 1 + } + if (collected.isEmpty()) return@withLock + rows.addAll(collected.map(::newRow)) while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(0) } } @@ -218,17 +234,7 @@ class QueueViewModel( if (from == to) return viewModelScope.launch { playbackController.move(from, to) - mutex.withLock { - val base = rows.minOfOrNull { it.position } ?: return@withLock - // The loaded order already reflects the move, so only the positions need updating. - 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) - } + mutex.withLock { reconcileLocked() } } } @@ -256,9 +262,9 @@ class QueueViewModel( rows.clear() return } - 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 start = (rows.minOfOrNull { it.position } ?: 0L).coerceIn(0, snapshot.size - 1) + val end = (rows.maxOfOrNull { it.position } ?: (start + QUEUE_CHUNK - 1)).coerceIn(start, snapshot.size - 1) + val loaded = queueRepository.range(snapshot, start, end).filter(::visible) val reused = reusedIds() val updated = loaded.map { rowFor(it, reused) } rows.clear() diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 1017926e..e0ee4eac 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -2497,6 +2497,37 @@ class PlaybackControllerTest { ) } + @Test + fun offlinePlayStartsAtTheNearestDownloadedWhenTheTargetIsNotDownloaded() { + seedAlbum(60, sourceId = 1) + markDownloaded(1, "s50") + setOffline() + + controller.playAlbum(1, "al1", 0) + + await { + controller.state.value.item + ?.id == "s50" + } + assertTrue(handle.items.isNotEmpty()) + } + + @Test + fun offlineShuffleStartsOnADownloadedTrack() { + seedAlbum(60, sourceId = 1) + markDownloaded(1, "s50") + markDownloaded(1, "s60") + setOffline() + + controller.shuffleAlbum(1, "al1") + + await { + controller.state.value.item + ?.id in listOf("s50", "s60") + } + assertTrue(handle.items.isNotEmpty()) + } + @Test fun offlinePlayAlbumFiltersTheWindowToDownloadedSongs() { seedAlbum(4, sourceId = 1) 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 9ca37e95..0fe05163 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -10,8 +10,10 @@ import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine import com.subtracks.data.model.AlbumSongItem +import com.subtracks.data.model.DownloadStatus import com.subtracks.data.model.QueueKind import com.subtracks.data.model.Song +import com.subtracks.data.model.SongDownload import com.subtracks.data.model.Source import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.repo.DownloadRepository @@ -24,10 +26,12 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.setMain import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout import okhttp3.OkHttpClient import org.junit.After import org.junit.Assert.assertEquals @@ -46,8 +50,9 @@ class QueueViewModelTest { private lateinit var db: SubtracksDatabase private lateinit var sources: SourceRepository - private lateinit var downloads: DownloadRepository private lateinit var queues: QueueRepository + private lateinit var downloads: DownloadRepository + private lateinit var downloadsDir: File private lateinit var controller: PlaybackController private lateinit var viewModel: QueueViewModel @@ -63,17 +68,18 @@ class QueueViewModelTest { sources = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art"))) queues = QueueRepository(db) val artwork = ArtworkStore(File(context.cacheDir, "art")) + downloadsDir = File(context.cacheDir, "downloads-${System.nanoTime()}") downloads = DownloadRepository( db, sources, FakeDownloadEngine(), - File(context.cacheDir, "downloads"), + downloadsDir, artworkStore = artwork, artworkFetcher = { ByteArray(0) }, - ) + ).also { it.start() } controller = PlaybackController(sources, queues, FakePlayerConnection(FakePlayerHandle()), downloads, dispatcher = dispatcher) - viewModel = QueueViewModel(queues, controller) + viewModel = QueueViewModel(queues, controller, sources, downloads) } @After @@ -411,6 +417,46 @@ class QueueViewModelTest { extendTo(399, first = 220) } + @Test + fun offlineHidesRowsThatAreNotDownloaded() { + runBlocking { seedSongs(6) } + markDownloaded(1, "s2") + markDownloaded(1, "s4") + setOffline() + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s2" + } + + viewModel.open() + await { viewModel.ready } + + assertEquals(listOf("s2", "s4"), rowsSnapshot().map { it.song.song.id }) + } + + private fun markDownloaded( + sourceId: Long, + songId: String, + ) { + val file = File(downloadsDir, "$sourceId/$songId") + runBlocking { + downloads.reconcile() + db.downloadDao().upsert(SongDownload(sourceId, songId, DownloadStatus.Completed)) + file.parentFile?.mkdirs() + file.writeBytes(byteArrayOf(1)) + downloads.reconcile() + } + await { downloads.localUri(songId) != null } + } + + private fun setOffline() { + runBlocking { + sources.setOfflineMode(true) + withTimeout(5_000) { sources.offline.first { it } } + } + } + private fun row(position: Int) = QueueRow( position.toLong(),