search

beetbox / mediafile

elegant audio file tagging
http://mediafile.readthedocs.io/
MIT License
100 stars 26 forks source link

More tag variants for `albumartists` #47

Closed JuniorJPDJ closed 2 years ago

JuniorJPDJ commented 3 years ago

Some fight about tag naming in Picard is still having place: https://tickets.metabrainz.org/browse/PICARD-700 Meanwhile different software using this tag uses other variants for it. e.g. Kodi can read ALBUMARTISTS and ALBUM ARTISTS while jaudiotagger is tagging files using ALBUM_ARTISTS tag.

JuniorJPDJ commented 3 years ago

Rebased on top of master and updated description. Tbh. I need some feedback.

sampsyo commented 3 years ago

Argh; really too bad that people haven't standardized this. I'm not really sure what to do about the concatenated values from multiple lists—it does not seem good! Ideally, we would have the first non-empty list take precedence, and any remaining values would be ignored. But I don't really understand where the concatenation is coming from in the first place… tracking down the source of that issue seems like an important first step.

JuniorJPDJ commented 3 years ago

This seems to come from there: https://github.com/beetbox/mediafile/blob/341db182c5699485e5d6d2495497c724036e9922/mediafile.py#L1275

Anyway - I would leave this as a draft for now and wait a bit for Picard's.

JuniorJPDJ commented 3 years ago

Ideally, we would have the first non-empty list take precedence, and any remaining values would be ignored.

Done ;)

JuniorJPDJ commented 3 years ago

.

JuniorJPDJ commented 3 years ago

Nothing is happening in Picard issue, so I think we should think about merging this one to maintain compatibility with all software ;/

What do you think about it? Does it need some changes?

Concatenated lists are fixed now ;)

JuniorJPDJ commented 2 years ago

bump, can we merge this?

sampsyo commented 2 years ago

Sorry for letting this slip through the cracks!! Seems like we should head toward merging it. A couple of high-level challenging questions:

Adding a changelog entry would also be great!

JuniorJPDJ commented 2 years ago

If we think either of these are somewhat uncommon, maybe it would make sense to mark one or both as read-only?

There's no actual standard for those. I'd go with marking the one with space as read-only as I personally don't like this one :D I remember there was some software unable to read one of ALBUMARTISTS or ALBUM_ARTISTS and other not being able to read the second so I'd leave those 2 tags as RW. There's some discussion about this and guys mention only ALBUMARTISTS or ALBUM_ARTISTS.

Maybe we should also dive back into the history to see why we originally made these values concatenated to see if there are specific fields that actually want that behavior.

TBH. I can't even imagine where it could be useful and I found no regression when testing this. This was probably by mistake as it was easier to write it like this. This would cause some issues for re-reading files with multiple RW tags with the same data - cloning values. AFAIK we didn't before have any multi-tags with list values.

JuniorJPDJ commented 2 years ago

Fixed mentioned read_only properties and added changelog entities to commits

JuniorJPDJ commented 2 years ago

I'd like also mention that readthedocs seems not to update automagically and seems to be not updated since looong time (v0.4.0).

JuniorJPDJ commented 2 years ago

@sampsyo bump ;)

JuniorJPDJ commented 2 years ago

bump! :D