GNOME Bugzilla – Bug 145327
patch: CORBA setRating
Last modified: 2004-12-22 21:47:04 UTC
Attached is a patch against Rhythmbox 0.8.4 that allows the current rating to be modified through Bonobo as well as making the shuffle and repeat properties read-write.
Created attachment 29171 [details] [review] patch to add setRating and make shuffle/repeat read-write
In rb_shell_corba_set_rating I'd add a g_value_unset when the gvalue is no longer needed. Apart from that, it looks nice
Created attachment 29313 [details] [review] updated patch updated patch to use g_value_unset
This looks good to me, I will apply after I get 0.9 mapped to CVS.
Created attachment 29498 [details] [review] extended to include corba bindings for volume/skipping etc. I've attached the second version of the patch with some modifications that I made for an IR daemon that controls rhythmbox (daemon will follow later). In addition to the shuffle/repeat/rating, it adds ; - toggleMute - toggleHide (does a gtk_window_(de)iconify) - get/setVolume - volumeUp/Down (changes current volume by +/-0.1) - skip (takes an offset in seconds to seek backwards/forwards) Also added commandline arguments to rhythmbox for the above plus --rate-up/down, so this also fixes bug 133061.
I haven't looked at the patch in detail, but here are a few random questions (maybe stupid, maybe relevant, just tell me ;) * rhythmbox is starting to have a lot of command line flags, maybe it would be a good idea to remove all the controlling flags and provide a rhythmbox-remote script/binary instead which implement all those flags ? This would also be a nice sample code for the bonobo interface * the corba code is getting a bit huge in rb-shell.c, maybe it could be moved to a rb-shell-corba.c file? (I don't remember if this code accesses private RBShell data, it probably does though :-/) * maybe the volume set/get and the mute toggle should be gobject properties in RBShellPlayer. Iirc it's a bit easier to wrap in bonobo, and you can also get notifications when those values change (probably not really useful though).
ad 1) yes, and test-corba is actually almost already such a tool. Personally I have no strong opinion on using rhythmbox or another binary to remotely control rhythmbox. ad 2) it's getting big yes, and it accesses private members. Don't know what the best solution on this one is, maybe move the code and add _get methods for the private members ? ad 3) That's a good point, I'll take a stab at making volume/mute/hide be properties instead.
For 2), a possible solution would be a #include "rb-shell-corba.c" in rb-shell.c, though this is a bit ugly
Thanks, I applied this: * committed rhythmbox-devel@gnome.org--2004/rhythmbox--main--0.8--patch-111 TODO: look into porting the changes to 0.9. It would be nice to add these to the D-BUS interface...