GNOME Bugzilla – Bug 338443
Make ipod support a plugin
Last modified: 2006-04-17 14:36:48 UTC
Making ipod support a plugin would be handy for packagers that want to not make the main Rhythmbox package depend on libgpod, but want ipod support too.
Created attachment 63448 [details] [review] patch Adds a signal to the removable media manager that plugins can connect to, to add sources when media is plugged in. Not having an ipod, I haven't actually tested that it works, but it should.
Created attachment 63452 [details] [review] fixed patch Here ones that doesn't explode all the time.
Created attachment 63631 [details] [review] Updated patch This new patch has been adjusted because of the commit of the patch from bug #338561
Created attachment 63643 [details] [review] Patch to apply on top of the previous one fixing various issues I made an interdiff between my current changes and the patch I posted previously to make it easier to spot the bug I encountered. There are still remaining issues with gtk_ui_manager_add_ui_from_file which I haven't investigated yet, and the iPod source isn't shown/hidden automatically when the plugin is activated/unactivated, but apart from that it's working (from a cursory test)
Created attachment 63650 [details] [review] Patch which should be committable So I figured out what was wrong with the GTKUIManager stuff. In the iPod source, I define a new iPodSourceRename action which is then used in the iPod popup menu UI file. However, if I move the call to gtk_ui_manager_add_ui_from_file from RBiPodSource to RBiPodPlugin, this can't work since the iPodSourceRename must be known to GTK+ *before* the UI file is loaded. And iPodSourceRename is registered with rb_source_register_action_group which uses gtk_action_group_add_actions, which creates the actions for use by the GtkUIManager, but also attaches a callback to those actions with the current RBiPodSource as 'user_data', so it's a bit difficult to move that to RBiPodPlugin. It's probably possible to use other parts of the GtkAction API to get that right (ie register the actions and install the callbacks separately), but I don't understand GtkAction perfectly, and I'm a bit tired of fighting with that stuff for now, so I decided to post that patch since it's working even if it could be better.
After thinking about it, registering the action for each source is probably the wrong thing to do anyway. I think it has the potential to break horribly is there were two ipod sources: a) source A gets created, and registers the action with A as the userdata b) source B gets created, and fails to register the action since A already did with that name c) source A gets deleted d) the user tries to rename B, and either the action doesn't exist (if the actions got unregistered in (c) ) the action callback gets run with userdata=A (if it didn't) What I think should happen is that the plugin registers the action with userdata=plugin. The plugins's action handler uses the shell (which the plugin has a reference to) to retrieve the currently selected source, check to make sure it's an ipod source, and then calls rb_ipod_source_do_rename ().
Created attachment 63666 [details] [review] patch with improvements This version does the above, and also hopefully detects ipods when the plugin is activates and removes then when deactivated. Testing appreciated.
Created attachment 63707 [details] [review] Updated patch This patch changes RBRemovableMediaManager a bit to remove sources from the volume_mapping hash when the "deleted" RBSource signal is received. This nearly makes iPod appear/disappear correctly when the plugin is loaded/unloaded. The only remaining issue is that when you start with the plugin disabled, the iPod is seen as a generic mass storage source, and when you enable the plugin, the iPod doesn't appear because of that.
Committed that last patch to CVS