GNOME Bugzilla – Bug 768520
AcoustID plugin: Support fetching recording artist for the "artist" key
Last modified: 2016-07-12 09:25:35 UTC
As acoustID source is meant to be used with a single audio piece at a time, its GRL_METADATA_KEY_ARTIST should contain the song/recording artist and not the album-artist (which could be from a compilation album). Instead the GRL_METADATA_KEY_ALBUM_ARTIST key will be used for storing the album-artist.
Created attachment 330999 [details] [review] Patch Allow fetching artist and album-artist both from acoustID source.
Review of attachment 330999 [details] [review]: Also, I would really appreciate a new test case for this. I don't think we can test this with the 4 cases I've included in the past. Many thanks! ::: src/lua-factory/sources/grl-acoustid.lua @@ +32,3 @@ + "title", + "album", + "album_artist", should be album-artist @@ +37,3 @@ + "mb-album-id", + "mb-artist-id" + }, It is fine to have this table in one line for now @@ +133,3 @@ -- and for that reason we are only returning first of all metadata + if record and record.artists and #record.artists > 0 then + artist = record.artists[1] Please, let's do it in two patches. The first one is this fix here... @@ +141,3 @@ + if album and album.artists and #album.artists > 0 then + album_artist = album.artists[1] + media.album_artist = keys.album_artist and album_artist.name or nil and the second one is including the album_artist
Created attachment 331118 [details] [review] Fetch artist or recording instead of release-group Victor, Thanks for the review. I made the changes as you had suggested. And there should be no need for new tests for this change. The samples you had provided originally are just fine (i.e., their recording-artist and release-group artists are both same for all 4 test samples, so acoustID-test still returns 'OK'). If there's need for correction, let me know.
Can you add another test where the album artist and the artist would be different, just to make sure that the functionality is working as expected?
Created attachment 331137 [details] [review] Add test samples to verify the adjustments Alright here you go. This one sample has different 'album-artist' ('Various Artists' for the first releasegroup) from the recording artist ('Radiohead' for the first recording). Sorry if the metadata file got a bit too large, I couldn't help it. The original problem itself occurs for songs that have been reused in many different compilations. Please let me know if this addition is alright.
Review of attachment 331118 [details] [review]: This patch fixes the artist metadata-key, so, you don't need to include album-artist Besides that, patch is fine ::: src/lua-factory/sources/grl-acoustid.lua @@ +29,3 @@ name = "Acoustid", description = "a source that provides audio identification", + supported_keys = {"title", "album", "album-artist", "artist", "mb-recording-id", "mb-album-id", "mb-artist-id" }, For this patch, you don't need to change supported-keys.
Review of attachment 331137 [details] [review]: Please, include in the commit log that this test is needed to check album-artist != artist which should be more clear with album-artist validation (could be together with album-artist support)
Created attachment 331203 [details] [review] Fetch artist or recording instead of release-group Removed extra supported key
Created attachment 331204 [details] [review] Add test samples to verify the artist retrieval Edited commit log to include the sample's importance.
Review of attachment 331203 [details] [review]: Sure
Review of attachment 331204 [details] [review]: Looks good