[fixed] Multiple music genres not handled correctly? (2 Viewers)

[MJGraf]

I just imported my whole music collection with the current master branch tvserver.

Many of my mp3-files have multiple genres. The separator used is ";". When I take the genre view in MP2-client, this is e.g. displayed as:

"Rock;Guitar"

That means the importer doesn't seem to realize that these are two genres "Rock" and "Guitar" - it treats this as one genre "Rock;Guitar".

The easiest explanation would be that I use the wong separator. But it works perfectly in MP1 so I doubt it...
Having a short glance at the code shows me that genres are coded as a many to many property, so this seems to be right. MP2-code always uses "string[]" for genres, which seems right to me as well. So I suppose the problem is within the taglib code, but I may also have overlooked a bug in the MP2 code...

Logs apparently don't show anything unusual. Please let me know when you need something else (or if should change my separator....)

Thanks,
Michael

 

[Smeulf]

Hi Michael,

May I ask the name of the software you use to tag your music ?

I can't save multiple tags with Windows Explorer, and I'd like to reproduce the exact same problem.

Thanks.

Smeulf.

 

[MJGraf]

I'm using MusicBrainz Picard: MusicBrainz Picard - MusicBrainz
with the "Last FM Plus" plugin: Picard Plugins - MusicBrainz

In the plugin you can chose how many genres should be saved.

OT: I'd love to have MusicBrainz support in MP2 because this site ensures that your files are tagged (nearly) absolutely consistently. I already had a look into doing it myself, but it is just too much for my programming knowledge... One way would be to just support it within kind of a ripping-plugin, which gets its metadata from this site. The more elegant way would be to write a MetadataExtractor, which gets its data from MusicBrainz...

Thanks,
Michael

 

[Smeulf]

I'm using MusicBrainz Picard: MusicBrainz Picard - MusicBrainz
with the "Last FM Plus" plugin: Picard Plugins - MusicBrainz

In the plugin you can chose how many genres should be saved.

OT: I'd love to have MusicBrainz support in MP2 because this site ensures that your files are tagged (nearly) absolutely consistently. I already had a look into doing it myself, but it is just too much for my programming knowledge... One way would be to just support it within kind of a ripping-plugin, which gets its metadata from this site. The more elegant way would be to write a MetadataExtractor, which gets its data from MusicBrainz...

Thanks,
Michael

Well, after some research, it seems we're dealing with a stardardisation problem :

ID3 Tag V2.3 defines the separator as a "/"
ID3 Tag V2.4 defines the separator as a null char

So the notation with a ";" as a separator does not match the ID3 specs.

But, I agree many software can use ";" as separator. Also, it seems ITunes uses "," and Foobar (does it still exists ?) uses "\"

I'm not the one that will descide what to do about this issue, it's just some points to discuss...

 

[MJGraf]

You're absolutely right, Smeulf!

This seems to have been discussed for MP1 as well: https://forum.team-mediaportal.com/listen-music-99/issues-album-artist-field-101372/

My problem was that e.g. MPTagThat accepts ";" and automatically replaces it with "/". Additionally, MP1 seems to accept ";" as well although this is not within the spec of ID3v2.3.
I, personally, don't like all these workaraounds so I would prefer MP2 being exactly with the specs, although that means fot me retagging all my files with "/". Maybe it would make even more sense to change to ID3v2.4 since for me "null" als separater makes a lot more sense than "/" (also found the "AC/DC-Exemption" in the code...). The only problem is that ID3v2.4 is not as widely supported as ID3v2.3.

I'll test in the next days with ID3v2.4 and ID3v2.3 with "/" as separator. If that works, I think we can mark this thread as "no bug".

Thanks,
Michael

And the really bad thing can be found in MP1-Source-Code file MusicDatabase.Updates.cs line 1263:

// When we got Multiple Entries of either Artist, Genre, Albumartist in WMP notation, separated by ";",
// we will store them separeted by "|"

The semicolon as separator seems to be the standard for WMP. And to make things even more complicated, MP1 stores the separator in its internal database as "|"...

Maybe at this point we should ask our commander in chief. Albert, do you have any plan how to handle this in MP2? I think at least we don't need to talk about the separator used in the database, because multiple values are in MP2 correctly stored in a separate database table (as it should be with many-to-many-relationships). The question is only how to handle the import. Although as mentioned above I'm not a fried of "hacks" besides the spec, I think many people use WMP to tag their files and I'm not sure whether it's a good idea to neglect that...

Michael

 

[MJGraf]

Over the weekend I tested the behaviour with "/" as separator for genres. I used MP3Tag to replace ";" with "/" in some of my MP3 and Flac music files and did a reimport of the music share.

Unfortunately, for me it doesn't work. It now shows "pop/disco" instead of "pop;disco" as a single genre.

Will have to do some further debugging...

Michael

 

[Albert]

AW: Multiple music genres not handled correctly?

Michael, your bug reports are very good and detailed, I like that very much.
The MusicMetadataExtractor is responsible for the extraction of MP3 tags. If you would like, you can debug the extractor method when you're in local browsing mode, maybe in a folder with only that single MP3 file. Then you can see if the extraction fails in the underlaying extraction library or in our code.
IIRC some months ago, I already had a look on the extraction of multiple artists because Smeulf reported another problem where multiple artists are not shown correctly in the media library. But I don't remember there was a problem with the parsing library for multiple artists separated with "/". Maybe it differs if you have MP3 tags of different versions.

 

[MJGraf]

Hi Albert,

Thanks, I'm trying my best . However, I have to correct my results from above. When I did the reimport there seems to have been a problem with my MP2 database (I changed the tags while the importworker was still importing). I deleted the database and did a fresh import of those files.

The result is that that MP2 behaves standard conform. I.e., multiple genres in ID3v2.3 tags separated by "/" are correctly imported as multiple genres. However, many of my files are flac-files. The vorbis comments in flac files do not have a standard separator. Vorbis comments should have multiple fields with the same name "genre" for multiple genres - which is not the case in my collection. As a result, the flac files still have genres like "pop/disco".

So definitely, this is not a bug - more kind of a feature request or a question as to how MP2 should behave in standard configuration.

To be honest, I don't know how MP2 should handle this. As mentioned above, I'm a fan of being as much standard compliant as possible. On the other hand, MP1-users being used to the customization possibilities of MP1 won't be happy with as it is now, I suppose. Furthermore, MP1 and MPTagThat seem to accept ";" as kind of an additional standard separator for all music metadata formats, which I personally don't like (although it leads to correct results with my music collection) since it seems to be completely hardcoded.

The following are just some thoughts and I'd like to hear your opinion about it before I try to start coding:
I could imagine an additional setting called "AdditionalSeparator" with a description like that:
"MP2 in general behaves 200% standard conform as to the behaviour of its metadata extractors. However, since many people use the same settings in their respective tagging software for every kind of media files, tagging information with multiple entries for one field (such as artists or genre) may be stored consistently across all kinds of media files but at the sime time non-compliant with some of the media metadata standards. As an example, many musing tagging programs store multiple values for one field in a single field with a ';' as separator between the multiple values. However, ';' is not a valid separator for most of the common music metadata formats. For ID3v2.3 the standard separator would be "/", in ID3v2.4 it would be a null value, while in Vorbis Comments, there are no separators at all since it is recommended to store multiple metadata fields with the same name for multiple values. If this setting is e.g. set to ';", the semicolon is accepted as an additional separator for metadata fields. That means that the standard separators of the vaious formats (such as '/' for ID3v2.3 or null for ID3v2.4) are still treated as separators for the respective metadata format. However, ';' is treated as an additional common separator across all metadata formats."

This would lead to the following question: How about the scope of this separator setting?
I could first imaging to make this setting absolutely global. One could implement it e.g. in the "SetCollectionAttribute" method of the MediaAspect. Howeer, I'm not sure what kind of metadata could be saved for media items such as internet streams or whatever may be possible in the future. If this kind of metadata contains lots of ';', this may be a problem.
The opposite way would be to implement it just for genres, but I think this scope would be too narrow since I could imagine that indeed lots of people (like me) use the same settings (e.g. ';' as separator) for all multiple value fields no matter what kind of metadata format is used.
In my opinion the best way would be to implement it on a "per metadata extractor basis". I.e., the setting would be valid for the MusicMetadataExtractor only, but all multiple value fields handled within it.

The final question would be: Should we provide for an "exemption-setting" such as for "AC/DC" in ID3v2.3-tags? I personally would implement this as a further setting to be able to cope with any situation that may arise.

As a result, I would implement these additional settings:
UseAdditionalSeparator: bool (Standard could be true to achieve the same behaviour as MP1 - or false to be standard compliant?)
AdditionalSeparator: char (Standard could be ';' for people using WMP as tagger?)
AdditionalSeparatorExceptions: string[] (No idea for a standard. Isn't P;nk originally written like that?!?)
These settings would be applicable to the MusicMetadataExtractor only, but within MusicMetadataExtractor it would be applicable for every multi value field (at the moment: Artists, AlbumArtists, Composers and Genres)

Ok, long text, I know, but please Albert or anyone else interested let me know your thoughts about it!

Best, Michael

Edit: Or maybe we should ask hwahrmann? I'm sure he got lots of more feature requests in that respect for MPTagThat than we will ever get for MP2

 

[MJGraf]

The following are just some more observations I made during browsing the code with respect to '/' as separator. I just wanted to write this down so that it doesn't get lost...

The starting point is TagLib/ID3v2/Frames/TextIdentificationFrame.cs lines 876 et seq.:
'/' is treated as separator for the following ID3v2 fields:
TCOM
TEXT
TOLY
TOPE
TPE1
TPE2
TPE3
TPE4
TCON

However, according to the ID3V2 specs, this only applies if the ID3v2 version is 3 or lower (i.e., ID3v2.2 and ID3v2.3; cf. TagLib/ID3v2/Frames/TextIdentificationFrame.cs line 863).

Therefore, as a first thought, in MusicMetadataExctractor PatchID3v2Enumeration should only be called if (Tag.TagTypes & TagLib.TagTypes.Id3v2 != 0) and (Tag.Version <= 3)

Then I had a look into TagLib/ID3v2/Tag.cs to find out, which of the above mentioned fields are actually used.

TCOM is mapped as public override string [] Composers
TEXT is not used at all
TOLY is not used at all
TOPE is not used at all
TPE1 is mapped as public override string [] Performers
TPE2 is mapped as public override string [] AlbumArtists
TPE3 is mapped as public override string Conductor [please note that this is not an array of strings but only a string - I don't understand why separators are taken into account at all for this field]
TPE 4 is not used at all
TCON is mapped as public override string [] Genres

In MusicMetadataExtractor, PatchID3v2Enumeration is currently called for the following values:
ATTR_ARTISTS (TPE1)
ATTR_ALBUMARTISTS (TPE2)
Based on the above and to be on the safe side, PatchID3v2Enumeration should therefore imho be called for ATTR_GENRES as well (although I can't imagine a genre with a '/' in it, but you never know what people do with their tags...). Futhermore, once an ATTR_COMPOSERS is implemented, PatchID3v2Enumeration should also be called for these values. This means at the same time that we may not only have DEFAULT_UNSPLITTABLE_ARTISTS, but also DEFAULT_UNSPLITTABLE_GENRES and DEFAULT_UNSPLITTABLE_COMPOSERS.

While the above only relates to the ID3v2 separator '/', I'm still thinking about my idea above of an "AdditionalSeparator". Deviating from the '/'-Prolem, I personally would apply this to all values we treat as multi-value-fields (i.e., where SetCollectionAttribute is called).
As I read again what Smeulf wrote above ("But, I agree many software can use ";" as separator. Also, it seems ITunes uses "," and Foobar (does it still exists ?) uses "\"), it would maybe make more sense to use "char[] AdditionalSeparators" instead of "char AdditionalSeparator" to be able to have multiple additional separators.

Again a lot of text, but I don't want to let my findings get lost....

Michael

 

[MJGraf]

The more I read the more I come to the conclusion that this whole tagging thing is a mess...
I had a deep look into the ID3v2.3 specs.

"/" as a separator should be used in TCOM, TEXT, TOLY, TOPE and TPE1 (Furthermore it's used in TPOS (to separate the "Part of Set" from the amount of all sets), in TRCK (to separate the actual track from the number of all tracks) and to separate prices in the commercial frame - but these are not relevant here).

This means first of all, that TagLib doesn't seem to be standard conform with regard to TPE2, since the spec does not provide for "/" as separator. This may, however, be related to the fact that the ID3v2 spec does not say that TPE2 is "AlbumArtist", but it says that it is "used for additional information about the performers in the recording". Nevertheless, it seems to be a de facto standard that TPE2 is used as AlbumArtist field and it just wouldn't make sense to have Artists (TPE1) separated by "/" and AlbumArtists not. So probably this is the reason for deviating from the standard.

For genres it gets even worse. The spec says:
The 'Content type', which previously was stored as a one byte numeric value only, is now a numeric string. You may use one or several of the types as ID3v1.1 did or, since the category list would be impossible to maintain with accurate and up to date categories, define your own.
References to the ID3v1 genres can be made by, as first byte, enter "(" followed by a number from the genres list (appendix A) and ended with a ")" character. This is optionally followed by a refinement, e.g. "(21)" or "(4)Eurodisco". Several references can be made in the same frame, e.g. "(51)(39)". If the refinement should begin with a "(" character it should be replaced with "((", e.g. "((I can figure out any genre)" or "(55)((I think...)". The following new content types is defined in ID3v2 and is implemented in the same way as the numerig content types, e.g. "(RX)".

The first paragrph says that you can "define your own" genres. However, there is nothing about a separator to be used when doing so. The second paragraph then only says something about using the old numeric predefined genres and refining them. From this one could conclude that either (1) this is the only possibility of "defining your own" genre, i.e., you can't just add a new genre without having a reference to a predefined one before or (2) when defining your own genre you should enclose it in "()", or (3) this has nothing to do with "defining your own" genre.

I tend to interpret this as mentioned in (3), but then the question remains what to use as separator. Maybe this is the reason why "/" as separator was used in kind of an analogy to TPE1, etc. However, I read in a post somwhere at taglibsharp (can't find it right now) that this is discussed and may be changed later.

To cut a long story short, in my opinion, we should rely on taglibsharp an it's behaviour as long as you can interpret the specs in a way that taglibsharp is within the specs - which is the case for my problem here.
Nevertheless, since the "specs" are not as unambiguous as they should be, we should imho include a possibility for an additional separator as described above to make MP2 as flexible as possible for the users and to give them the possibility to use the tagging tool they like even if this tool is not completely within the specs.

I'll try to implement something and provide a patch. Let me know what you think about it.
Michael

-----

...and for those of you who want to have a good laugh, have a look there: SoundClick artist: semicolon - Saul Williams, Rage Against the Machine, good, best, worst, hate, Metal, Rock, Primus, Laswell, Kill
Someone should tell this band what headache they cause with their creative name for programmers...