[Rhythmbox-devel] D-Bus interface enhancements patch



As part of my efforts to get Rhythmbox Applet [0] working [1] with the
development version of Rhythmbox, I've prepared a patch against
Rhythmbox CVS that adds some extra functionality to the D-Bus interface.
This patch provides all the features that my applet needs; it does not
provide all the features offered by the Bonobo interface.  (Of course,
there's no reason why they can't be added later, but first things
first.)

Here's an overview of the changes this patch makes:

 * To the Player object exposed via D-Bus:

   - Adds a getPlaying method to Player that returns true if a song is
     currently playing, and false otherwise.

   - Adds a getPlayingUri method to Player that returns the URI of the
     current song.

   - Adds a getElapsed method to Player that returns the elapsed playback
     time in seconds of the current song.

   - Adds a setElapsed method to Player that allows for seeking to a
     particular time index in the current song.

   - Adds an elapsedChanged signal that fires whenever the value that would
     be returned by getElapsed changes.

 * To RBShellPlayer:

   - Adds an "elapsed_changed" signal to implement elapsedChanged in the
     D-Bus interface.  It fires whenever the (oddly-named)
     "duration_changed" signal does, but provides the elapsed playback
     time in seconds instead of a string of the form "ELAPSED of
     DURATION".

   - Changes when "elapsed_changed" and "duration_changed" signals are
     fired.  Instead of firing them after every RB_PLAYER_TICK_HZ ticks,
     it fires them whenever the elapsed time changes.  This helps reduce
     delays in these signals firing when Rhythmbox is busy with
     something else. (*)

   - Modifies the rb_shell_player_get_playing, _get_playing_path,
     _get_playing_time, and _set_playing_time functions to be callable
     via D-Bus.

   - Modifies the rb_shell_player_get_playing_path function so that it
     does actually return what it's supposed to.

   - Removes the have_url and url private fields, which didn't seem to
     ever actually store the URI of the current song. (*)

 * To RBShell:

   - Adjusts calls to the RBShellPlayer functions that had their
     interfaces modified.

The changes marked with (*) don't strictly need to be part of this
patch, and could be broken out by themselves if desired.  They just
seemed like reasonable changes to make while I was already messing with
those parts of the code.

With these changes, Rhythmbox Applet can be made to happily use the
D-Bus interface instead of the Bonobo interface without any loss in its
functionality.  (The code is implemented and works nicely, but I'm
tentatively waiting to check those changes into my repo until I find out
what changes I might need to make to this patch.)

In [1] I had mentioned wanting to fix the ABI changes in the Bonobo
interface.  After giving that some thought, I suspect it'd be easier to
revert the interface back to how it was in 0.8.8 and add whatever new
RPC features are desired only to the D-Bus interface.  I think it's
*possible* to both preserve compatibility and allow for Bonobo
enhancements if the interface is versioned, but it strikes me as adding
far more complexity than it's worth.

So, if this patch looks good, feel free to add it into CVS.  If there
are problems, let me know and I'll try to take care of them.  Thanks.


[0] http://web.ics.purdue.edu/~kuliniew/rhythmbox-applet/

[1] See thread starting at
    http://mail.gnome.org/archives/rhythmbox-devel/2005-September/msg00056.html
Index: shell/rb-shell-player.c
===================================================================
RCS file: /cvs/gnome/rhythmbox/shell/rb-shell-player.c,v
retrieving revision 1.172
diff -u -r1.172 rb-shell-player.c
--- shell/rb-shell-player.c	27 Sep 2005 11:30:24 -0000	1.172
+++ shell/rb-shell-player.c	5 Oct 2005 03:16:58 -0000
@@ -197,8 +197,6 @@
 	GList *active_uris;
 
 	char *song;
-	gboolean have_url;
-	char *url;
 
 	gboolean have_previous_entry;
 
@@ -251,6 +249,7 @@
 {
 	WINDOW_TITLE_CHANGED,
 	DURATION_CHANGED,
+	ELAPSED_CHANGED,
 	PLAYING_SOURCE_CHANGED,
 	PLAYING_CHANGED,
 	PLAYING_SONG_CHANGED,
@@ -436,6 +435,17 @@
 			      1,
 			      G_TYPE_STRING);
 
+	rb_shell_player_signals[ELAPSED_CHANGED] =
+		g_signal_new ("elapsed_changed",
+			      G_OBJECT_CLASS_TYPE (object_class),
+			      G_SIGNAL_RUN_LAST,
+			      G_STRUCT_OFFSET (RBShellPlayerClass, elapsed_changed),
+			      NULL, NULL,
+			      g_cclosure_marshal_VOID__UINT,
+			      G_TYPE_NONE,
+			      1,
+			      G_TYPE_UINT);
+
 	rb_shell_player_signals[PLAYING_SOURCE_CHANGED] =
 		g_signal_new ("playing-source-changed",
 			      G_OBJECT_CLASS_TYPE (object_class),
@@ -544,8 +554,10 @@
 	gchar *volume_mount_point;
 	RhythmDBEntry *entry;
 	const char *uri;
+	gboolean playing;
 
-	if (rb_shell_player_get_playing (player)) {
+	rb_shell_player_get_playing (player, &playing, NULL);
+	if (playing) {
 		return;
 	}
 
@@ -1752,6 +1764,7 @@
 	char *title;
 	RhythmDBEntry *entry;
 	char *duration;
+	glong elapsed;
 
 	entry = rb_shell_player_get_playing_entry (player);
 	rb_debug ("playing source: %p, active entry: %p", player->priv->source, entry);
@@ -1761,13 +1774,8 @@
 		artist = rb_refstring_get (entry->artist);
 	}
 
-	if (player->priv->have_url)
-		rb_header_set_urldata (player->priv->header_widget,
-				       entry_title,
-				       player->priv->url);
-	else
-		rb_header_set_urldata (player->priv->header_widget,
-				       NULL, NULL);
+	rb_header_set_urldata (player->priv->header_widget,
+			       NULL, NULL);
 
 	if (player->priv->song && entry_title)
 		title = g_strdup_printf ("%s (%s)", player->priv->song,
@@ -1780,11 +1788,14 @@
 		title = NULL;
 
 	duration = rb_header_get_elapsed_string (player->priv->header_widget);
+	elapsed = rb_player_get_time (player->priv->mmplayer);
 
 	g_signal_emit (G_OBJECT (player), rb_shell_player_signals[WINDOW_TITLE_CHANGED], 0,
 		       title);
 	g_signal_emit (G_OBJECT (player), rb_shell_player_signals[DURATION_CHANGED], 0,
 		       duration);
+	g_signal_emit (G_OBJECT (player), rb_shell_player_signals[ELAPSED_CHANGED], 0,
+		       (guint) elapsed);
 	g_free (duration);
 
 	/* Sync the player */
@@ -1935,11 +1946,8 @@
 		rb_play_order_playing_source_changed (player->priv->play_order);
 
 
-	g_free (player->priv->url);
 	g_free (player->priv->song);
 	player->priv->song = NULL;
-	player->priv->url = NULL;
-	player->priv->have_url = FALSE;
 
 	if (source == NULL)
 		rb_shell_player_stop (player);
@@ -1971,11 +1979,17 @@
 }
 
 gboolean
-rb_shell_player_get_playing (RBShellPlayer *player)
+rb_shell_player_get_playing (RBShellPlayer *player,
+			     gboolean *playing,
+			     GError **error)
 {
-	g_return_val_if_fail (RB_IS_SHELL_PLAYER (player), -1);
+	if (playing != NULL) {
+		*playing = rb_player_playing (player->priv->mmplayer);
+	} else {
+		*playing = FALSE;
+	}
 
-	return rb_player_playing (player->priv->mmplayer);
+	return TRUE;
 }
 
 RBPlayer *
@@ -1986,21 +2000,30 @@
 	return player->priv->mmplayer;
 }
 
-long
-rb_shell_player_get_playing_time (RBShellPlayer *player)
+gboolean
+rb_shell_player_get_playing_time (RBShellPlayer *player,
+				  guint *time,
+				  GError **error)
 {
-	g_return_val_if_fail (RB_IS_SHELL_PLAYER (player), 0);
-	
-	return rb_player_get_time (player->priv->mmplayer);
+	*time = (guint) rb_player_get_time (player->priv->mmplayer);
+	return TRUE;
 }
 
-void
-rb_shell_player_set_playing_time (RBShellPlayer *player, long time)
+gboolean
+rb_shell_player_set_playing_time (RBShellPlayer *player,
+				  guint time,
+				  GError **error)
 {
-	g_return_if_fail (RB_IS_SHELL_PLAYER (player));
-	
-	if (rb_player_seekable (player->priv->mmplayer))
-		rb_player_set_time (player->priv->mmplayer, time);
+	if (rb_player_seekable (player->priv->mmplayer)) {
+		rb_player_set_time (player->priv->mmplayer, (long) time);
+		return TRUE;
+	} else {
+		g_set_error (error,
+			     RB_SHELL_PLAYER_ERROR,
+			     RB_SHELL_PLAYER_ERROR_NOT_SEEKABLE,
+			     _("Current song is not seekable"));
+		return FALSE;
+	}
 }
 
 void
@@ -2039,8 +2062,6 @@
 	{
 		rb_debug ("no playing source, new source is %p", player->priv->selected_source);
 
-		player->priv->have_url = rb_source_have_url (player->priv->selected_source);
-
 		rb_shell_player_sync_with_source (player);
 	}
 }
@@ -2167,16 +2188,17 @@
 	rb_header_sync_time (player->priv->header_widget);
 
 	if (rb_player_playing (mmplayer)) {
-		static int callback_runs = 0;
-		callback_runs++;
-		if (callback_runs >= RB_PLAYER_TICK_HZ) {
+		static long last_elapsed = -1;
+		if (last_elapsed != elapsed) {
 			gchar *duration;
 
 			duration = rb_header_get_elapsed_string (player->priv->header_widget);
 			g_signal_emit (G_OBJECT (player), rb_shell_player_signals[DURATION_CHANGED],
 				       0, duration);
+			g_signal_emit (G_OBJECT (player), rb_shell_player_signals[ELAPSED_CHANGED],
+				       0, (guint) elapsed);
 			g_free (duration);
-			callback_runs = 0;
+			last_elapsed = elapsed;
 		}
 	}
 
@@ -2317,10 +2339,21 @@
 }
 
 
-const char *
-rb_shell_player_get_playing_path (RBShellPlayer *shell_player)
+gboolean
+rb_shell_player_get_playing_path (RBShellPlayer *shell_player,
+				  const gchar **path,
+				  GError **error)
 {
-	return shell_player->priv->url;
+	RhythmDBEntry *entry;
+
+	entry = rb_play_order_get_playing_entry (shell_player->priv->play_order);
+	if (entry != NULL) {
+		*path = rhythmdb_entry_get_string (entry, RHYTHMDB_PROP_LOCATION);
+	} else {
+		*path = NULL;
+	}
+
+	return TRUE;
 }
 
 #ifdef HAVE_MMKEYS
@@ -2387,7 +2420,9 @@
 		rb_shell_player_playpause (player, TRUE, NULL);
 		return GDK_FILTER_REMOVE;
 	} else if (XKeysymToKeycode (GDK_DISPLAY (), XF86XK_AudioPause) == key->keycode) {	
-		if (rb_shell_player_get_playing	(player)) {
+		gboolean playing;
+		rb_shell_player_get_playing (player, &playing, NULL);
+		if (playing) {
 			rb_shell_player_playpause (player, TRUE, NULL);
 		}
 		return GDK_FILTER_REMOVE;
Index: shell/rb-shell-player.h
===================================================================
RCS file: /cvs/gnome/rhythmbox/shell/rb-shell-player.h,v
retrieving revision 1.32
diff -u -r1.32 rb-shell-player.h
--- shell/rb-shell-player.h	28 Aug 2005 11:56:54 -0000	1.32
+++ shell/rb-shell-player.h	5 Oct 2005 03:16:58 -0000
@@ -44,7 +44,8 @@
 {
 	RB_SHELL_PLAYER_ERROR_PLAYLIST_PARSE_ERROR,
 	RB_SHELL_PLAYER_ERROR_END_OF_PLAYLIST,
-	RB_SHELL_PLAYER_ERROR_NOT_PLAYING
+	RB_SHELL_PLAYER_ERROR_NOT_PLAYING,
+        RB_SHELL_PLAYER_ERROR_NOT_SEEKABLE,
 } RBShellPlayerError;
 
 #define RB_SHELL_PLAYER_ERROR rb_shell_player_error_quark ()
@@ -66,6 +67,7 @@
 
 	void (*window_title_changed) (RBShellPlayer *player, const char *window_title);
 	void (*duration_changed) (RBShellPlayer *player, const char *duration);
+	void (*elapsed_changed) (RBShellPlayer *player, guint elapsed);
 	void (*playing_changed) (RBShellPlayer *player, gboolean playing);
 	void (*playing_source_changed) (RBShellPlayer *player, RBSource *source);
 	void (*playing_uri_changed) (RBShellPlayer *player, const char *uri);
@@ -94,16 +96,24 @@
 gboolean                rb_shell_player_do_previous	(RBShellPlayer *player, GError **error);
 gboolean		rb_shell_player_do_next		(RBShellPlayer *player, GError **error);
 
-long			rb_shell_player_get_playing_time(RBShellPlayer *player);
-void			rb_shell_player_set_playing_time(RBShellPlayer *player, long time);
+gboolean		rb_shell_player_get_playing_time(RBShellPlayer *player,
+                                                         guint *time,
+                                                         GError **error);
+gboolean		rb_shell_player_set_playing_time(RBShellPlayer *player,
+                                                         guint time,
+                                                         GError **error);
 void			rb_shell_player_seek		(RBShellPlayer *player, long offset);
 long			rb_shell_player_get_playing_song_duration (RBShellPlayer *player);
 
 RBPlayer *		rb_shell_player_get_mm_player	(RBShellPlayer *shell_player);
 
-gboolean		rb_shell_player_get_playing	(RBShellPlayer *shell_player);
-
-const char *		rb_shell_player_get_playing_path(RBShellPlayer *shell_player);
+gboolean		rb_shell_player_get_playing	(RBShellPlayer *shell_player,
+							 gboolean *playing,
+							 GError **error);
+
+gboolean		rb_shell_player_get_playing_path(RBShellPlayer *shell_player,
+							 const gchar **path,
+							 GError **error);
 
 void			rb_shell_player_sync_buttons	(RBShellPlayer *player);
 
Index: shell/rb-shell-player.xml
===================================================================
RCS file: /cvs/gnome/rhythmbox/shell/rb-shell-player.xml,v
retrieving revision 1.2
diff -u -r1.2 rb-shell-player.xml
--- shell/rb-shell-player.xml	28 Aug 2005 11:56:54 -0000	1.2
+++ shell/rb-shell-player.xml	5 Oct 2005 03:16:58 -0000
@@ -16,9 +16,33 @@
       <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="rb_shell_player_do_next"/>
     </method>
 
+    <method name="getPlaying">
+      <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="rb_shell_player_get_playing"/>
+      <arg type="b" name="playing" direction="out"/>
+    </method>
+
+    <method name="getPlayingUri">
+      <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="rb_shell_player_get_playing_path"/>
+      <arg type="s" name="uri" direction="out">
+        <annotation name="org.freedesktop.DBus.GLib.Const" value=""/>
+      </arg>
+    </method>
+
+    <method name="getElapsed">
+      <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="rb_shell_player_get_playing_time"/>
+      <arg type="u" name="elapsed" direction="out"/>
+    </method>
+
+    <method name="setElapsed">
+      <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="rb_shell_player_set_playing_time"/>
+      <arg type="u" name="elapsed" direction="in"/>
+    </method>
+
+
     <!-- <property name="playing" access="readwrite"/> -->
     <signal name="playingChanged"/>
     <signal name="playingUriChanged"/>
+    <signal name="elapsedChanged"/>
 
   </interface>
 </node>
Index: shell/rb-shell.c
===================================================================
RCS file: /cvs/gnome/rhythmbox/shell/rb-shell.c,v
retrieving revision 1.338
diff -u -r1.338 rb-shell.c
--- shell/rb-shell.c	28 Sep 2005 14:28:25 -0000	1.338
+++ shell/rb-shell.c	5 Oct 2005 03:16:58 -0000
@@ -1550,9 +1550,11 @@
 static void
 rb_shell_set_duration (RBShell *shell, const char *duration)
 {
-	gboolean playing = rb_shell_player_get_playing (shell->priv->player_shell);
+	gboolean playing;
 	char *tooltip;
 
+	rb_shell_player_get_playing (shell->priv->player_shell, &playing, NULL);
+
 	if (shell->priv->cached_title == NULL) 
 		tooltip = g_strdup (_("Not playing"));
 	else if (!playing) {
@@ -1582,9 +1584,11 @@
 				      _("Music Player"));
 	}
 	else {
-		gboolean playing = rb_shell_player_get_playing (shell->priv->player_shell);
+		gboolean playing;
 		char *title;
 
+		rb_shell_player_get_playing (shell->priv->player_shell, &playing, NULL);
+
 		if (shell->priv->cached_title &&
 		    !strcmp (shell->priv->cached_title, window_title)) {
 			return;
@@ -2512,21 +2516,25 @@
 rb_shell_playing_impl (RBRemoteProxy *proxy)
 {
 	RBShellPlayer *player = RB_SHELL (proxy)->priv->player_shell;
-	return rb_shell_player_get_playing (player);
+	gboolean playing;
+	rb_shell_player_get_playing (player, &playing, NULL);
+	return playing;
 }
 
 static long
 rb_shell_get_playing_time_impl (RBRemoteProxy *proxy)
 {
 	RBShellPlayer *player = RB_SHELL (proxy)->priv->player_shell;
-	return rb_shell_player_get_playing_time (player);
+	guint time;
+	rb_shell_player_get_playing_time (player, &time, NULL);
+	return (long) time;
 }
 
 static void
 rb_shell_set_playing_time_impl (RBRemoteProxy *proxy, long time)
 {
 	RBShellPlayer *player = RB_SHELL (proxy)->priv->player_shell;
-	rb_shell_player_set_playing_time (player, time);
+	rb_shell_player_set_playing_time (player, (guint) time, NULL);
 }
 
 static gchar*

Attachment: signature.asc
Description: Digital signature



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]