GNOME Bugzilla – Bug 602840
Fails to correctly handle "Album Artist" on import
Last modified: 2010-07-09 20:16:47 UTC
On reimport the 'Album Artist' field shows 'Various Artists' for tracks that had the same value for 'Artist' and 'Album Artist' before the reimport. Example album Foo: 01 - Artist A - Track 01.mp3 02 - Artist B - Track 02.mp3 03 - Artist C - Track 03.mp3 After tagging 'Track', 'Artist', 'Album', and 'A' for 'Album Artist' the album overview looks like: Foo A (contains tracks from A, B, and C) After removing and reimporting the album I get: Foo A (contains tracks from B and C) Foo Various Artists (contains track from A) Not sure if writing the metadata or handling it during import is the problem. Banshee 1.6 Beta 4 (1.5.3) from the Ubuntu daily-live PPA (banshee 1.5.3+git20091122.r1.530cb5e-0ubuntu1).
Still an issue with version 1.6.0+git20100409.r1.094be72-0ubuntu1+karmic
I can reproduce this. It only happens (to me) when the TCMP flag is set. To reproduce this in a way which doesn't involve banshee too much: 1. Grab two mp3 files, called in this example 1.mp3 and 2.mp3 2. Clear all their id3 tags. I'm using mid3v2: mid3v2 --delete-all [12].mp3 3. Tag them as follows (banshee should interpret TPE2 as AlbumArtist): mid3v2 --album=AnAlbum --song=S1 --artist=A --track=1/2 --TPE2 A --TCMP 1 1.mp3 mid3v2 --album=AnAlbum --song=S2 --artist=B --track=2/2 --TPE2 A --TCMP 1 2.mp3 4. Import them to banshee via Import-->"Local Folder" The result of this is that banshee shows Track 1 as having AlbumArtist equal to "Various Artists". This is the incorrect behavior described in the bug. If you repeat the whole experiment but omit the tag "--TCMP 1" in step 3, then banshee imports the files correctly. However, if you later edit the metadata using banshee (e.g. change the album title), then banshee quietly adds the TCMP flag again; so when you remove the files from the library and re-import them, the problem appears.
I can confirm this in current git for Ogg Vorbis tracks as well, though for those it’s apparently using the iscompilation tag rather than an id3 tag.
Created attachment 164093 [details] [review] Potential patch to fix this bug This fixes the problem on my system. Can someone who is experiencing it with MP3 files or other files please test? I don't think it's necessarily the right long-term solution, but perhaps will shed some light on the problem for someone more familiar with Banshee's codebase.
Seems to work with MP3 files.
Thanks. For those reviewing the patch: The actual problem is created because the DatabaseTrackInfo.AlbumArtist property's set accessor uses CleanseString to try to make sure that the new value is not the same as the old value (comparing against TrackInfo.ArtistName). Unfortunately, the get accessor for TrackInfo.ArtistName will fall back to the track artist if the album is not marked as a compilation. Since the track artist is the same as the album artist, CleanseString thinks the old value is the same as the new value, so the new value doesn't get stored as the album artist. Then, a little bit later the isCompilation property is set, and thereafter it is possible to retrieve the actual AlbumArtist value (which, since it never got set, is still null and is therefore retrieved as "Various Artists"). Obviously this patch makes banshee set the isCompilation flag before trying to set the albumartist values. Alternatives: * Allow the set accessor to store the album artist even if it's the same as the old one. (probably bad since it would result in extra db writes.) * Change TrackInfo.AlbumArtist so that it doesn't treat the track artist as a fallback. (probably bad since it would mean that single-artist albums wouldn't get an albumartist.) * Reverse the roles of TrackInfo.AlbumArtist and TrackInfo: Modify ArtistName so that it retrieves the AlbumArtist if the ArtistName is otherwise null. (This has potential, I think?)
Review of attachment 164093 [details] [review]: Thanks Alex, looks good. Can you attach the patch outputted from `git format-patch HEAD~1` - eg with the commit msg, etc?
Created attachment 164465 [details] [review] Patch for this bug with Git info included Attached. I also added a comment in the code so that someone looking in future can recognize that the order of operations is important.
Review of attachment 164465 [details] [review]: Committed, thanks Alex!