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.

Sign up for GitHub

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

Jump to bottom

PICARD-700: Support albumartists tag #553

Closed
wants to merge 2 commits into from
Closed

PICARD-700: Support albumartists tag #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa41519
Display albumartists in UI
samj1912 Jan 13, 2017
a237599
Save albumartists tag to files
samj1912 Jan 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions picard/formats/asf.py
View file
Open in desktop
Expand Up @@ -138,6 +138,7 @@ class ASFFile(File):
'djmixer': 'WM/DJMixer',
'mixer': 'WM/Mixer',
'artists': 'WM/ARTISTS',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. I wonder why this is like this, when it's 'artistsort': 'WM/ArtistSortOrder', further up for artistsort. Maybe 'artists': 'WM/Artists', would have been better. Does mutagen use all-uppercase? :/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between "artist" and "artists"? mutagen properly supports multiple "Author" values since 1.26.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

artists is simply a multi valued field with all the artists, artist includes things like 'feat.' etc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we don't have something like that in quodlibet. We have a "artist" tag containing a list of artists and save it in "Author" as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we have had some discussion on this see PICARD-586

I think @zas @mwiencek @mineo should be consulted since this is a big change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Jaudiotagger Tag Mappings

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can;t remember whether tags in ASF format are case independent or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little bit of internet research suggests that in WMP 11 and below they were case insensitive, but in WMP they are case sensitive. I suspect that Jaikoz created the tag first and we then adopted it with the case defined, but that may be wrong. Regardless, the case is now fixed by historical usage.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jaudiotagger tag mappings doc is out of date actually best to use the spreadsheet link below, the jaudiotagger source code on the songkong help http://www.jthink.net/songkong/help/help.html#d5e2051for uptodate mappings

'albumartists': 'WM/ALBUMARTISTS',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe 'albumartists': 'WM/AlbumArtists',? (Based on 'albumartistsort': 'WM/AlbumArtistSortOrder',.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar question, what's the difference between "albumartist" and "albumartists"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lazka same as artist and artists. artist is a string, artists is a multi valued field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eg Artist: Chakuza & RAF Camora
and Artists: Chakuza/RAF Camora

'work': 'WM/Work',
'website': 'WM/AuthorURL',
}
Expand Down
1 change: 1 addition & 0 deletions picard/formats/id3.py
View file
Open in desktop
Expand Up @@ -153,6 +153,7 @@ class ID3File(File):
'ASIN': 'asin',
'MusicMagic Fingerprint': 'musicip_fingerprint',
'Artists': 'artists',
'Album Artists': 'albumartists',
'Work': 'work',
'Writer': 'writer',
}
Expand Down
1 change: 1 addition & 0 deletions picard/formats/mp4.py
View file
Open in desktop
Expand Up @@ -102,6 +102,7 @@ class MP4File(File):
"----:com.apple.iTunes:SCRIPT": "script",
"----:com.apple.iTunes:LANGUAGE": "language",
"----:com.apple.iTunes:ARTISTS": "artists",
"----:com.apple.iTunes:ALBUMARTISTS": "albumartists",
"----:com.apple.iTunes:WORK": "work",
"----:com.apple.iTunes:initialkey": "key",
}
Expand Down
2 changes: 1 addition & 1 deletion picard/mbxml.py
View file
Open in desktop
Expand Up @@ -195,7 +195,7 @@ def artist_credit_to_metadata(node, m, release=False):
m["musicbrainz_albumartistid"] = ids
m["albumartist"] = artist
m["albumartistsort"] = artistsort
m["~albumartists"] = artists
m["albumartists"] = artists
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we'll want to keep ~albumartists (alongside albumartists) for the 1.4.x releases, marking it as deprecated and remove for 2.0?

Copy link
Collaborator Author

@samj1912 samj1912 Jan 13, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the '~' simple means its a 'hidden/protected' tag and is appended to hide it from UI/saving from file tags.

As for it's use in scripting, we can update the documentation maybe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created ~albumartists.

I would suggest that we dispose of this now, and instead create update code that replaces %_albumartists% with %albumartists% in any scripts.

It might break plugins, but with the exception of my sort_multivalue_tags plugin (which strangely already has albumartists instead of ~albumartists) there are no other plugins I know of which use this.

m["~albumartists_sort"] = artistssort
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created this one too.

See also ~artists_sort.

I suggest we keep these both as hidden and not saved to tags.

else:
m["musicbrainz_artistid"] = ids
Expand Down