From 9ca4f70315872c131aca161791f54e79f3a28cd6 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sun, 17 Jul 2022 14:15:36 -0600 Subject: [PATCH] music: add id3v2.3 full date support [#159] Add support for the ID3v2.3 TDAT and TIME frames to the ExoPlayer parser. --- app/src/main/AndroidManifest.xml | 2 +- .../java/org/oxycblt/auxio/music/Music.kt | 55 ++++++++++++------- .../java/org/oxycblt/auxio/music/MusicUtil.kt | 4 +- .../auxio/music/system/ExoPlayerBackend.kt | 25 ++++++++- .../auxio/music/system/MediaStoreBackend.kt | 2 +- .../playback/system/MediaSessionComponent.kt | 3 +- .../oxycblt/auxio/ui/recycler/ViewHolders.kt | 2 +- info/ARCHITECTURE.md | 2 +- 8 files changed, 66 insertions(+), 29 deletions(-) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 675d4c529..fc9b42d46 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -7,7 +7,7 @@ - + diff --git a/app/src/main/java/org/oxycblt/auxio/music/Music.kt b/app/src/main/java/org/oxycblt/auxio/music/Music.kt index ed7beeb14..f11a563ac 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -139,7 +139,7 @@ data class Song( * The raw artist name for this song in particular. First uses the artist tag, and then falls * back to the album artist tag (i.e parent artist name). Null if name is unknown. */ - val individualRawArtistName: String? + val individualArtistRawName: String? get() = _artistName ?: album.artist.rawName /** @@ -301,8 +301,8 @@ data class Genre(override val rawName: String?, override val songs: List) * or reject valid-ish dates. * * Date instances are immutable and their internal implementation is hidden. To instantiate one, use - * [fromYear] or [parseTimestamp]. The string representation of a Date is RFC 3339, with granular - * position depending on the presence of particular tokens. + * [from] or [from]. The string representation of a Date is RFC 3339, with granular position + * depending on the presence of particular tokens. * * Please, **Do not use this for anything important related to time.** I cannot stress this enough. * This class will blow up if you try to do that. @@ -364,14 +364,14 @@ class Date private constructor(private val tokens: List) : Comparable override fun toString() = StringBuilder().appendDate().toString() private fun StringBuilder.appendDate(): StringBuilder { - // I assume RFC 3339 allows partial precision, i.e YYYY-MM, but I'm not sure. + // RFC 3339 does not allow partial precision. append(year.toFixedString(4)) append("-${(month ?: return this).toFixedString(2)}") append("-${(day ?: return this).toFixedString(2)}") append("T${(hour ?: return this).toFixedString(2)}") - append(":${(minute ?: return this).toFixedString(2)}") - append(":${(second ?: return this).toFixedString(2)}Z") - return this + append(":${(minute ?: return this.append('Z')).toFixedString(2)}") + append(":${(second ?: return this.append('Z')).toFixedString(2)}") + return this.append('Z') } private fun Int.toFixedString(len: Int) = toString().padStart(len, '0') @@ -381,27 +381,40 @@ class Date private constructor(private val tokens: List) : Comparable Regex( """^(\d{4,})([-.](\d{2})([-.](\d{2})([T ](\d{2})([:.](\d{2})([:.](\d{2}))?)?)?)?)?$""") - fun fromYear(year: Int) = year.nonZeroOrNull()?.let { Date(listOf(it)) } + fun from(year: Int) = fromTokens(listOf(year)) - fun parseTimestamp(timestamp: String): Date? { - val groups = (ISO8601_REGEX.matchEntire(timestamp) ?: return null).groupValues - val tokens = mutableListOf() - populateTokens(groups, tokens) + fun from(year: Int, month: Int, day: Int) = fromTokens(listOf(year, month, day)) - if (tokens.isEmpty()) { + fun from(year: Int, month: Int, day: Int, hour: Int, minute: Int) = + fromTokens(listOf(year, month, day, hour, minute)) + + fun from(timestamp: String): Date? { + val groups = + (ISO8601_REGEX.matchEntire(timestamp) ?: return null) + .groupValues.mapIndexedNotNull { index, s -> + if (index % 2 != 0) s.toIntOrNull() else null + } + + return fromTokens(groups) + } + + private fun fromTokens(tokens: List): Date? { + val out = mutableListOf() + validateTokens(tokens, out) + if (out.isEmpty()) { return null } - return Date(tokens) + return Date(out) } - private fun populateTokens(groups: List, tokens: MutableList) { - tokens.add(groups.getOrNull(1)?.toIntOrNull()?.nonZeroOrNull() ?: return) - tokens.add(groups.getOrNull(3)?.toIntOrNull()?.inRangeOrNull(1..12) ?: return) - tokens.add(groups.getOrNull(5)?.toIntOrNull()?.inRangeOrNull(1..31) ?: return) - tokens.add(groups.getOrNull(7)?.toIntOrNull()?.inRangeOrNull(0..23) ?: return) - tokens.add(groups.getOrNull(9)?.toIntOrNull()?.inRangeOrNull(0..59) ?: return) - tokens.add(groups.getOrNull(11)?.toIntOrNull()?.inRangeOrNull(0..59) ?: return) + private fun validateTokens(src: List, dst: MutableList) { + dst.add(src.getOrNull(0)?.nonZeroOrNull() ?: return) + dst.add(src.getOrNull(1)?.inRangeOrNull(1..12) ?: return) + dst.add(src.getOrNull(2)?.inRangeOrNull(1..31) ?: return) + dst.add(src.getOrNull(3)?.inRangeOrNull(0..23) ?: return) + dst.add(src.getOrNull(4)?.inRangeOrNull(0..59) ?: return) + dst.add(src.getOrNull(5)?.inRangeOrNull(0..59) ?: return) } } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicUtil.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicUtil.kt index 79ee1c49d..ca52c9964 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicUtil.kt @@ -78,10 +78,10 @@ fun Int.unpackDiscNo() = div(1000).nonZeroOrNull() fun String.parsePositionNum() = split('/', limit = 2)[0].toIntOrNull()?.nonZeroOrNull() /** Parse a plain year from the field into a [Date]. */ -fun String.parseYear() = toIntOrNull()?.let(Date::fromYear) +fun String.parseYear() = toIntOrNull()?.let(Date::from) /** Parse an ISO-8601 time-stamp from this field into a [Date]. */ -fun String.parseTimestamp() = Date.parseTimestamp(this) +fun String.parseTimestamp() = Date.from(this) /** Shortcut to resolve a year from a nullable date. Will return "No Date" if it is null. */ fun Date?.resolveYear(context: Context) = diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/ExoPlayerBackend.kt b/app/src/main/java/org/oxycblt/auxio/music/system/ExoPlayerBackend.kt index e3b147165..03bd61a04 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/ExoPlayerBackend.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/ExoPlayerBackend.kt @@ -19,11 +19,13 @@ package org.oxycblt.auxio.music.system import android.content.Context import android.database.Cursor +import androidx.core.text.isDigitsOnly import com.google.android.exoplayer2.MediaItem import com.google.android.exoplayer2.MetadataRetriever import com.google.android.exoplayer2.metadata.Metadata import com.google.android.exoplayer2.metadata.id3.TextInformationFrame import com.google.android.exoplayer2.metadata.vorbis.VorbisComment +import org.oxycblt.auxio.music.Date import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.audioUri import org.oxycblt.auxio.music.parseId3GenreName @@ -226,7 +228,7 @@ class Task(context: Context, private val audio: MediaStoreBackend.Audio) { // 5. ID3v2.3 Release Year, as it is the most common date type (tags["TDOR"]?.parseTimestamp() ?: tags["TDRC"]?.parseTimestamp() ?: tags["TDRL"]?.parseTimestamp() - ?: tags["TORY"]?.parseYear() ?: tags["TYER"]?.parseYear()) + ?: parseId3v23Date(tags)) ?.let { audio.date = it } // (Sort) Album @@ -245,6 +247,27 @@ class Task(context: Context, private val audio: MediaStoreBackend.Audio) { tags["TCON"]?.let { audio.genre = it.parseId3GenreName() } } + private fun parseId3v23Date(tags: Map): Date? { + val year = tags["TORY"]?.toIntOrNull() ?: tags["TYER"]?.toIntOrNull() ?: return null + + val mmdd = tags["TDAT"] + return if (mmdd != null && mmdd.length == 4 && mmdd.isDigitsOnly()) { + val mm = mmdd.substring(0..1).toInt() + val dd = mmdd.substring(2..3).toInt() + + val hhmi = tags["TIME"] + if (hhmi != null && hhmi.length == 4 && hhmi.isDigitsOnly()) { + val hh = hhmi.substring(0..1).toInt() + val mi = hhmi.substring(2..3).toInt() + Date.from(year, mm, dd, hh, mi) + } else { + Date.from(year, mm, dd) + } + } else { + return Date.from(year) + } + } + private fun populateVorbis(tags: Map) { // (Sort) Title tags["TITLE"]?.let { audio.title = it } diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/MediaStoreBackend.kt b/app/src/main/java/org/oxycblt/auxio/music/system/MediaStoreBackend.kt index 47d4bc82c..7bd1bdf6e 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/MediaStoreBackend.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/MediaStoreBackend.kt @@ -293,7 +293,7 @@ abstract class MediaStoreBackend : Indexer.Backend { audio.displayName = cursor.getStringOrNull(displayNameIndex) audio.duration = cursor.getLong(durationIndex) - audio.date = cursor.getIntOrNull(yearIndex)?.let(Date::fromYear) + audio.date = cursor.getIntOrNull(yearIndex)?.let(Date::from) // A non-existent album name should theoretically be the name of the folder it contained // in, but in practice it is more often "0" (as in /storage/emulated/0), even when it the diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt index 04a2d68f7..ed0698f0a 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt @@ -109,6 +109,8 @@ class MediaSessionComponent( return } + // We would leave the artist field null if it didn't exist and let downstream consumers + // handle it, but that would break the notification display. val title = song.resolveName(context) val artist = song.resolveIndividualArtistName(context) val builder = @@ -152,7 +154,6 @@ class MediaSessionComponent( // // Neither of these are good, but 1 is the only one that will work on all versions // without the notification being eaten by rate-limiting. - provider.load( song, object : BitmapProvider.Target { diff --git a/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt b/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt index 55b151fc1..45b348395 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt @@ -65,7 +65,7 @@ class SongViewHolder private constructor(private val binding: ItemSongBinding) : object : SimpleItemCallback() { override fun areItemsTheSame(oldItem: Song, newItem: Song) = oldItem.rawName == newItem.rawName && - oldItem.individualRawArtistName == oldItem.individualRawArtistName + oldItem.individualArtistRawName == oldItem.individualArtistRawName } } } diff --git a/info/ARCHITECTURE.md b/info/ARCHITECTURE.md index 00ea603aa..0f45720f2 100644 --- a/info/ARCHITECTURE.md +++ b/info/ARCHITECTURE.md @@ -129,7 +129,7 @@ Things to keep in mind while working with music data: unique, non-subjective fields of the music data. Attempting to use it as a `MediaStore` ID will result in errors. - Any field or method beginning with `_` is off-limits. These fields are meant for use within -`MusicLoader` and generally provide poor UX to the user. The only reason they are public is to +the indexer and generally provide poor UX to the user. The only reason they are public is to simplify the loading process, as there is no reason to remove internal fields given that it won't free memory. - `rawName` is used when doing internal work, such as saving music data or diffing items