On Sun, Feb 27, 2011 at 23:58, phantomjinx <firstname.lastname@example.org>
On 27/02/11 17:04, Javier Kohen wrote:
> I built HEAD from git and noticed that my iPod doesn't load in a
> reasonable time (I've waited tops one or two minutes). A git bisect
> identified the following commit 42ff0255bd1d6989dc67c7d924dabda904c95000
> Author: phantomjinx <email@example.com
> Date: Tue Feb 22 23:10:41 2011 +0000Yes. Totally confirmed and it looks like that patch was completely naive
> Fix embedded artwork not being loaded
> * Embedded artwork is initially detected and displayed. However,
> on quitting
> and restarting the artwork is no longer displayed.
> Without this patch the iPod is loaded in one or two seconds.
in trying to solve the bug.
The idea of the patch was the following:
1) init gtkpod
2) import itdbs
3) Load all apic (embedded) artwork for all tracks in each itdb
Further investigation this evening has shown the following:
a) APIC / embedded artwork for ipods is always loaded since the
itdb_parse(mountpoint) function is used so the above patch was pointless
b) APIC / embedded artwork for local itdbs is not loaded because it used
itdb_parse_file(filename) which has no access to a related Artwork
database (hence the original bug)
c) I am an idiot for not realizing this sooner!!
To fix embedded artwork for local itdbs requires loading the artwork for
each track. This is expensive for performance since it must read and
analyse each file.
A better way would be to save off an artwork database for local itdbs.
Christophe did suggest some time ago deprecating the idb_parse_file
function and this bug adds credence to this notion. It may be a whole
lot better to have local itdbs in the .gtkpod directory stored as
sub-directories, ie. fake mountpoints complete with all the required
files, including the Artwork database. This way a call to
itdb_parse(mountpoint) would be trivial and all artwork accounted for.
Obviously some migration would be required to convert existing local
itdbs to the new directory layout.
Is this workable? Do people think this is appropriate and whether this
should be done before releasing version 2?
Thanks for looking into this.
Version 2.0 has quite a lot of external changes and more under the hood. It sounds like it's at a good point for the fixing the last bugs and then releasing. Is this additional change you are discussing simple enough? Do you think it could make the jump from version 1 harder for users, or increase the likelihood that something will go wrong? I suspect it shouldn't go in this version, but you know better.