fix downloaded playback and tighten the download state machine

Review findings on the first downloads block:

- ExoPlayer could not open a downloaded file at all: the service's data source
  factory was OkHttp only, so a file:// URI went to the HTTP stack and failed
  with a malformed URL. Wrap it in DefaultDataSource so the file, asset and
  content schemes are resolved before the stream chain, and cover it with a
  test that reads a local file through the production factory.
- Reconcile no longer restarts a failed download, or a running one whose engine
  entry vanished: only a queued row is started again, and the rest become failed
  so the explicit Retry in the UI means something.
- song_downloads also references songs with CASCADE now, so a song pruned from
  the mirror takes its download row with it and the sweep can reclaim the file.
- The mutating methods run on the injected dispatcher rather than the caller's,
  which was the main thread for every call site, and the repository exposes one
  shared state flow instead of a second map plus five subscriptions.
- Playback stops when the active source disappears, including deleting the last
  source while a downloaded track is playing.
- Dropped the unused request mime type, the queued-at column nothing read, the
  metered constructor parameter nobody varied, and a dead index guard.
- The running indicator is asserted positively, the engine's status and reason
  mapping has a test, and the failure paths use the fake's failure and forget
  helpers.
This commit is contained in:
austinried
2026-10-04 00:30:01 +00:00
parent 2b104f1387
commit 31c1fec322
17 changed files with 285 additions and 135 deletions
+16 -10
View File
@@ -2,7 +2,7 @@
"formatVersion": 1, "formatVersion": 1,
"database": { "database": {
"version": 18, "version": 18,
"identityHash": "5af02d01d5774106a81b2d031b6bd215", "identityHash": "982f1044d9532868990a4e14017ad3d1",
"entities": [ "entities": [
{ {
"tableName": "sources", "tableName": "sources",
@@ -1105,7 +1105,7 @@
}, },
{ {
"tableName": "song_downloads", "tableName": "song_downloads",
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`sourceId` INTEGER NOT NULL, `songId` TEXT NOT NULL, `status` TEXT NOT NULL, `engineId` INTEGER, `bytes` INTEGER NOT NULL DEFAULT 0, `total` INTEGER NOT NULL DEFAULT 0, `error` TEXT, `queuedAt` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`sourceId`, `songId`), FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`sourceId` INTEGER NOT NULL, `songId` TEXT NOT NULL, `status` TEXT NOT NULL, `engineId` INTEGER, `bytes` INTEGER NOT NULL DEFAULT 0, `total` INTEGER NOT NULL DEFAULT 0, `error` TEXT, PRIMARY KEY(`sourceId`, `songId`), FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`sourceId`, `songId`) REFERENCES `songs`(`sourceId`, `id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
"fields": [ "fields": [
{ {
"fieldPath": "sourceId", "fieldPath": "sourceId",
@@ -1148,13 +1148,6 @@
"fieldPath": "error", "fieldPath": "error",
"columnName": "error", "columnName": "error",
"affinity": "TEXT" "affinity": "TEXT"
},
{
"fieldPath": "queuedAt",
"columnName": "queuedAt",
"affinity": "INTEGER",
"notNull": true,
"defaultValue": "0"
} }
], ],
"primaryKey": { "primaryKey": {
@@ -1195,13 +1188,26 @@
"referencedColumns": [ "referencedColumns": [
"id" "id"
] ]
},
{
"table": "songs",
"onDelete": "CASCADE",
"onUpdate": "NO ACTION",
"columns": [
"sourceId",
"songId"
],
"referencedColumns": [
"sourceId",
"id"
]
} }
] ]
} }
], ],
"setupQueries": [ "setupQueries": [
"CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
"INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '5af02d01d5774106a81b2d031b6bd215')" "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '982f1044d9532868990a4e14017ad3d1')"
] ]
} }
} }
@@ -249,8 +249,9 @@ val MIGRATION_17_18 =
connection.execSQL( connection.execSQL(
"CREATE TABLE IF NOT EXISTS `song_downloads` (`sourceId` INTEGER NOT NULL, `songId` TEXT NOT NULL, " + "CREATE TABLE IF NOT EXISTS `song_downloads` (`sourceId` INTEGER NOT NULL, `songId` TEXT NOT NULL, " +
"`status` TEXT NOT NULL, `engineId` INTEGER, `bytes` INTEGER NOT NULL DEFAULT 0, " + "`status` TEXT NOT NULL, `engineId` INTEGER, `bytes` INTEGER NOT NULL DEFAULT 0, " +
"`total` INTEGER NOT NULL DEFAULT 0, `error` TEXT, `queuedAt` INTEGER NOT NULL DEFAULT 0, " + "`total` INTEGER NOT NULL DEFAULT 0, `error` TEXT, " +
"PRIMARY KEY(`sourceId`, `songId`), FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) " + "PRIMARY KEY(`sourceId`, `songId`), FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) " +
"ON UPDATE NO ACTION ON DELETE CASCADE, FOREIGN KEY(`sourceId`, `songId`) REFERENCES `songs`(`sourceId`, `id`) " +
"ON UPDATE NO ACTION ON DELETE CASCADE )", "ON UPDATE NO ACTION ON DELETE CASCADE )",
) )
connection.execSQL("CREATE INDEX IF NOT EXISTS `index_song_downloads_sourceId` ON `song_downloads` (`sourceId`)") connection.execSQL("CREATE INDEX IF NOT EXISTS `index_song_downloads_sourceId` ON `song_downloads` (`sourceId`)")
@@ -4,7 +4,6 @@ data class EngineRequest(
val uri: String, val uri: String,
val path: String, val path: String,
val title: String, val title: String,
val mimeType: String? = null,
val allowMetered: Boolean = false, val allowMetered: Boolean = false,
) )
@@ -9,15 +9,14 @@ class SystemDownloadEngine(
private val context: Context, private val context: Context,
) : DownloadEngine { ) : DownloadEngine {
override fun enqueue(request: EngineRequest): Long { override fun enqueue(request: EngineRequest): Long {
val builder = val download =
DownloadManager DownloadManager
.Request(Uri.parse(request.uri)) .Request(Uri.parse(request.uri))
.setTitle(request.title) .setTitle(request.title)
.setDestinationInExternalFilesDir(context, Environment.DIRECTORY_MUSIC, request.path) .setDestinationInExternalFilesDir(context, Environment.DIRECTORY_MUSIC, request.path)
.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE) .setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE)
.setAllowedOverMetered(request.allowMetered) .setAllowedOverMetered(request.allowMetered)
request.mimeType?.let(builder::setMimeType) return manager().enqueue(download)
return manager().enqueue(builder)
} }
override fun download(id: Long): EngineDownload? { override fun download(id: Long): EngineDownload? {
@@ -31,15 +30,8 @@ class SystemDownloadEngine(
cursor cursor
.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_REASON)) .getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_REASON))
.takeIf { status == DownloadManager.STATUS_FAILED } .takeIf { status == DownloadManager.STATUS_FAILED }
val mapped =
when (status) {
DownloadManager.STATUS_SUCCESSFUL -> EngineStatus.Completed
DownloadManager.STATUS_FAILED -> EngineStatus.Failed
DownloadManager.STATUS_RUNNING -> EngineStatus.Running
else -> EngineStatus.Pending
}
return EngineDownload( return EngineDownload(
status = mapped, status = engineStatus(status),
bytes = bytes.coerceAtLeast(0), bytes = bytes.coerceAtLeast(0),
total = total.coerceAtLeast(0), total = total.coerceAtLeast(0),
error = reason?.let(::failureMessage), error = reason?.let(::failureMessage),
@@ -52,15 +44,23 @@ class SystemDownloadEngine(
} }
private fun manager(): DownloadManager = context.getSystemService(DownloadManager::class.java) private fun manager(): DownloadManager = context.getSystemService(DownloadManager::class.java)
private fun failureMessage(reason: Int): String =
when (reason) {
DownloadManager.ERROR_INSUFFICIENT_SPACE -> "Not enough space to download"
DownloadManager.ERROR_DEVICE_NOT_FOUND -> "Storage is unavailable"
DownloadManager.ERROR_FILE_ERROR, DownloadManager.ERROR_FILE_ALREADY_EXISTS -> "Could not write the download"
DownloadManager.ERROR_HTTP_DATA_ERROR, DownloadManager.ERROR_UNHANDLED_HTTP_CODE -> "The server refused to send this track"
DownloadManager.ERROR_TOO_MANY_REDIRECTS -> "The server redirected too many times"
DownloadManager.ERROR_CANNOT_RESUME -> "The download could not be resumed"
else -> "Download failed"
}
} }
internal fun engineStatus(status: Int): EngineStatus =
when (status) {
DownloadManager.STATUS_SUCCESSFUL -> EngineStatus.Completed
DownloadManager.STATUS_FAILED -> EngineStatus.Failed
DownloadManager.STATUS_RUNNING -> EngineStatus.Running
else -> EngineStatus.Pending
}
internal fun failureMessage(reason: Int): String =
when (reason) {
DownloadManager.ERROR_INSUFFICIENT_SPACE -> "Not enough space to download"
DownloadManager.ERROR_DEVICE_NOT_FOUND -> "Storage is unavailable"
DownloadManager.ERROR_FILE_ERROR, DownloadManager.ERROR_FILE_ALREADY_EXISTS -> "Could not write the download"
DownloadManager.ERROR_HTTP_DATA_ERROR, DownloadManager.ERROR_UNHANDLED_HTTP_CODE -> "The server refused to send this track"
DownloadManager.ERROR_TOO_MANY_REDIRECTS -> "The server redirected too many times"
DownloadManager.ERROR_CANNOT_RESUME -> "The download could not be resumed"
else -> "Download failed"
}
@@ -18,6 +18,12 @@ enum class DownloadStatus { Queued, Running, Completed, Failed }
childColumns = ["sourceId"], childColumns = ["sourceId"],
onDelete = ForeignKey.CASCADE, onDelete = ForeignKey.CASCADE,
), ),
ForeignKey(
entity = Song::class,
parentColumns = ["sourceId", "id"],
childColumns = ["sourceId", "songId"],
onDelete = ForeignKey.CASCADE,
),
], ],
indices = [Index("sourceId"), Index("engineId")], indices = [Index("sourceId"), Index("engineId")],
) )
@@ -29,7 +35,6 @@ data class SongDownload(
@ColumnInfo(defaultValue = "0") val bytes: Long = 0, @ColumnInfo(defaultValue = "0") val bytes: Long = 0,
@ColumnInfo(defaultValue = "0") val total: Long = 0, @ColumnInfo(defaultValue = "0") val total: Long = 0,
val error: String? = null, val error: String? = null,
@ColumnInfo(defaultValue = "0") val queuedAt: Long = 0,
) { ) {
val progress: Float? val progress: Float?
get() = total.takeIf { it > 0 }?.let { (bytes.toFloat() / it).coerceIn(0f, 1f) } get() = total.takeIf { it > 0 }?.let { (bytes.toFloat() / it).coerceIn(0f, 1f) }
@@ -17,14 +17,18 @@ import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import java.io.File import java.io.File
class DownloadRepository( class DownloadRepository(
@@ -32,9 +36,8 @@ class DownloadRepository(
private val sourceRepository: SourceRepository, private val sourceRepository: SourceRepository,
private val engine: DownloadEngine, private val engine: DownloadEngine,
private val downloadsDir: File, private val downloadsDir: File,
private val allowMetered: Boolean = false,
private val showMessage: (String) -> Unit = {}, private val showMessage: (String) -> Unit = {},
dispatcher: CoroutineDispatcher = Dispatchers.IO, private val dispatcher: CoroutineDispatcher = Dispatchers.IO,
) { ) {
private val scope = CoroutineScope(SupervisorJob() + dispatcher) private val scope = CoroutineScope(SupervisorJob() + dispatcher)
private val mutex = Mutex() private val mutex = Mutex()
@@ -44,94 +47,106 @@ class DownloadRepository(
@Volatile @Volatile
private var started = false private var started = false
private val completedFiles = MutableStateFlow<Map<String, File>>(emptyMap()) private val sharedStates: StateFlow<Map<String, SongDownload>> by lazy {
downloadStates()
.onEach { loaded.complete(Unit) }
.stateIn(scope, SharingStarted.Eagerly, emptyMap())
}
fun start() { fun start() {
if (started) return if (started) return
started = true started = true
downloadsDir.mkdirs() downloadsDir.mkdirs()
File(downloadsDir, NO_MEDIA).createNewFile() File(downloadsDir, NO_MEDIA).createNewFile()
sharedStates
scope.launch { scope.launch {
sourceRepository.activeSourceId().distinctUntilChanged().collect { reconcile() } sourceRepository.activeSourceId().distinctUntilChanged().collect { reconcile() }
} }
scope.launch {
states().collect { rows ->
completedFiles.value =
rows
.filterValues { it.status == DownloadStatus.Completed }
.mapValues { (_, row) -> file(row.sourceId, row.songId) }
.filterValues { it.exists() }
loaded.complete(Unit)
}
}
} }
fun close() { fun close() {
scope.cancel() scope.cancel()
} }
fun states(): Flow<Map<String, SongDownload>> = fun states(): StateFlow<Map<String, SongDownload>> = sharedStates
sourceRepository
.activeSourceId()
.flatMapLatest { sourceId -> if (sourceId == null) flowOf(emptyList()) else db.downloadDao().downloads(sourceId) }
.map { rows -> rows.associateBy { it.songId } }
suspend fun awaitLoaded() { suspend fun awaitLoaded() {
if (started) loaded.await() if (!started) return
states()
loaded.await()
} }
fun localUri(songId: String): String? = completedFiles.value[songId]?.takeIf { it.exists() }?.let { Uri.fromFile(it).toString() } fun localUri(songId: String): String? =
sharedStates.value[songId]
?.takeIf { it.status == DownloadStatus.Completed }
?.let { file(it.sourceId, it.songId) }
?.takeIf { it.exists() }
?.let { Uri.fromFile(it).toString() }
suspend fun download( suspend fun download(
sourceId: Long, sourceId: Long,
songId: String, songId: String,
) = mutex.withLock { ) = withContext(dispatcher) {
if (sourceRepository.activeSourceIdOnce() != sourceId) { mutex.withLock {
showMessage("Can't download from a source that isn't active") if (sourceRepository.activeSourceIdOnce() != sourceId) {
return@withLock showMessage("Can't download from a source that isn't active")
return@withLock
}
val existing = db.downloadDao().find(sourceId, songId)
if (existing != null && existing.status != DownloadStatus.Failed) return@withLock
existing?.engineId?.let(engine::cancel)
val url = sourceRepository.downloadUri(songId)
if (url == null) {
showMessage("Can't download: the server address is unavailable")
return@withLock
}
enqueue(sourceId, songId, url)
} }
val existing = db.downloadDao().find(sourceId, songId)
if (existing != null && existing.status != DownloadStatus.Failed) return@withLock
existing?.engineId?.let(engine::cancel)
val url = sourceRepository.downloadUri(songId)
if (url == null) {
showMessage("Can't download: the server address is unavailable")
return@withLock
}
enqueue(sourceId, songId, url)
} }
suspend fun remove( suspend fun remove(
sourceId: Long, sourceId: Long,
songId: String, songId: String,
) = mutex.withLock { ) = withContext(dispatcher) {
val row = db.downloadDao().find(sourceId, songId) ?: return@withLock mutex.withLock {
row.engineId?.let(engine::cancel) val row = db.downloadDao().find(sourceId, songId) ?: return@withLock
file(sourceId, songId).delete() row.engineId?.let(engine::cancel)
db.downloadDao().delete(sourceId, songId) file(sourceId, songId).delete()
db.downloadDao().delete(sourceId, songId)
}
} }
suspend fun removeSource(sourceId: Long) = suspend fun removeSource(sourceId: Long) =
mutex.withLock { withContext(dispatcher) {
db mutex.withLock {
.downloadDao() db
.all() .downloadDao()
.filter { it.sourceId == sourceId } .all()
.forEach { row -> row.engineId?.let(engine::cancel) } .filter { it.sourceId == sourceId }
db.downloadDao().deleteSource(sourceId) .forEach { row -> row.engineId?.let(engine::cancel) }
dir(sourceId).deleteRecursively() db.downloadDao().deleteSource(sourceId)
dir(sourceId).deleteRecursively()
}
} }
suspend fun reconcile() = suspend fun reconcile() =
mutex.withLock { withContext(dispatcher) {
db.downloadDao().all().forEach { row -> mutex.withLock {
val state = row.engineId?.let(engine::download) db.downloadDao().all().forEach { row ->
if (state == null) recover(row) else apply(row, state) val state = row.engineId?.let(engine::download)
if (state == null) recover(row) else apply(row, state)
}
sweep(db.downloadDao().all())
ensurePolling()
} }
sweep(db.downloadDao().all())
ensurePolling()
} }
private fun downloadStates(): Flow<Map<String, SongDownload>> =
sourceRepository
.activeSourceId()
.flatMapLatest { sourceId -> if (sourceId == null) flowOf(emptyList()) else db.downloadDao().downloads(sourceId) }
.map { rows -> rows.associateBy { it.songId } }
private suspend fun enqueue( private suspend fun enqueue(
sourceId: Long, sourceId: Long,
songId: String, songId: String,
@@ -142,40 +157,32 @@ class DownloadRepository(
val title = db.libraryDao().songOnce(sourceId, songId)?.title ?: songId val title = db.libraryDao().songOnce(sourceId, songId)?.title ?: songId
val engineId = val engineId =
engine.enqueue( engine.enqueue(
EngineRequest(uri = url, path = enginePath(sourceId, songId), title = title, allowMetered = allowMetered), EngineRequest(uri = url, path = enginePath(sourceId, songId), title = title, allowMetered = false),
) )
db.downloadDao().upsert( db.downloadDao().upsert(
SongDownload( SongDownload(sourceId = sourceId, songId = songId, status = DownloadStatus.Queued, engineId = engineId),
sourceId = sourceId,
songId = songId,
status = DownloadStatus.Queued,
engineId = engineId,
queuedAt = System.currentTimeMillis(),
),
) )
ensurePolling() ensurePolling()
} }
private suspend fun recover(row: SongDownload) { private suspend fun recover(row: SongDownload) {
when { when (row.status) {
row.status == DownloadStatus.Completed && file(row.sourceId, row.songId).exists() -> { DownloadStatus.Completed -> {
Unit if (!file(row.sourceId, row.songId).exists()) markFailed(row, MISSING_FILE)
} }
row.status == DownloadStatus.Completed -> { DownloadStatus.Queued -> {
markFailed(row, "The downloaded file is missing") if (sourceRepository.activeSourceIdOnce() != row.sourceId) return
}
sourceRepository.activeSourceIdOnce() != row.sourceId -> {
Unit
}
else -> {
val url = sourceRepository.downloadUri(row.songId) val url = sourceRepository.downloadUri(row.songId)
when (url) { if (url == null) markFailed(row, NO_ADDRESS) else enqueue(row.sourceId, row.songId, url)
null -> markFailed(row, "Can't download: the server address is unavailable") }
else -> enqueue(row.sourceId, row.songId, url)
} DownloadStatus.Running -> {
markFailed(row, STOPPED)
}
DownloadStatus.Failed -> {
Unit
} }
} }
} }
@@ -257,5 +264,8 @@ class DownloadRepository(
const val ENGINE_DIR = "downloads" const val ENGINE_DIR = "downloads"
const val NO_MEDIA = ".nomedia" const val NO_MEDIA = ".nomedia"
const val POLL_MS = 1_000L const val POLL_MS = 1_000L
const val MISSING_FILE = "The downloaded file is missing"
const val NO_ADDRESS = "Can't download: the server address is unavailable"
const val STOPPED = "The download stopped unexpectedly"
} }
} }
@@ -144,7 +144,7 @@ class PlaybackController(
init { init {
scope.launch { scope.launch {
sourceRepository.activeSourceId().collect { sourceId -> sourceRepository.activeSourceId().collect { sourceId ->
if (queueSourceId != null && sourceId != null && sourceId != queueSourceId) stop() if (queueSourceId != null && sourceId != queueSourceId) stop()
} }
} }
scope.launch { scope.launch {
@@ -965,10 +965,8 @@ class PlaybackController(
val snapshot = snapshot ?: return val snapshot = snapshot ?: return
if (snapshot.size == 0L) return if (snapshot.size == 0L) return
if (player.isEnded) return if (player.isEnded) return
val index = player.currentIndex val position = (windowStart + player.currentIndex).coerceIn(0, snapshot.size - 1)
val position = (windowStart + index).coerceIn(0, snapshot.size - 1)
val positionMs = player.currentPositionMs val positionMs = player.currentPositionMs
if (player.currentIndex != index) return
loadWindow(position, autoplay = player.playWhenReady, startPositionMs = positionMs) loadWindow(position, autoplay = player.playWhenReady, startPositionMs = positionMs)
} }
@@ -1,6 +1,7 @@
package com.subtracks.playback package com.subtracks.playback
import android.app.PendingIntent import android.app.PendingIntent
import android.content.Context
import android.content.Intent import android.content.Intent
import android.os.SystemClock import android.os.SystemClock
import android.util.Log import android.util.Log
@@ -9,6 +10,8 @@ import androidx.media3.common.AudioAttributes
import androidx.media3.common.C import androidx.media3.common.C
import androidx.media3.common.util.ExperimentalApi import androidx.media3.common.util.ExperimentalApi
import androidx.media3.common.util.UnstableApi import androidx.media3.common.util.UnstableApi
import androidx.media3.datasource.DataSource
import androidx.media3.datasource.DefaultDataSource
import androidx.media3.datasource.okhttp.OkHttpDataSource import androidx.media3.datasource.okhttp.OkHttpDataSource
import androidx.media3.exoplayer.ExoPlayer import androidx.media3.exoplayer.ExoPlayer
import androidx.media3.exoplayer.source.DefaultMediaSourceFactory import androidx.media3.exoplayer.source.DefaultMediaSourceFactory
@@ -41,9 +44,7 @@ class PlaybackService : MediaSessionService() {
.setMediaSourceFactory( .setMediaSourceFactory(
DefaultMediaSourceFactory(this) DefaultMediaSourceFactory(this)
.setEnableClippingInMediaPeriod(true) .setEnableClippingInMediaPeriod(true)
.setDataSourceFactory( .setDataSourceFactory(mediaDataSourceFactory(this, OkHttpDataSource.Factory(streamingClient()))),
KnownLengthDataSourceFactory(OkHttpDataSource.Factory(streamingClient())),
),
).build() ).build()
val loader = val loader =
CoverArtBitmapLoader( CoverArtBitmapLoader(
@@ -88,10 +89,6 @@ class PlaybackService : MediaSessionService() {
response response
}.build() }.build()
private companion object {
const val STREAM_TAG = "SubtracksPlayback"
}
override fun onDestroy() { override fun onDestroy() {
artworkLoader?.shutdown() artworkLoader?.shutdown()
artworkLoader = null artworkLoader = null
@@ -102,4 +99,14 @@ class PlaybackService : MediaSessionService() {
session = null session = null
super.onDestroy() super.onDestroy()
} }
private companion object {
const val STREAM_TAG = "SubtracksPlayback"
}
} }
@OptIn(UnstableApi::class)
internal fun mediaDataSourceFactory(
context: Context,
upstream: DataSource.Factory,
): DataSource.Factory = DefaultDataSource.Factory(context, KnownLengthDataSourceFactory(upstream))
@@ -13,12 +13,10 @@ import com.subtracks.data.repo.SourceRepository
import com.subtracks.playback.PlaybackController import com.subtracks.playback.PlaybackController
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
@@ -37,8 +35,7 @@ class AlbumDetailViewModel(
val discs: Flow<List<Disc>> = sourceId.flatMapLatest { libraryRepository.albumDiscs(it, albumId) } val discs: Flow<List<Disc>> = sourceId.flatMapLatest { libraryRepository.albumDiscs(it, albumId) }
val downloads: StateFlow<Map<String, SongDownload>> = val downloads: StateFlow<Map<String, SongDownload>> = downloadRepository.states()
downloadRepository.states().stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyMap())
fun coverArt( fun coverArt(
coverArt: String?, coverArt: String?,
@@ -15,12 +15,10 @@ import com.subtracks.data.repo.SourceRepository
import com.subtracks.playback.PlaybackController import com.subtracks.playback.PlaybackController
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
@@ -41,7 +39,7 @@ class PlaylistDetailViewModel(
.cachedIn(viewModelScope) .cachedIn(viewModelScope)
val downloads: StateFlow<Map<String, SongDownload>> = val downloads: StateFlow<Map<String, SongDownload>> =
downloadRepository.states().stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyMap()) downloadRepository.states()
fun download(song: Song) { fun download(song: Song) {
viewModelScope.launch { downloadRepository.download(song.sourceId, song.id) } viewModelScope.launch { downloadRepository.download(song.sourceId, song.id) }
@@ -20,6 +20,8 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip import androidx.compose.ui.draw.clip
import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.Color
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
@@ -117,9 +119,16 @@ private fun DownloadBadge(download: SongDownload?) {
DownloadStatus.Queued, DownloadStatus.Running -> { DownloadStatus.Queued, DownloadStatus.Running -> {
val progress = download.progress val progress = download.progress
if (progress == null) { if (progress == null) {
CircularProgressIndicator(modifier = Modifier.size(14.dp), strokeWidth = 2.dp) CircularProgressIndicator(
modifier = Modifier.size(14.dp).semantics { contentDescription = "Downloading" },
strokeWidth = 2.dp,
)
} else { } else {
CircularProgressIndicator(progress = { progress }, modifier = Modifier.size(14.dp), strokeWidth = 2.dp) CircularProgressIndicator(
progress = { progress },
modifier = Modifier.size(14.dp).semantics { contentDescription = "Downloading" },
strokeWidth = 2.dp,
)
} }
} }
@@ -104,7 +104,7 @@ fun NowPlayingRoute(
val positionMs by controller.positionMs.collectAsStateWithLifecycle() val positionMs by controller.positionMs.collectAsStateWithLifecycle()
val sourceId = state.context?.sourceId val sourceId = state.context?.sourceId
val songId = state.item?.id val songId = state.item?.id
val downloads by downloadRepository.states().collectAsStateWithLifecycle(initialValue = emptyMap()) val downloads by downloadRepository.states().collectAsStateWithLifecycle()
val download = songId?.let { downloads[it] } val download = songId?.let { downloads[it] }
val scope = rememberCoroutineScope() val scope = rememberCoroutineScope()
val song by val song by
@@ -279,7 +279,7 @@ fun QueueRoute(
downloadRepository: DownloadRepository = koinInject(), downloadRepository: DownloadRepository = koinInject(),
) { ) {
val playback by controller.state.collectAsStateWithLifecycle() val playback by controller.state.collectAsStateWithLifecycle()
val downloads by downloadRepository.states().collectAsStateWithLifecycle(initialValue = emptyMap()) val downloads by downloadRepository.states().collectAsStateWithLifecycle()
val queueContext = playback.context val queueContext = playback.context
// The item's album keeps the header stable while the async name lookup runs, so it does not // The item's album keeps the header stable while the async name lookup runs, so it does not
// flash the fallback when tapping between context and manually queued tracks. // flash the fallback when tapping between context and manually queued tracks.
@@ -0,0 +1,31 @@
package com.subtracks.data.download
import android.app.DownloadManager
import androidx.test.ext.junit.runners.AndroidJUnit4
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
@RunWith(AndroidJUnit4::class)
class SystemDownloadEngineTest {
@Test
fun successfulRunningAndFailedStatusesAreMapped() {
assertEquals(EngineStatus.Completed, engineStatus(DownloadManager.STATUS_SUCCESSFUL))
assertEquals(EngineStatus.Running, engineStatus(DownloadManager.STATUS_RUNNING))
assertEquals(EngineStatus.Failed, engineStatus(DownloadManager.STATUS_FAILED))
}
@Test
fun aWaitingDownloadIsPending() {
assertEquals(EngineStatus.Pending, engineStatus(DownloadManager.STATUS_PENDING))
assertEquals(EngineStatus.Pending, engineStatus(DownloadManager.STATUS_PAUSED))
assertEquals(EngineStatus.Pending, engineStatus(Int.MIN_VALUE))
}
@Test
fun failureReasonsReadAsSentences() {
assertEquals("Not enough space to download", failureMessage(DownloadManager.ERROR_INSUFFICIENT_SPACE))
assertEquals("Storage is unavailable", failureMessage(DownloadManager.ERROR_DEVICE_NOT_FOUND))
assertEquals("Download failed", failureMessage(DownloadManager.ERROR_UNKNOWN))
}
}
@@ -173,6 +173,62 @@ class DownloadRepositoryTest {
assertEquals(DownloadStatus.Queued, row(1, "s1")?.status) assertEquals(DownloadStatus.Queued, row(1, "s1")?.status)
} }
@Test
fun aFailedDownloadIsNotRestartedByReconcile() =
runTest {
seedLibrary()
db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Failed, engineId = null, error = "boom"))
runBlocking { repository.reconcile() }
runBlocking { repository.reconcile() }
assertTrue(engine.requests.isEmpty())
assertEquals(DownloadStatus.Failed, row(1, "s1")?.status)
assertEquals("boom", row(1, "s1")?.error)
}
@Test
fun anEngineFailureIsMirroredWithItsMessage() =
runTest {
seedLibrary()
runBlocking { repository.download(1, "s1") }
val id = engine.requests.single().first
engine.fail(id)
runBlocking { repository.reconcile() }
assertEquals(DownloadStatus.Failed, row(1, "s1")?.status)
assertEquals("Download failed", row(1, "s1")?.error)
}
@Test
fun aRunningDownloadWhoseEngineEntryVanishesIsFailedRatherThanRestarted() =
runTest {
seedLibrary()
repository.start()
runBlocking { repository.download(1, "s1") }
val id = engine.requests.single().first
engine.running(id, bytes = 40, total = 100)
await { row(1, "s1")?.status == DownloadStatus.Running }
engine.forget(id)
runBlocking { repository.reconcile() }
assertEquals(DownloadStatus.Failed, row(1, "s1")?.status)
assertEquals(1, engine.requests.size)
}
@Test
fun aSongRemovedFromTheServerTakesItsDownloadRowWithIt() =
runTest {
seedLibrary()
runBlocking { repository.download(1, "s1") }
runBlocking { db.libraryDao().deleteSongs(1, listOf("s1")) }
assertNull(row(1, "s1"))
}
@Test @Test
fun downloadingFromASourceThatIsNotActiveIsRefused() = fun downloadingFromASourceThatIsNotActiveIsRefused() =
runTest { runTest {
@@ -0,0 +1,32 @@
package com.subtracks.playback
import android.content.Context
import android.net.Uri
import androidx.media3.datasource.DataSpec
import androidx.media3.datasource.okhttp.OkHttpDataSource
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import okhttp3.OkHttpClient
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import java.io.File
@RunWith(AndroidJUnit4::class)
class MediaDataSourceTest {
@Test
fun aDownloadedFileIsReadThroughTheStreamingChain() {
val context = ApplicationProvider.getApplicationContext<Context>()
val file = File(context.cacheDir, "downloaded-${System.nanoTime()}").apply { writeBytes(byteArrayOf(1, 2, 3, 4)) }
val dataSource = mediaDataSourceFactory(context, OkHttpDataSource.Factory(OkHttpClient())).createDataSource()
val length = dataSource.open(DataSpec(Uri.fromFile(file)))
val buffer = ByteArray(4)
val read = dataSource.read(buffer, 0, buffer.size)
dataSource.close()
assertEquals(4L, length)
assertEquals(4, read)
assertEquals(listOf<Byte>(1, 2, 3, 4), buffer.toList())
}
}
@@ -45,10 +45,11 @@ class SongRowDownloadTest {
} }
@Test @Test
fun aRunningDownloadShowsNoDownloadedIndicator() { fun aRunningDownloadShowsProgressRatherThanTheDownloadedIndicator() {
render(download(DownloadStatus.Running)) render(download(DownloadStatus.Running))
composeRule.onNodeWithContentDescription("Downloaded").assertDoesNotExist() composeRule.onNodeWithContentDescription("Downloaded").assertDoesNotExist()
composeRule.onNodeWithContentDescription("Downloading").assertIsDisplayed()
} }
@Test @Test