mopidy / mopidy-local

Mopidy extension for playing music from your local music archive
https://mopidy.com/ext/local/
Apache License 2.0
61 stars 26 forks source link

Put back the use_album_mbid_uri option that was in local-sqlite #26

Closed fatg3erman closed 4 years ago

fatg3erman commented 4 years ago

Using MBIDs to create uris can have unintended consequences. With local-sqlite I could switch this off but this option seems to have been removed in the new mopidy-local.

As an example, I have 2 albums that I tagged with Musicbrainz Picard.

One has 'Artist = Brian Eno' One has 'Artist = Brian Eno & Jah Wobble'

They're also tagged with musicbrainz IDs for the artists. The second album has two artist ID tags even though it only has one artist. (This is the way Musicbrainz reccomend it is done). Since the scanner can only use one mbid to create the artist URI, both albums end up under 'Brian Eno and Jah Wobble'.

With the old local-sqlite setting use_album_mbid_uri = false in mopidy.conf fixed this.

tkem commented 4 years ago

@fatg3erman: Oops, my bad! I was trying to reduce the number of configuration options to make this somewhat less confusing for end users. However, I seem to have overlooked the fact that use_artist_mbid_uri was set to false by default in mopidy-local-sqlite, apparently for good reason. However, instead of re-adding these config options, maybe it would be preferable to remove the mbid URI scheme altogether for simplicity? I guess this would affect only a minority of users with tracks which actually contain Musicbrainz IDs, and even for those who care, it's probably more hassle than it's worth...

fatg3erman commented 4 years ago

Yeah I'm not entirely sure what the purpose of the mbid scheme was - except perhaps that in the case where you need a system to create a unique URI, using a handy mbid is faster than creating an MD5 hash of the entire data structure - I did notice the rescan of my files when I tested this commit took a little longer, but not significantly.

In any case the idea seems to have been based on the assumption that artist mbids are always unique and correct, which certainly isn't the case. I don't see why anybody would care what the actual URI is, so long as they get one and it's unique.

tkem commented 4 years ago

In any case the idea seems to have been based on the assumption that artist mbids are always unique and correct

Yes, this was the original intent ;-)

fatg3erman commented 4 years ago

The problem is, that assumption doesn' work. I've got a few albums where the AlbumArtist is 'Artist A & Artist B', but there is no mbid for 'Artist A & Artist B' because they're individuals with their own mbids, so instead Picard tags it with 2 artist mbids - one for Artist A and one for Artist B. Or sometimes just one for Artist A, or one for Artist B, depending on how it feels on the day :) This leads to albums being assiged to the wrong artist if you use the mbid to determine the artist.

tkem commented 4 years ago

@fatg3erman: I noticed this too, back in the days, and that's why use_artist_mbid_uri was set to false originally. Just forgot about it when merging/porting this. So I'd strongly suggest removing the mbid URI scheme altogether instead of re-introducing those config options.