GNOME Bugzilla – Bug 549166
Should transfer Cover Art in a configurable way
Last modified: 2008-09-12 21:09:16 UTC
It should be possible to transfer cover art to mass storage audio players that support it. I suggest to create three keys in .is_audio_player and add them to MediaCapabilities (and eventually submit a patch against the HAL specs). 1. CoverArtSize: Int32 to specify the height/width pixelsize 2. CoverArtFileType: To specify the image format 3. CoverArtFileName: To specify the filename that is supposed to be put into the album folder. I am working on a patch right now, please let me know if you have any input on this.
Created attachment 117326 [details] [review] Patch to implement cover art transfer to MassStorage DAPs I implemented the first version of this patch. CoverArtSize CoverArtFileName and CoverArtFileType are taken from .is_audio_player file but are also worked into the banshee HAL back-end and MediaCapabilities so that they can be retrieved from HAL once supported. Perhaps there should be a schema to implement a maximum size for the image in pixel as the variable now can basically be any Int32 value. I need some input on this, specifically where it makes sense to check the value against the schema value (I was thinking in MassStorageSource.CoverArtSize.get() and set() - therefore also correcting possible HAL values that conflict with that schema). Please let me know what you think.
Created attachment 118111 [details] [review] Patch to implement cover art transfer to MassStorage DAPs 0.2 I modified the patch to make it work with podcast cover art. Both, album art and podcast logos are now transfered according to what is configured in .is_audio_player. I decided against adding a schema for maximum album art size since I think it makes sense to trust HAL data if we submit a patch against HAL and it makes equal sense to trust the user to specify the right size for their devices. I think this patch is ready, let me know what you think...
This is looking good. I'd like to see a way to specify in the cover_art_size 'unlimited' - maybe 0 would work, and -1 or blank means not supported? I'd also like to see an optimization for jpegs (which all files in ~/.cache/album-art/ are) and cover_art_size = unlimited that just copies the files over using Banshee.IO.File.Copy. Also, please use track.ArtworkId instead of CoverArtSpec.CreateArtistAlbumId, and please cache the ArtworkService into a local variable to avoid doing the Get() call repeatedly.
Created attachment 118209 [details] [review] Patch to implement cover art transfer to MassStorage DAPs 0.3 Done. If cover_art_size is set to 0, banshee does not resize the cover art anymore. If on top of that the format is jpg (or jpeg, I also made that case insensitive), the cover is just copied using Banshee.IO.File.Copy. If the track is no podcast, I use track.ArtworkId now, else I assemble the coverart_id myself. This should probably be fixed in the Podcast module but I don't want this patch to be more invasive than it has to (the podcast module should set the artwork id correctly). The ArtworkService is now cached into a local variable.
The podcast extension is already setting the ArtworkId properly, AFAIK.
Great job on this patch, David. I've committed it. Thanks a lot! One tiny style note, we call variables "string foo" not "String foo", but use String everywhere else. I also changed the Podcast check to use TrackInfo.HasAttribute instead of just checking the genre (although in 99%+ of cases that should have been too). Another minor style note, we put spaces between method names and the arg list, eg foo () not foo() - though this isn't something we used to do, so you'll still see it the old way in some code. See HACKING for full details. Thanks again!
*** Bug 434030 has been marked as a duplicate of this bug. ***