beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.72k stars 1.81k forks source link

`beet write`: false positive for changes in fields with multiple values #5371

Open celynw opened 1 month ago

celynw commented 1 month ago

Problem

It seems that for fields which allow multiple tags, the write command might incorrectly say there are changes to be made.

Backstory

I am using the albumtypes plugin to set the 'soundtrack' album type.

My relevant config looks like this:

albumtypes:
  types:
    - soundtrack: "OST"
    - ep: "EP"
    - single: "Single"
    - live: "Live"
    - compilation: "Anthology"
    - remix: "Remix"
  ignore_va: compilation
paths:
  default: $albumartist/$year - $album%aunique{}/%if{$multidisc,$paddisc-}$padtrack $title
  albumtype:soundtrack: Soundtracks/$album/%if{$multidisc,$paddisc-}$padtrack $title
  albumtypes:soundtrack: Soundtracks/$album/%if{$multidisc,$paddisc-}$padtrack $title
musicbrainz:

(I opened a related discussion, which was solved.)

Basically, if 'soundtrack' appears at all in MusicBrainz's albumtypes list, I'll set the main albumtype to soundtrack. This is because I prefer soundtrack to be the highest priority when something belongs to multiple types.

This does work; I checked the database manually, and the types are set correctly:

albumtype albumtypes
soundtrack album; soundtrack

I also checked the files themselves and found the data to also be correct.

RELEASETYPE     : album;soundtrack
MUSICBRAINZ_ALBUMTYPE: album;soundtrack

My issue is that when I run beet write, it tells me there are many changes to be made like:

albumtype: album -> soundtrack

This never clears; if I run the command again, it tries to do this again.

The issue seems to related to this part in mediafile:

    albumtypes = ListMediaField(
        MP3ListDescStorageStyle('MusicBrainz Album Type', split_v23=True),
        MP4ListStorageStyle('----:com.apple.iTunes:MusicBrainz Album Type'),
        ListStorageStyle('RELEASETYPE'),
        ListStorageStyle('MUSICBRAINZ_ALBUMTYPE'),
        ASFStorageStyle('MusicBrainz/Album Type'),
    )
    albumtype = albumtypes.single_field()

ListMediaField.single_field returns the first item in albumtypes.

This comes up in the write command, write_items(), where a 'clean_item` is created from the file on disk:

https://github.com/beetbox/beets/blob/b88c09720c3f0782b90f83df74e65680c050392f/beets/ui/commands.py#L2285

It's a bit tricky, because both the file and the database are already correct, but because the write command thought there is a difference, it will attempt to write the corrections anyway.

If the above is always true, a workaround which works for me is to add

        if item.albumtype in clean_item.albumtypes:
            clean_item.albumtype = item.albumtype

before checking for the changes here:

https://github.com/beetbox/beets/blob/b88c09720c3f0782b90f83df74e65680c050392f/beets/ui/commands.py#L2293

I'm not sure if this is the most robust though.

Also, I guess it could apply to other 'multiple' fields too:

Setup

celynw commented 1 month ago

And similarly for beet update: adding

            old_item = lib.get_item(item.id)
            if old_item.albumtype in item.albumtypes:
                item.albumtype = old_item.albumtype

before checking for the changes here:

https://github.com/beetbox/beets/blob/b88c09720c3f0782b90f83df74e65680c050392f/beets/ui/commands.py#L1685

snejus commented 3 weeks ago

Thank you, this is an amazing investigation! Linking this to the issue where this problem has been reported first: https://github.com/beetbox/beets/issues/5265