Re: [Gtkpod-devel] IPod never loaded after "Fix embedded artwork not being loaded"

[Javier K. <jk...@gm...>]

On Sun, Feb 27, 2011 at 23:58, phantomjinx <p.g...@ph...
> wrote:

> 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 <p.g...@ph...
> > <mailto:p.g...@ph...>>
> > Date:   Tue Feb 22 23:10:41 2011 +0000
> >
> >     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.
> >
>
> Yes. Totally confirmed and it looks like that patch was completely naive
> 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
> for ipods
> 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.