musikr: stable playlist song identity across UID changes#1321
Open
Victor239 wants to merge 4 commits intoOxygenCobalt:devfrom
Open
musikr: stable playlist song identity across UID changes#1321Victor239 wants to merge 4 commits intoOxygenCobalt:devfrom
Victor239 wants to merge 4 commits intoOxygenCobalt:devfrom
Conversation
When a song was indexed without a MusicBrainz ID its playlist entries were stored under a hash-based UID. If an external app later tagged the file with a MusicBrainz ID, the next index run would assign the song a new MusicBrainz UID, leaving the stored hash UID unresolved — silently dropping the song from all playlists with no recovery path. Fix this with a two-part approach: 1. TagInterpreter now always computes the three hash-based UIDs (v363/v400/v401) unconditionally, even when a MusicBrainz ID is present. When a MusicBrainz ID supersedes them they are stored as PreSong.legacyUids; otherwise legacyUids is empty and behaviour is unchanged for songs without a MusicBrainz ID. 2. During graph playlist resolution, each song's legacyUids are probed against the stored playlist pointerMap in addition to the three existing canonical probes. A successful legacy-UID match resolves the song correctly for the current run and records an oldHash → newMbUid entry in a uidMigrations map on the built MusicGraph. After the graph is built, EvaluateStep calls StoredPlaylists.migrate(uidMigrations), which rewrites PlaylistSong and PlaylistSongCrossRef rows in the DB from the old hash UID to the new MusicBrainz UID. Subsequent index runs resolve cleanly via canonical probes and the migration is a no-op (empty map).
The previous migration only handled the case where a song gains a MusicBrainz ID without any other tag changes, relying on the legacyUids probe populated during playlist resolution. This extends the approach with a persistent URI → UID index (SongUriRecord, DB version 30→31). After every index run the current mapping is saved. On the next run the stored mapping is diffed against the new one and any songs whose UID changed (for any reason: MB ID added alongside other tag edits, pure tag-hash drift, etc.) are included in the migration set alongside the legacyUids-derived migrations. Changes: - Add SongUriRecord entity and DB migration 30→31 - Add computeUriMigrations / updateUriIndex to StoredPlaylists - EvaluateStep merges both migration sources before calling migrate() - Minor KDoc reformatting (spotless)
…elease > hash
Previously the canonical song UID was either the MusicBrainz release track ID
(MUSICBRAINZ_RELEASETRACKID) or a hash of tag metadata. This introduces a fuller
priority hierarchy so that the most stable available identifier is always used:
1. ACOUSTID_FINGERPRINT — content-based fingerprint; survives any metadata edit
2. MUSICBRAINZ_TRACKID — recording ID; stable across different releases of the same
recording
3. MUSICBRAINZ_RELEASETRACKID — release-specific track ID (previous behaviour)
4. Hash of tag metadata (previous fallback; URI-index handles drift)
Changes:
- Music.UID: new ACOUSTID format ('c' micronamespace, "org.acoustid" namespace);
Music.UID.acoustid() factory SHA-256 hashes the fingerprint string into a UUID;
fromString() updated to parse the new format
- TagFields: musicBrainzRecordingId() reads MUSICBRAINZ_TRACKID across Xiph/MP4/ID3v2;
acoustidFingerprint() reads ACOUSTID_FINGERPRINT across the same formats
- ParsedTags / TagParser: plumb the two new fields through from raw metadata
- TagInterpreter: stableUid selected by priority; legacyUids now includes all
lower-priority UIDs (MB recording, MB release track, all three hash variants) so
any playlist stored under any previous UID resolves and migrates on next rescan
…rescan Previously, URI-based UID migrations were computed and written to the DB after MusicGraph.build() had already resolved playlist vertices using the pre-migration DB state. This meant songs whose UID changed in a given rescan appeared missing from playlists until a second rescan applied the now-correct DB state. Fix by calling graph.applyMigrations(uriMigrations) after the DB is updated but before libraryFactory.create() consumes the graph. This walks the playlist vertices and fills in any positions that the URI migration resolved, so the corrected playlist contents are visible immediately in the same rescan that triggered the UID change.
Owner
|
Thanks for looking at this. This seems to be made my Claude so I'll have to look at this deeply & also have to rewrite. Possible that changes like these can be extremely naive and actually make things worse. |
Author
|
No problem. I have extensively tested and reviewed the code, just had Claude create it as I'm unfamiliar with Kotlin. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is it?
Description of changes
Songs in playlists could silently disappear whenever their UID changed — either because a MusicBrainz ID was added to a previously hash-identified song, or because tag edits produced a different hash. This PR fixes that end-to-end.
ACOUSTID_FINGERPRINT(content-based, most stable) >MUSICBRAINZ_TRACKID(recording ID) >MUSICBRAINZ_RELEASETRACKID(release track ID, previous behaviour) > hash of tag metadataMusic.UID.Format.ACOUSTIDandMusic.UID.acoustid()factory; updatefromString()to parse the new formatTagFields.musicBrainzRecordingId()andTagFields.acoustidFingerprint()tag accessors across Xiph/MP4/ID3v2PreSong.legacyUids: all lower-priority UIDs a song carried before a stable identifier superseded them. DuringMusicGraph.build(), each song probes playlist pointer maps with its legacy UIDs and records any matches inuidMigrations, which are written to the DB after indexingSongUriRecord(DB version 30→31, non-destructive migration): a persistentfileUri → songUidindex updated after every rescan.computeUriMigrationsdiffs the stored mapping against the current one to catch any UID change thelegacyUidsprobe cannot — specifically when the tags that make up the hash are also edited, or when a unique identifier like a MusicBrainz ID is gained at the same time other tags changeMusicGraph.applyMigrations(): patches playlist vertices in-memory after URI migrations are computed so corrected playlist contents are visible in the same rescan rather than requiring a second oneFixes the following issues
Songs added to a playlist disappear after retagging — whether that is editing metadata fields, adding a MusicBrainz ID, or both at once. Also #1004.
Any additional information
The two migration mechanisms are complementary:
The
legacyUidsprobe works purely from tag data computed at index time. It handles the clean case of a song gaining a stable identifier (MusicBrainz/AcoustID) without other tag changes, and requires no DB read beyond the playlist itself.The URI index is the fallback for everything else. Because it tracks the last-known UID per file path across rescans, it can detect any UID change regardless of cause — hash drift from tag edits, gaining a stable ID alongside other edits, or any future UID format change. Both sources are merged before being applied.
APK testing
debug.zip
Due diligence