I paste the content of a new ipod, because I plan to use it soon

(((baptiste@linbox)))-> ls -lR /media/ipod/
/media/ipod/:
total 16
drwx------ 2 baptiste baptiste 4096 1999-12-31 23:35 Calendars
drwx------ 2 baptiste baptiste 4096 1999-12-31 23:35 Contacts
drwx------ 5 baptiste baptiste 4096 2006-06-27 10:36 iPod_Control
drwx------ 2 baptiste baptiste 4096 1999-12-31 23:35 Notes

/media/ipod/Calendars:
total 0

/media/ipod/Contacts:
total 8
-rwx------ 1 baptiste baptiste 104 1999-12-31 23:35 ipod_created_instructions.vcf
-rwx------ 1 baptiste baptiste 676 1999-12-31 23:35 ipod_created_sample.vcf

/media/ipod/iPod_Control:
total 12
drwx------ 2 baptiste baptiste 4096 1999-12-31 23:35 Accessories
drwx------ 2 baptiste baptiste 4096 1999-12-31 23:35 Device
drwx------ 2 baptiste baptiste 4096 2006-06-27 10:36 iTunes

/media/ipod/iPod_Control/Accessories:
total 0

/media/ipod/iPod_Control/Device:
total 12
-rwx------ 1 baptiste baptiste  133 2000-01-01 13:54 clock
-rwx------ 1 baptiste baptiste 2912 2000-01-01 13:54 Preferences
-rwx------ 1 baptiste baptiste  128 2000-01-01 13:54 radio
-rwx------ 1 baptiste baptiste    0 1999-12-31 23:35 SysInfo

/media/ipod/iPod_Control/iTunes:
total 0
-rwx------ 1 baptiste baptiste 0 2006-06-27 10:36 firsttime

/media/ipod/Notes:
total 4
-rwx------ 1 baptiste baptiste 14 1999-12-31 23:35 Instructions

Baptiste,

I am also a Rhythmbox user yet haven't tried the ipod support yet. One thing I know though is that it's true a freshly formatted/new ipod generally cannot be used in most 3rd party, non-iTunes tools (yamipod for me at the time).

For these purposes and others, I always have a copy of gnupod laying around. Doing a gnupod_init and mktunes commands after having mounted your ipod as a mass storage disk creates the necessary files on the ipod so that the other ipod management tools can see and manage tracks properly.

Hope that helps

Created attachment 77294 [details] [review]
start of a patch

First stab at implementing this. The code to build the model menu is stolen from GtkPod.

FYI, this didn't work for me with an 80 GB 5th generation video iPod. However the problem might actually be lower down in the mysterious stack that governs such things...

(HAL claims that the iPod only contains one 37.7 GB volume (sda1) when in fact it contains two (sda1 being ~91 MB and sda2 being ~74 GB). g-v-m mounts neither when the iPod is inserted. When the iPod is removed, HAL claims the volume still exists!)

Once I sorted out the problems with HAL, and symlinked ipod_init.glade into /usr/lib/rhythmbox/plugins/ipod (it is not installed by default, I guess plugins/ipod/Makefile.am needs to be modified somehow) then the plugin worked perfectly--thanks!

Created attachment 79594 [details] [review]
better patch

Auto-detection of the model now works for me. The "do you want to do this" message is a bit long, so should probably be replaced with something shorted.

Source tree surgery broke this patch.  I can update it if required.

I didn't test that patch at all, but does it pop up a dialog asking the user to fill in which iPod model it owns? While popping out a dialog before writing is nice, I'm sure we can auto-detect the iPod model instead of asking the user though it might be hard to do without some special permissions :-/

The dialog asks for a name for the ipod and the model, but if all goes well, the model will be autodetected.

Doc, Is there any chance this patch could be updated?

(I'm currently testing the patches in #436319 with various iPods -- and it'd be nice to do it all from one program than having to fire up gnupod -> gtkpod -> RB for each of them).

Created attachment 89476 [details] [review]
updated patch

Updated for trunk, although I haven't tested this for more than compiling successfully.

If you decide to apply it, can this wait until the patch from bug #436319 goes in? (I'm waiting for Paul's feedback before aplpying it). I'll update the patch from that bug once it's in.

Aren't the configure.ac, shell/rb-shell.c and the data/* changes totally unrelated ? ipod_init.glade seems to be missing from that patch as well.

Created attachment 89565 [details] [review]
Updated patch

I updated the patch against SVN trunk and removed the bits which felt out of place and added the missing glade file. Only compile tested.

Created attachment 111056 [details] [review]
small bugfix

I had to apply that patch on top of patch #89565 to get the "new ipod" dialog when plugging a brand new ipod classic. It had an iPod_Control/iTunes directory, which is what the previous patch uses to detect empty ipods, that patch changes it to look for iPod_Control/iTunes/iTunesDB.

This ipod contains an iPod_Control/iTunes/firsttime file, maybe it would be better to use that file to trigger the display of that 1st time dialog. Anyway, all that initialization could be done automatically using hal, but that's more work :-/

Created attachment 111057 [details] [review]
Cumulative patch with patch #89565 and patch #111056 (against svn trunk)

Created attachment 113159 [details] [review]
More work based on doctau's patch

I worked a bit on that new ipod dialog and made some UI improvements : the model list only show ipod model names along with their colors and there are no longer cryptic model numbers appearing in the UI. I changed the GtkComboBoxEntry to a GtkComboBox while I was at it.
I can provide a patch applying on top of patch #111057 if needed

Created attachment 113160 [details]
Screenshot of the dialog

I'd be a bit cautious about using the word 'corrupted' when someone plugs in their shiny new ipod straight out of the box.  Can we tell the difference at all?

What song metadata is lost when reinitialising an ipod?

Isn't it possible to detect the type of iPod? At least using Podsleuth's HAL data if it's available.

We could probably at _least_ detect the size of the device to only offer matching sizes, and probably remove the Unix device node from dialogue.

Jonathan: new ipods have a "firsttime" file on their FS which is removed once the ipod has been initialized once, it might be a good thing to rely on that to detect if we've got a brand new ipod or if we're looking at an ipod with some necessary files that have been removed (we currently detect if the ipod is new by checking if some necessary files is missing).

I don't think itdb_init_ipod (the libgpod function doing the actual initialization) can lose any data, it's just making sure that all the expected directories exist. It can corrupt the ipod model though, which would mean that artwork may not function correctly afterwards. The code in that patch displays the dialog if the ipod doesn't contain a database (ie it doesn't have any song metadata), so in that case we don't risk losing song metadata ;)

Bastien: I don't know why I didn't think about filtering the lists of ipod depending on the size of the plugged device, that's a great idea, I'll post an updated patch when I get a chance to code that ;)

I wasn't sure if the Unix device node should be shown or not, I guess I'll just remove it.

As for autodetecting the iPod type, with the next release of libgpod (and a working libgpod hal callout), the iPod type will be autodetected and with the patch attached to that bug, the detected iPod type will be automatically selected, the combo box is here to let the user choose something else if the autodetection went wrong. Dunno if it's worth coding some additionnal autodetection using podsleuth ?

(In reply to comment #22)
> Bastien: I don't know why I didn't think about filtering the lists of ipod
> depending on the size of the plugged device, that's a great idea, I'll post an
> updated patch when I get a chance to code that ;)

Glad to be of help :)

> I wasn't sure if the Unix device node should be shown or not, I guess I'll just
> remove it.

It should probably appear somewhere in the debug.

> As for autodetecting the iPod type, with the next release of libgpod (and a
> working libgpod hal callout), the iPod type will be autodetected and with the
> patch attached to that bug, the detected iPod type will be automatically
> selected, the combo box is here to let the user choose something else if the
> autodetection went wrong. Dunno if it's worth coding some additionnal
> autodetection using podsleuth ?

Probably not useful to have 2 detection mechanisms (especially as libgpod is already a dependency), but given that the libgpod hal callout will write data into HAL, it might be a good idea for it to use the same properties as podsleuth to do so, no?

Created attachment 113208 [details] [review]
New iteration of the patch

* integrates a reworked version of the glade file from crevette
* removes the device name from the UI, shows it in rb_debug
* only show the models whose capacity matches the capacity of the plugged in device

I wasn't sure how to get the capacity so I used a statvfs and rounded what I got up to the next 512MB. statvfs has portability issues, hal probably provides that info as well, but also has its own portability/availability issues.

At the moment, the models in the combo box are necessarily listed in a 2 level hierarchy but some top level items only have 1 ipod model in them, maybe it would be better show those models in the top level combo, and to only use submenus when we have several items (usually different colors for the same model) ?

(In reply to comment #23)
> 
> Probably not useful to have 2 detection mechanisms (especially as libgpod is
> already a dependency), but given that the libgpod hal callout will write data
> into HAL, it might be a good idea for it to use the same properties as
> podsleuth to do so, no?

For now, the hal callout only talks to the device to get its scsi info page (or whatever that is called) and dumps it to a file (SysInfoExtended) on the ipod. libgpod then parses it and uses what it finds here to infer which ipod model we have. 
I also have some work in progress code in libgpod to put that information in the hal device tree, and the plan is indeed to be as compatible as possible with podsleuth (including key names).

Created attachment 119849 [details] [review]
Patch ported to GIO

This patch is identical to the previous one except that I removed the use of gnome-vfs and used GIO instead.

get_ipod_capacity is probably leaking the GFileInfo object in the patch above.

Created attachment 121395 [details] [review]
Patch applying on svn trunk and without the leak

The only slight concern I have with this is that if HAL is disabled, it looks like we'll display the first time dialog for every mount.  I'm not sure who builds rhythmbox without HAL, though..

Good point, I'll add an additional test to avoid that. A brand new ipod has a characteristic directory structure even if the iTunesDB file itself isn't there, so we can check if it looks like an ipod before displaying the "new ipod" dialog even in the non hal case

Hmm, for now configure.ac has : 

if test "x$have_libgpod" = "xyes"; then
    if test "x$with_hal" = xyes && test "x$enable_hal" = xno; then
        AC_MSG_ERROR([iPod explicitly requested but HAL not found or too ol
    fi
    if test "x$enable_hal" = xyes; then
        AC_DEFINE(WITH_IPOD_SUPPORT, 1, [Define if iPod support is enabled]
                  use_ipod=yes
    fi
.....

ie ipod support is disabled if there is no HAL support. I'll have to find out why, I thought it was supposed to work ;)

Created attachment 122642 [details] [review]
New version of the patch

New version of the patch, the init dialog will only be shown for ipods both in the hal and non-hal case. I moved the responsibility for displaying the "1st time" dialog to rb-ipod-plugin.c instead of doing it in rb-ipod-source.c and I moved some code from rb-ipod-source.c to a new file with helper functions (rb-ipod-helpers.c)

Looks fine to me.

We need agreement from the gtkpod guys to add the gstreamer exception to their code since the original patch was based on that, they seem to be ok with that, Jorg, can you confirm ?

Hi Christophe,

I see no problem in basing the patch on gtkpod's code and agree to the re-licensing (I've written that part myself).

JCS.

I committed the latest patch in this bug :) Thanks everyone!