musikr: add back tag whitespace fixes

Requires me to rejig the JNI integration, but it's overall good since
it allows me to strip away a lot of the logic.

musikr/proguard-rules.pro

@@ -23,3 +23,4 @@
 -keep class org.oxycblt.musikr.metadata.NativeInputStream { *; }
 -keep class org.oxycblt.musikr.metadata.Metadata { *; }
 -keep class org.oxycblt.musikr.metadata.Properties { *; }
+-keep class org.oxycblt.musikr.metadata.NativeTagMap { *; }

musikr/src/main/cpp/JVMMetadataBuilder.cpp

@@ -18,7 +18,7 @@
  
 #include "JVMMetadataBuilder.h"
 
-#include "log.h"
+#include "util.h"
 
 #include <taglib/mp4tag.h>
 #include <taglib/textidentificationframe.h>
@@ -37,18 +37,18 @@ void JVMMetadataBuilder::setId3v2(const TagLib::ID3v2::Tag &tag) {
     for (auto frame : tag.frameList()) {
         if (auto txxxFrame =
                 dynamic_cast<TagLib::ID3v2::UserTextIdentificationFrame*>(frame)) {
+            TagLib::String id = frame->frameID();
             TagLib::StringList frameText = txxxFrame->fieldList();
             // Frame text starts with the description then the remaining values
             auto begin = frameText.begin();
-            TagLib::String key = TagLib::String(frame->frameID()) + ":"
+            TagLib::String description = *begin;
-                    + begin->upper();
             frameText.erase(begin);
-            id3v2.add(key, frameText);
+            id3v2.add_combined(id, description, frameText);
         } else if (auto textFrame =
                 dynamic_cast<TagLib::ID3v2::TextIdentificationFrame*>(frame)) {
             TagLib::String key = frame->frameID();
             TagLib::StringList frameText = textFrame->fieldList();
-            id3v2.add(key, frameText);
+            id3v2.add_id(key, frameText);
         } else {
             continue;
         }
@@ -59,7 +59,23 @@ void JVMMetadataBuilder::setXiph(const TagLib::Ogg::XiphComment &tag) {
     for (auto field : tag.fieldListMap()) {
         auto key = field.first.upper();
         auto values = field.second;
-        xiph.add(key, values);
+        xiph.add_custom(key, values);
+    }
+}
+
+template<typename T>
+void mp4AddImpl(JVMTagMap &map, TagLib::String &itemName, T itemValue) {
+    if (itemName.startsWith("----")) {
+        // Split this into it's atom name and description
+        auto split = itemName.split(":");
+        if (split.size() != 2) {
+            throw std::runtime_error("Invalid atom name");
+        }
+        auto atomName = split[0];
+        auto atomDescription = split[1];
+        map.add_combined(atomName, atomDescription, itemValue);
+    } else {
+        map.add_id(itemName, itemValue);
     }
 }
 
@@ -67,47 +83,36 @@ void JVMMetadataBuilder::setMp4(const TagLib::MP4::Tag &tag) {
     auto map = tag.itemMap();
     for (auto item : map) {
         auto itemName = item.first;
-        if (itemName.startsWith("----")) {
-            // Capitalize description atoms only
-            // Other standard atoms are cased so we want to avoid collissions there.
-            itemName = itemName.upper();
-        }
         auto itemValue = item.second;
         auto type = itemValue.type();
-        // Only read out the atoms for the reasonable tags we are expecting.
+        std::string serializedValue;
-        // None of the crazy binary atoms.
+        switch (type) {
-        if (type == TagLib::MP4::Item::Type::StringList) {
+            // Normal expected MP4 items
-            auto value = itemValue.toStringList();
+            case TagLib::MP4::Item::Type::StringList:
-            mp4.add(itemName, value);
+                mp4AddImpl(mp4, itemName, itemValue.toStringList());
-            continue;
+                break;
-        }
+                // Weird MP4 items I'm 90% sure I'll encounter.
-
+            case TagLib::MP4::Item::Type::Int:
-        // Assume that taggers will be unhinged and store track numbers
+                serializedValue = std::to_string(itemValue.toInt());
-        // as ints, uints, or longs.
+                break;
-        if (type == TagLib::MP4::Item::Type::Int) {
+            case TagLib::MP4::Item::Type::UInt:
-            auto value = std::to_string(itemValue.toInt());
+                serializedValue = std::to_string(itemValue.toUInt());
-            id3v2.add(itemName, value);
+                break;
-            continue;
+            case TagLib::MP4::Item::Type::LongLong:
-        }
+                serializedValue = std::to_string(itemValue.toLongLong());
-        if (type == TagLib::MP4::Item::Type::UInt) {
+                break;
-            auto value = std::to_string(itemValue.toUInt());
+            case TagLib::MP4::Item::Type::IntPair:
-            id3v2.add(itemName, value);
+                // It's inefficient going from the integer representation back into
-            continue;
+                // a string, but I fully expect taggers to just write "NN/TT" strings
-        }
+                // anyway, and musikr doesn't have to do as much fiddly variant handling.
-        if (type == TagLib::MP4::Item::Type::LongLong) {
+                serializedValue = std::to_string(itemValue.toIntPair().first) + "/"
-            auto value = std::to_string(itemValue.toLongLong());
+                             + std::to_string(itemValue.toIntPair().second);
-            id3v2.add(itemName, value);
+                break;
-            continue;
+            default:
-        }
+                // Don't care about the other types
-        if (type == TagLib::MP4::Item::Type::IntPair) {
+                continue;
-            // It's inefficient going from the integer representation back into
-            // a string, but I fully expect taggers to just write "NN/TT" strings
-            // anyway, and musikr doesn't have to do as much fiddly variant handling.
-            auto value = std::to_string(itemValue.toIntPair().first) + "/"
-                    + std::to_string(itemValue.toIntPair().second);
-            id3v2.add(itemName, value);
-            continue;
         }
+        mp4AddImpl(mp4, itemName, TagLib::String(serializedValue));
     }
 }
 

musikr/src/main/cpp/JVMTagMap.cpp

@@ -18,76 +18,86 @@
  
 #include "JVMTagMap.h"
 
-JVMTagMap::JVMTagMap(JNIEnv *env) : env(env) {
+#include "util.h"
-    jclass hashMapClass = env->FindClass("java/util/HashMap");
-    jmethodID init = env->GetMethodID(hashMapClass, "<init>", "()V");
-    hashMap = env->NewObject(hashMapClass, init);
-    hashMapGetMethod = env->GetMethodID(hashMapClass, "get",
-            "(Ljava/lang/Object;)Ljava/lang/Object;");
-    hashMapPutMethod = env->GetMethodID(hashMapClass, "put",
-            "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
-    env->DeleteLocalRef(hashMapClass);
 
-    jclass arrayListClass = env->FindClass("java/util/ArrayList");
+JVMTagMap::JVMTagMap(JNIEnv *env) : env(env) {
+    jclass tagMapClass = env->FindClass("org/oxycblt/musikr/metadata/NativeTagMap");
+    jmethodID init = env->GetMethodID(tagMapClass, "<init>", "()V");
+    tagMap = env->NewObject(tagMapClass, init);
+    tagMapAddIdSingleMethod = env->GetMethodID(tagMapClass, "addID",
+            "(Ljava/lang/String;Ljava/lang/String;)V");
+    tagMapAddIdListMethod = env->GetMethodID(tagMapClass, "addID",
+            "(Ljava/lang/String;Ljava/util/List;)V");
+    tagMapAddCustomSingleMethod = env->GetMethodID(tagMapClass, "addCustom",
+            "(Ljava/lang/String;Ljava/lang/String;)V");
+    tagMapAddCustomListMethod = env->GetMethodID(tagMapClass, "addCustom",
+            "(Ljava/lang/String;Ljava/util/List;)V");
+    tagMapAddCombinedSingleMethod = env->GetMethodID(tagMapClass, "addCombined",
+            "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V");
+    tagMapAddCombinedListMethod = env->GetMethodID(tagMapClass, "addCombined",
+            "(Ljava/lang/String;Ljava/lang/String;Ljava/util/List;)V");
+    tagMapGetObjectMethod = env->GetMethodID(tagMapClass, "getObject",
+            "()Ljava/util/Map;");
+    env->DeleteLocalRef(tagMapClass);
+
+    arrayListClass = env->FindClass("java/util/ArrayList");
     arrayListInitMethod = env->GetMethodID(arrayListClass, "<init>", "()V");
     arrayListAddMethod = env->GetMethodID(arrayListClass, "add",
             "(Ljava/lang/Object;)Z");
-    env->DeleteLocalRef(arrayListClass);
 }
 
 JVMTagMap::~JVMTagMap() {
-    env->DeleteLocalRef(hashMap);
+    env->DeleteLocalRef(tagMap);
+    env->DeleteLocalRef(arrayListClass);
 }
 
-void JVMTagMap::add(TagLib::String &key, std::string_view value) {
+void JVMTagMap::add_id(TagLib::String &id, TagLib::String &value) {
-    jstring jKey = env->NewStringUTF(key.toCString(true));
+    env->CallVoidMethod(tagMap, tagMapAddIdSingleMethod,
-    jstring jValue = env->NewStringUTF(value.data());
+            env->NewStringUTF(id.toCString(true)), env->NewStringUTF(value.toCString(true)));
-
-    // check if theres already a value arraylist in the map
-    jobject existingValue = env->CallObjectMethod(hashMap, hashMapGetMethod,
-            jKey);
-    // if there is, add to the value to the existing arraylist
-    if (existingValue != nullptr) {
-        env->CallBooleanMethod(existingValue, arrayListAddMethod, jValue);
-    } else {
-        // if there isn't, create a new arraylist and add the value to it
-        jclass arrayListClass = env->FindClass("java/util/ArrayList");
-        jobject arrayList = env->NewObject(arrayListClass, arrayListInitMethod);
-        env->CallBooleanMethod(arrayList, arrayListAddMethod, jValue);
-        env->CallObjectMethod(hashMap, hashMapPutMethod, jKey, arrayList);
-        env->DeleteLocalRef(arrayListClass);
-    }
 }
 
-void JVMTagMap::add(TagLib::String &key, TagLib::StringList &value) {
+void JVMTagMap::add_id(TagLib::String &id, TagLib::StringList &value) {
-    if (value.isEmpty()) {
+    jobject arrayList = env->NewObject(arrayListClass, arrayListInitMethod);
-        // Nothing to add
+    for (auto &item : value) {
-        return;
+        env->CallBooleanMethod(arrayList, arrayListAddMethod,
+                env->NewStringUTF(item.toCString(true)));
     }
-    jstring jKey = env->NewStringUTF(key.toCString(true));
+    env->CallVoidMethod(tagMap, tagMapAddIdListMethod,
+            env->NewStringUTF(id.toCString(true)), arrayList);
+}
 
-    // check if theres already a value arraylist in the map
+void JVMTagMap::add_custom(TagLib::String &description, TagLib::String &value) {
-    jobject existingValue = env->CallObjectMethod(hashMap, hashMapGetMethod,
+    env->CallVoidMethod(tagMap, tagMapAddCustomSingleMethod,
-            jKey);
+            env->NewStringUTF(description.toCString(true)), env->NewStringUTF(value.toCString(true)));
-    // if there is, add to the value to the existing arraylist
+}
-    if (existingValue != nullptr) {
+
-        for (auto &val : value) {
+void JVMTagMap::add_custom(TagLib::String &description, TagLib::StringList &value) {
-            jstring jValue = env->NewStringUTF(val.toCString(true));
+    jobject arrayList = env->NewObject(arrayListClass, arrayListInitMethod);
-            env->CallBooleanMethod(existingValue, arrayListAddMethod, jValue);
+    for (auto &item : value) {
-        }
+        env->CallBooleanMethod(arrayList, arrayListAddMethod,
-    } else {
+                env->NewStringUTF(item.toCString(true)));
-        // if there isn't, create a new arraylist and add the value to it
-        jclass arrayListClass = env->FindClass("java/util/ArrayList");
-        jobject arrayList = env->NewObject(arrayListClass, arrayListInitMethod);
-        for (auto &val : value) {
-            jstring jValue = env->NewStringUTF(val.toCString(true));
-            env->CallBooleanMethod(arrayList, arrayListAddMethod, jValue);
-        }
-        env->CallObjectMethod(hashMap, hashMapPutMethod, jKey, arrayList);
-        env->DeleteLocalRef(arrayListClass);
     }
+    env->CallVoidMethod(tagMap, tagMapAddCustomListMethod,
+            env->NewStringUTF(description.toCString(true)), arrayList);
+}
+
+void JVMTagMap::add_combined(TagLib::String &id, TagLib::String &description, TagLib::String &value) {
+    env->CallVoidMethod(tagMap, tagMapAddCombinedSingleMethod,
+            env->NewStringUTF(id.toCString(true)), env->NewStringUTF(description.toCString(true)),
+            env->NewStringUTF(value.toCString(true)));
+}
+
+void JVMTagMap::add_combined(TagLib::String &id, TagLib::String &description, TagLib::StringList &value) {
+    jobject arrayList = env->NewObject(arrayListClass, arrayListInitMethod);
+    for (auto &item : value) {
+        env->CallBooleanMethod(arrayList, arrayListAddMethod,
+                env->NewStringUTF(item.toCString(true)));
+    }
+    env->CallVoidMethod(tagMap, tagMapAddCombinedListMethod,
+            env->NewStringUTF(id.toCString(true)), env->NewStringUTF(description.toCString(true)),
+            arrayList);
 }
 
 jobject JVMTagMap::getObject() {
-    return hashMap;
+    return env->CallObjectMethod(tagMap, tagMapGetObjectMethod);
 }

musikr/src/main/cpp/JVMTagMap.h

@@ -32,16 +32,28 @@ public:
     JVMTagMap(const JVMTagMap&) = delete;
     JVMTagMap& operator=(const JVMTagMap&) = delete;
 
-    void add(TagLib::String &key, std::string_view value);
+    void add_id(TagLib::String &id, TagLib::String &value);
-    void add(TagLib::String &key, TagLib::StringList &value);
+    void add_id(TagLib::String &id, TagLib::StringList &value);
+
+    void add_custom(TagLib::String &description, TagLib::String &value);
+    void add_custom(TagLib::String &description, TagLib::StringList &value);
+
+    void add_combined(TagLib::String &id, TagLib::String &description, TagLib::String &value);
+    void add_combined(TagLib::String &id, TagLib::String &description, TagLib::StringList &value);
 
     jobject getObject();
 
 private:
     JNIEnv *env;
-    jobject hashMap;
+    jobject tagMap;
-    jmethodID hashMapGetMethod;
+    jmethodID tagMapAddIdSingleMethod;
-    jmethodID hashMapPutMethod;
+    jmethodID tagMapAddIdListMethod;
+    jmethodID tagMapAddCustomSingleMethod;
+    jmethodID tagMapAddCustomListMethod;
+    jmethodID tagMapAddCombinedSingleMethod;
+    jmethodID tagMapAddCombinedListMethod;
+    jmethodID tagMapGetObjectMethod;
+    jclass arrayListClass;
     jmethodID arrayListInitMethod;
     jmethodID arrayListAddMethod;
 };

musikr/src/main/cpp/taglib_jni.cpp

@@ -20,7 +20,7 @@
 #include <string>
 #include "JVMInputStream.h"
 #include "JVMMetadataBuilder.h"
-#include "log.h"
+#include "util.h"
 
 #include "taglib/fileref.h"
 #include "taglib/flacfile.h"

musikr/src/main/cpp/util.h

@@ -16,9 +16,10 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
  
-#ifndef AUXIO_LOG_H
+#ifndef AUXIO_UTIL_H
-#define AUXIO_LOG_H
+#define AUXIO_UTIL_H
 
+#include <jni.h>
 #include <android/log.h>
 
 #define LOG_TAG "taglib_jni"
@@ -27,4 +28,4 @@
 #define LOGD(...) \
   ((void)__android_log_print(ANDROID_LOG_DEBUG, LOG_TAG, __VA_ARGS__))
 
-#endif //AUXIO_LOG_H
+#endif //AUXIO_UTIL_H

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

@@ -0,0 +1,35 @@
+package org.oxycblt.musikr.metadata
+
+import org.oxycblt.musikr.util.correctWhitespace
+
+class NativeTagMap {
+    private val map = mutableMapOf<String, List<String>>()
+
+    fun addID(id: String, value: String) {
+        addID(id, listOf(value))
+    }
+
+    fun addID(id: String, values: List<String>) {
+        map[id] = values.mapNotNull { it.correctWhitespace() }
+    }
+
+    fun addCustom(description: String, value: String) {
+        addCustom(description, listOf(value))
+    }
+
+    fun addCustom(description: String, values: List<String>) {
+        map[description.uppercase()] = values.mapNotNull { it.correctWhitespace() }
+    }
+
+    fun addCombined(id: String, description: String, value: String) {
+        addCombined(id, description, listOf(value))
+    }
+
+    fun addCombined(id: String, description: String, values: List<String>) {
+        map["$id:${description.uppercase()}"] = values.mapNotNull { it.correctWhitespace() }
+    }
+
+    fun getObject(): Map<String, List<String>> {
+        return map
+    }
+}