From 8b3d7cae9c33f132452f962a9062519c86b602c1 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Fri, 27 Dec 2024 15:11:09 -0500 Subject: [PATCH] musikr: handle missing covers on recaching Now that we have effectively two caches (The main cache and the covers), we have to handle the case where we have cached data, but the cover data is missing. This is a real-world edge case once album covers are made configurable as they were previously. --- .../auxio/music/covers/SiloedCovers.kt | 26 +++++++------ .../main/java/org/oxycblt/musikr/Config.kt | 4 +- .../java/org/oxycblt/musikr/cache/Cache.kt | 4 +- .../org/oxycblt/musikr/cache/CacheDatabase.kt | 19 +++++++-- .../org/oxycblt/musikr/cache/StoredCache.kt | 9 +++-- .../org/oxycblt/musikr/cover/CoverFiles.kt | 6 +-- .../cover/{StoredCovers.kt => Covers.kt} | 39 ++++++++++++------- .../oxycblt/musikr/pipeline/ExtractStep.kt | 4 +- 8 files changed, 68 insertions(+), 43 deletions(-) rename musikr/src/main/java/org/oxycblt/musikr/cover/{StoredCovers.kt => Covers.kt} (64%) diff --git a/app/src/main/java/org/oxycblt/auxio/music/covers/SiloedCovers.kt b/app/src/main/java/org/oxycblt/auxio/music/covers/SiloedCovers.kt index ac67a5363..d60cd375b 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/covers/SiloedCovers.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/covers/SiloedCovers.kt @@ -28,23 +28,25 @@ import org.oxycblt.musikr.cover.Cover import org.oxycblt.musikr.cover.CoverFiles import org.oxycblt.musikr.cover.CoverFormat import org.oxycblt.musikr.cover.CoverParams -import org.oxycblt.musikr.cover.MutableStoredCovers -import org.oxycblt.musikr.cover.StoredCovers +import org.oxycblt.musikr.cover.Covers +import org.oxycblt.musikr.cover.MutableCovers +import org.oxycblt.musikr.cover.ObtainResult class SiloedCovers( private val rootDir: File, private val silo: CoverSilo, - private val inner: MutableStoredCovers -) : MutableStoredCovers { - override suspend fun obtain(id: String): SiloedCover? { - val coverId = SiloedCoverId.parse(id) ?: return null - if (coverId.silo != silo) return null - return inner.obtain(coverId.id)?.let { SiloedCover(coverId.silo, it) } + private val inner: MutableCovers +) : MutableCovers { + override suspend fun obtain(id: String): ObtainResult { + val coverId = SiloedCoverId.parse(id) ?: return ObtainResult.Miss + if (coverId.silo != silo) return ObtainResult.Miss + return when (val result = inner.obtain(coverId.id)) { + is ObtainResult.Hit -> ObtainResult.Hit(SiloedCover(silo, result.cover)) + is ObtainResult.Miss -> ObtainResult.Miss + } } - override suspend fun write(data: ByteArray): SiloedCover? { - return inner.write(data)?.let { SiloedCover(silo, it) } - } + override suspend fun write(data: ByteArray) = SiloedCover(silo, inner.write(data)) override suspend fun cleanup(assuming: Library) { inner.cleanup(assuming) @@ -66,7 +68,7 @@ class SiloedCovers( } val files = CoverFiles.at(revisionDir) val format = CoverFormat.jpeg(silo.params) - return SiloedCovers(rootDir, silo, StoredCovers.from(files, format)) + return SiloedCovers(rootDir, silo, Covers.from(files, format)) } } } diff --git a/musikr/src/main/java/org/oxycblt/musikr/Config.kt b/musikr/src/main/java/org/oxycblt/musikr/Config.kt index c3d720847..cdd5a3f56 100644 --- a/musikr/src/main/java/org/oxycblt/musikr/Config.kt +++ b/musikr/src/main/java/org/oxycblt/musikr/Config.kt @@ -19,14 +19,14 @@ package org.oxycblt.musikr import org.oxycblt.musikr.cache.Cache -import org.oxycblt.musikr.cover.MutableStoredCovers +import org.oxycblt.musikr.cover.MutableCovers import org.oxycblt.musikr.playlist.db.StoredPlaylists import org.oxycblt.musikr.tag.interpret.Naming import org.oxycblt.musikr.tag.interpret.Separators data class Storage( val cache: Cache.Factory, - val storedCovers: MutableStoredCovers, + val storedCovers: MutableCovers, val storedPlaylists: StoredPlaylists ) diff --git a/musikr/src/main/java/org/oxycblt/musikr/cache/Cache.kt b/musikr/src/main/java/org/oxycblt/musikr/cache/Cache.kt index 94d37726e..277495d3a 100644 --- a/musikr/src/main/java/org/oxycblt/musikr/cache/Cache.kt +++ b/musikr/src/main/java/org/oxycblt/musikr/cache/Cache.kt @@ -18,12 +18,12 @@ package org.oxycblt.musikr.cache -import org.oxycblt.musikr.cover.StoredCovers +import org.oxycblt.musikr.cover.Covers import org.oxycblt.musikr.fs.DeviceFile import org.oxycblt.musikr.pipeline.RawSong abstract class Cache { - internal abstract suspend fun read(file: DeviceFile, storedCovers: StoredCovers): CacheResult + internal abstract suspend fun read(file: DeviceFile, covers: Covers): CacheResult internal abstract suspend fun write(song: RawSong) diff --git a/musikr/src/main/java/org/oxycblt/musikr/cache/CacheDatabase.kt b/musikr/src/main/java/org/oxycblt/musikr/cache/CacheDatabase.kt index dd4d8df63..9f7678089 100644 --- a/musikr/src/main/java/org/oxycblt/musikr/cache/CacheDatabase.kt +++ b/musikr/src/main/java/org/oxycblt/musikr/cache/CacheDatabase.kt @@ -31,7 +31,8 @@ import androidx.room.RoomDatabase import androidx.room.Transaction import androidx.room.TypeConverter import androidx.room.TypeConverters -import org.oxycblt.musikr.cover.StoredCovers +import org.oxycblt.musikr.cover.Covers +import org.oxycblt.musikr.cover.ObtainResult import org.oxycblt.musikr.fs.DeviceFile import org.oxycblt.musikr.metadata.Properties import org.oxycblt.musikr.pipeline.RawSong @@ -117,8 +118,17 @@ internal data class CachedSong( val replayGainAlbumAdjustment: Float?, val coverId: String?, ) { - suspend fun intoRawSong(file: DeviceFile, storedCovers: StoredCovers) = - RawSong( + suspend fun intoRawSong(file: DeviceFile, covers: Covers): RawSong? { + val cover = + when (val result = coverId?.let { covers.obtain(it) }) { + // We found the cover. + is ObtainResult.Hit -> result.cover + // We actually didn't find the cover, can't safely convert. + is ObtainResult.Miss -> return null + // No cover in the first place, can ignore. + null -> null + } + return RawSong( file, Properties(mimeType, durationMs, bitrateHz, sampleRateHz), ParsedTags( @@ -143,8 +153,9 @@ internal data class CachedSong( genreNames = genreNames, replayGainTrackAdjustment = replayGainTrackAdjustment, replayGainAlbumAdjustment = replayGainAlbumAdjustment), - coverId?.let { storedCovers.obtain(it) }, + cover = cover, addedMs = addedMs) + } object Converters { @TypeConverter diff --git a/musikr/src/main/java/org/oxycblt/musikr/cache/StoredCache.kt b/musikr/src/main/java/org/oxycblt/musikr/cache/StoredCache.kt index 5f4c95d70..8e5144f67 100644 --- a/musikr/src/main/java/org/oxycblt/musikr/cache/StoredCache.kt +++ b/musikr/src/main/java/org/oxycblt/musikr/cache/StoredCache.kt @@ -19,7 +19,7 @@ package org.oxycblt.musikr.cache import android.content.Context -import org.oxycblt.musikr.cover.StoredCovers +import org.oxycblt.musikr.cover.Covers import org.oxycblt.musikr.fs.DeviceFile import org.oxycblt.musikr.pipeline.RawSong @@ -53,7 +53,7 @@ private abstract class BaseStoredCache(protected val writeDao: CacheWriteDao) : private class VisibleStoredCache(private val visibleDao: VisibleCacheDao, writeDao: CacheWriteDao) : BaseStoredCache(writeDao) { - override suspend fun read(file: DeviceFile, storedCovers: StoredCovers): CacheResult { + override suspend fun read(file: DeviceFile, covers: Covers): CacheResult { val song = visibleDao.selectSong(file.uri.toString()) ?: return CacheResult.Miss(file, null) if (song.modifiedMs != file.lastModified) { // We *found* this file earlier, but it's out of date. @@ -63,7 +63,8 @@ private class VisibleStoredCache(private val visibleDao: VisibleCacheDao, writeD } // Valid file, update the touch time. visibleDao.touch(file.uri.toString()) - return CacheResult.Hit(song.intoRawSong(file, storedCovers)) + val rawSong = song.intoRawSong(file, covers) ?: return CacheResult.Miss(file, song.addedMs) + return CacheResult.Hit(rawSong) } class Factory(private val cacheDatabase: CacheDatabase) : Cache.Factory() { @@ -76,7 +77,7 @@ private class InvisibleStoredCache( private val invisibleCacheDao: InvisibleCacheDao, writeDao: CacheWriteDao ) : BaseStoredCache(writeDao) { - override suspend fun read(file: DeviceFile, storedCovers: StoredCovers) = + override suspend fun read(file: DeviceFile, covers: Covers) = CacheResult.Miss(file, invisibleCacheDao.selectAddedMs(file.uri.toString())) class Factory(private val cacheDatabase: CacheDatabase) : Cache.Factory() { diff --git a/musikr/src/main/java/org/oxycblt/musikr/cover/CoverFiles.kt b/musikr/src/main/java/org/oxycblt/musikr/cover/CoverFiles.kt index 39a35cf93..d3b960892 100644 --- a/musikr/src/main/java/org/oxycblt/musikr/cover/CoverFiles.kt +++ b/musikr/src/main/java/org/oxycblt/musikr/cover/CoverFiles.kt @@ -30,7 +30,7 @@ import kotlinx.coroutines.withContext interface CoverFiles { suspend fun find(name: String): CoverFile? - suspend fun write(name: String, block: suspend (OutputStream) -> Unit): CoverFile? + suspend fun write(name: String, block: suspend (OutputStream) -> Unit): CoverFile suspend fun deleteWhere(block: (String) -> Boolean) @@ -63,7 +63,7 @@ private class CoverFilesImpl(private val dir: File) : CoverFiles { } } - override suspend fun write(name: String, block: suspend (OutputStream) -> Unit): CoverFile? { + override suspend fun write(name: String, block: suspend (OutputStream) -> Unit): CoverFile { val fileMutex = getMutexForFile(name) return fileMutex.withLock { val targetFile = File(dir, name) @@ -77,7 +77,7 @@ private class CoverFilesImpl(private val dir: File) : CoverFiles { CoverFileImpl(targetFile) } catch (e: IOException) { tempFile.delete() - null + throw e } } } else { diff --git a/musikr/src/main/java/org/oxycblt/musikr/cover/StoredCovers.kt b/musikr/src/main/java/org/oxycblt/musikr/cover/Covers.kt similarity index 64% rename from musikr/src/main/java/org/oxycblt/musikr/cover/StoredCovers.kt rename to musikr/src/main/java/org/oxycblt/musikr/cover/Covers.kt index 6f68851a8..d2fa7304d 100644 --- a/musikr/src/main/java/org/oxycblt/musikr/cover/StoredCovers.kt +++ b/musikr/src/main/java/org/oxycblt/musikr/cover/Covers.kt @@ -1,6 +1,6 @@ /* * Copyright (c) 2024 Auxio Project - * StoredCovers.kt is part of Auxio. + * Covers.kt is part of Auxio. * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -20,37 +20,48 @@ package org.oxycblt.musikr.cover import org.oxycblt.musikr.Library -interface StoredCovers { - suspend fun obtain(id: String): Cover? +interface Covers { + suspend fun obtain(id: String): ObtainResult companion object { fun from( coverFiles: CoverFiles, coverFormat: CoverFormat, identifier: CoverIdentifier = CoverIdentifier.md5() - ): MutableStoredCovers = FileStoredCovers(coverFiles, coverFormat, identifier) + ): MutableCovers = FileCovers(coverFiles, coverFormat, identifier) } } -interface MutableStoredCovers : StoredCovers { - suspend fun write(data: ByteArray): Cover? +interface MutableCovers : Covers { + suspend fun write(data: ByteArray): Cover suspend fun cleanup(assuming: Library) } -private class FileStoredCovers( +sealed interface ObtainResult { + data class Hit(val cover: Cover) : ObtainResult + + data object Miss : ObtainResult +} + +private class FileCovers( private val coverFiles: CoverFiles, private val coverFormat: CoverFormat, private val coverIdentifier: CoverIdentifier, -) : StoredCovers, MutableStoredCovers { - override suspend fun obtain(id: String) = - coverFiles.find(getFileName(id))?.let { FileCover(id, it) } +) : Covers, MutableCovers { + override suspend fun obtain(id: String): ObtainResult { + val file = coverFiles.find(getFileName(id)) + return if (file != null) { + ObtainResult.Hit(FileCover(id, file)) + } else { + ObtainResult.Miss + } + } - override suspend fun write(data: ByteArray): Cover? { + override suspend fun write(data: ByteArray): Cover { val id = coverIdentifier.identify(data) - return coverFiles - .write(getFileName(id)) { coverFormat.transcodeInto(data, it) } - ?.let { FileCover(id, it) } + val file = coverFiles.write(getFileName(id)) { coverFormat.transcodeInto(data, it) } + return FileCover(id, file) } override suspend fun cleanup(assuming: Library) { diff --git a/musikr/src/main/java/org/oxycblt/musikr/pipeline/ExtractStep.kt b/musikr/src/main/java/org/oxycblt/musikr/pipeline/ExtractStep.kt index d42a17389..ad34d8f82 100644 --- a/musikr/src/main/java/org/oxycblt/musikr/pipeline/ExtractStep.kt +++ b/musikr/src/main/java/org/oxycblt/musikr/pipeline/ExtractStep.kt @@ -35,7 +35,7 @@ import org.oxycblt.musikr.Storage import org.oxycblt.musikr.cache.Cache import org.oxycblt.musikr.cache.CacheResult import org.oxycblt.musikr.cover.Cover -import org.oxycblt.musikr.cover.MutableStoredCovers +import org.oxycblt.musikr.cover.MutableCovers import org.oxycblt.musikr.fs.DeviceFile import org.oxycblt.musikr.metadata.MetadataExtractor import org.oxycblt.musikr.metadata.Properties @@ -62,7 +62,7 @@ private class ExtractStepImpl( private val metadataExtractor: MetadataExtractor, private val tagParser: TagParser, private val cacheFactory: Cache.Factory, - private val storedCovers: MutableStoredCovers + private val storedCovers: MutableCovers ) : ExtractStep { @OptIn(ExperimentalCoroutinesApi::class) override fun extract(nodes: Flow): Flow {