After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 338443 - Make ipod support a plugin
Make ipod support a plugin
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: Removable Media
HEAD
Other Linux
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks: 338564
 
 
Reported: 2006-04-14 10:09 UTC by James "Doc" Livingston
Modified: 2006-04-17 14:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (11.77 KB, patch)
2006-04-14 10:10 UTC, James "Doc" Livingston
none Details | Review
fixed patch (12.66 KB, patch)
2006-04-14 12:04 UTC, James "Doc" Livingston
none Details | Review
Updated patch (17.31 KB, patch)
2006-04-16 11:05 UTC, Christophe Fergeau
none Details | Review
Patch to apply on top of the previous one fixing various issues (5.12 KB, patch)
2006-04-16 15:37 UTC, Christophe Fergeau
none Details | Review
Patch which should be committable (22.26 KB, patch)
2006-04-16 17:58 UTC, Christophe Fergeau
none Details | Review
patch with improvements (25.51 KB, patch)
2006-04-17 05:38 UTC, James "Doc" Livingston
none Details | Review
Updated patch (29.67 KB, patch)
2006-04-17 13:38 UTC, Christophe Fergeau
none Details | Review

Description James "Doc" Livingston 2006-04-14 10:09:30 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.
Comment 1 James "Doc" Livingston 2006-04-14 10:10:48 UTC
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.
Comment 2 James "Doc" Livingston 2006-04-14 12:04:14 UTC
Created attachment 63452 [details] [review]
fixed patch

Here ones that doesn't explode all the time.
Comment 3 Christophe Fergeau 2006-04-16 11:05:18 UTC
Created attachment 63631 [details] [review]
Updated patch

This new patch has been adjusted because of the commit of the patch from bug #338561
Comment 4 Christophe Fergeau 2006-04-16 15:37:54 UTC
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)
Comment 5 Christophe Fergeau 2006-04-16 17:58:01 UTC
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.
Comment 6 James "Doc" Livingston 2006-04-17 04:22:27 UTC
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 ().
Comment 7 James "Doc" Livingston 2006-04-17 05:38:00 UTC
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.
Comment 8 Christophe Fergeau 2006-04-17 13:38:42 UTC
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.
Comment 9 Christophe Fergeau 2006-04-17 14:36:48 UTC
Committed that last patch to CVS