rpms/rhythmbox/F-12 rb-playbin2-track-changes.patch, NONE, 1.1 rhythmbox.spec, 1.275, 1.276

[Bastien Nocera]

Author: hadess

Update of /cvs/pkgs/rpms/rhythmbox/F-12
In directory cvs1.fedora.phx.redhat.com:/tmp/cvs-serv13089

Modified Files:
	rhythmbox.spec 
Added Files:
	rb-playbin2-track-changes.patch 
Log Message:
* Wed Jan 06 2010 Bastien Nocera <bnocera at redhat.com> 0.12.6-3
- Fix potential hangs on track changes


rb-playbin2-track-changes.patch:
 rb-player-gst.c |  131 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 88 insertions(+), 43 deletions(-)

--- NEW FILE rb-playbin2-track-changes.patch ---
commit c258166cef4aa61a59f78c142ea7140490126ce4
Author: Jonathan Matthew <jonathan at d14n.org>
Date:   Mon Dec 28 17:10:15 2009 +1000

    playbin2: fix track changes (bug #601524, bug #602957)
    
    Rather than emitting the playing stream change signal immediately, and
    pretending the playback position is 0 until the next tick, we wait until
    either we get a playbin2-stream-changed signal or a position query
    returns a position inside the first second.  This means the playback
    position should always be consistent, and the track change is reflected
    in the UI closer to when it actually occurs.
    
    This also means we don't emit the playing-stream signal on a streaming
    thread, which fixes some threading problems with a11y enabled.

diff --git a/backends/gstreamer/rb-player-gst.c b/backends/gstreamer/rb-player-gst.c
index 8d485fd..9028130 100644
--- a/backends/gstreamer/rb-player-gst.c
+++ b/backends/gstreamer/rb-player-gst.c
@@ -91,6 +91,8 @@ struct _RBPlayerGstPrivate
 	char *uri;
 	gpointer stream_data;
 	GDestroyNotify stream_data_destroy;
+	gpointer next_stream_data;
+	GDestroyNotify next_stream_data_destroy;
 
 	GstElement *playbin;
 	GstElement *audio_sink;
@@ -101,7 +103,7 @@ struct _RBPlayerGstPrivate
 
 	gboolean stream_change_pending;
 	gboolean current_track_finishing;
-	gboolean fake_playing_position;
+	gboolean playbin_stream_changing;
 
 	gboolean emitted_error;
 
@@ -122,19 +124,25 @@ struct _RBPlayerGstPrivate
 	GstElement *filterbin;
 };
 
-static gboolean
-tick_timeout (RBPlayerGst *mp)
+static void
+_destroy_stream_data (RBPlayerGst *player)
 {
-	if (mp->priv->playing) {
-		_rb_player_emit_tick (RB_PLAYER (mp),
-				      mp->priv->stream_data,
-				      rb_player_get_time (RB_PLAYER (mp)),
-				      -1);
-		mp->priv->fake_playing_position = FALSE;
+	if (player->priv->stream_data && player->priv->stream_data_destroy) {
+		player->priv->stream_data_destroy (player->priv->stream_data);
 	}
-	return TRUE;
+	player->priv->stream_data = NULL;
+	player->priv->stream_data_destroy = NULL;
 }
 
+static void
+_destroy_next_stream_data (RBPlayerGst *player)
+{
+	if (player->priv->next_stream_data && player->priv->next_stream_data_destroy) {
+		player->priv->next_stream_data_destroy (player->priv->next_stream_data);
+	}
+	player->priv->next_stream_data = NULL;
+	player->priv->next_stream_data_destroy = NULL;
+}
 
 static void
 about_to_finish_cb (GstElement *playbin, RBPlayerGst *player)
@@ -214,6 +222,35 @@ process_tag (const GstTagList *list, const gchar *tag, RBPlayerGst *player)
 }
 
 static void
+emit_playing_stream_and_tags (RBPlayerGst *player, gboolean track_change)
+{
+	GList *t;
+
+	if (track_change) {
+		/* swap stream data */
+		_destroy_stream_data (player);
+		player->priv->stream_data = player->priv->next_stream_data;
+		player->priv->stream_data_destroy = player->priv->next_stream_data_destroy;
+		player->priv->next_stream_data = NULL;
+		player->priv->next_stream_data_destroy = NULL;
+	}
+
+	_rb_player_emit_playing_stream (RB_PLAYER (player), player->priv->stream_data);
+
+	/* process any tag lists we received while starting the stream */
+	for (t = player->priv->stream_tags; t != NULL; t = t->next) {
+		GstTagList *tags;
+
+		tags = (GstTagList *)t->data;
+		rb_debug ("processing buffered taglist");
+		gst_tag_list_foreach (tags, (GstTagForeachFunc) process_tag, player);
+		gst_tag_list_free (tags);
+	}
+	g_list_free (player->priv->stream_tags);
+	player->priv->stream_tags = NULL;
+}
+
+static void
 handle_missing_plugin_message (RBPlayerGst *player, GstMessage *message)
 {
 	char **details;
@@ -351,8 +388,14 @@ bus_cb (GstBus *bus, GstMessage *message, RBPlayerGst *mp)
 		break;
 
 	case GST_MESSAGE_ELEMENT:
+		structure = gst_message_get_structure (message);
 		if (gst_is_missing_plugin_message (message)) {
 			handle_missing_plugin_message (mp, message);
+		} else if (mp->priv->playbin_stream_changing &&
+			   gst_structure_has_name (structure, "playbin2-stream-changed")) {
+			rb_debug ("got playbin2-stream-changed message");
+			mp->priv->playbin_stream_changing = FALSE;
+			emit_playing_stream_and_tags (mp, TRUE);
 		}
 		break;
 
@@ -637,16 +680,32 @@ set_state_and_wait (RBPlayerGst *player, GstState target, GError **error)
 	return result;
 }
 
-static void
-_destroy_stream_data (RBPlayerGst *player)
+static gboolean
+tick_timeout (RBPlayerGst *mp)
 {
-	if (player->priv->stream_data && player->priv->stream_data_destroy) {
-		player->priv->stream_data_destroy (player->priv->stream_data);
+	if (mp->priv->playing) {
+		gint64 position;
+
+		position = rb_player_get_time (RB_PLAYER (mp));
+
+		/* if we don't have stream-changed messages, do the track change when
+		 * the playback position is less than one second into the current track,
+		 * which pretty much has to be the new one.
+		 */
+		if (mp->priv->playbin_stream_changing && (position < GST_SECOND)) {
+			emit_playing_stream_and_tags (mp, TRUE);
+			mp->priv->playbin_stream_changing = FALSE;
+		}
+
+		_rb_player_emit_tick (RB_PLAYER (mp),
+				      mp->priv->stream_data,
+				      position,
+				      -1);
 	}
-	player->priv->stream_data = NULL;
-	player->priv->stream_data_destroy = NULL;
+	return TRUE;
 }
 
+
 static gboolean
 impl_close (RBPlayer *player, const char *uri, GError **error)
 {
@@ -662,6 +721,9 @@ impl_close (RBPlayer *player, const char *uri, GError **error)
 	mp->priv->current_track_finishing = FALSE;
 
 	_destroy_stream_data (mp);
+	if (uri == NULL) {
+		_destroy_next_stream_data (mp);
+	}
 	g_free (mp->priv->uri);
 	g_free (mp->priv->prev_uri);
 	mp->priv->uri = NULL;
@@ -701,12 +763,12 @@ impl_open (RBPlayer *player,
 	}
 
 	rb_debug ("setting new uri to %s", uri);
-	_destroy_stream_data (mp);
+	_destroy_next_stream_data (mp);
 	g_free (mp->priv->prev_uri);
 	mp->priv->prev_uri = mp->priv->uri;
 	mp->priv->uri = g_strdup (uri);
-	mp->priv->stream_data = stream_data;
-	mp->priv->stream_data_destroy = stream_data_destroy;
+	mp->priv->next_stream_data = stream_data;
+	mp->priv->next_stream_data_destroy = stream_data_destroy;
 	mp->priv->emitted_error = FALSE;
 	mp->priv->stream_change_pending = TRUE;
 
@@ -745,23 +807,21 @@ impl_play (RBPlayer *player, RBPlayerPlayType play_type, gint64 crossfade, GErro
 {
 	RBPlayerGst *mp = RB_PLAYER_GST (player);
 	gboolean result;
+	gboolean track_change = TRUE;
 
 	g_return_val_if_fail (mp->priv->playbin != NULL, FALSE);
 
 	if (mp->priv->stream_change_pending == FALSE) {
 		rb_debug ("no stream change pending, just restarting playback");
 		result = set_state_and_wait (mp, GST_STATE_PLAYING, error);
+		track_change = FALSE;
 
 	} else if (mp->priv->current_track_finishing) {
 		rb_debug ("current track finishing -> just setting URI on playbin");
 		g_object_set (mp->priv->playbin, "uri", mp->priv->uri, NULL);
 		result = TRUE;
 
-		/* since we don't know exactly when the change occurs, pretend the playing position
-		 * is 0 until the next tick.  otherwise we sometimes get the playing position from the
-		 * previous track.
-		 */
-		mp->priv->fake_playing_position = TRUE;
+		mp->priv->playbin_stream_changing = TRUE;
 
 	} else {
 		gboolean reused = FALSE;
@@ -797,13 +857,13 @@ impl_play (RBPlayer *player, RBPlayerPlayType play_type, gint64 crossfade, GErro
 	mp->priv->stream_change_pending = FALSE;
 
 	if (result) {
-		GList *t;
-
 		mp->priv->current_track_finishing = FALSE;
 		mp->priv->buffering = FALSE;
 		mp->priv->playing = TRUE;
 
-		_rb_player_emit_playing_stream (RB_PLAYER (mp), mp->priv->stream_data);
+		if (mp->priv->playbin_stream_changing == FALSE) {
+			emit_playing_stream_and_tags (mp, track_change);
+		}
 
 		if (mp->priv->tick_timeout_id == 0) {
 			mp->priv->tick_timeout_id =
@@ -833,18 +893,6 @@ impl_play (RBPlayer *player, RBPlayerPlayType play_type, gint64 crossfade, GErro
 
 			mp->priv->volume_applied = mp->priv->volume_changed;
 		}
-
-		/* process any tag lists we received while starting the stream */
-		for (t = mp->priv->stream_tags; t != NULL; t = t->next) {
-			GstTagList *tags;
-
-			tags = (GstTagList *)t->data;
-			rb_debug ("processing buffered taglist");
-			gst_tag_list_foreach (tags, (GstTagForeachFunc) process_tag, mp);
-			gst_tag_list_free (tags);
-		}
-		g_list_free (mp->priv->stream_tags);
-		mp->priv->stream_tags = NULL;
 	}
 
 	return result;
@@ -1011,10 +1059,7 @@ impl_get_time (RBPlayer *player)
 {
 	RBPlayerGst *mp = RB_PLAYER_GST (player);
 
-	if (mp->priv->fake_playing_position) {
-		/* track transition occurring, so pretend we're at the start of the new one */
-		return 0;
-	} else if (mp->priv->playbin != NULL) {
+	if (mp->priv->playbin != NULL) {
 		gint64 position = -1;
 		GstFormat fmt = GST_FORMAT_TIME;
 


Index: rhythmbox.spec
===================================================================
RCS file: /cvs/pkgs/rpms/rhythmbox/F-12/rhythmbox.spec,v
retrieving revision 1.275
retrieving revision 1.276
diff -u -p -r1.275 -r1.276
--- rhythmbox.spec	10 Dec 2009 15:45:23 -0000	1.275
+++ rhythmbox.spec	6 Jan 2010 13:36:26 -0000	1.276
@@ -3,7 +3,7 @@
 Name: rhythmbox
 Summary: Music Management Application
 Version: 0.12.6
-Release: 2%{?dist}
+Release: 3%{?dist}
 License: GPLv2+ with exceptions and GFDL
 Group: Applications/Multimedia
 URL: http://projects.gnome.org/rhythmbox/
@@ -55,6 +55,9 @@ Patch0: rb-no-HEAD-for-podcasts.patch
 # https://bugzilla.redhat.com/show_bug.cgi?id=540672
 # https://bugzilla.gnome.org/show_bug.cgi?id=602140
 Patch1: rb-webkit-crasher.patch
+# https://bugzilla.gnome.org/show_bug.cgi?id=601524
+# https://bugzilla.gnome.org/show_bug.cgi?id=602957
+Patch2: rb-playbin2-track-changes.patch
 
 %description
 Rhythmbox is an integrated music management application based on the powerful
@@ -221,6 +224,9 @@ fi
 %{_libdir}/rhythmbox/plugins/upnp_coherence
 
 %changelog
+* Wed Jan 06 2010 Bastien Nocera <bnocera at redhat.com> 0.12.6-3
+- Fix potential hangs on track changes
+
 * Thu Dec 10 2009 Bastien Nocera <bnocera at redhat.com> 0.12.6-2
 - Fix crasher in WebKit when using the context pane (#540672)