Merge branch 'music-fixes' into 3.5.0

.github/workflows/android.yml

@@ -2,9 +2,9 @@ name: Android CI
 
 on:
   push:
-    branches: [ "dev" ]
+    branches: []
   pull_request:
-    branches: [ "dev" ]
+    branches: []
     
 jobs:
   build:

app/src/main/java/org/oxycblt/auxio/image/extractor/CoverExtractor.kt

@@ -157,8 +157,8 @@ constructor(
         }
 
     private suspend fun extractQualityCover(cover: Cover.Embedded) =
-        extractAospMetadataCover(cover)
+        extractExoplayerCover(cover)
-            ?: extractExoplayerCover(cover) ?: extractMediaStoreCover(cover)
+            ?: extractAospMetadataCover(cover) ?: extractMediaStoreCover(cover)
 
     private fun extractAospMetadataCover(cover: Cover.Embedded): InputStream? =
         MediaMetadataRetriever().run {

app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt

@@ -164,7 +164,10 @@ constructor(
                 }
 
                 val deviceLibrary = musicRepository.deviceLibrary ?: return@launch
-                val songs = importedPlaylist.paths.mapNotNull(deviceLibrary::findSongByPath)
+                val songs =
+                    importedPlaylist.paths.mapNotNull {
+                        it.firstNotNullOfOrNull(deviceLibrary::findSongByPath)
+                    }
 
                 if (songs.isEmpty()) {
                     logE("No songs found")

app/src/main/java/org/oxycblt/auxio/music/external/ExternalPlaylistManager.kt

@@ -76,7 +76,9 @@ data class ExportConfig(val absolute: Boolean, val windowsPaths: Boolean)
  * @see ExternalPlaylistManager
  * @see M3U
  */
-data class ImportedPlaylist(val name: String?, val paths: List<Path>)
+data class ImportedPlaylist(val name: String?, val paths: List<PossiblePaths>)
+
+typealias PossiblePaths = List<Path>
 
 class ExternalPlaylistManagerImpl
 @Inject

app/src/main/java/org/oxycblt/auxio/music/external/M3U.kt

@@ -29,9 +29,12 @@ import javax.inject.Inject
 import org.oxycblt.auxio.music.Playlist
 import org.oxycblt.auxio.music.fs.Components
 import org.oxycblt.auxio.music.fs.Path
+import org.oxycblt.auxio.music.fs.Volume
+import org.oxycblt.auxio.music.fs.VolumeManager
 import org.oxycblt.auxio.music.metadata.correctWhitespace
 import org.oxycblt.auxio.music.resolveNames
 import org.oxycblt.auxio.util.logE
+import org.oxycblt.auxio.util.unlikelyToBeNull
 
 /**
  * Minimal M3U file format implementation.
@@ -72,10 +75,16 @@ interface M3U {
     }
 }
 
-class M3UImpl @Inject constructor(@ApplicationContext private val context: Context) : M3U {
+class M3UImpl
+@Inject
+constructor(
+    @ApplicationContext private val context: Context,
+    private val volumeManager: VolumeManager
+) : M3U {
     override fun read(stream: InputStream, workingDirectory: Path): ImportedPlaylist? {
+        val volumes = volumeManager.getVolumes()
         val reader = BufferedReader(InputStreamReader(stream))
-        val paths = mutableListOf<Path>()
+        val paths = mutableListOf<PossiblePaths>()
         var name: String? = null
 
         consumeFile@ while (true) {
@@ -112,39 +121,13 @@ class M3UImpl @Inject constructor(@ApplicationContext private val context: Conte
             }
 
             // There is basically no formal specification of file paths in M3U, and it differs
-            // based on the US that generated it. These are the paths though that I assume most
+            // based on the programs that generated it. I more or less have to consider any possible
-            // programs will generate.
+            // interpretation as valid.
-            val components =
+            val interpretations = interpretPath(path)
-                when {
+            val possibilities =
-                    path.startsWith('/') -> {
+                interpretations.flatMap { expandInterpretation(it, workingDirectory, volumes) }
-                        // Unix absolute path. Note that we still assume this absolute path is in
-                        // the same volume as the M3U file. There's no sane way to map the volume
-                        // to the phone's volumes, so this is the only thing we can do.
-                        Components.parseUnix(path)
-                    }
-                    path.startsWith("./") -> {
-                        // Unix relative path, resolve it
-                        Components.parseUnix(path).absoluteTo(workingDirectory.components)
-                    }
-                    path.matches(WINDOWS_VOLUME_PREFIX_REGEX) -> {
-                        // Windows absolute path, we should get rid of the volume prefix, but
-                        // otherwise
-                        // the rest should be fine. Again, we have to disregard what the volume
-                        // actually
-                        // is since there's no sane way to map it to the phone's volumes.
-                        Components.parseWindows(path.substring(2))
-                    }
-                    path.startsWith(".\\") -> {
-                        // Windows relative path, we need to remove the .\\ prefix
-                        Components.parseWindows(path).absoluteTo(workingDirectory.components)
-                    }
-                    else -> {
-                        // No clue, parse by all separators and assume it's relative.
-                        Components.parseAny(path).absoluteTo(workingDirectory.components)
-                    }
-                }
 
-            paths.add(Path(workingDirectory.volume, components))
+            paths.add(possibilities)
         }
 
         return if (paths.isNotEmpty()) {
@@ -155,6 +138,44 @@ class M3UImpl @Inject constructor(@ApplicationContext private val context: Conte
         }
     }
 
+    private data class InterpretedPath(val components: Components, val likelyAbsolute: Boolean)
+
+    private fun interpretPath(path: String): List<InterpretedPath> =
+        when {
+            path.startsWith('/') -> listOf(InterpretedPath(Components.parseUnix(path), true))
+            path.startsWith("./") -> listOf(InterpretedPath(Components.parseUnix(path), false))
+            path.matches(WINDOWS_VOLUME_PREFIX_REGEX) ->
+                listOf(InterpretedPath(Components.parseWindows(path.substring(2)), true))
+            path.startsWith("\\") -> listOf(InterpretedPath(Components.parseWindows(path), true))
+            path.startsWith(".\\") -> listOf(InterpretedPath(Components.parseWindows(path), false))
+            else ->
+                listOf(
+                    InterpretedPath(Components.parseUnix(path), false),
+                    InterpretedPath(Components.parseWindows(path), true))
+        }
+
+    private fun expandInterpretation(
+        path: InterpretedPath,
+        workingDirectory: Path,
+        volumes: List<Volume>
+    ): List<Path> {
+        val absoluteInterpretation = Path(workingDirectory.volume, path.components)
+        val relativeInterpretation =
+            Path(workingDirectory.volume, path.components.absoluteTo(workingDirectory.components))
+        val volumeExactMatch = volumes.find { it.components?.contains(path.components) == true }
+        val volumeInterpretation =
+            volumeExactMatch?.let {
+                val components =
+                    unlikelyToBeNull(volumeExactMatch.components).containing(path.components)
+                Path(volumeExactMatch, components)
+            }
+        return if (path.likelyAbsolute) {
+            listOfNotNull(volumeInterpretation, absoluteInterpretation, relativeInterpretation)
+        } else {
+            listOfNotNull(relativeInterpretation, volumeInterpretation, absoluteInterpretation)
+        }
+    }
+
     override fun write(
         playlist: Playlist,
         outputStream: OutputStream,

app/src/main/java/org/oxycblt/auxio/music/fs/Fs.kt

@@ -158,6 +158,8 @@ value class Components private constructor(val components: List<String>) {
         return components == other.components.take(components.size)
     }
 
+    fun containing(other: Components) = Components(other.components.drop(components.size))
+
     companion object {
         /**
          * Parses a path string into a [Components] instance by the unix path separator (/).

app/src/main/java/org/oxycblt/auxio/music/metadata/TagWorker.kt

@@ -140,7 +140,9 @@ private class TagWorkerImpl(
 
     private fun populateWithId3v2(textFrames: Map<String, List<String>>) {
         // Song
-        textFrames["TXXX:musicbrainz release track id"]?.let { rawSong.musicBrainzId = it.first() }
+        (textFrames["TXXX:musicbrainz release track id"]
+                ?: textFrames["TXXX:musicbrainz_releasetrackid"])
+            ?.let { rawSong.musicBrainzId = it.first() }
         textFrames["TIT2"]?.let { rawSong.name = it.first() }
         textFrames["TSOT"]?.let { rawSong.sortName = it.first() }
 
@@ -170,7 +172,9 @@ private class TagWorkerImpl(
             ?.let { rawSong.date = it }
 
         // Album
-        textFrames["TXXX:musicbrainz album id"]?.let { rawSong.albumMusicBrainzId = it.first() }
+        (textFrames["TXXX:musicbrainz album id"] ?: textFrames["TXXX:musicbrainz_albumid"])?.let {
+            rawSong.albumMusicBrainzId = it.first()
+        }
         textFrames["TALB"]?.let { rawSong.albumName = it.first() }
         textFrames["TSOA"]?.let { rawSong.albumSortName = it.first() }
         (textFrames["TXXX:musicbrainz album type"]
@@ -180,7 +184,9 @@ private class TagWorkerImpl(
             ?.let { rawSong.releaseTypes = it }
 
         // Artist
-        textFrames["TXXX:musicbrainz artist id"]?.let { rawSong.artistMusicBrainzIds = it }
+        (textFrames["TXXX:musicbrainz artist id"] ?: textFrames["TXXX:musicbrainz_artistid"])?.let {
+            rawSong.artistMusicBrainzIds = it
+        }
         (textFrames["TXXX:artists"] ?: textFrames["TPE1"])?.let { rawSong.artistNames = it }
         (textFrames["TXXX:artistssort"]
                 ?: textFrames["TXXX:artists_sort"] ?: textFrames["TXXX:artists sort"]
@@ -188,9 +194,9 @@ private class TagWorkerImpl(
             ?.let { rawSong.artistSortNames = it }
 
         // Album artist
-        textFrames["TXXX:musicbrainz album artist id"]?.let {
+        (textFrames["TXXX:musicbrainz album artist id"]
-            rawSong.albumArtistMusicBrainzIds = it
+                ?: textFrames["TXXX:musicbrainz_albumartistid"])
-        }
+            ?.let { rawSong.albumArtistMusicBrainzIds = it }
         (textFrames["TXXX:albumartists"]
                 ?: textFrames["TXXX:album_artists"] ?: textFrames["TXXX:album artists"]
                     ?: textFrames["TPE2"])
@@ -261,7 +267,9 @@ private class TagWorkerImpl(
 
     private fun populateWithVorbis(comments: Map<String, List<String>>) {
         // Song
-        comments["musicbrainz_releasetrackid"]?.let { rawSong.musicBrainzId = it.first() }
+        (comments["musicbrainz_releasetrackid"] ?: comments["musicbrainz release track id"])?.let {
+            rawSong.musicBrainzId = it.first()
+        }
         comments["title"]?.let { rawSong.name = it.first() }
         comments["titlesort"]?.let { rawSong.sortName = it.first() }
 
@@ -290,20 +298,28 @@ private class TagWorkerImpl(
             ?.let { rawSong.date = it }
 
         // Album
-        comments["musicbrainz_albumid"]?.let { rawSong.albumMusicBrainzId = it.first() }
+        (comments["musicbrainz_albumid"] ?: comments["musicbrainz album id"])?.let {
+            rawSong.albumMusicBrainzId = it.first()
+        }
         comments["album"]?.let { rawSong.albumName = it.first() }
         comments["albumsort"]?.let { rawSong.albumSortName = it.first() }
-        comments["releasetype"]?.let { rawSong.releaseTypes = it }
+        (comments["releasetype"] ?: comments["musicbrainz album type"])?.let {
+            rawSong.releaseTypes = it
+        }
 
         // Artist
-        comments["musicbrainz_artistid"]?.let { rawSong.artistMusicBrainzIds = it }
+        (comments["musicbrainz_artistid"] ?: comments["musicbrainz artist id"])?.let {
+            rawSong.artistMusicBrainzIds = it
+        }
         (comments["artists"] ?: comments["artist"])?.let { rawSong.artistNames = it }
         (comments["artistssort"]
                 ?: comments["artists_sort"] ?: comments["artists sort"] ?: comments["artistsort"])
             ?.let { rawSong.artistSortNames = it }
 
         // Album artist
-        comments["musicbrainz_albumartistid"]?.let { rawSong.albumArtistMusicBrainzIds = it }
+        (comments["musicbrainz_albumartistid"] ?: comments["musicbrainz album artist id"])?.let {
+            rawSong.albumArtistMusicBrainzIds = it
+        }
         (comments["albumartists"]
                 ?: comments["album_artists"] ?: comments["album artists"]
                     ?: comments["albumartist"])

app/src/main/java/org/oxycblt/auxio/util/StateUtil.kt

@@ -32,6 +32,7 @@ import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withTimeout
+import org.oxycblt.auxio.BuildConfig
 
 /**
  * A wrapper around [StateFlow] exposing a one-time consumable event.
@@ -166,7 +167,13 @@ suspend fun <E> SendChannel<E>.sendWithTimeout(element: E, timeout: Long = DEFAU
     try {
         withTimeout(timeout) { send(element) }
     } catch (e: TimeoutCancellationException) {
-        throw TimeoutException("Timed out sending element $element to channel: $e")
+        logE("Failed to send element to channel $e in ${timeout}ms.")
+        if (BuildConfig.DEBUG) {
+            throw TimeoutException("Timed out sending element to channel: $e")
+        } else {
+            logE(e.stackTraceToString())
+            send(element)
+        }
     }
 }
 
@@ -203,7 +210,13 @@ suspend fun <E> ReceiveChannel<E>.forEachWithTimeout(
                 subsequent = true
             }
         } catch (e: TimeoutCancellationException) {
-            throw TimeoutException("Timed out receiving element from channel: $e")
+            logE("Failed to send element to channel $e in ${timeout}ms.")
+            if (BuildConfig.DEBUG) {
+                throw TimeoutException("Timed out sending element to channel: $e")
+            } else {
+                logE(e.stackTraceToString())
+                handler()
+            }
         }
     }
 }