Adapt to new iTunes ID3 mappings for "grouping" and "work"

[sampsyo]

trafficstars

Apparently, some iTunes version around 12.5 changed the "grouping" field to map to the ID3 tag GRP1, while it was previously TIT1. It now uses TIT1 for the work title. Picard apparently has a special config option just to deal with this mapping. There's more info in a post on the mp3tag forum.

[sandreas]

Thank you... In case someone needs sample files see https://github.com/beetbox/beets/pull/1515#issuecomment-501622982

[sandreas]

I took a closer look at the code: https://github.com/beetbox/mediafile/blob/deb597f747b0959350bb74cabcea177d2ea1de3c/mediafile.py#L1651-L1656

In my opinion ALWAYS writing TIT1 and GRP1 is not reasonable... i think a specific setting would be needed, like in Picard (e.g. itunes_compatible_grouping=1), to decide which field should be used - but i did not see a way to pass such a setting to MediaFile.

Since a StorageStyle can have only one single key, its difficult to decide how to fix this. I would have prepared a pull request, but my python is not good enough to touch such a sensitive area without further instructions.

Any Suggestions?

[sampsyo]

That’s troubling. Making it a config option would actually be pretty disappointing—a nice thing about MediaFile as it currently stands is that it basically works, most of the time, for most software. It maximizes compatibility without fuss on the part of the user.

Maybe there’s some policy we can devise that would do the right thing in most cases? For example, maybe we could flip the implementation of “work” depending on the presence of GRP1?

[sandreas]

Did you mean something like this?:

class MP3StorageStyle(StorageStyle):
    """Store data in ID3 frames.
    """
    formats = ['MP3', 'AIFF', 'DSF']

    def __init__(self, key, id3_lang=None, **kwargs):
        """Create a new ID3 storage style. `id3_lang` is the value for
        the language field of newly created frames.
        """
        self.id3_lang = id3_lang
        super(MP3StorageStyle, self).__init__(key, **kwargs)

    def fetch(self, mutagen_file):
        try:
            return mutagen_file[self.key].text[0]
        except (KeyError, IndexError):
            """Switch missing TIT1 key to GRP1 if present for iTunes compatibility"""
            if self.key == 'TIT1' and 'GRP1' in mutagen_file:
                self.key = 'GRP1'
                return mutagen_file[self.key]
            return None

    def store(self, mutagen_file, value):
        frame = mutagen.id3.Frames[self.key](encoding=3, text=[value])
        mutagen_file.tags.setall(self.key, [frame])

Would that work in every case? Event when converting files in beets for example?

Or did you mean, that store should also have such a check and only overwrite the tag if its present and leave self.key?, e.g.:

class MP3StorageStyle(StorageStyle):
    """Store data in ID3 frames.
    """
    formats = ['MP3', 'AIFF', 'DSF']

    def __init__(self, key, id3_lang=None, **kwargs):
        """Create a new ID3 storage style. `id3_lang` is the value for
        the language field of newly created frames.
        """
        self.id3_lang = id3_lang
        super(MP3StorageStyle, self).__init__(key, **kwargs)

    def fetch(self, mutagen_file):
        try:
            return mutagen_file[self.key].text[0]
        except (KeyError, IndexError):
            """Switch missing TIT1 key to GRP1 if present for iTunes compatibility"""
            if self.key == 'TIT1' and 'GRP1' in mutagen_file:
                return mutagen_file['GRP1']
            return None

    def store(self, mutagen_file, value):
        if self.key == 'TIT1' and 'GRP1' in mutagen_file:
            frame = mutagen.id3.Frames['GRP1'](encoding=3, text=[value])
            mutagen_file.tags.setall('GRP1', [frame])
                
        frame = mutagen.id3.Frames[self.key](encoding=3, text=[value])
        mutagen_file.tags.setall(self.key, [frame])

[sampsyo]

Something like this! I guess it would be useful to think through the right policy and what each option’s positives and negatives are. We can code up any policy we like, even if it requires some changes to the MediaField interfaces...