Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect mapping of Metadata to tag fields #1302

Closed
1 task
bryanparadis opened this issue Apr 22, 2024 · 6 comments
Closed
1 task

Incorrect mapping of Metadata to tag fields #1302

bryanparadis opened this issue Apr 22, 2024 · 6 comments
Assignees
Labels

Comments

@bryanparadis
Copy link

bryanparadis commented Apr 22, 2024

Version

Media3 main branch

More version details

These mappings are mixed up and are causing music to be sorted wrongly in Auxio Music Player. Artist Vitalism was showing up in among artists starting with D and it ended up due to having a sort album field in the mp4 tag with an album starting with D.

All 3 below seem to be mixed up.

      } else if (type == TYPE_SORT_ALBUM) {
        return parseTextAttribute(type, "TSO2", ilst);

TYPE_SORT_ALBUM should be TSOA

      } else if (type == TYPE_SORT_ARTIST) {
        return parseTextAttribute(type, "TSOA", ilst);

TYPE_SORT_ARTIST should be TSOP

      } else if (type == TYPE_SORT_ALBUM_ARTIST) {
        return parseTextAttribute(type, "TSOP", ilst);

TYPE_SORT_ALBUM_ARTIST should be TSO2

Devices that reproduce the issue

Fairphone 4 running Android 13

Devices that do not reproduce the issue

n/a

Reproducible in the demo app?

Not tested

Reproduction steps

  1. Grab a M4A file with sort album field in tag or add the field
  2. Get the Metadata and check TYPE_SORT_ARTIST contains the sort album value

Expected result

TYPE_SORT_ARTIST contains the value of the sort artist tag field

Actual result

TYPE_SORT_ARTIST contains the value of sort album tag field

Media

Not Applicable

Bug Report

@icbaker
Copy link
Collaborator

icbaker commented Apr 22, 2024

Thanks for flagging - I agree things aren't quite right here, but I'm not exactly sure what the right mapping is.

With reference to https://mutagen-specs.readthedocs.io/en/latest/id3/id3v2.4.0-frames.html#other-text-frames

TYPE_SORT_ALBUM should be TSOA

I agree with this, it seems unambiguous from the ID3 spec I linked:

The ‘Album sort order’ frame defines a string which should be used instead of the album name (TALB) for sorting purposes. E.g. an album named “A Soundtrack” might preferably be sorted as “Soundtrack”.


TYPE_SORT_ARTIST should be TSOP

This isn't obvious to me. We map SHORT_TYPE_ARTIST to TPE1 which ID3 describes as:

The ‘Lead artist/Lead performer/Soloist/Performing group’ is used for the main artist

Whereas the ID3 spec describes TSOP as (emphasis mine):

The ‘Performer sort order’ frame defines a string which should be used instead of the performer (TPE2) for sorting purposes.

And TPE2 is defined as (and we map this to TYPE_ALBUM_ARTIST):

The ‘Band/Orchestra/Accompaniment’ frame is used for additional information about the performers in the recording.

So is it just a typo in the ID3 spec and it should just say "[...] instead of the performer (TPE1) [...]"?


TYPE_SORT_ALBUM_ARTIST should be TSO2

I couldn't see TSO2 in the page I linked above, but I do see it here: https://wiki.hydrogenaud.io/index.php?title=Foobar2000:ID3_Tag_Mapping (and there TSOP is more clearly linked to TPE1, and TPE2 is more clearly considered 'album artist').


I'll send a change based on the hydrogenaud.io link.

@bryanparadis
Copy link
Author

bryanparadis commented Apr 22, 2024

The ‘Album sort order’ frame defines a string which should be used instead of the album name (TALB) for sorting purposes. E.g. an album named “A Soundtrack” might preferably be sorted as “Soundtrack”.

TYPE_SORT_ARTIST should be TSOP

This isn't obvious to me. We map SHORT_TYPE_ARTIST to TPE1 which ID3 describes as:

The ‘Lead artist/Lead performer/Soloist/Performing group’ is used for the main artist

Whereas the ID3 spec describes TSOP as (emphasis mine):

The ‘Performer sort order’ frame defines a string which should be used instead of the performer (TPE2) for sorting purposes.

And TPE2 is defined as (and we map this to TYPE_ALBUM_ARTIST):

The ‘Band/Orchestra/Accompaniment’ frame is used for additional information about the performers in the recording.

So is it just a typo in the ID3 spec and it should just say "[...] instead of the performer (TPE1) [...]"?

All of these are different artist/performers
TPE1 Lead performer(s)/Soloist(s)
TPE2 Band/orchestra/accompaniment
TPE3 Conductor/performer refinement
TPE4 Interpreted, remixed, or otherwise modified by

This would be I believe what most people should be using for Artist value in applications?
TPE1 Lead performer(s)/Soloist(s)

and then this would be your alternative sort value
TSOP Performer sort order

Edit: TPE2 must be a typo. Why would they not use the main field for artist/performer? I think that TSO2 is what is used as an alternative sort for TPE2. Artist = TPE1 and Album Artist = TPE2

TYPE_SORT_ALBUM_ARTIST should be TSO2

I couldn't see TSO2 in the page I linked above, but I do see it here: https://wiki.hydrogenaud.io/index.php?title=Foobar2000:ID3_Tag_Mapping (and there TSOP is more clearly linked to TPE1, and TPE2 is more clearly considered 'album artist').

I'll send a change based on the hydrogenaud.io link.

It is listed on mutagen-specs in the Off-Spec Frames section:

https://mutagen-specs.readthedocs.io/en/latest/id3/id3v2-other-frames.html?highlight=tso2

I am not sure how supported it is in ID3 or not as it's not my area of expertise. It looks to be referenced a fair number of places.

@bryanparadis
Copy link
Author

Everything that I can find while searching online shows TPE1 as the main artist and TPE2 as additional/album artist.

https://mutagen-specs.readthedocs.io/en/latest/id3/id3v2.4.0-frames.html?highlight=tsop#tpe1

TPE1
The ‘Lead artist/Lead performer/Soloist/Performing group’ is used for the main artist.

TPE2
The ‘Band/Orchestra/Accompaniment’ frame is used for additional information about the performers in the recording.

here as well https://docs.mp3tag.de/mapping/#albumartist

@icbaker
Copy link
Collaborator

icbaker commented Apr 23, 2024

Edit: TPE2 must be a typo. Why would they not use the main field for artist/performer? I think that TSO2 is what is used as an alternative sort for TPE2. Artist = TPE1 and Album Artist = TPE2

Yeah, this makes sense to me - sorry my comment was a bit confusing.

It is listed on mutagen-specs in the Off-Spec Frames section:

Ah great, thanks for that link, I hadn't seen that.

copybara-service bot pushed a commit that referenced this issue Apr 23, 2024
The ID3 tags are documented here:
https://wiki.hydrogenaud.io/index.php?title=Foobar2000:ID3_Tag_Mapping

The MP4 fourcc types are documented here:
https://mutagen.readthedocs.io/en/latest/api/mp4.html#mutagen.mp4.MP4Tags

From the field definitions at the top of this file:

* `TYPE_SORT_ALBUM = 0x736f616c = 'soal'`
* `TYPE_SORT_ARTIST = 0x736f6172 = 'soar'`
* `TYPE_SORT_ALBUM_ARTIST = 0x736f6161 = 'soaa'`

Issue: #1302

#minor-release

PiperOrigin-RevId: 627486902
@icbaker
Copy link
Collaborator

icbaker commented Apr 23, 2024

This should be fixed by the commit above.

@icbaker icbaker closed this as completed Apr 23, 2024
@bryanparadis
Copy link
Author

bryanparadis commented Apr 23, 2024

Edit: TPE2 must be a typo. Why would they not use the main field for artist/performer? I think that TSO2 is what is used as an alternative sort for TPE2. Artist = TPE1 and Album Artist = TPE2

Yeah, this makes sense to me - sorry my comment was a bit confusing.

No worries!

It is listed on mutagen-specs in the Off-Spec Frames section:

Ah great, thanks for that link, I hadn't seen that.

Glad I could help. Thanks for fixing this!

OxygenCobalt pushed a commit to OxygenCobalt/media that referenced this issue May 18, 2024
The ID3 tags are documented here:
https://wiki.hydrogenaud.io/index.php?title=Foobar2000:ID3_Tag_Mapping

The MP4 fourcc types are documented here:
https://mutagen.readthedocs.io/en/latest/api/mp4.html#mutagen.mp4.MP4Tags

From the field definitions at the top of this file:

* `TYPE_SORT_ALBUM = 0x736f616c = 'soal'`
* `TYPE_SORT_ARTIST = 0x736f6172 = 'soar'`
* `TYPE_SORT_ALBUM_ARTIST = 0x736f6161 = 'soaa'`

Issue: androidx#1302

PiperOrigin-RevId: 627486902
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants