Merge pull request #1009 from OxygenCobalt/dev

v4.0.1

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 4.0.1
+
+#### What's Fixed
+- Fixed music loading hanging on files without tags
+- Fixed playlists being destroyed in poorly tagged libraries
+
 ## 4.0.0
 
 #### What's New

README.md

@@ -2,8 +2,8 @@
 <h1 align="center"><b>Auxio</b></h1>
 <h4 align="center">A simple, rational music player for android.</h4>
 <p align="center">
-    <a href="https://github.com/oxygencobalt/Auxio/releases/tag/v4.0.0">
+    <a href="https://github.com/oxygencobalt/Auxio/releases/tag/v4.0.1">
-        <img alt="Latest Version" src="https://img.shields.io/static/v1?label=tag&message=v4.0.0&color=64B5F6&style=flat">
+        <img alt="Latest Version" src="https://img.shields.io/static/v1?label=tag&message=v4.0.1&color=64B5F6&style=flat">
     </a>
     <a href="https://github.com/oxygencobalt/Auxio/releases/">
         <img alt="Releases" src="https://img.shields.io/github/downloads/OxygenCobalt/Auxio/total.svg?color=4B95DE&style=flat">

app/build.gradle

@@ -18,8 +18,8 @@ android {
 
     defaultConfig {
         applicationId namespace
-        versionName "4.0.0"
+        versionName "4.0.1"
-        versionCode 59
+        versionCode 60
 
         minSdk min_sdk
         targetSdk target_sdk

fastlane/metadata/android/en-US/changelogs/60.txt

@@ -0,0 +1,4 @@
+Auxio 4.0.0 completely overhauls the user experience, with a refreshed design based on the latest Material Design specs
+and a brand new music loader with signifigant improvements to device and tag support.
+This issue fixes critical issues with music loading.
+For more information, see https://github.com/OxygenCobalt/Auxio/releases/tag/v4.0.1.

musikr/src/main/cpp/JInputStream.cpp

@@ -34,7 +34,7 @@ JInputStream::JInputStream(JNIEnv *env, jobject jInputStream) : env(env), jInput
     jmethodID jInputStreamNameMethod = jInputStreamClass.method("name",
             "()Ljava/lang/String;");
     jInputStreamReadBlockMethod = jInputStreamClass.method("readBlock",
-            "(Ljava/nio/ByteBuffer;)Z");
+            "(Ljava/nio/ByteBuffer;)I");
     jInputStreamIsOpenMethod = jInputStreamClass.method("isOpen", "()Z");
     jInputStreamSeekFromBeginningMethod = jInputStreamClass.method(
             "seekFromBeginning", "(J)Z");
@@ -58,22 +58,31 @@ TagLib::FileName /* const char * */JInputStream::name() const {
     return _name.toCString(true);
 }
 
-TagLib::ByteVector JInputStream::readBlock(size_t length) {
+jint JInputStream::readBlockImpl(TagLib::ByteVector &buf) {
-    // We have to invert the buffer allocation here siits not a perfect system (vykeen instead of korvax0 but i warped all over the hub and i dont think its possible to find a "perfect" purple system like you would withnce the JVM ByteBuffer allocation system
-    // uses a bugged caching mechanism that leaks memory if used in multithreaded contexts.
-    TagLib::ByteVector buf { static_cast<unsigned int>(length), 0 };
     jobject wrappedByteBuffer = env->NewDirectByteBuffer(buf.data(),
             buf.size());
     if (wrappedByteBuffer == nullptr) {
         throw std::runtime_error("Failed to wrap ByteBuffer");
     }
-    JObjectRef byteBuffer = { env, wrappedByteBuffer };
+    JObjectRef byteBuffer { env, wrappedByteBuffer };
-    jboolean result = env->CallBooleanMethod(jInputStream,
+    jint read = env->CallIntMethod(jInputStream, jInputStreamReadBlockMethod,
-            jInputStreamReadBlockMethod, *byteBuffer);
+            *byteBuffer);
-    if (!result) {
+    return read;
+}
+
+TagLib::ByteVector JInputStream::readBlock(size_t length) {
+    // We have to invert the buffer allocation here
+    TagLib::ByteVector buf { static_cast<unsigned int>(length), 0 };
+    jint read = readBlockImpl(buf);
+    if (read >= 0) {
+        buf.resize(read);
+        return buf;
+    } else if (read == -1) {
+        buf.resize(0);
+        return buf;
+    } else {
         throw std::runtime_error("Failed to read block, see logs");
     }
-    return buf;
 }
 
 void JInputStream::writeBlock(const TagLib::ByteVector &data) {

musikr/src/main/cpp/JInputStream.h

@@ -124,6 +124,7 @@ private:
     jmethodID jInputStreamSeekFromEndMethod;
     jmethodID jInputStreamTellMethod;
     jmethodID jInputStreamLengthMethod;
+    jint readBlockImpl(TagLib::ByteVector &buf);
 };
 
 #endif //AUXIO_JINPUTSTREAM_H

musikr/src/main/java/org/oxycblt/musikr/cache/CacheDatabase.kt

@@ -41,7 +41,7 @@ import org.oxycblt.musikr.tag.parse.ParsedTags
 import org.oxycblt.musikr.util.correctWhitespace
 import org.oxycblt.musikr.util.splitEscaped
 
-@Database(entities = [CachedSong::class], version = 58, exportSchema = false)
+@Database(entities = [CachedSong::class], version = 60, exportSchema = false)
 internal abstract class CacheDatabase : RoomDatabase() {
     abstract fun visibleDao(): VisibleCacheDao
 
@@ -97,7 +97,7 @@ internal data class CachedSong(
     val bitrateHz: Int,
     val sampleRateHz: Int,
     val musicBrainzId: String?,
-    val name: String,
+    val name: String?,
     val sortName: String?,
     val track: Int?,
     val disc: Int?,

musikr/src/main/java/org/oxycblt/musikr/graph/MusicGraph.kt

@@ -55,7 +55,7 @@ private class MusicGraphBuilderImpl : MusicGraph.Builder {
     private val playlistVertices = mutableSetOf<PlaylistVertex>()
 
     override fun add(preSong: PreSong) {
-        val uid = preSong.uid
+        val uid = preSong.v363Uid
         if (songVertices.containsKey(uid)) {
             return
         }
@@ -140,8 +140,10 @@ private class MusicGraphBuilderImpl : MusicGraph.Builder {
             vertex.genreVertices = vertex.genreVertices.distinct().toMutableList()
 
             playlistVertices.forEach {
-                val pointer = SongPointer.UID(entry.key)
+                val v363Pointer = SongPointer.UID(entry.key)
-                it.pointerMap[pointer]?.forEach { index -> it.songVertices[index] = vertex }
+                it.pointerMap[v363Pointer]?.forEach { index -> it.songVertices[index] = vertex }
+                val v400Pointer = SongPointer.UID(entry.value.preSong.v400Uid)
+                it.pointerMap[v400Pointer]?.forEach { index -> it.songVertices[index] = vertex }
             }
         }
 

musikr/src/main/java/org/oxycblt/musikr/metadata/NativeInputStream.kt

@@ -28,13 +28,12 @@ internal class NativeInputStream(private val deviceFile: DeviceFile, fis: FileIn
 
     fun name() = requireNotNull(deviceFile.path.name)
 
-    fun readBlock(buf: ByteBuffer): Boolean {
+    fun readBlock(buf: ByteBuffer): Int {
         try {
-            channel.read(buf)
+            return channel.read(buf)
-            return true
         } catch (e: Exception) {
             Log.d("NativeInputStream", "Error reading block", e)
-            return false
+            return -2
         }
     }
 

musikr/src/main/java/org/oxycblt/musikr/model/AlbumImpl.kt

@@ -50,7 +50,7 @@ class AlbumImpl internal constructor(private val core: AlbumCore) : Album {
                 // I don't know if there is any situation where an artist will have two albums with
                 // the exact same name, but if there is, I would love to know.
                 update(preAlbum.rawName)
-                update(preAlbum.preArtists.map { it.rawName })
+                update(preAlbum.preArtists.mapNotNull { it.rawName })
             }
     override val name = preAlbum.name
     override val releaseType = preAlbum.releaseType

musikr/src/main/java/org/oxycblt/musikr/model/LibraryImpl.kt

@@ -37,6 +37,7 @@ internal data class LibraryImpl(
     private val playlistInterpreter: PlaylistInterpreter
 ) : MutableLibrary {
     private val songUidMap = songs.associateBy { it.uid }
+    private val v400SongUidMap = songs.associateBy { it.v400Uid }
     private val albumUidMap = albums.associateBy { it.uid }
     private val artistUidMap = artists.associateBy { it.uid }
     private val genreUidMap = genres.associateBy { it.uid }
@@ -44,7 +45,8 @@ internal data class LibraryImpl(
 
     override fun empty() = songs.isEmpty()
 
-    override fun findSong(uid: Music.UID) = songUidMap[uid]
+    // Compat hack. See TagInterpreter for why this needs to be done
+    override fun findSong(uid: Music.UID) = songUidMap[uid] ?: v400SongUidMap[uid]
 
     override fun findSongByPath(path: Path) = songs.find { it.path == path }
 

musikr/src/main/java/org/oxycblt/musikr/model/SongImpl.kt

@@ -42,7 +42,10 @@ internal interface SongCore {
 internal class SongImpl(private val handle: SongCore) : Song {
     private val preSong = handle.preSong
 
-    override val uid = preSong.uid
+    override val uid = preSong.v363Uid
+
+    val v400Uid = preSong.v400Uid
+
     override val name = preSong.name
     override val track = preSong.track
     override val disc = preSong.disc

musikr/src/main/java/org/oxycblt/musikr/pipeline/ExtractStep.kt

@@ -131,7 +131,7 @@ private class ExtractStepImpl(
             metadata
                 .map { fileWith ->
                     if (fileWith.with != null) {
-                        val tags = tagParser.parse(fileWith.file, fileWith.with)
+                        val tags = tagParser.parse(fileWith.with)
                         val cover = fileWith.with.cover?.let { storedCovers.write(it) }
                         RawSong(fileWith.file, fileWith.with.properties, tags, cover, addingMs)
                     } else {

musikr/src/main/java/org/oxycblt/musikr/pipeline/PipelineException.kt

@@ -18,7 +18,6 @@
  
 package org.oxycblt.musikr.pipeline
 
-import android.util.Log
 import org.oxycblt.musikr.fs.DeviceFile
 import org.oxycblt.musikr.playlist.PlaylistFile
 import org.oxycblt.musikr.playlist.interpret.PrePlaylist
@@ -55,45 +54,35 @@ sealed interface WhileProcessing {
 
 internal suspend fun <R> wrap(file: DeviceFile, block: suspend (DeviceFile) -> R): R =
     try {
-        Log.d("wrap", "Processing DeviceFile ${file.path}")
         block(file)
     } catch (e: Exception) {
-        Log.e("wrap", "Error while processing DeviceFile ${file.path}", e)
         throw PipelineException(WhileProcessing.AFile(file), e)
     }
 
 internal suspend fun <R> wrap(song: RawSong, block: suspend (RawSong) -> R): R =
     try {
-        Log.d("wrap", "Processing RawSong ${song.file.path}")
         block(song)
     } catch (e: Exception) {
-        Log.e("wrap", "Error while processing RawSong ${song.file.path}", e)
         throw PipelineException(WhileProcessing.ARawSong(song), e)
     }
 
 internal suspend fun <R> wrap(file: PlaylistFile, block: suspend (PlaylistFile) -> R): R =
     try {
-        Log.d("wrap", "Processing PlaylistFile ${file.name}")
         block(file)
     } catch (e: Exception) {
-        Log.e("wrap", "Error while processing PlaylistFile ${file.name}", e)
         throw PipelineException(WhileProcessing.APlaylistFile(file), e)
     }
 
 internal suspend fun <R> wrap(song: PreSong, block: suspend (PreSong) -> R): R =
     try {
-        Log.d("wrap", "Processing PreSong ${song.path}")
         block(song)
     } catch (e: Exception) {
-        Log.e("wrap", "Error while processing PreSong ${song.path}", e)
         throw PipelineException(WhileProcessing.APreSong(song), e)
     }
 
 internal suspend fun <R> wrap(playlist: PrePlaylist, block: suspend (PrePlaylist) -> R): R =
     try {
-        Log.d("wrap", "Processing PrePlaylist ${playlist.name}")
         block(playlist)
     } catch (e: Exception) {
-        Log.e("wrap", "Error while processing PrePlaylist ${playlist.name}", e)
         throw PipelineException(WhileProcessing.APrePlaylist(playlist), e)
     }

musikr/src/main/java/org/oxycblt/musikr/tag/interpret/PreMusic.kt

@@ -29,9 +29,10 @@ import org.oxycblt.musikr.tag.Disc
 import org.oxycblt.musikr.tag.Name
 import org.oxycblt.musikr.tag.ReleaseType
 import org.oxycblt.musikr.tag.ReplayGainAdjustment
-import org.oxycblt.musikr.util.update
 
 internal data class PreSong(
+    val v363Uid: Music.UID,
+    val v400Uid: Music.UID,
     val musicBrainzId: UUID?,
     val name: Name.Known,
     val rawName: String,
@@ -52,24 +53,7 @@ internal data class PreSong(
     val preAlbum: PreAlbum,
     val preArtists: List<PreArtist>,
     val preGenres: List<PreGenre>
-) {
+) {}
-    val uid =
-        musicBrainzId?.let { Music.UID.musicBrainz(Music.UID.Item.SONG, it) }
-            ?: Music.UID.auxio(Music.UID.Item.SONG) {
-                // Song UIDs are based on the raw data without parsing so that they remain
-                // consistent across music setting changes. Parents are not held up to the
-                // same standard since grouping is already inherently linked to settings.
-                update(rawName)
-                update(preAlbum.rawName)
-                update(date)
-
-                update(track)
-                update(disc?.number)
-
-                update(preArtists.map { artist -> artist.rawName })
-                update(preAlbum.preArtists.map { artist -> artist.rawName })
-            }
-}
 
 internal data class PreAlbum(
     val musicBrainzId: UUID?,

musikr/src/main/java/org/oxycblt/musikr/tag/interpret/TagInterpreter.kt

@@ -19,6 +19,8 @@
 package org.oxycblt.musikr.tag.interpret
 
 import org.oxycblt.musikr.Interpretation
+import org.oxycblt.musikr.Music
+import org.oxycblt.musikr.fs.DeviceFile
 import org.oxycblt.musikr.fs.Format
 import org.oxycblt.musikr.pipeline.RawSong
 import org.oxycblt.musikr.tag.Disc
@@ -29,6 +31,7 @@ import org.oxycblt.musikr.tag.ReplayGainAdjustment
 import org.oxycblt.musikr.tag.format.parseId3GenreNames
 import org.oxycblt.musikr.tag.parse.ParsedTags
 import org.oxycblt.musikr.util.toUuidOrNull
+import org.oxycblt.musikr.util.update
 
 internal interface TagInterpreter {
     fun interpret(song: RawSong): PreSong
@@ -53,22 +56,65 @@ private class TagInterpreterImpl(private val interpretation: Interpretation) : T
                 song.tags.albumArtistSortNames,
                 interpretation)
         val preAlbum =
-            makePreAlbum(song.tags, individualPreArtists, albumPreArtists, interpretation)
+            makePreAlbum(
+                song.tags, song.file, individualPreArtists, albumPreArtists, interpretation)
         val rawArtists =
             individualPreArtists.ifEmpty { albumPreArtists }.ifEmpty { listOf(unknownPreArtist()) }
         val rawGenres =
             makePreGenres(song.tags, interpretation).ifEmpty { listOf(unknownPreGenre()) }
         val uri = song.file.uri
+
+        val songNameOrFile = song.tags.name ?: requireNotNull(song.file.path.name)
+        val songNameOrFileWithoutExt =
+            song.tags.name ?: requireNotNull(song.file.path.name).split('.').first()
+        val albumNameOrDir = song.tags.albumName ?: song.file.path.directory.name
+
+        val musicBrainzId = song.tags.musicBrainzId?.toUuidOrNull()
+        val v363uid =
+            musicBrainzId?.let { Music.UID.musicBrainz(Music.UID.Item.SONG, it) }
+                ?: Music.UID.auxio(Music.UID.Item.SONG) {
+                    update(songNameOrFileWithoutExt)
+                    update(albumNameOrDir)
+                    update(song.tags.date)
+
+                    update(song.tags.track)
+                    update(song.tags.disc)
+
+                    update(song.tags.artistNames)
+                    update(song.tags.albumArtistNames)
+                }
+
+        // I was an idiot and accidentally changed the UID spec in v4.0.0, so we need to calculate
+        // the broken UID too and maintain compat for that version.
+        val v400uid =
+            musicBrainzId?.let { Music.UID.musicBrainz(Music.UID.Item.SONG, it) }
+                ?: Music.UID.auxio(Music.UID.Item.SONG) {
+                    update(songNameOrFile)
+                    update(song.tags.albumName)
+                    update(song.tags.date)
+
+                    update(song.tags.track)
+                    update(song.tags.disc)
+
+                    val artistNames = interpretation.separators.split(song.tags.artistNames)
+                    update(artistNames.ifEmpty { listOf(null) })
+                    val albumArtistNames =
+                        interpretation.separators.split(song.tags.albumArtistNames)
+                    update(albumArtistNames.ifEmpty { artistNames }.ifEmpty { listOf(null) })
+                }
+
         return PreSong(
+            v363Uid = v363uid,
+            v400Uid = v400uid,
             uri = uri,
             path = song.file.path,
             size = song.file.size,
             format = Format.infer(song.file.mimeType, song.properties.mimeType),
             modifiedMs = song.file.modifiedMs,
             addedMs = song.addedMs,
-            musicBrainzId = song.tags.musicBrainzId?.toUuidOrNull(),
+            musicBrainzId = musicBrainzId,
-            name = interpretation.naming.name(song.tags.name, song.tags.sortName),
+            name = interpretation.naming.name(songNameOrFileWithoutExt, song.tags.sortName),
-            rawName = song.tags.name,
+            rawName = songNameOrFileWithoutExt,
             track = song.tags.track,
             disc = song.tags.disc?.let { Disc(it, song.tags.subtitle) },
             date = song.tags.date,
@@ -88,16 +134,16 @@ private class TagInterpreterImpl(private val interpretation: Interpretation) : T
 
     private fun makePreAlbum(
         parsedTags: ParsedTags,
+        deviceFile: DeviceFile,
         individualPreArtists: List<PreArtist>,
         albumPreArtists: List<PreArtist>,
         interpretation: Interpretation
     ): PreAlbum {
+        val name = parsedTags.albumName ?: deviceFile.path.directory.name
         return PreAlbum(
             musicBrainzId = parsedTags.albumMusicBrainzId?.toUuidOrNull(),
-            name =
+            name = interpretation.naming.name(name, parsedTags.albumSortName, Placeholder.ALBUM),
-                interpretation.naming.name(
+            rawName = name,
-                    parsedTags.albumName, parsedTags.albumSortName, Placeholder.ALBUM),
-            rawName = parsedTags.albumName,
             releaseType =
                 ReleaseType.parse(interpretation.separators.split(parsedTags.releaseTypes))
                     ?: ReleaseType.Album(null),

musikr/src/main/java/org/oxycblt/musikr/tag/parse/ParsedTags.kt

@@ -25,7 +25,7 @@ internal data class ParsedTags(
     val replayGainTrackAdjustment: Float? = null,
     val replayGainAlbumAdjustment: Float? = null,
     val musicBrainzId: String? = null,
-    val name: String,
+    val name: String? = null,
     val sortName: String? = null,
     val track: Int? = null,
     val disc: Int? = null,

musikr/src/main/java/org/oxycblt/musikr/tag/parse/TagParser.kt

@@ -18,12 +18,10 @@
  
 package org.oxycblt.musikr.tag.parse
 
-import org.oxycblt.musikr.fs.DeviceFile
 import org.oxycblt.musikr.metadata.Metadata
-import org.oxycblt.musikr.util.unlikelyToBeNull
 
 internal interface TagParser {
-    fun parse(file: DeviceFile, metadata: Metadata): ParsedTags
+    fun parse(metadata: Metadata): ParsedTags
 
     companion object {
         fun new(): TagParser = TagParserImpl
@@ -31,14 +29,14 @@ internal interface TagParser {
 }
 
 private data object TagParserImpl : TagParser {
-    override fun parse(file: DeviceFile, metadata: Metadata): ParsedTags {
+    override fun parse(metadata: Metadata): ParsedTags {
         val compilation = metadata.isCompilation()
         return ParsedTags(
             durationMs = metadata.properties.durationMs,
             replayGainTrackAdjustment = metadata.replayGainTrackAdjustment(),
             replayGainAlbumAdjustment = metadata.replayGainAlbumAdjustment(),
             musicBrainzId = metadata.musicBrainzId(),
-            name = metadata.name() ?: unlikelyToBeNull(file.path.name),
+            name = metadata.name(),
             sortName = metadata.sortName(),
             track = metadata.track(),
             disc = metadata.disc(),