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

[sampsyo]

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 beetbox/beets#1515 (comment)

[sandreas]

I took a closer look at the code:

mediafile/mediafile.py

Lines 1651 to 1656 in deb597f

	grouping = MediaField(
	MP3StorageStyle('TIT1'),
	MP4StorageStyle('\xa9grp'),
	StorageStyle('GROUPING'),
	ASFStorageStyle('WM/ContentGroupDescription'),
	)

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...

[Morikko]

I wanted to open such issue as I discovered that the grouping field on MP3 is writing to work. The issue popped since I upgraded to navidrome 0.55 that supports now grouping.

A few things to point:

1. The grouping is correctly using @grp for MP4 style
2. While it is using TIT1 for MP3 usually mapped to work

    grouping = MediaField(
        MP3StorageStyle('TIT1'),
        MP4StorageStyle('\xa9grp'),
        StorageStyle('GROUPING'),
        ASFStorageStyle('WM/ContentGroupDescription'),
    )

According to Kid3:

• grouping -> GRP1/@grp
• work -> TIT1/@wrk

Source: https://kid3.sourceforge.io/kid3_en.html#frame-list

What about:

• Map grouping to MP3 GRP1 to be align with most tools out there. It is a breaking change but it looks more a bug to use TIT1.
• Add a new work field that is mapped according to Kid3 as it may be also useful

Would you accept a contribution ?