Merge pull request #1009 from OxygenCobalt/dev

v4.0.1
OxygenCobalt · Feb 24, 2025 · 8a8fd0f · 8a8fd0f
2 parents 442abb7 + 9829972
commit 8a8fd0f
Show file tree

Hide file tree

Showing 18 changed files with 114 additions and 71 deletions.
diff --git a/CHANGELOG.md b/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

diff --git a/README.md b/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">
-        <img alt="Latest Version" src="https://img.shields.io/static/v1?label=tag&message=v4.0.0&color=64B5F6&style=flat">
+    <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.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">

diff --git a/app/build.gradle b/app/build.gradle
@@ -18,8 +18,8 @@ android {
 
     defaultConfig {
         applicationId namespace
-        versionName "4.0.0"
-        versionCode 59
+        versionName "4.0.1"
+        versionCode 60
 
         minSdk min_sdk
         targetSdk target_sdk

diff --git a/fastlane/metadata/android/en-US/changelogs/60.txt b/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.
diff --git a/musikr/src/main/cpp/JInputStream.cpp b/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) {
-    // 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 };
+jint JInputStream::readBlockImpl(TagLib::ByteVector &buf) {
     jobject wrappedByteBuffer = env->NewDirectByteBuffer(buf.data(),
             buf.size());
     if (wrappedByteBuffer == nullptr) {
         throw std::runtime_error("Failed to wrap ByteBuffer");
     }
-    JObjectRef byteBuffer = { env, wrappedByteBuffer };
-    jboolean result = env->CallBooleanMethod(jInputStream,
-            jInputStreamReadBlockMethod, *byteBuffer);
-    if (!result) {
+    JObjectRef byteBuffer { env, wrappedByteBuffer };
+    jint read = env->CallIntMethod(jInputStream, jInputStreamReadBlockMethod,
+            *byteBuffer);
+    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) {

diff --git a/musikr/src/main/cpp/JInputStream.h b/musikr/src/main/cpp/JInputStream.h
@@ -124,6 +124,7 @@ class JInputStream: public TagLib::IOStream {
     jmethodID jInputStreamSeekFromEndMethod;
     jmethodID jInputStreamTellMethod;
     jmethodID jInputStreamLengthMethod;
+    jint readBlockImpl(TagLib::ByteVector &buf);
 };
 
 #endif //AUXIO_JINPUTSTREAM_H
diff --git a/musikr/src/main/java/org/oxycblt/musikr/cache/CacheDatabase.kt b/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?,

diff --git a/musikr/src/main/java/org/oxycblt/musikr/graph/MusicGraph.kt b/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)
-                it.pointerMap[pointer]?.forEach { index -> it.songVertices[index] = vertex }
+                val v363Pointer = SongPointer.UID(entry.key)
+                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 }
             }
         }
 

diff --git a/musikr/src/main/java/org/oxycblt/musikr/metadata/NativeInputStream.kt b/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 true
+            return channel.read(buf)
         } catch (e: Exception) {
             Log.d("NativeInputStream", "Error reading block", e)
-            return false
+            return -2
         }
     }
 

diff --git a/musikr/src/main/java/org/oxycblt/musikr/model/AlbumImpl.kt b/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

diff --git a/musikr/src/main/java/org/oxycblt/musikr/model/LibraryImpl.kt b/musikr/src/main/java/org/oxycblt/musikr/model/LibraryImpl.kt
@@ -37,14 +37,16 @@ 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 }
     private val playlistUidMap = playlists.associateBy { it.uid }
 
     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 }
 

diff --git a/musikr/src/main/java/org/oxycblt/musikr/model/SongImpl.kt b/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

diff --git a/musikr/src/main/java/org/oxycblt/musikr/pipeline/ExtractStep.kt b/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 {

diff --git a/musikr/src/main/java/org/oxycblt/musikr/pipeline/PipelineException.kt b/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)
     }
diff --git a/musikr/src/main/java/org/oxycblt/musikr/tag/interpret/PreMusic.kt b/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?,