Re: [Gtkpod-devel] Incoming libgpod release

[Christophe F. <cfe...@gm...>]

Hi Joel,

On Mon, Aug 30, 2010 at 08:24:17PM -0600, Joel Smith wrote:
> Of course I'm biased since it's my patches getting left out, but I think 
> that several of my changes are important, especially for TV Shows and 
> Audiobooks on iPhone/iPod Touch.

Yeah, I'm really sorry for that, it takes me a lot of time to get them in
while they all seem really useful. One of the reason it took me so long to
get a release out is that I didn't want to release without your patches,
but for various external reasons (banshee release needing the mono
bindings, various distro releases getting closer), it's really better if a
release is done now.

> What do you think of committing them 
> on a trial basis post 0.7.94 so that they have a better chance of being 
> tested by a wider audience?  Additionally, I'm happy to spend time 
> discussing the patches, and doing alternate versions, if appropriate. 

For most of the patches, I either wanted to spend more time thinking about
or to rework the code a bit, that's one of the reason things didn't move
really fast with them :-/

> The six patches that I have outstanding are:

Yep, I think I've got them all in
http://gitorious.org/~teuf/libgpod/teuf-sandbox/commits/iphone
> 
> 1. Fix for video_info table to sort by TV Show title

For this one, I'm really not a big fan of comparing for both NULL strings
and "\0" strings, and since hawke's patch that didn't make it in was also
such a change, I want to spend a bit more time playing with iTunesDBs
trying to understand why we get "\0" strings. It seems iTunes writes such
strings, but I want to double check that first.
> 
> 2. make some WContents functions available to other libgpod modules

Dunno if that's the best course of action for the next patches, if that's
done, I think I'd move them to a new file and rename the funcs, I have to
play a bit with that and make up my mind

> 
> 3. update chapter parsing to handle (and skip over) child atoms other 
> than name
> 
>      Since chapters can have several child atoms besides name (such as url ,
>      urlt, or ploc/trak), the parser now properly skips them and only looks
>      at the name child atom

On retrospect, I probably should have taken this one ;)


> 
> 4. Add chapter support for sqlite tables

I'm not a big fan of putting this adhoc wcontents code in itdb_sqlite,
maybe it would be better to have a helper function in itdb_itunesdb.c,
maybe not, once again, I need to think more about it and make up my mind.
> 
> 5. update plist parsing to return 64-bit ints when a number overflows a
>      32-bit int

Yep, I'm  uncomfortable with this one, is this needed to handle
PlayCounts.plist? Could you send me such a file?

> 
> 6. Support for reading PlayCounts.plist for play counts, stats and
>      bookmarks.  iPhone/iPod Touch 3.x use this file, so without this
>      support, bookmarks are lost during sync.

For this one, I'm not really happy with adding a pcounts2 member to
FImport, especially since the way we use FImport::playcounts is awkward so
I wanted to use this patch as an opportunity to clean up the existing code.

Once again, I'm really sorry for blocking this useful code for so long :-/
I probably should have sent a feedback email much before. I'll try to play
with this code this week (maybe the playcount code) ;)

Cheers,

Christophe