[fixed] Multiple music genres not handled correctly? (1 Viewer)

[MJGraf]

Hi everybody,

patch attached.
It corrects the ID3v2.3 problem ("AC/DC"-problem) for composers and genres additionally to artists and albumartists (explanation see above).
It introduces an AdditionalSeparator as described above. This change is optional as it is implemented as setting.

I included a lot of comments, so it should be fairly self-explanatory.
The patch works perfectly with my whole music collection, but I'd be grateful if someone with more programming knowledge could have a look at it because I'm not sure if my implementation is very elegant.

Let me know what you think!

cheers,
Michael

 

[Albert]

AW: Multiple music genres not handled correctly?

Thanks Michael very much!
Your patch is almost perfect! I don't say that to many people...
There were only little syntactic problems: Some ending dots missing, a parameter was not referenced correctly in the param docs, some odd <remarks> tags. I corrected those problems.

Albert

 

[hwahrmann]

Michael asked me to have a peek on that issue and respond.

Taglib-sharp should handle the instances with multiple values for a Tag pretty well, provided that the Tagger does it's job correctly.
for the various music formats we have the following rules:

MP3
----
Id3v1 -- Not supported
Id3v2 v3 - the "/" (forward slash) is used as the separator
id3v2 v4 - the null char is used as separator

Ogg Vorbis (used in ogg and FLAC)
----------
We have mutiple tags of the same tag type.
e.g. Genre=Rock Genre=Hard Rock

M4a (apple AAC Format)
-----------------------
uses ";" as a separator

If a tagger did the tagging in the above described method, then taglib-sharp delivers the multi value fields as an Array.

In the Tagreader implementation within MP1, I join the Array to a string separated with ";". An AC/DC song stored in Id3 v3 would be returned as "AC;DC", so i handle this as an exception.

If a Tagger now didn't follow the rules and stored the mullti values separated with ";", then Taglib will return an array with just 1 element.
Because i'm looking for ";" in mp1 anyhow, this doesn't cause any troubles.

If a Tagger now uses "\" or "|" or something else for separation, this will be handled not as a multi value by Taglib.

If you want to have flexibility for non-standard taggers, you need to add exception handling into MP2, but i fear this will just add confusion.
We should "teach" our users to follow the standards and retag their crappy files, with standard taggers.
i've tagged my complete music collection with MPTagThat and don't have any troubles with that.

Note: We need also to be careful telling the users to switch to ID3 v4 tagging, cause at least the old Apple iOS used on older iPods can't handle all the tags of it. e.g Lyrics from a v4 can't be shown.
Maybe this has changed at the new versions, but i can't v erify that.

 

[MJGraf]

hwahrmann: Thanks a lot, Helmut, your input is highly appreciated. Now I understand why MP1 and MPTagThat treats ';' as a separator.

Albert: Thanks for committing the patch. There is still one little bug in it. Today I tried to import my "untagged" or "half-tagged" files as well and got an exception in the logs, which was caused by some tag fields not containing a value. I forgot to check for null values and empty enumerations in ApplyAdditionalSeparator. I corrected this in the attached patches (Sorry for two patches - I'm still struggeling with Git...). I'd be glad if you could commit these as well (or mabe easier just add the two lines contained in the two patches).

Thanks to you all and sorry for the bug...
Michael

 

[MJGraf]

Just as a last confirmation:

I just compiled the latest dev-branch and did a complete, 2-hour-lasting fresh import of my music collection. It finished without any error and the behaviour is now as (at least) I would expect it to be for all fields (including genre).

again for committing!