Library and GUI for Apple's iPod

Hi Andrew,
> 
> I've made a proof of concept patch using gdbm.   It speeds up "sync" and 
> "add".  A test simple test shows uncached sync takes 2 mins and 30 seconds.  
> With the patch it takes 8 seconds.  
Wow! Thats one hell of an improvement! Very nice... ;)
> 
> Anyone like it?  Comment?  Ideas?  gdbm seems safe, but I'm not a gtk 
> programmer (kde guy) so maybe there's some gtk way to do it?  I haven't done 
> the clean up code and a few other things.  I'm just looking for feedback.  
> You have to link with -lgdbm
At first glance, I like this a lot. I'm not that familiar with gdbm
(didn't know it existed until now) but I think using a database like
this is a great idea. AFAIK gtk doesn't have anything like this, but I'm
relatively new to gtk, so I could be wrong. But, what I think you should
do before trying to get this into CVS is to add a configure check for
gdbm if you can, so we don't break gtkpod compiles on systems that don't
have gdbm. 
> 
> Patch:
> 
> Index: mp3file.c
> ===================================================================
> RCS file: /cvsroot/gtkpod/gtkpod/src/mp3file.c,v
> retrieving revision 1.58
> diff -u -r1.58 mp3file.c
> --- mp3file.c   23 Nov 2005 04:16:12 -0000      1.58
> +++ mp3file.c   27 Jan 2006 22:46:01 -0000
> @@ -113,11 +113,14 @@
>  #include <sys/types.h>
>  #include <unistd.h>
> 
> +#include <gdbm.h>
> +
>  #include "mp3file.h"
>  #include "charset.h"
>  #include "itdb.h"
>  #include "file.h"
>  #include "misc.h"
> +#include "prefs.h"
> 
> 
>  /* MIN_CONSEC_GOOD_FRAMES defines how many consecutive valid MP3 frames
> @@ -156,6 +159,8 @@
>      gint milliseconds;
>      gint frames;
>      gint badframes;
> +    time_t modtime;
> +    time_t lastseen;
>  } MP3Info;
> 
>  /* This is for xmms code */
> @@ -349,10 +354,39 @@
>    MP3Header header;
>    struct stat filestat;
>    off_t data_start=0;
> -
> +  GDBM_FILE dbf;
> +  datum key, data;
> +  gchar *db_file=NULL;
> 
>    stat(mp3->filename,&filestat);
>    mp3->datasize=filestat.st_size;
> +  mp3->modtime = filestat.st_mtime;
> +  mp3->lastseen = time(NULL);
> +
> +  db_file = g_build_filename(prefs_get_cfgdir(), "mp3_data.db", NULL);
> +
> +  dbf = gdbm_open(db_file, 0, GDBM_WRCREAT, 0600, 0);
> +  if(dbf){
> +
> +      key.dsize = strlen(mp3->filename) + 1;
> +      key.dptr = mp3->filename;
> +      data = gdbm_fetch(dbf, key);
> +      if(data.dsize == sizeof(MP3Info)){
> +         MP3Info *cmp3 = (MP3Info *)data.dptr;
> +         if(cmp3->modtime == mp3->modtime){
> +             memcpy(mp3, cmp3, sizeof(MP3Info));
> +             cmp3->lastseen = time(NULL);
> +             gdbm_store(dbf, key, data, GDBM_REPLACE);
> +             free(data.dptr);
> +             gdbm_close(dbf);
> +              g_free(db_file);
> +              return;
> +         }
> +      }
> +
> +  }
> +
> +
> 
>    if(get_first_header(mp3,0L)) {
>        data_start=ftell(mp3->file);
> @@ -384,6 +418,17 @@
>           mp3->vbr=1;
>        }
>    }
> +
> +  if(dbf){
> +      key.dsize = strlen(mp3->filename) + 1;
> +      key.dptr = mp3->filename;
> +      data.dsize = sizeof(MP3Info);
> +      data.dptr = (char *)mp3;
> +      gdbm_store(dbf, key, data, GDBM_REPLACE);
> +      gdbm_close(dbf);
> +  }
> +  g_free(db_file);
> +
>  }
> 
> 
> ---
> Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
> EE, LongEz N87KJ             |                           -- Walt Disney
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Gtkpod-devel mailing list
> Gtkpod-devel@...
> https://lists.sourceforge.net/lists/listinfo/gtkpod-devel

Hi James, hi Andrew,

why not just storing modification date in the "extended database"
(iTunesDB.ext)? Filesize and last access time are already available in the
iTunesDB (size, time_added).

Then, instead of using the MD5 checksum to determine whether a change has
taken place, filesize, access time and modification date would be used.

No real need for gdbm I think.

Cheers,


JCS.

On Fri, Jan 27, 2006 at 05:22:16PM -0800, James Liggett wrote:
> Hi Andrew,
> > 
> > I've made a proof of concept patch using gdbm.   It speeds up "sync" and 
> > "add".  A test simple test shows uncached sync takes 2 mins and 30 seconds.  
> > With the patch it takes 8 seconds.  
> Wow! Thats one hell of an improvement! Very nice... ;)
> > 
> > Anyone like it?  Comment?  Ideas?  gdbm seems safe, but I'm not a gtk 
> > programmer (kde guy) so maybe there's some gtk way to do it?  I haven't done 
> > the clean up code and a few other things.  I'm just looking for feedback.  
> > You have to link with -lgdbm
> At first glance, I like this a lot. I'm not that familiar with gdbm
> (didn't know it existed until now) but I think using a database like
> this is a great idea. AFAIK gtk doesn't have anything like this, but I'm
> relatively new to gtk, so I could be wrong. But, what I think you should
> do before trying to get this into CVS is to add a configure check for
> gdbm if you can, so we don't break gtkpod compiles on systems that don't
> have gdbm. 
> > 
> > Patch:
> > 
> > Index: mp3file.c
> > ===================================================================
> > RCS file: /cvsroot/gtkpod/gtkpod/src/mp3file.c,v
> > retrieving revision 1.58
> > diff -u -r1.58 mp3file.c
> > --- mp3file.c   23 Nov 2005 04:16:12 -0000      1.58
> > +++ mp3file.c   27 Jan 2006 22:46:01 -0000
> > @@ -113,11 +113,14 @@
> >  #include <sys/types.h>
> >  #include <unistd.h>
> > 
> > +#include <gdbm.h>
> > +
> >  #include "mp3file.h"
> >  #include "charset.h"
> >  #include "itdb.h"
> >  #include "file.h"
> >  #include "misc.h"
> > +#include "prefs.h"
> > 
> > 
> >  /* MIN_CONSEC_GOOD_FRAMES defines how many consecutive valid MP3 frames
> > @@ -156,6 +159,8 @@
> >      gint milliseconds;
> >      gint frames;
> >      gint badframes;
> > +    time_t modtime;
> > +    time_t lastseen;
> >  } MP3Info;
> > 
> >  /* This is for xmms code */
> > @@ -349,10 +354,39 @@
> >    MP3Header header;
> >    struct stat filestat;
> >    off_t data_start=0;
> > -
> > +  GDBM_FILE dbf;
> > +  datum key, data;
> > +  gchar *db_file=NULL;
> > 
> >    stat(mp3->filename,&filestat);
> >    mp3->datasize=filestat.st_size;
> > +  mp3->modtime = filestat.st_mtime;
> > +  mp3->lastseen = time(NULL);
> > +
> > +  db_file = g_build_filename(prefs_get_cfgdir(), "mp3_data.db", NULL);
> > +
> > +  dbf = gdbm_open(db_file, 0, GDBM_WRCREAT, 0600, 0);
> > +  if(dbf){
> > +
> > +      key.dsize = strlen(mp3->filename) + 1;
> > +      key.dptr = mp3->filename;
> > +      data = gdbm_fetch(dbf, key);
> > +      if(data.dsize == sizeof(MP3Info)){
> > +         MP3Info *cmp3 = (MP3Info *)data.dptr;
> > +         if(cmp3->modtime == mp3->modtime){
> > +             memcpy(mp3, cmp3, sizeof(MP3Info));
> > +             cmp3->lastseen = time(NULL);
> > +             gdbm_store(dbf, key, data, GDBM_REPLACE);
> > +             free(data.dptr);
> > +             gdbm_close(dbf);
> > +              g_free(db_file);
> > +              return;
> > +         }
> > +      }
> > +
> > +  }
> > +
> > +
> > 
> >    if(get_first_header(mp3,0L)) {
> >        data_start=ftell(mp3->file);
> > @@ -384,6 +418,17 @@
> >           mp3->vbr=1;
> >        }
> >    }
> > +
> > +  if(dbf){
> > +      key.dsize = strlen(mp3->filename) + 1;
> > +      key.dptr = mp3->filename;
> > +      data.dsize = sizeof(MP3Info);
> > +      data.dptr = (char *)mp3;
> > +      gdbm_store(dbf, key, data, GDBM_REPLACE);
> > +      gdbm_close(dbf);
> > +  }
> > +  g_free(db_file);
> > +
> >  }
> > 
> > 
> > ---
> > Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
> > EE, LongEz N87KJ             |                           -- Walt Disney
> > 
> > 
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> > for problems?  Stop!  Download the new AJAX search engine that makes
> > searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> > _______________________________________________
> > Gtkpod-devel mailing list
> > Gtkpod-devel@...
> > https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
> 
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Gtkpod-devel mailing list
> Gtkpod-devel@...
> https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
>

Hi Jorg,
On Sat, 2006-01-28 at 11:09 +0900, Jorg Schuler wrote:
> Hi James, hi Andrew,
> 
> why not just storing modification date in the "extended database"
> (iTunesDB.ext)? Filesize and last access time are already available in the
> iTunesDB (size, time_added).
> 
> Then, instead of using the MD5 checksum to determine whether a change has
> taken place, filesize, access time and modification date would be used.
That's not a bad idea either. Maybe we should try it? If we can get
results like what Andrew does, that would be neat. And if we could do it
without additional dependencies that would be even better. 
> 
> No real need for gdbm I think.
> 
> Cheers,
> 
> 
> JCS.
> 
> On Fri, Jan 27, 2006 at 05:22:16PM -0800, James Liggett wrote:
> > Hi Andrew,
> > > 
> > > I've made a proof of concept patch using gdbm.   It speeds up "sync" and 
> > > "add".  A test simple test shows uncached sync takes 2 mins and 30 seconds.  
> > > With the patch it takes 8 seconds.  
> > Wow! Thats one hell of an improvement! Very nice... ;)
> > > 
> > > Anyone like it?  Comment?  Ideas?  gdbm seems safe, but I'm not a gtk 
> > > programmer (kde guy) so maybe there's some gtk way to do it?  I haven't done 
> > > the clean up code and a few other things.  I'm just looking for feedback.  
> > > You have to link with -lgdbm
> > At first glance, I like this a lot. I'm not that familiar with gdbm
> > (didn't know it existed until now) but I think using a database like
> > this is a great idea. AFAIK gtk doesn't have anything like this, but I'm
> > relatively new to gtk, so I could be wrong. But, what I think you should
> > do before trying to get this into CVS is to add a configure check for
> > gdbm if you can, so we don't break gtkpod compiles on systems that don't
> > have gdbm. 
> > > 
> > > Patch:
> > > 
> > > Index: mp3file.c
> > > ===================================================================
> > > RCS file: /cvsroot/gtkpod/gtkpod/src/mp3file.c,v
> > > retrieving revision 1.58
> > > diff -u -r1.58 mp3file.c
> > > --- mp3file.c   23 Nov 2005 04:16:12 -0000      1.58
> > > +++ mp3file.c   27 Jan 2006 22:46:01 -0000
> > > @@ -113,11 +113,14 @@
> > >  #include <sys/types.h>
> > >  #include <unistd.h>
> > > 
> > > +#include <gdbm.h>
> > > +
> > >  #include "mp3file.h"
> > >  #include "charset.h"
> > >  #include "itdb.h"
> > >  #include "file.h"
> > >  #include "misc.h"
> > > +#include "prefs.h"
> > > 
> > > 
> > >  /* MIN_CONSEC_GOOD_FRAMES defines how many consecutive valid MP3 frames
> > > @@ -156,6 +159,8 @@
> > >      gint milliseconds;
> > >      gint frames;
> > >      gint badframes;
> > > +    time_t modtime;
> > > +    time_t lastseen;
> > >  } MP3Info;
> > > 
> > >  /* This is for xmms code */
> > > @@ -349,10 +354,39 @@
> > >    MP3Header header;
> > >    struct stat filestat;
> > >    off_t data_start=0;
> > > -
> > > +  GDBM_FILE dbf;
> > > +  datum key, data;
> > > +  gchar *db_file=NULL;
> > > 
> > >    stat(mp3->filename,&filestat);
> > >    mp3->datasize=filestat.st_size;
> > > +  mp3->modtime = filestat.st_mtime;
> > > +  mp3->lastseen = time(NULL);
> > > +
> > > +  db_file = g_build_filename(prefs_get_cfgdir(), "mp3_data.db", NULL);
> > > +
> > > +  dbf = gdbm_open(db_file, 0, GDBM_WRCREAT, 0600, 0);
> > > +  if(dbf){
> > > +
> > > +      key.dsize = strlen(mp3->filename) + 1;
> > > +      key.dptr = mp3->filename;
> > > +      data = gdbm_fetch(dbf, key);
> > > +      if(data.dsize == sizeof(MP3Info)){
> > > +         MP3Info *cmp3 = (MP3Info *)data.dptr;
> > > +         if(cmp3->modtime == mp3->modtime){
> > > +             memcpy(mp3, cmp3, sizeof(MP3Info));
> > > +             cmp3->lastseen = time(NULL);
> > > +             gdbm_store(dbf, key, data, GDBM_REPLACE);
> > > +             free(data.dptr);
> > > +             gdbm_close(dbf);
> > > +              g_free(db_file);
> > > +              return;
> > > +         }
> > > +      }
> > > +
> > > +  }
> > > +
> > > +
> > > 
> > >    if(get_first_header(mp3,0L)) {
> > >        data_start=ftell(mp3->file);
> > > @@ -384,6 +418,17 @@
> > >           mp3->vbr=1;
> > >        }
> > >    }
> > > +
> > > +  if(dbf){
> > > +      key.dsize = strlen(mp3->filename) + 1;
> > > +      key.dptr = mp3->filename;
> > > +      data.dsize = sizeof(MP3Info);
> > > +      data.dptr = (char *)mp3;
> > > +      gdbm_store(dbf, key, data, GDBM_REPLACE);
> > > +      gdbm_close(dbf);
> > > +  }
> > > +  g_free(db_file);
> > > +
> > >  }
> > > 
> > > 
> > > ---
> > > Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
> > > EE, LongEz N87KJ             |                           -- Walt Disney
> > > 
> > > 
> > > -------------------------------------------------------
> > > This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> > > for problems?  Stop!  Download the new AJAX search engine that makes
> > > searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> > > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> > > _______________________________________________
> > > Gtkpod-devel mailing list
> > > Gtkpod-devel@...
> > > https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
> > 
> > 
> > 
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> > for problems?  Stop!  Download the new AJAX search engine that makes
> > searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> > _______________________________________________
> > Gtkpod-devel mailing list
> > Gtkpod-devel@...
> > https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
> > 
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Gtkpod-devel mailing list
> Gtkpod-devel@...
> https://lists.sourceforge.net/lists/listinfo/gtkpod-devel

On Fri, Jan 27, 2006 at 08:26:37PM -0800, James Liggett wrote:
> Hi Jorg,
> On Sat, 2006-01-28 at 11:09 +0900, Jorg Schuler wrote:
> > Hi James, hi Andrew,
> > 
> > why not just storing modification date in the "extended database"
> > (iTunesDB.ext)? Filesize and last access time are already available in the
> > iTunesDB (size, time_added).
> > 
> > Then, instead of using the MD5 checksum to determine whether a change has
> > taken place, filesize, access time and modification date would be used.
> That's not a bad idea either. Maybe we should try it? If we can get
> results like what Andrew does, that would be neat. And if we could do it
> without additional dependencies that would be even better. 

We definitely should try it -- are you up to it?

Add the modification date to ExtraTrackData (displa_itdb.h) and make sure
it's written to iTunesDB.ext in file_itunesdb.c (write_extended_info(),
fill_in_extended_info(), read_extended_info()).

Cheers,


JCS.

Now, I'm not 100% sure why get_mp3_info() is being called, but it's not 
apparent to me where the md5sum is being done if at all for this operate.  
I'm seeing up to 10 seconds per file for this function to run and it looks 
like it's just parsing and filling in the mp3 tag information.

I've only spent an hour or two looking at gtkpod total, so probably know 
something I don't, I just profiled code till I found where time was being 
spent.  Make sure you profile it before fixing it. ;)  

Maybe files are being read twice and we could save some time doing md5sums 
caching?   But I ended up with this patch based after tunneling down to where 
the time was being spent.

I'll spend some time looking at this and my other bug this weekend.

Cheers,

-Andrew


On Friday 27 January 2006 23:52, Jorg Schuler wrote:
> On Fri, Jan 27, 2006 at 08:26:37PM -0800, James Liggett wrote:
> > Hi Jorg,
> >
> > On Sat, 2006-01-28 at 11:09 +0900, Jorg Schuler wrote:
> > > Hi James, hi Andrew,
> > >
> > > why not just storing modification date in the "extended database"
> > > (iTunesDB.ext)? Filesize and last access time are already available in
> > > the iTunesDB (size, time_added).
> > >
> > > Then, instead of using the MD5 checksum to determine whether a change
> > > has taken place, filesize, access time and modification date would be
> > > used.
> >
> > That's not a bad idea either. Maybe we should try it? If we can get
> > results like what Andrew does, that would be neat. And if we could do it
> > without additional dependencies that would be even better.
>
> We definitely should try it -- are you up to it?
>
> Add the modification date to ExtraTrackData (displa_itdb.h) and make sure
> it's written to iTunesDB.ext in file_itunesdb.c (write_extended_info(),
> fill_in_extended_info(), read_extended_info()).
>
> Cheers,
>
>
> JCS.
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Gtkpod-devel mailing list
> Gtkpod-devel@...
> https://lists.sourceforge.net/lists/listinfo/gtkpod-devel

-- 
---
Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
EE, LongEz N87KJ             |                           -- Walt Disney

OK,

there are two update functions:

1) Update: should update all information from file in case you messed up
your Tags. This function sould be renamed to "Refresh Meta Info from Files".

2) Sync Dirs: should probably skip files that have already been read after
their last modification date.

The check on whether or not to update the tags or not should therefore done
before calling get_track_info_from_file() when syncing dirs.

Cheers,

JCS.


On Sat, Jan 28, 2006 at 12:39:52AM -0500, Andrew Stanley-Jones wrote:
> 
> Now, I'm not 100% sure why get_mp3_info() is being called, but it's not 
> apparent to me where the md5sum is being done if at all for this operate.  
> I'm seeing up to 10 seconds per file for this function to run and it looks 
> like it's just parsing and filling in the mp3 tag information.
> 
> I've only spent an hour or two looking at gtkpod total, so probably know 
> something I don't, I just profiled code till I found where time was being 
> spent.  Make sure you profile it before fixing it. ;)  
> 
> Maybe files are being read twice and we could save some time doing md5sums 
> caching?   But I ended up with this patch based after tunneling down to where 
> the time was being spent.
> 
> I'll spend some time looking at this and my other bug this weekend.
> 
> Cheers,
> 
> -Andrew
> 
> 
> On Friday 27 January 2006 23:52, Jorg Schuler wrote:
> > On Fri, Jan 27, 2006 at 08:26:37PM -0800, James Liggett wrote:
> > > Hi Jorg,
> > >
> > > On Sat, 2006-01-28 at 11:09 +0900, Jorg Schuler wrote:
> > > > Hi James, hi Andrew,
> > > >
> > > > why not just storing modification date in the "extended database"
> > > > (iTunesDB.ext)? Filesize and last access time are already available in
> > > > the iTunesDB (size, time_added).
> > > >
> > > > Then, instead of using the MD5 checksum to determine whether a change
> > > > has taken place, filesize, access time and modification date would be
> > > > used.
> > >
> > > That's not a bad idea either. Maybe we should try it? If we can get
> > > results like what Andrew does, that would be neat. And if we could do it
> > > without additional dependencies that would be even better.
> >
> > We definitely should try it -- are you up to it?
> >
> > Add the modification date to ExtraTrackData (displa_itdb.h) and make sure
> > it's written to iTunesDB.ext in file_itunesdb.c (write_extended_info(),
> > fill_in_extended_info(), read_extended_info()).
> >
> > Cheers,
> >
> >
> > JCS.
> >
> >
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> > files for problems?  Stop!  Download the new AJAX search engine that makes
> > searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> > _______________________________________________
> > Gtkpod-devel mailing list
> > Gtkpod-devel@...
> > https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
> 
> -- 
> ---
> Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
> EE, LongEz N87KJ             |                           -- Walt Disney
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Gtkpod-devel mailing list
> Gtkpod-devel@...
> https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
>

Ok I have a patch that uses items itunesdb.ext like Jorg suggested yesterday, 
it fits in very nicely into update_track_from_file().  I added a boolean that 
can be used to force a reload of the meta info.  

This saves fetching both the meta info and the md5 hash.  The sync is now very 
very fast, and update was renamed and refetches all the information.   The 
patch is very simple too, but touched a couple files.

I wish I could generate a patch but sourceforge is being a pita and spitting 
out:   "can't create temporary directory /tmp/cvs-serv6674"    When sf starts 
working again I'll send it.  I'm running while : ; do cvs diff; done =)

-Andrew

On Saturday 28 January 2006 09:48, Jorg Schuler wrote:
> OK,
>
> there are two update functions:
>
> 1) Update: should update all information from file in case you messed up
> your Tags. This function sould be renamed to "Refresh Meta Info from
> Files".
>
> 2) Sync Dirs: should probably skip files that have already been read after
> their last modification date.
>
> The check on whether or not to update the tags or not should therefore done
> before calling get_track_info_from_file() when syncing dirs.
>
> Cheers,
>
> JCS.
>
> On Sat, Jan 28, 2006 at 12:39:52AM -0500, Andrew Stanley-Jones wrote:
> > Now, I'm not 100% sure why get_mp3_info() is being called, but it's not
> > apparent to me where the md5sum is being done if at all for this operate.
> > I'm seeing up to 10 seconds per file for this function to run and it
> > looks like it's just parsing and filling in the mp3 tag information.
> >
> > I've only spent an hour or two looking at gtkpod total, so probably know
> > something I don't, I just profiled code till I found where time was being
> > spent.  Make sure you profile it before fixing it. ;)
> >
> > Maybe files are being read twice and we could save some time doing
> > md5sums caching?   But I ended up with this patch based after tunneling
> > down to where the time was being spent.
> >
> > I'll spend some time looking at this and my other bug this weekend.
> >
> > Cheers,
> >
> > -Andrew
> >
> > On Friday 27 January 2006 23:52, Jorg Schuler wrote:
> > > On Fri, Jan 27, 2006 at 08:26:37PM -0800, James Liggett wrote:
> > > > Hi Jorg,
> > > >
> > > > On Sat, 2006-01-28 at 11:09 +0900, Jorg Schuler wrote:
> > > > > Hi James, hi Andrew,
> > > > >
> > > > > why not just storing modification date in the "extended database"
> > > > > (iTunesDB.ext)? Filesize and last access time are already available
> > > > > in the iTunesDB (size, time_added).
> > > > >
> > > > > Then, instead of using the MD5 checksum to determine whether a
> > > > > change has taken place, filesize, access time and modification date
> > > > > would be used.
> > > >
> > > > That's not a bad idea either. Maybe we should try it? If we can get
> > > > results like what Andrew does, that would be neat. And if we could do
> > > > it without additional dependencies that would be even better.
> > >
> > > We definitely should try it -- are you up to it?
> > >
> > > Add the modification date to ExtraTrackData (displa_itdb.h) and make
> > > sure it's written to iTunesDB.ext in file_itunesdb.c
> > > (write_extended_info(), fill_in_extended_info(), read_extended_info()).
> > >
> > > Cheers,
> > >
> > >
> > > JCS.
> > >
> > >
> > > -------------------------------------------------------
> > > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> > > files for problems?  Stop!  Download the new AJAX search engine that
> > > makes searching your log files as easy as surfing the  web.  DOWNLOAD
> > > SPLUNK!
> > > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=12164
> > >2 _______________________________________________
> > > Gtkpod-devel mailing list
> > > Gtkpod-devel@...
> > > https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
> >
> > --
> > ---
> > Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
> > EE, LongEz N87KJ             |                           -- Walt Disney
> >
> >
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> > files for problems?  Stop!  Download the new AJAX search engine that
> > makes searching your log files as easy as surfing the  web.  DOWNLOAD
> > SPLUNK!
> > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> > _______________________________________________
> > Gtkpod-devel mailing list
> > Gtkpod-devel@...
> > https://lists.sourceforge.net/lists/listinfo/gtkpod-devel

-- 
---
Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
EE, LongEz N87KJ             |                           -- Walt Disney

Sorry for the delay, I've been traveling and just got home, plus sf wasn't 
cooperating on Sunday so I couldn't do a checkout with my account correctly.

This patch needs a bug fix, it doesn't save the information right the first 
time it sees the file.  I also had to toss a bunch of lines from the patch 
for checking on the double free on double sync so it may not apply cleanly.  

Anyways, it'll give you an idea of what my idea was.  I'll clean it up over 
the next couple of days.

---
Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
EE, LongEz N87KJ             |                           -- Walt Disney

diff -u -r1.53 context_menus.c
--- context_menus.c	14 Dec 2005 14:58:55 -0000	1.53
+++ context_menus.c	2 Feb 2006 01:14:58 -0000
@@ -378,7 +378,7 @@
 		   G_CALLBACK (create_playlist_file), NULL);
 	hookup_mi (menu[type], _("Edit Details"), NULL,
 		   G_CALLBACK (edit_details_entries), NULL);
-	hookup_mi (menu[type], _("Update"), GTK_STOCK_REFRESH,
+	hookup_mi (menu[type], _("Refresh Meta Info from Files"), GTK_STOCK_REFRESH,
 		   G_CALLBACK (update_entries), NULL);
 	hookup_mi (menu[type], _("Sync Dirs"), GTK_STOCK_REFRESH,
 		   G_CALLBACK (sync_dirs_entries), NULL);
diff -u -r1.24 display_itdb.h
--- display_itdb.h	5 Dec 2005 16:35:17 -0000	1.24
+++ display_itdb.h	2 Feb 2006 01:14:59 -0000
@@ -78,6 +78,7 @@
   gchar   *hostname;      /* name of host this file has been imported on*/
   gchar   *md5_hash;      /* md5 hash of file (or NULL)              */
   gchar   *charset;       /* charset used for ID3 tags               */
+  time_t  mtime;          /* file modification time                  */
   gint32 sortindex;       /* used for stable sorting (current order) */
   gboolean tchanged;      /* temporary use, e.g. in detail.c         */
 } ExtraTrackData;
Index: file.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/file.c,v
retrieving revision 1.152
diff -u -r1.152 file.c
--- file.c	10 Dec 2005 17:04:28 -0000	1.152
+++ file.c	2 Feb 2006 01:15:06 -0000
@@ -328,7 +328,7 @@
 	    }
 	    else if (add_track_by_filename (itdb, filename, plitem,
 					    prefs_get_add_recursively (),
-					    addtrackfunc, data))
+					    addtrackfunc, data, 0))
 	    {
 		++tracks;
 	    }
@@ -367,7 +367,8 @@
    position in the track view */
 gboolean add_directory_by_name (iTunesDB *itdb, gchar *name,
 				Playlist *plitem, gboolean descend,
-				AddTrackFunc addtrackfunc, gpointer data)
+				AddTrackFunc addtrackfunc, gpointer data,
+				gboolean nocache)
 {
   gboolean result = TRUE;
 
@@ -389,7 +390,7 @@
 		      !g_file_test (nextfull, G_FILE_TEST_IS_DIR))
 		      result &= add_directory_by_name (
 			  itdb, nextfull, plitem,
-			  descend, addtrackfunc, data);
+			  descend, addtrackfunc, data, nocache);
 		  g_free (nextfull);
 	      }
 	  } while (next != NULL);
@@ -400,7 +401,7 @@
   else
   {
       result = add_track_by_filename (itdb, name, plitem,
-				      descend, addtrackfunc, data);
+				      descend, addtrackfunc, data, nocache);
   }
   return result;
 }
@@ -756,6 +763,7 @@
     g_free (eto->charset);
     g_free (eto->thumb_path_utf8);
     g_free (eto->thumb_path_locale);
+
     itdb_artwork_free (to->artwork);
     /* copy strings */
     to->album = g_strdup (from->album);
@@ -787,6 +795,7 @@
     eto->peak_signal_set = efrom->peak_signal_set;
     eto->radio_gain_set = efrom->radio_gain_set;
     eto->audiophile_gain_set = efrom->audiophile_gain_set;
+    eto->mtime = efrom->mtime;
     to->time_added = from->time_added;
     to->time_modified = from->time_modified;
     to->year = from->year;
@@ -1169,7 +1183,7 @@
 					  get_track_info (track, TRUE));
 	    gtkpod_statusbar_message (buf);
 	    g_free (buf);
-	    update_track_from_file (track->itdb, track);
+	    update_track_from_file (track->itdb, track, 1);
 	}
     }
     release_widgets ();
@@ -1297,7 +1311,7 @@
     g_free (dir_utf8);
 
     /* sync dir */
-    add_directory_by_name (itdb, dir, pl, FALSE, sync_addtrackfunc, NULL);
+    add_directory_by_name (itdb, dir, pl, FALSE, sync_addtrackfunc, NULL, 0);
 
     /* reset charset */
     if (!prefs_get_update_charset () && charset)
@@ -1856,14 +1870,17 @@
    a list of those can be displayed by calling "remove_duplicate
    (NULL, NULL)", that list can be deleted by calling
    "remove_duplicate (NULL, (void *)-1)"*/
-void update_track_from_file (iTunesDB *itdb, Track *track)
+void update_track_from_file (iTunesDB *itdb, Track *track, gboolean nocache)
 {
     ExtraTrackData *etr;
     Track *oldtrack;
     gchar *prefs_charset = NULL;
     gchar *trackpath = NULL;
     gint32 oldsize = 0;
+    gint32 no_change = 0;
     gboolean charset_set;
+    struct stat filestat;
+
 
     g_return_if_fail (itdb);
     g_return_if_fail (track);
@@ -1893,14 +1910,34 @@
 	get_track_info_from_file () since track->pc_path gets g_freed
 	there */
 	trackpath = g_strdup (etr->pc_path_locale);
+
+	if(nocache == 0){
+	    if (stat(trackpath,&filestat) == 0){
+		if(filestat.st_size == track->size &&
+		   filestat.st_mtime == etr->mtime){
+		    no_change = 1;
+		}
+	    }
+	    else {
+		perror("Can't stat file");
+	    }
+	}
     }
 
     if (!(etr->pc_path_locale && *etr->pc_path_locale))
     { /* no path available */
 	display_non_updated (track, _("no filename available"));
     }
+    else if (no_change){
+    }
     else if (get_track_info_from_file (trackpath, track))
     { /* update successfull */
+	/* save the modification time so updates are faster */
+	if(stat(trackpath,&filestat) == 0){
+	    printf("Size: %ld date: %ld\n", filestat.st_size, filestat.st_mtime);
+	    etr->mtime = filestat.st_mtime;
+	}
+
 	/* remove track from md5 hash and reinsert it
 	   (hash value may have changed!) */
 	gchar *oldhash = etr->md5_hash;
@@ -1978,7 +2015,7 @@
    position in the track view */
 gboolean add_track_by_filename (iTunesDB *itdb, gchar *fname,
 				Playlist *plitem, gboolean descend,
-			       AddTrackFunc addtrackfunc, gpointer data)
+			       AddTrackFunc addtrackfunc, gpointer data, gboolean nocache)
 {
   static gint count = 0; /* do a gtkpod_tracks_statusbar_update() every
 			    10 tracks */
@@ -1997,9 +2034,11 @@
 
   if (g_file_test (fname, G_FILE_TEST_IS_DIR))
   {
-      return add_directory_by_name (itdb, fname, plitem, descend, 
addtrackfunc, data);
+      return add_directory_by_name (itdb, fname, plitem, descend, 
addtrackfunc, data, nocache);
   }
 
+
+
   /* check if file is a playlist */
   switch (determine_file_type(fname))
   {
@@ -2043,7 +2082,7 @@
   {
       if (prefs_get_update_existing ())
       {   /* update the information */
-	  update_track_from_file (itdb, oldtrack);
+	  update_track_from_file (itdb, oldtrack, nocache);
       }
       /* add to current playlist if it's not already in there */
       if (!itdb_playlist_is_mpl (plitem))
Index: file.h
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/file.h,v
retrieving revision 1.47
diff -u -r1.47 file.h
--- file.h	10 Dec 2005 17:04:28 -0000	1.47
+++ file.h	2 Feb 2006 01:15:06 -0000
@@ -70,16 +70,16 @@
 FileType determine_file_type(gchar *path);
 gboolean add_track_by_filename (iTunesDB *itdb, gchar *name,
 				Playlist *plitem, gboolean descend,
-				AddTrackFunc addtrackfunc, gpointer data);
+				AddTrackFunc addtrackfunc, gpointer data, gboolean nocache);
 gboolean add_directory_by_name (iTunesDB *itdb, gchar *name,
 				Playlist *plitem, gboolean descend,
-				AddTrackFunc addtrackfunc, gpointer data);
+				AddTrackFunc addtrackfunc, gpointer data, gboolean nocache);
 Playlist *add_playlist_by_filename (iTunesDB *itdb, gchar *plfile,
 				    Playlist *plitem, gint plitem_pos,
 				    AddTrackFunc addtrackfunc,
 				    gpointer data);
 gboolean write_tags_to_file(Track *s);
-void update_track_from_file (iTunesDB *itdb, Track *track);
+void update_track_from_file (iTunesDB *itdb, Track *track, gboolean nocache);
 void update_tracks (GList *selected_tracks);
 void mserv_from_file_tracks (GList *selected_tracks);
 void sync_tracks (GList *selected_tracks);
Index: file_itunesdb.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/file_itunesdb.c,v
retrieving revision 1.72
diff -u -r1.72 file_itunesdb.c
--- file_itunesdb.c	4 Dec 2005 16:00:39 -0000	1.72
+++ file_itunesdb.c	2 Feb 2006 01:15:09 -0000
@@ -64,6 +64,7 @@
     gchar *hostname;
     gchar *ipod_path;
     gint32 oldsize;
+    gint32 mtime;
     guint32 playcount;
     guint32 rating;        /* still read but never written */
     guint32 peak_signal;
@@ -146,6 +147,7 @@
       if (sei->hostname && !etr->hostname)
 	  etr->hostname = g_strdup (sei->hostname);
       etr->oldsize = sei->oldsize;
+      etr->mtime = sei->mtime;
       track->playcount += sei->playcount;
       if (sei->peak_signal_set)
       {
@@ -382,6 +384,8 @@
 		sei->rating = atoi (arg);
 	    else if (g_ascii_strcasecmp (line, "transferred") == 0)
 		sei->transferred = atoi (arg);
+	    else if (g_ascii_strcasecmp (line, "mtime") == 0)
+		sei->mtime = atoi (arg);
 	    else if (g_ascii_strcasecmp (line, "filename_ipod") == 0)
 		sei->ipod_path = g_strdup (arg);
 	    else if (g_ascii_strcasecmp (line, "peak_signal") == 0)
@@ -931,6 +935,8 @@
 	  fprintf (fp, "md5_hash=%s\n", etr->md5_hash);
       if (etr->charset && *etr->charset)
 	  fprintf (fp, "charset=%s\n", etr->charset);
+      if (etr->mtime != 0)
+	  fprintf (fp, "mtime=%ld\n", etr->mtime);
       if (!track->transferred && etr->oldsize)
 	  fprintf (fp, "oldsize=%d\n", etr->oldsize);
       if (etr->peak_signal_set)
Index: fileselection.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/fileselection.c,v
retrieving revision 1.4
diff -u -r1.4 fileselection.c
--- fileselection.c	4 Dec 2005 10:24:38 -0000	1.4
+++ fileselection.c	2 Feb 2006 01:15:10 -0000
@@ -105,7 +105,7 @@
 					(gchar*)current->data,
 					playlist,
 					prefs_get_add_recursively(),
-					NULL, NULL);
+					NULL, NULL, 0);
 	current = current->next;
     }
 	
@@ -460,7 +460,7 @@
 	Playlist *plitem = pm_get_selected_playlist ();
 	add_directory_by_name (db_active_itdb, dir, plitem,
 			       prefs_get_add_recursively (),
-			       NULL, NULL);
+			       NULL, NULL, 0);
 	prefs_set_last_dir_browse(dir);
 	gtkpod_tracks_statusbar_update();
     }
Index: misc_playlist.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/misc_playlist.c,v
retrieving revision 1.27
diff -u -r1.27 misc_playlist.c
--- misc_playlist.c	23 Nov 2005 04:16:12 -0000	1.27
+++ misc_playlist.c	2 Feb 2006 01:15:13 -0000
@@ -1138,7 +1138,7 @@
 		    {
 			add_track_by_filename(itdb,
 					      fn_orphaned, pl_orphaned,
-					      FALSE, NULL, NULL);
+					      FALSE, NULL, NULL, 0);
 		    }
 		    g_free (fn_orphaned);
 		    g_free (num_str);
Index: misc_track.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/misc_track.c,v
retrieving revision 1.33
diff -u -r1.33 misc_track.c
--- misc_track.c	10 Dec 2005 17:04:29 -0000	1.33
+++ misc_track.c	2 Feb 2006 01:15:14 -0000
@@ -1348,7 +1348,7 @@
 		    }
 		    add_directory_by_name (itdb, decoded_file, pl,
 					   prefs_get_add_recursively (),
-					   trackaddfunc, data);
+					   trackaddfunc, data, 0);
 		    added = TRUE;
 		}
 		if (g_file_test (decoded_file, G_FILE_TEST_IS_REGULAR))
@@ -1373,7 +1373,7 @@
 			}
 			add_track_by_filename (itdb, decoded_file, pl,
 					       prefs_get_add_recursively (),
-					       trackaddfunc, data);
+					       trackaddfunc, data, 0);
 			added = TRUE;
 			break;
 		    case FILE_TYPE_M3U:

Ok, here's the patch, I cleaned everything up and it should patch cleanly to 
current cvs.  It sets the mtime on file add so new files aren't checked on 
the next sync if the the mtime/size/name all match.

-Andrew

---
Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
EE, LongEz N87KJ             |                           -- Walt Disney

Index: context_menus.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/context_menus.c,v
retrieving revision 1.54
diff -u -r1.54 context_menus.c
--- context_menus.c	1 Feb 2006 12:50:02 -0000	1.54
+++ context_menus.c	5 Feb 2006 01:54:36 -0000
@@ -378,7 +378,7 @@
 		   G_CALLBACK (create_playlist_file), NULL);
 	hookup_mi (menu[type], _("Edit Details"), NULL,
 		   G_CALLBACK (edit_details_entries), NULL);
-	hookup_mi (menu[type], _("Update"), GTK_STOCK_REFRESH,
+	hookup_mi (menu[type], _("Refresh Meta Info from Files"), GTK_STOCK_REFRESH,
 		   G_CALLBACK (update_entries), NULL);
 	hookup_mi (menu[type], _("Sync Dirs"), GTK_STOCK_REFRESH,
 		   G_CALLBACK (sync_dirs_entries), NULL);
Index: display_itdb.h
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/display_itdb.h,v
retrieving revision 1.24
diff -u -r1.24 display_itdb.h
--- display_itdb.h	5 Dec 2005 16:35:17 -0000	1.24
+++ display_itdb.h	5 Feb 2006 01:54:38 -0000
@@ -78,6 +78,7 @@
   gchar   *hostname;      /* name of host this file has been imported on*/
   gchar   *md5_hash;      /* md5 hash of file (or NULL)              */
   gchar   *charset;       /* charset used for ID3 tags               */
+  time_t  mtime;          /* file modification time                  */
   gint32 sortindex;       /* used for stable sorting (current order) */
   gboolean tchanged;      /* temporary use, e.g. in detail.c         */
 } ExtraTrackData;
Index: display_songs.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/display_songs.c,v
retrieving revision 1.94
diff -u -r1.94 display_songs.c
--- display_songs.c	3 Dec 2005 16:36:26 -0000	1.94
+++ display_songs.c	5 Feb 2006 01:54:42 -0000
@@ -942,6 +942,7 @@
 
   text = track_get_text (track, TM_to_T (column));
 
+
   switch (column)
   {
   case TM_COLUMN_TITLE:
Index: file.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/file.c,v
retrieving revision 1.152
diff -u -r1.152 file.c
--- file.c	10 Dec 2005 17:04:28 -0000	1.152
+++ file.c	5 Feb 2006 01:54:45 -0000
@@ -328,7 +328,7 @@
 	    }
 	    else if (add_track_by_filename (itdb, filename, plitem,
 					    prefs_get_add_recursively (),
-					    addtrackfunc, data))
+					    addtrackfunc, data, 0))
 	    {
 		++tracks;
 	    }
@@ -367,7 +367,8 @@
    position in the track view */
 gboolean add_directory_by_name (iTunesDB *itdb, gchar *name,
 				Playlist *plitem, gboolean descend,
-				AddTrackFunc addtrackfunc, gpointer data)
+				AddTrackFunc addtrackfunc, gpointer data,
+				gboolean nocache)
 {
   gboolean result = TRUE;
 
@@ -389,7 +390,7 @@
 		      !g_file_test (nextfull, G_FILE_TEST_IS_DIR))
 		      result &= add_directory_by_name (
 			  itdb, nextfull, plitem,
-			  descend, addtrackfunc, data);
+			  descend, addtrackfunc, data, nocache);
 		  g_free (nextfull);
 	      }
 	  } while (next != NULL);
@@ -400,7 +401,7 @@
   else
   {
       result = add_track_by_filename (itdb, name, plitem,
-				      descend, addtrackfunc, data);
+				      descend, addtrackfunc, data, nocache);
   }
   return result;
 }
@@ -756,6 +757,7 @@
     g_free (eto->charset);
     g_free (eto->thumb_path_utf8);
     g_free (eto->thumb_path_locale);
+
     itdb_artwork_free (to->artwork);
     /* copy strings */
     to->album = g_strdup (from->album);
@@ -787,6 +789,7 @@
     eto->peak_signal_set = efrom->peak_signal_set;
     eto->radio_gain_set = efrom->radio_gain_set;
     eto->audiophile_gain_set = efrom->audiophile_gain_set;
+    eto->mtime = efrom->mtime;
     to->time_added = from->time_added;
     to->time_modified = from->time_modified;
     to->year = from->year;
@@ -1169,7 +1172,7 @@
 					  get_track_info (track, TRUE));
 	    gtkpod_statusbar_message (buf);
 	    g_free (buf);
-	    update_track_from_file (track->itdb, track);
+	    update_track_from_file (track->itdb, track, 1);
 	}
     }
     release_widgets ();
@@ -1297,7 +1300,7 @@
     g_free (dir_utf8);
 
     /* sync dir */
-    add_directory_by_name (itdb, dir, pl, FALSE, sync_addtrackfunc, NULL);
+    add_directory_by_name (itdb, dir, pl, FALSE, sync_addtrackfunc, NULL, 0);
 
     /* reset charset */
     if (!prefs_get_update_charset () && charset)
@@ -1856,14 +1859,17 @@
    a list of those can be displayed by calling "remove_duplicate
    (NULL, NULL)", that list can be deleted by calling
    "remove_duplicate (NULL, (void *)-1)"*/
-void update_track_from_file (iTunesDB *itdb, Track *track)
+void update_track_from_file (iTunesDB *itdb, Track *track, gboolean nocache)
 {
     ExtraTrackData *etr;
     Track *oldtrack;
     gchar *prefs_charset = NULL;
     gchar *trackpath = NULL;
     gint32 oldsize = 0;
+    gint32 no_change = 0;
     gboolean charset_set;
+    struct stat filestat;
+
 
     g_return_if_fail (itdb);
     g_return_if_fail (track);
@@ -1893,14 +1899,33 @@
 	get_track_info_from_file () since track->pc_path gets g_freed
 	there */
 	trackpath = g_strdup (etr->pc_path_locale);
+
+	if(nocache == 0){
+	    if (stat(trackpath,&filestat) == 0){
+		if(filestat.st_size == track->size &&
+		   filestat.st_mtime == etr->mtime){
+		    no_change = 1;
+		}
+	    }
+	    else {
+		perror("Can't stat file");
+	    }
+	}
     }
 
     if (!(etr->pc_path_locale && *etr->pc_path_locale))
     { /* no path available */
 	display_non_updated (track, _("no filename available"));
     }
+    else if (no_change){
+    }
     else if (get_track_info_from_file (trackpath, track))
     { /* update successfull */
+	/* save the modification time so updates are faster */
+	if(stat(trackpath,&filestat) == 0){
+	    etr->mtime = filestat.st_mtime;
+	}
+
 	/* remove track from md5 hash and reinsert it
 	   (hash value may have changed!) */
 	gchar *oldhash = etr->md5_hash;
@@ -1978,7 +2003,7 @@
    position in the track view */
 gboolean add_track_by_filename (iTunesDB *itdb, gchar *fname,
 				Playlist *plitem, gboolean descend,
-			       AddTrackFunc addtrackfunc, gpointer data)
+			       AddTrackFunc addtrackfunc, gpointer data, gboolean nocache)
 {
   static gint count = 0; /* do a gtkpod_tracks_statusbar_update() every
 			    10 tracks */
@@ -1987,6 +2012,7 @@
   gchar *basename;
   Playlist *mpl;
   gboolean result = TRUE;
+  struct stat filestat;
 
   g_return_val_if_fail (fname, FALSE);
   g_return_val_if_fail (itdb, FALSE);
@@ -1997,7 +2023,7 @@
 
   if (g_file_test (fname, G_FILE_TEST_IS_DIR))
   {
-      return add_directory_by_name (itdb, fname, plitem, descend, 
addtrackfunc, data);
+      return add_directory_by_name (itdb, fname, plitem, descend, 
addtrackfunc, data, nocache);
   }
 
   /* check if file is a playlist */
@@ -2043,7 +2069,7 @@
   {
       if (prefs_get_update_existing ())
       {   /* update the information */
-	  update_track_from_file (itdb, oldtrack);
+	  update_track_from_file (itdb, oldtrack, nocache);
       }
       /* add to current playlist if it's not already in there */
       if (!itdb_playlist_is_mpl (plitem))
@@ -2069,6 +2095,10 @@
 	  track->id = 0;
 	  track->transferred = FALSE;
 
+	  if(stat(fname,&filestat) == 0){
+	      etr->mtime = filestat.st_mtime;
+	  }
+
 	  /* is 'fname' on the iPod? -- if yes mark as transfered, if
 	   * it's in the ipod_control directory */
 	  if (itdb->usertype & GP_ITDB_TYPE_IPOD)
Index: file.h
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/file.h,v
retrieving revision 1.47
diff -u -r1.47 file.h
--- file.h	10 Dec 2005 17:04:28 -0000	1.47
+++ file.h	5 Feb 2006 01:54:46 -0000
@@ -70,16 +70,16 @@
 FileType determine_file_type(gchar *path);
 gboolean add_track_by_filename (iTunesDB *itdb, gchar *name,
 				Playlist *plitem, gboolean descend,
-				AddTrackFunc addtrackfunc, gpointer data);
+				AddTrackFunc addtrackfunc, gpointer data, gboolean nocache);
 gboolean add_directory_by_name (iTunesDB *itdb, gchar *name,
 				Playlist *plitem, gboolean descend,
-				AddTrackFunc addtrackfunc, gpointer data);
+				AddTrackFunc addtrackfunc, gpointer data, gboolean nocache);
 Playlist *add_playlist_by_filename (iTunesDB *itdb, gchar *plfile,
 				    Playlist *plitem, gint plitem_pos,
 				    AddTrackFunc addtrackfunc,
 				    gpointer data);
 gboolean write_tags_to_file(Track *s);
-void update_track_from_file (iTunesDB *itdb, Track *track);
+void update_track_from_file (iTunesDB *itdb, Track *track, gboolean nocache);
 void update_tracks (GList *selected_tracks);
 void mserv_from_file_tracks (GList *selected_tracks);
 void sync_tracks (GList *selected_tracks);
Index: file_itunesdb.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/file_itunesdb.c,v
retrieving revision 1.73
diff -u -r1.73 file_itunesdb.c
--- file_itunesdb.c	4 Feb 2006 12:04:52 -0000	1.73
+++ file_itunesdb.c	5 Feb 2006 01:54:48 -0000
@@ -64,6 +64,7 @@
     gchar *hostname;
     gchar *ipod_path;
     gint32 oldsize;
+    gint32 mtime;
     guint32 playcount;
     guint32 rating;        /* still read but never written */
     guint32 peak_signal;
@@ -146,6 +147,7 @@
       if (sei->hostname && !etr->hostname)
 	  etr->hostname = g_strdup (sei->hostname);
       etr->oldsize = sei->oldsize;
+      etr->mtime = sei->mtime;
       track->playcount += sei->playcount;
       if (sei->peak_signal_set)
       {
@@ -382,6 +384,8 @@
 		sei->rating = atoi (arg);
 	    else if (g_ascii_strcasecmp (line, "transferred") == 0)
 		sei->transferred = atoi (arg);
+	    else if (g_ascii_strcasecmp (line, "mtime") == 0)
+		sei->mtime = atoi (arg);
 	    else if (g_ascii_strcasecmp (line, "filename_ipod") == 0)
 		sei->ipod_path = g_strdup (arg);
 	    else if (g_ascii_strcasecmp (line, "peak_signal") == 0)
@@ -931,6 +935,8 @@
 	  fprintf (fp, "md5_hash=%s\n", etr->md5_hash);
       if (etr->charset && *etr->charset)
 	  fprintf (fp, "charset=%s\n", etr->charset);
+      if (etr->mtime != 0)
+	  fprintf (fp, "mtime=%ld\n", etr->mtime);
       if (!track->transferred && etr->oldsize)
 	  fprintf (fp, "oldsize=%d\n", etr->oldsize);
       if (etr->peak_signal_set)
Index: fileselection.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/fileselection.c,v
retrieving revision 1.4
diff -u -r1.4 fileselection.c
--- fileselection.c	4 Dec 2005 10:24:38 -0000	1.4
+++ fileselection.c	5 Feb 2006 01:54:50 -0000
@@ -105,7 +105,7 @@
 					(gchar*)current->data,
 					playlist,
 					prefs_get_add_recursively(),
-					NULL, NULL);
+					NULL, NULL, 0);
 	current = current->next;
     }
 	
@@ -460,7 +460,7 @@
 	Playlist *plitem = pm_get_selected_playlist ();
 	add_directory_by_name (db_active_itdb, dir, plitem,
 			       prefs_get_add_recursively (),
-			       NULL, NULL);
+			       NULL, NULL, 0);
 	prefs_set_last_dir_browse(dir);
 	gtkpod_tracks_statusbar_update();
     }
Index: misc_playlist.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/misc_playlist.c,v
retrieving revision 1.27
diff -u -r1.27 misc_playlist.c
--- misc_playlist.c	23 Nov 2005 04:16:12 -0000	1.27
+++ misc_playlist.c	5 Feb 2006 01:54:53 -0000
@@ -1138,7 +1138,7 @@
 		    {
 			add_track_by_filename(itdb,
 					      fn_orphaned, pl_orphaned,
-					      FALSE, NULL, NULL);
+					      FALSE, NULL, NULL, 0);
 		    }
 		    g_free (fn_orphaned);
 		    g_free (num_str);
Index: misc_track.c
===================================================================
RCS file: /cvsroot/gtkpod/gtkpod/src/misc_track.c,v
retrieving revision 1.33
diff -u -r1.33 misc_track.c
--- misc_track.c	10 Dec 2005 17:04:29 -0000	1.33
+++ misc_track.c	5 Feb 2006 01:54:55 -0000
@@ -1348,7 +1348,7 @@
 		    }
 		    add_directory_by_name (itdb, decoded_file, pl,
 					   prefs_get_add_recursively (),
-					   trackaddfunc, data);
+					   trackaddfunc, data, 0);
 		    added = TRUE;
 		}
 		if (g_file_test (decoded_file, G_FILE_TEST_IS_REGULAR))
@@ -1373,7 +1373,7 @@
 			}
 			add_track_by_filename (itdb, decoded_file, pl,
 					       prefs_get_add_recursively (),
-					       trackaddfunc, data);
+					       trackaddfunc, data, 0);
 			added = TRUE;
 			break;
 		    case FILE_TYPE_M3U:

Hi Andrew,
Overall, the patch looks great, but I have some small nitpicks:



> +	    update_track_from_file (track->itdb, track, 1);
For that last arguement, why not use TRUE/FALSE instead of 1/0 for
booleans? IMHO that would make it a little easier to read.

> +    gint32 no_change = 0;
Why did you make this a gint32 instead of a gboolean?
> +
> +	if(nocache == 0){
There's no need to check a boolean like this; it'd be a bit cleaner if
you tried something like if (!nocache).
> +	    if (stat(trackpath,&filestat) == 0){
> +		if(filestat.st_size == track->size &&
> +		   filestat.st_mtime == etr->mtime){
> +		    no_change = 1;
> +		}
> +	    }
> +	    else {
> +		perror("Can't stat file");
I'm not sure if you should use perror here. Try using gtkpod_warning
instead, something like gtkpod_warning(_("Can't stat file"));

Thanks for your work on this! :)

James

Heh looks good.  Most of the reasons are "I don't know gtk" :)  

The reasoned I used perror() was because I didn't really want to pop up a 
warning box it seemed so minor, and it was quick and easy in development to 
see if I got the filename right. ;)  If it fails to stat() it doesn't really 
break anything, just makes it slower later.  It's easy enough to change over 
though.

I actually don't like if(!nocache) the double negatives I think makes it 
harder to read, and the ! is easily missed.  I should change the variable to 
"cache".  if(cache) seems nicer to me.

I'll fix it up latter today.

-Andrew

On Saturday 04 February 2006 23:57, James Liggett wrote:
> Hi Andrew,
>
> Overall, the patch looks great, but I have some small nitpicks:
> > +	    update_track_from_file (track->itdb, track, 1);
>
> For that last arguement, why not use TRUE/FALSE instead of 1/0 for
> booleans? IMHO that would make it a little easier to read.
>
> > +    gint32 no_change = 0;
>
> Why did you make this a gint32 instead of a gboolean?
>
> > +
> > +	if(nocache == 0){
>
> There's no need to check a boolean like this; it'd be a bit cleaner if
> you tried something like if (!nocache).
>
> > +	    if (stat(trackpath,&filestat) == 0){
> > +		if(filestat.st_size == track->size &&
> > +		   filestat.st_mtime == etr->mtime){
> > +		    no_change = 1;
> > +		}
> > +	    }
> > +	    else {
> > +		perror("Can't stat file");
>
> I'm not sure if you should use perror here. Try using gtkpod_warning
> instead, something like gtkpod_warning(_("Can't stat file"));
>
> Thanks for your work on this! :)
>
> James
>
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Gtkpod-devel mailing list
> Gtkpod-devel@...
> https://lists.sourceforge.net/lists/listinfo/gtkpod-devel

-- 
---
Andrew Stanley-Jones         | "It's kind of fun to do the impossible."
EE, LongEz N87KJ             |                           -- Walt Disney

Hi Andrew,

first of all thanks for your work. I second the comments made by Andrew,
which are mostly cosmetic except for the gtkpod_warning() and
internationalization issue _("")

I'm currently away from my computer and won't be able to upload any patches
and probably also won't be able to think much about any inquiries for at
least 10 days...

Cheers,


JCS.

> --- Ursprüngliche Nachricht ---
> Hi Andrew,
> Overall, the patch looks great, but I have some small nitpicks:
> 
> 
> 
> > +	    update_track_from_file (track->itdb, track, 1);
> For that last arguement, why not use TRUE/FALSE instead of 1/0 for
> booleans? IMHO that would make it a little easier to read.
> 
> > +    gint32 no_change = 0;
> Why did you make this a gint32 instead of a gboolean?
> > +
> > +	if(nocache == 0){
> There's no need to check a boolean like this; it'd be a bit cleaner if
> you tried something like if (!nocache).
> > +	    if (stat(trackpath,&filestat) == 0){
> > +		if(filestat.st_size == track->size &&
> > +		   filestat.st_mtime == etr->mtime){
> > +		    no_change = 1;
> > +		}
> > +	    }
> > +	    else {
> > +		perror("Can't stat file");
> I'm not sure if you should use perror here. Try using gtkpod_warning
> instead, something like gtkpod_warning(_("Can't stat file"));
> 
> Thanks for your work on this! :)
> 
> James
> 
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Gtkpod-devel mailing list
> Gtkpod-devel@...
> https://lists.sourceforge.net/lists/listinfo/gtkpod-devel
> 

-- 
What's the difference between eating sugar (e.g. candy bar) and
eating carbon hydrates (let's say a loaf of bread)?

Telefonieren Sie schon oder sparen Sie noch?
NEU: GMX Phone_Flat http://www.gmx.net/de/go/telefonie

Hi,

> The reasoned I used perror() was because I didn't really want to pop up a 
> warning box it seemed so minor, and it was quick and easy in development
> to 
> see if I got the filename right. ;)  If it fails to stat() it doesn't
> really 
> break anything, just makes it slower later.  It's easy enough to change
> over 
> though.

If it's really unimportant you could also display it in the statusbar -- but
it will quickly be overwritten by other status messages...

> I actually don't like if(!nocache) the double negatives I think makes it 
> harder to read, and the ! is easily missed.  I should change the variable
> to 
> "cache".  if(cache) seems nicer to me.

Sounds even better.


Cheers,


JCS.

-- 
What's the difference between eating sugar (e.g. candy bar) and
eating carbon hydrates (let's say a loaf of bread)?

10 GB Mailbox, 100 FreeSMS/Monat http://www.gmx.net/de/go/topmail
+++ GMX - die erste Adresse für Mail, Message, More +++

On Sun, 2006-02-05 at 09:21 -0500, Andrew Stanley-Jones wrote:
> Heh looks good.  Most of the reasons are "I don't know gtk" :)
If it makes you feel better, I'm not too experienced with gtk either. :)
>   
> 
> The reasoned I used perror() was because I didn't really want to pop up a 
> warning box it seemed so minor, and it was quick and easy in development to 
> see if I got the filename right. ;)  If it fails to stat() it doesn't really 
> break anything, just makes it slower later.  It's easy enough to change over 
> though.
OK. I wasn't sure if this was really a big deal or not. 
> 
> I actually don't like if(!nocache) the double negatives I think makes it 
> harder to read, and the ! is easily missed.  I should change the variable to 
> "cache".  if(cache) seems nicer to me.
Yes, good idea. Again, nice job on this one... ;-)
> 
> I'll fix it up latter today.
Great...looking forward to seeing it in CVS!

James