MUSICBRAINZ_TRACKID is not unique, so use MUSICBRAINZ_RELEASETRACKID if present.

[darrell-k]

As requested by @mikeysas , this is a simplistic (perhaps too simplistic) solution to #1372 which (in my understanding) proposes the following:

1. The scanner should prefer MUSICBRAINZ_RELEASETRACKID (if it is present in the tags) over MUSICBRAINZ_TRACKID. The reason for this is that MusicBrainz a while ago changed the usage of MUSICBRAINZ_TRACKID so that it is no longer unique: the same ID would be used for the same track on different albums. MUSICBRAINZ_RELEASETRACKID is now the identifier which uniquely identifies the actual album track.
   Our continued use of MUSICBRAINZ_TRACKID can cause problems with persist.db as LMS assumes MUSICBRAINZ_TRACKID will uniquely identify the track. The user experiences this problem in ratings and the new music list.

2. We could also introduce our own custom tag LYRION_UNIQUETRACKID for users who would like to override MusicBrainz values without actually overwriting their MB tags.

3. We could add a pre or post scan analysis of MUSICBRAINZ_TRACKID to inform the user in scanner.log if they have existing duplicates of this tag across their library.

This PR so far only implements proposal 1.

Proposal 2 would be easy enough and using our own namespace (LYRION_*) should avoid unintended consequences.

I had some reservations about the effect of proposal 3 on scanner run-time, but having experimented with the required SQL, it's likely to be insignificant compared to total scan run-time.

If we agree to go forward with this, it will need testing with all music file formats (as indicated by the changed files in this PR).

And before releasing, we need to understand and document the detailed effects this could have on the user's existing persist.db.

Testers: either pull this PR or the branch from my LMS fork - https://github.com/darrell-k/slimserver/tree/MUSICBRAINZ_RELEASETRACKID

[mikeysas]

Thanks for starting this draft @darrell-k !

I do think it is worth considering forcing a Wipe & Rescan with this change. I can't say it is technically necessary, I am thinking it is better to have full scan vs. some tracks being updated over time as tags are changed and the rest when the user finally does a full wipe and rescan. I think the biggest risk where this could cause unpredictable issues is a user reorganizing their library or changing a filenames between the last time a track was scanned with the current mbid logic to the first time it is scanned with the new mbid logic. Hence, I believe forcing a full wipe & rescan is the safest way to significantly reduce that risk. In the release notes I would also include a warning that if you have recently changed your library structure or renamed files that you should run a full wipe and rescan before upgrading to be safe.

Outside of this concern, I have outlined below what I think of think of the side effects and impact from this change. I hope my detailed analysis does not scare anyone away from this as correcting this data integrity issue is long overdue IMHO.

[ TL;DR: I believe that most users will not notice any side effects from this and of the few that do most will understand why]

The full response if you can bear to read through it
(sorry for the length, but did not know how else to make it clear and this is without the use of any AI)

I believe the side effects introduce here are minimal and limited to those that currently have duplicate mbid's in their tracks table. To clarify, I believe there are three distinct outcomes that result from scanning duplicate mbid's and we should consider the side effects here for each.

Outcomes from the issue
(1) Missing tracks_persistent records -- this is the outcome when a user scans a new to their library tracks that contain a duplicate mbid's. In this scenario, the new url is replaced over the existing tacks_persistent record leaving the previous url in the tracks table pointing to the tracks_persistent record with the url from the new duplicate track. As a result there is no longer a tracks_persistent record that contains the previous url. Of course a future scan that includes a change to the previous url or a full rescan could change the url on the record with the duplicate mbid, but either way at any point time one of the duplicate url's does not have a record in tracks_persistent like it should.

(2) Two tracks_persistent records with the same url and another missing -- this is the outcome of a duplicate mbid being added to a track that was previously scanned without an mbid. The url of the existing record with the mbid is replaced by the new duplicate leaving two records in track_persistent with the same url and the original url with the mbid is no longer exists on a record in tracks_persistent. See (3) for possible impact subsequent scans.

(3) Orphaned tracks_persistent records -- this is the outcome when a subsequent scan takes the url that is missing as a result of (2) and updates the duplicate mbid record back to the original url before the duplicate was scanned. At this point we have two records in tracks_persistent each with a unique url that map to each duplicate mbid track, but only the one with the mbid is being updated with play stats and the other is what I am calling the "orphan" (maybe not the right term, but hopefully this makes sense).

In addition to breaking the design assumption that there is a 1:1 relationship between tracks and tracks_persistent records, each of these 3 outcomes create potential issues with play stats, ratings, and/or the sorting of the New Music browse menu. I believe the user community falls into 4 groups also here that we should consider (I ordered them by my estimate of relative size of the group with A being the largest)

User Groupings
(A) Do not have an issue because they either do not tag with mbid's or do not have any duplicates in their library naturally (no manual updates to eliminate duplicates). This group should not be impacted by the change unless they have bad data in their MUSICBRAINZ_RELEASETRACKID tags which creates duplicates.

(B) Not aware of the issue even though they have duplicate mbid's because they do not care about play stats, ratings, or the New Music browse menu. Technically side effects will happen to this group, but given they have not noticed the issues with the duplicates, It would be surprising for them to notice any of the side effects as a result of this change. There was recent example of user with >1k duplicates who sounds like they discovered when they were adjusting ratings so this group can become aware at any moment.

(C) Aware of the issue and has manually manipulated their mbid tags to eliminate duplicates. This group should not be impacted by the change unless they have done some unusual with their tag manipulation of have bad tag data.

(D) Aware of the issue but have not adjusted their tags because they use other software that needs accurate mb_trackid's. I guess they just live with the issues or seek alternatives to LMS. This group will see new or different side effects as a result of this change, but they should be well understood given the issues they live with today and ultimately this will improve their situation.

Side effects after the this fix is released
Here are my predictions of the side effects that Group D and maybe a small # of group B users will see:

• New track_persistent records will be created for the missing records (1) & (2) which if they encompass an entire album will move it to the top of the New Music Browse list. And of course these new records will have no play stats or ratings.
• Orphaned track_persistent records from (3) should still contain their original Added Date so New Music should not be impacted, but the play stats and ratings may be incorrect.
• The duplicate url record from (2) is the wild card here that probably should be confirmed. If the scanner does not find a matching mbid in tracks_persistent, but does find two records with matching md5 hash of the url, I don't know which record it will update with the new mbid as coded today. If it is easy enough accommodate, the preference would be that it update the record that does not currently contain an mbid as in this instance that should be the correct original record for that url and as such would be the better choice in my opinion, but don't see this showstopper to be concerned about plus I can't guarantee that logic is 100% correct as there are probably some anomalies from edge cases like if the user has rescanned after deleting mbid's that I have not accounted for here. And bottom line it will still not be 100% correct and I believe that very few will actually notice this.

[mikeysas]

@darrell-k - I am going to work on setting up a test environment and dataset so I can simulate the 3 outcomes from duplicate mbid's that I outlined in my previous comment. I will then add PR and test for the side effects that I am expecting.

If you are up for adding (2) the LYRION_UNIQUETRACKID, I can run the test with both options.

We could add a pre or post scan analysis of MUSICBRAINZ_TRACKID to inform the user in scanner.log if they have existing duplicates of this tag across their library.

IMO for this to be effective you actually have to prevent the duplicate from being added as in a tracks record with an identified duplicate will added without an mbid and a tracks_persisitent record with the matching url (if it does not already exist) is created (also without an mbid). The problem is what happens on a subsequent scan that looks at the duplicates in a different order. I beginning to think there is not a simple way to prevent duplicates. The Visual Statistics plugin see the # of tracks with duplicate mbid's and save them as a playlist to review. And maybe that is the best we can do with these changes you have outline in (1) & (2).

[darrell-k]

@mikeysas I've given this some more thought. I don't think we should use the existing tracks.musicbrainz_id database column to store a LYRION_UNIQUETRACKID. If we did so:

• It would have to pass validation by having the same hexadecimal format as MUSICBRAINZ_TRACKID/MUSICBRAINZ_RELEASETRACKID.
• And who knows what plugins would use/display it as MUSICBRAINZ_ID, leading to possible confusion.

So I think to implement (2) we would want to populate new database column and condition the link between tracks and tracks_persistent on its presence.

This adds complication, so I would say test (1) on its own for now. The change at present is very simple: all it does is override MUSICBRAINZ_TRACKID with MUSICBRAINZ_RELEASETRACKID at tag reading time.

Of course, for users who know what they are doing and understand the consequences, they can still edit MUSICBRAINZ_RELEASETRACKID (or for that matter MUSICBRAINZ_TRACKID) in their tagging software.

[mikeysas]

So I think to implement (2) we would want to populate new database column and condition the link between tracks and tracks_persistent on its presence.

Well I would vote to not add further complications also and get what you have here into 9.2 as I believe will be big help here.

Including (2) was to satisfy a few who voiced that option in the forum on multiple threads. I am not sure what the hexadecimal validation is for this, but there was one forum user who claimed to successfully make their mb_trackid unique by concatenating mb_trackid+mb_albumid which I did not think would work. When I challenged they said they confirmed it in their persist.db 🤷‍♂️. I am not aware of any plugins today that use track mbid currently, but (a) there is no way to know for sure and (b) I can see the potential for that issue from a future developer especially given the explosion of vibe coded plugins.

@darrell-k - Right now I have just been testing with FLAC as that is all I use. Have you tested each file type or do you need help with that also? I suppose I could convert some to test, but did not want to spend time on that if you have that covered already. Thx.

[darrell-k]

@mikeysas if you could test the other formats it would be very useful.

[mikeysas]

Well that took me a while.... I had issues with converting to APE and WMA which I never touch and had to retag the m4a files because I lost the mbid's on conversion.

Summary Conclusion
I feeling generally good about this and from my testing I believe the odds of side effects are even smaller than I thought. I found a small issue with APE and WMA which I cannot be sure is not related to the trouble I had with converting and it would be good if you could verify this @darrell-k.

Potential Issue
On APE and WMA I observed that it seemed to only read mb_releasetrackid. I tested each file type as follows:

1. With both mb_trackid and mb_releasetrackid
2. just mb_trackid (no mb_releasetrackid)
3. just mb_releasetrackid (no mb_trackid)
   For (2) both my APE and WMA files did not populate the mbid column in tracks or tracks_persistent.

I saying this should be verified by someone else since I had issues converting those. All the other files were converted using FileFlows (which uses ffmpeg). I could not get that to work for APE and WMA so I ended up using dbPowerAmp for WMA and Foobar2000 for APE conversion which is what I could get to work. Given they were each converted with a different tool it seems odd, but maybe it is a problem on my side with the conversion. They both worked fine for (1) and (3).

Full Testing Details
[TL;DR - Side effects I expected for outcome (2) have been mostly eliminated and and the chance of side effects from a revised outcome (3) are too small to be concerned about]

I successfully tested FLAC, OGG, MP3, and m4a with the same 3 use cases and all were correct.

Additionally with FLAC, I tested the three outcome scenarios that I listed in my first reply above before applying the patch and then tested what would happen after the patch is applied. Interestingly, the results of the outcomes were different, but it has been a while and either I am misremembering or some changes since then have changed the behavior.

(1) Missing tracks_persistent records -- this was as I described above

(2) Two tracks_persistent records with the same url and another missing -- this did not happen. Instead what I saw was that the duplicate mbid was added to the tracks_persistent row that was newly scanned with an updated new mbid (previously it had null). The result then is two rows in tracks_persistent with same mbid and each with a unique url. However all updates (play stats and ratings*) were only applied to the record that has the sequently lower index (added to the table earlier). This means that after a clear and rescan the situation stays stable. The same record get updated and the url's are maintained (this is a good thing for this patch).

(*)There was one interesting side effect after the clear and rescan, ratings were being applied to the track based on url so each track was rated correctly despite the duplicate mbid. I use Ratings Light plugin so I suspect that this logic has been worked around in that plugin, but not sure why it took a rescan for that to happen.

(3) Orphaned tracks_persistent records -- this no longer happens because in (2) each record kept the correct url and has an mbid. However, what I did not test and don't know what would happen is if someone reorganized their directory structure with multiple tracks_persistent row with the same mbid from (2). My theory is that the sequentially lower record in the table will be updated with the new url regardless of which record was reorganized. This will create two outcomes depending on which record was changed. If the just the lower sequential record is changed then the situation should be the same. If the higher was changed or both were changed then the lower will have the new url of the higher sequential dup and the lower url will be lost and the higher will now be an "orphan". This all seems very unlikely but still technically possible.

The good news here is that with (2), the only side effect is incorrect play stats the new music sort is not impacted. ratings will be mixed but correct as of the next full rescan. After the dup was first scanned. The side effects of (1) are the same as I described above. Again I did not test the revised theory for (3) and I think it is very unlikely, but I would expect the side effects to be as I described above for (30, but the good news is much lower odds of this happening now.