GNOME Bugzilla – Bug 555329
Ipod Coverart Size
Last modified: 2010-08-27 17:07:56 UTC
Please describe the problem: Ipod Coverart is taking a lot of space. Almost 1mb for each track. Right now i have 369 tracks on my ipod and 647MB of coverart and that's in a fresh restored ipod. Steps to reproduce: 1. copy songs to ipod 2. see that "Other" and the ipod's artwork folder is taking a lot of space. Actual results: Expected results: Does this happen every time? yes Other information: ipod nano 3rd generation banshee-svn (shows 1.3.2)
Hrm, I was just looking at the iPod artwork code the other day - it's probably b/c we set it for each format, so potentially multiple times per file (which makes sense - I doubt any of your album art is over 1 MB).
I have found a forum topic that can guide you guys: http://amarok.kde.org/forum/index.php?topic=15032.0
I'd like to add that coverflow becomes VERY SLOW and almost not usable due to the huge coverart database.
*** Bug 560883 has been marked as a duplicate of this bug. ***
Created attachment 122748 [details] A screenshot of the artwork folder A screenshot of the artwork folder
Created attachment 122749 [details] Ipod Music Folder
I'm experiencing the same thing to an extreme. I have 2.3GB of music on my iPod and my Artwork folder is a whopping 1.3GB! That is over half the size of the music itself! This is on my iPod Classic 120GB (Black).
Created attachment 124696 [details] [review] ipod-sharp-0.8.1-ArtworkFormat-equality.patch I think the problem here is two fold, one small issue and one big issue. First the small issue, this problem might be real or it might be my misunderstanding on the meaning of the == operator in C# (I'm a Java programmer for work and a C/C++ programmer for fun). LookupThumbnail in Photo.cs is testing equality on ArtworkFormat objects using the == operator. My understanding is that this checks whether the two objects are the same instance instead of checking if they are the same logical format. I've made and attached a patch to address this issue. As I said in my previous post, I had imported 2.3GB of music to my iPod and my Artwork folder was 1.3GB. Now, with this patch, after importing the exact same music to my clean iPod I have an Artwork folder about half the size at ~620MB. The second larger problem is the way iPod-Sharp handles the artwork database in comparison to how iTunes (and now libgpod) handles the database. It seems iTunes only stores album art for one or two tracks on an album and reuses this art for all tracks. The bug report for this same (solved) issue for libgpod can be found here: http://sourceforge.net/tracker/index.php?func=detail&aid=1882327&group_id=67873&atid=519273
It looks like #559923 is a duplicate of this bug.
I can confirm this issue as well on a 8GB nano 4g. The cover art takes up almost 2GB when having around 4GB of music.
*** Bug 559923 has been marked as a duplicate of this bug. ***
Thanks for the patch Nicholas, I look forward to one for the second problem. :)
I'm using Banshee 1.4.2 on Ubuntu Hardy. Another aspect of this bug, apparently, is that the iPod database is not saved in certain cases, resulting in albums not getting synced to the iPod. I first noticed this after syncing my music library with my iPod classic, when Banshee indicated first that all music was synced (the number of songs in my library was the same as the number on the iPod), but then, after ejecting the iPod and reconnecting, the last four artists in my alphabetical list in the library (and all of their albums) did not show up on the iPod. Restarting Banshee in debugging mode, and dragging one of these albums from my library to the iPod resulted in the usual "Transcoding file" messages (from FLAC to mp3), and then: [Warn 21:37:14.608] Failed to save iPod database - Argument is out of range. Parameter name: Length is less than 0 (in `mscorlib') at System.IO.FileStream.SetLength (Int64 length) [0x00000] at IPod.PhotoDatabase.SaveThumbnails (System.Collections.Generic.List`1 existingNames, System.Collections.Generic.List`1 newNames, System.Collections.Generic.List`1 removedNames, IPod.ArtworkFormat format) [0x001ca] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1401 at IPod.PhotoDatabase.SaveThumbnails () [0x0009d] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1308 at IPod.PhotoDatabase.Save () [0x0003a] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1101 Failed to save database (in `ipod-sharp') at IPod.PhotoDatabase.Save () [0x001ab] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1137 at IPod.TrackDatabase.Save () [0x0026c] in /build/buildd/ipod-sharp-0.8.2/src/TrackDatabase.cs:2566 Failed to save database (in `ipod-sharp') at IPod.TrackDatabase.Save () [0x00391] in /build/buildd/ipod-sharp-0.8.2/src/TrackDatabase.cs:2629 at IPod.Device.Save () [0x0000b] in /build/buildd/ipod-sharp-0.8.2/src/Device.cs:282 at Banshee.Dap.Ipod.IpodSource.PerformSyncThreadCycle () [0x002dc] in /build/buildd/banshee-1.4.2/src/Dap/Banshee.Dap.Ipod/Banshee.Dap.Ipod/IpodSource.cs:629 [Warn 21:37:20.728] Failed to save iPod database - Object reference not set to an instance of an object (in `ipod-sharp') at IPod.ImageNameRecord.SetData (System.IO.Stream stream, System.Byte[] data, Int32 offset) [0x00020] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:748 at IPod.ImageNameRecord.SetData (System.IO.Stream stream, System.Byte[] data) [0x00000] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:738 at IPod.PhotoDatabase.SaveThumbnails (System.Collections.Generic.List`1 existingNames, System.Collections.Generic.List`1 newNames, System.Collections.Generic.List`1 removedNames, IPod.ArtworkFormat format) [0x00142] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1387 at IPod.PhotoDatabase.SaveThumbnails () [0x0009d] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1308 at IPod.PhotoDatabase.Save () [0x0003a] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1101 Failed to save database (in `ipod-sharp') at IPod.PhotoDatabase.Save () [0x001ab] in /build/buildd/ipod-sharp-0.8.2/src/PhotoDatabase.cs:1137 at IPod.TrackDatabase.Save () [0x0026c] in /build/buildd/ipod-sharp-0.8.2/src/TrackDatabase.cs:2566 Failed to save database (in `ipod-sharp') at IPod.TrackDatabase.Save () [0x00391] in /build/buildd/ipod-sharp-0.8.2/src/TrackDatabase.cs:2629 at IPod.Device.Save () [0x0000b] in /build/buildd/ipod-sharp-0.8.2/src/Device.cs:282 at Banshee.Dap.Ipod.IpodSource.PerformSyncThreadCycle () [0x002dc] in /build/buildd/banshee-1.4.2/src/Dap/Banshee.Dap.Ipod/Banshee.Dap.Ipod/IpodSource.cs:629 The iPod itself registers a large "Other" section (in Settings -> About) and only 670 songs. My library contains 946, and the iPod seems to have many more songs stored on it: doing ls {{mount point}}/iPod_Control/Music/F??/*.mp3 | wc finds 1357 mp3 files.
twilson, that looks like a separate issue to me. Nicholas, did you mean to point him at a different bug?
I actually did mean to point him here. The length variable (for the file length of the photo database) is overflowing and becoming negative after the coverart DB grows past ~2GB (it grows this big due to this bug). This would work if the artwork DB did the following: 1. Split the DB in to multiple files like iTunes or libgpod. (a good metric would be ~256MB or so) 2. Fix problem 2 from my comment #8. It seems that, if the iPod supports it, each track can have some sort of coverart ID that points in to the artwork DB to get the appropriate coverart (this can avoid 'a lot' of duplication). Right now the coverart DB is indexed by track index meaning that the coverart is duplicated for every track.
Still not working although the svn logs for ipod-sharp claim there's a fix. Banshee SVN revision 5209 ipod-sharp SVN revision 132081 I'm getting up to 5Gb of Artwork for 7000 songs
The patch from comment #8 is in SVN and was listed in the changelog but that is only one very small problem fixed. The big major problem that still exists is with the second problem listed in comment #8. There is no fix made for that yet and that is what is making the /massive/ coverart databases.
Gabriel: I'd say you're right. Twilson: that seems like bug 564503 or 578701 (which have a patch that you can try).
I am going to have to disagree with comment #18 and say that this bug and Twilson's problem are separate from both bug 564503 and bug 578701. I added some explanation in comment 28 of bug 578701.
Also, I don't want to sound like a pest but I think the severity and/or priority for this bug should be raised because (to recap) the bug has these implications for iPod users: - Album art databases are excessively large. They are easily larger than one third the size of the music on the actual iPod. For users with iPods with smaller capacities this drastically effects the amount of music that can be stored on the device. The album art database gets large because album art data is getting duplicated for every track of an album instead of being stored once per album. - Users with (what I would estimate to be) iPods with 8GBs of capacity or higher will encounter the problem in comment #13 after loading enough music. This problem effectively blocks users from adding any more (usable) music to their iPod because both the photo database and the music database fail to get written after any file in the photo database exceeds 2GB (which happens quickly). Once any file in the photo database exceeds 2GB, a variable (signed 32bit integer) that stores the size of the file will overflow and become negative causing "Failed to save iPod database - Argument is out of range. Parameter name: Length is less than 0 (in `mscorlib')". This means that anyone with an iPod of capacity ~8GB or higher is unable to use any of that extra capacity.
(In reply to comment #19) > I am going to have to disagree with comment #18 and say that this bug and > Twilson's problem are separate from both bug 564503 and bug 578701. > > I added some explanation in comment 28 of bug 578701. Ok, but at least we all agree that Twilson's problem is not related to *this* bug :) (In reply to comment #20) > Once any file in the photo database exceeds 2GB, a variable (signed > 32bit integer) that stores the size of the file will overflow and become > negative causing "Failed to save iPod database - Argument is out of range. > Parameter name: Length is less than 0 (in `mscorlib')". And another bug could be created to request that ipod-sharp detects this gracefully, instead of throwing that weird exception, in order to make Banshee able of at least showing more information about the problem to the user.
I've extracted bug 584860 from this bug to address the 'filesize > 2GB' problem encountered in this bug. This bug can be about fixing the tremendous amounts of duplication in the album art database that grows it uncontrollably fast and causes bug 584860 to rear its ugly head.
I've started idly working on this in free time. I might be able to produce something in the nearish future depending on how busy and how motivated I am. No guarantees though.
*** Bug 578701 has been marked as a duplicate of this bug. ***
Created attachment 151231 [details] [review] Sparse artwork support for ipod-sharp Here's a rough patch adding sparse artwork support that I would like some feedback on. It works by simply looking for covers that are the same and share an album title, and if so then the covers are shared. A few things I'm thinking: 1) Currently banshee does not set full size covers, only thumbnails. Since ipods support multiple formats, with different models using different ones, we have to kinda guess which thumbnail format we want to use to compare. I tried simply using the first supported cover and comparing that--which should be set based on my quick glance at Banshee.Dap.Ipod--but that didn't work so I wound up needing to use the first supported and actually used cover. This seems a bit messy. Any thoughts on a way around this? 2) Updating a shared cover on a track will change it for all tracks referencing that cover. (Hence the album art name check.) This includes deleting. It would be easy to fix/improve by having it duplicate the cover on Set operations and only unreference it on Remove so then no other tracks are changed. Does that sound like the better thing to do? If this is what winds up happening then there really is no reason to do the Album check. 3) Currently without sparse artwork the covers are removed immediately, whereas with they are removed on save. It would probably be best to make it so they are all removed on save to reduce duplication and simplify things. Thanks for any feedback.
Review of attachment 151231 [details] [review]: Looking good, this will be extremely appreciated by a lot of people. ::: src/Utility.cs @@ +88,3 @@ + + public static bool Equals(byte[] a, byte[] b) { + if (a.Length != b.Length) maybe check for null too, to be safe? @@ +99,3 @@ + } + + public static int Sum(byte[] a) { it's pretty easy to overflow this, no? ::: src/TrackDatabase.cs @@ +2727,3 @@ + ArtworkFormat comparisonFormat = null; + + Dictionary<int, List<Track>> artworkLookup = new Dictionary<int, List<Track>> (); feel free to use 'var' to make this more readable @@ +2742,3 @@ + } + + if (comparisonFormat == null || track.GetCoverArt(comparisonFormat) == null) you end up calling GetCoverArt (comparisonFormat) (and the successful call with 'format' in the loop above) quite a few times. Is it trivially cheap, or should we be caching the result of it? @@ +2752,3 @@ + { + foreach (Track matchingTrack in possibleMatches) + { be more consistent with putting the brace for if/foreach/else etc either on newline or on same - I think on same is better, but I don't remember if that's the ipod-sharp default @@ +2762,3 @@ + // check to see if thumb and album name are the same + if (Utility.Equals(track.GetCoverArt(comparisonFormat), matchingTrack.GetCoverArt(comparisonFormat)) && + track.Album.Equals(matchingTrack.Album)) Since the track.Album comparison is probably quite a bit shorter/cheaper, put it first @@ +2789,3 @@ + if (!usedArtwork.ContainsKey(photo.Id)) + artdb.RemovePhoto(photo); + } Have you done any performance testing on this? The two loops over all the tracks, and all the List/Dictionaries created worry me a little bit.
(In reply to comment #25) > 1) Currently banshee does not set full size covers, only thumbnails. Since > ipods support multiple formats, with different models using different ones, we > have to kinda guess which thumbnail format we want to use to compare. I tried > simply using the first supported cover and comparing that--which should be set > based on my quick glance at Banshee.Dap.Ipod--but that didn't work so I wound > up needing to use the first supported and actually used cover. This seems a bit > messy. Any thoughts on a way around this? I think it's fine. > 2) Updating a shared cover on a track will change it for all tracks referencing > that cover. (Hence the album art name check.) This includes deleting. It would > be easy to fix/improve by having it duplicate the cover on Set operations and > only unreference it on Remove so then no other tracks are changed. Does that > sound like the better thing to do? If this is what winds up happening then > there really is no reason to do the Album check. You're saying that with your patch, if you delete one track on a album from the ipod, it will remove the (shared) cover art for the album? Sounds like the unref-on-remove is the way to go. Any downsides to that? > 3) Currently without sparse artwork the covers are removed immediately, whereas > with they are removed on save. It would probably be best to make it so they are > all removed on save to reduce duplication and simplify things. Yeah. As-is it won't cause any problems, right?
Created attachment 151582 [details] [review] Sparse artwork property for podsleuth Export the sparse cover art property
> You're saying that with your patch, if you delete one track on a album from the > ipod, it will remove the (shared) cover art for the album? Sounds like the > unref-on-remove is the way to go. Any downsides to that? Right. The only real downside is that making it consistent, so modifying the cover also does not change the whole album, is a bit weird. I think I know a way to make it fine though, so I'll describe that when I resubmit. > > 3) Currently without sparse artwork the covers are removed immediately, whereas > > with they are removed on save. It would probably be best to make it so they are > > all removed on save to reduce duplication and simplify things. > > Yeah. As-is it won't cause any problems, right? Nah, no problem. Nothing actually happens until save, so it winds up being effectively the same either way aside from the duplication in the code.
Created attachment 151696 [details] [review] Sparse artwork support for ipod-sharp Okay, this should address the issues from your review, Gabriel. About the only thing left is the inconsistent formatting, and with that I'm just trying to match surrounding style (which differs between files, ack). Let me know if you would rather have it consistent within the patch. The bottleneck seems to be disk IO since it has to read every thumbnail to hash, so the second loop just going over the db in memory is fine. To make the first loop faster I fixed the thumbnail format selection to try to pick the smallest one. For me this makes it take ~10 seconds, maybe a little less. (The smaller thumb file at 200MB divided by 20MB per sec, approximately.) Also, it says this in the comments but I'll note it here as well: In SetCoverArt I assume that no one is going to update one of the thumbnail formats on a track but not others. (Banshee loops over all formats and sets them, so it's fine.) This allows SetCoverArt to work as expected and not change all tracks sharing the same cover.
Review of attachment 151696 [details] [review]: Thanks Andy, getting close. ::: src/Utility.cs @@ +105,3 @@ + int sum = 0; + foreach (byte b in a) { + unchecked { I don't like this 'unchecked' - if this overflows, it's will make this function return an unexpected result. ::: src/TrackDatabase.cs @@ +2725,3 @@ + // Optimize for sparse artwork if supported (use smallest thumbnail for speed) + var formats = new List<ArtworkFormat> (device.ArtworkFormats); + formats = formats.FindAll(delegate(ArtworkFormat a) { return a.Usage == ArtworkUsage.Cover; }); replace this with the sleeker "FindAll(a => a.Usage == ArtworkUsage.Cover)" @@ +2726,3 @@ + var formats = new List<ArtworkFormat> (device.ArtworkFormats); + formats = formats.FindAll(delegate(ArtworkFormat a) { return a.Usage == ArtworkUsage.Cover; }); + formats.Sort(delegate(ArtworkFormat a, ArtworkFormat b) { return (a.Width * a.Height).CompareTo(b.Width * b.Height); }); same idea here @@ +2738,3 @@ + byte[] coverArt = track.GetCoverArt(comparisonFormat); + + if (comparisonFormat == null || coverArt == null) if comparisonFOrmat was null, no reason to check it inside this loop and continue for each one - check it before this foreach @@ +2741,3 @@ + continue; + + int hash = Utility.Sum(coverArt); The artwork could be quite big and easily overflow an int or a long. Instead of this, why not actually calculate a real hash? Create and MD5 instance outside of this method (or even a static one) with System.Security.Cryptography.MD5.Create (); and then use its md5.ComputeHash method.
Review of attachment 151582 [details] [review]: Thanks, committed.
> ::: src/Utility.cs > @@ +105,3 @@ > + int sum = 0; > + foreach (byte b in a) { > + unchecked { > > I don't like this 'unchecked' - if this overflows, it's will make this function > return an unexpected result. > > @@ +2741,3 @@ > + continue; > + > + int hash = Utility.Sum(coverArt); > > The artwork could be quite big and easily overflow an int or a long. Instead > of this, why not actually calculate a real hash? > > Create and MD5 instance outside of this method (or even a static one) with > System.Security.Cryptography.MD5.Create (); > > and then use its md5.ComputeHash method. Shouldn't unchecked overflow just truncate? I avoided a cypto hash because it is overkill. We'd be doing lots of extra computation, but it wouldn't get us anything. All the hashing function needs to do is take an input and always generate the same output, and be reasonably distributed when we give it different inputs. Not that summing is necessarily the best answer, but it's quick and easy.
Granted, MD5 takes all of something like 5 cycles/byte when implemented well, and really this is disk bound so who is counting.
Created attachment 152372 [details] [review] Sparse artwork support for ipod-sharp From irc this now just compares album artist and album strings to see if art should be shared. Concatenating the two together with a pipe for a key seems a bit hacky--though it is also used earlier in the file--but then again creating a class with a GetHashCode seems overly verbose and complicated for such a simple thing.
I tested out the patches in this bug. I'm running banshee 1.5.6-1~hyper1~karmic on Ubuntu 9.10 x86_64. I have an iPod classic 6th gen, 120GB. I patched podsleuth-0.6.5 with the sparse artwork property patch (id 151582). I patched ipod-sharp-0.8.5 with the sparse artwork patch (id 152372). I skipped the artworkformat-equality patch, since it looked like it had already been incorporated. I built both podsleuth and ipod-sharp and installed them. I added 10174 songs - about 80 GB - to the iPod using banshee. The good news is that all songs got added. The bad news is that only about half of the album artwork got added. About 15% of the MP3s have artwork that I've manually added. Most of this artwork made it onto the device, but not all. The missing artwork is represented by a solid black square. During the sync operation, I got an alert that my /tmp filesystem was full. I don't know if this means this particular bug is not quite fixed, or if this is another bug. I don't know of an option in banshee to specify a temp directory to be used during iPod sync operations. I did not run banshee from a terminal, so I don't have any error output at this time.
I forgot to mention in my last update the artwork database size on the device after the update: $ ls -l /media/IPOD/iPod_Control/Artwork/ total 5528224 -rwx------ 1 steve root 4537096 2010-03-26 14:19 ArtworkDB -rwx------ 1 steve root 1355358208 2010-03-26 14:19 F1055_1.ithmb -rwx------ 1 steve root 1927151616 2010-03-26 14:19 F1060_1.ithmb -rwx------ 1 steve root 1133965568 2010-03-26 14:19 F1061_1.ithmb -rwx------ 1 steve root 1239862400 2010-03-26 14:19 F1068_1.ithmb $ ls -lh /media/IPOD/iPod_Control/Artwork/ total 5.3G -rwx------ 1 steve root 4.4M 2010-03-26 14:19 ArtworkDB -rwx------ 1 steve root 1.3G 2010-03-26 14:19 F1055_1.ithmb -rwx------ 1 steve root 1.8G 2010-03-26 14:19 F1060_1.ithmb -rwx------ 1 steve root 1.1G 2010-03-26 14:19 F1061_1.ithmb -rwx------ 1 steve root 1.2G 2010-03-26 14:19 F1068_1.ithmb
(In reply to comment #36) > During the sync operation, I got an alert that my /tmp filesystem > was full. I don't know if this means this particular bug is not > quite fixed, or if this is another bug. I don't know of an option > in banshee to specify a temp directory to be used during iPod sync > operations. I did not run banshee from a terminal, so I don't have > any error output at this time. Banshee not handling tmp running out of space well is another bug (#608530). Running into that does make it hard to tell if this fix is working though, as the best check would be to compare artdb size before and after (as well as ensuring the artwork is correct). Ultimately you may just need to free more tmp space up, as the proper fix for the other bug is likely to error out of the sync.
> Banshee not handling tmp running out of space well is another bug (#608530). Erm... bug 608530 for the autohappy linking.
(In reply to comment #35) > Created an attachment (id=152372) [details] [review] > Sparse artwork support for ipod-sharp > > From irc this now just compares album artist and album strings to see if art > should be shared. Concatenating the two together with a pipe for a key seems a > bit hacky--though it is also used earlier in the file--but then again creating > a class with a GetHashCode seems overly verbose and complicated for such a > simple thing. I just tested out this patch and I'm getting a boatload of warnings: http://pastebin.ubuntu.com/440866/ It seems that NO cover art at all is transferred to my iPod. Personally, I'm not complaining since I save a lot of space that way but I guess that wasn't what you tried to do :)
By the way, the exception mentioned in comment 13 above seems to have disappeared. I should also mention that I have switched from just dragging individual tracks onto to iPod to synchronizing playlists. Not sure if that is related in any way.
(regarding comment 40) I've managed to get more detailed exceptions, now with references to the exact line where things seem to go wrong. An example: [8 Warn 16:48:47.503] Failed to set cover art on iPod from /home/mp/.cache/media-art/album-4ffb7e8db864d2a9647e6d4897645ec6.jpg - System.NullReferenceException: Object reference not set to an instance of an object (in `ipod-sharp') at IPod.Track.SetCoverArt (IPod.ArtworkFormat format, System.Byte[] data) [0x00000] in /home/mp/ipod-sharp/src/Track.cs:429 at Banshee.Dap.Ipod.IpodTrackInfo.SetIpodCoverArt (IPod.Device device, IPod.Track track, System.String path) [0x00034] in /home/mp/banshee/src/Dap/Banshee.Dap.Ipod/Banshee.Dap.Ipod/IpodTrackInfo.cs:285 The others are pretty much identical except for the 'album-xxxxxxxxxxx.jpg' which seem to be different for each album.
We're moving to a libgpod based ipod extension soon. Closing this as obsolete in light of that -- artwork should work much better with it.
You should reopen this bug. Last git revisions of banshee and libgpod-sharp still have not solved this issue for me. Artwork is not displayed on my ipod touch (3.1.3 1st version JB)
See bug #627945 (FYI). Thank You lorenzo
lorenzo, are you sure the issue that you're experiencing is related to this bug? This bug report is about enormous amounts of space on iPods being used to store cover art, and as Gabriel said, it should be obsolete with libgpod.
Well Michael I really don't know....and yes you're right this bug seems to be different from the one I've reported. By the way I'd really love to say this bug has been solved and obsolete but, unfortunately, I cannot verify :-)Thanks for you answer