[rhythmbox] fix interaction with pulseaudio flat volume (bug #584958)

[Jonathan Matthew <jmatthew src gnome org>]

commit 65f1f82ef9fda2f878be45aeef5ab26dbcf0beab
Author: Jonathan Matthew <jonathan d14n org>
Date:   Mon Jun 15 22:12:33 2009 +1000

    fix interaction with pulseaudio flat volume (bug #584958)
    
    First, we count the number of volume changes that have been made, and
    keep track of the last change to be applied to the pipeline.  Then,
    when starting the pipeline, we apply any volume changes that were made
    while the pipeline was not running.
    
    The extra trick here is that when the audio sink provides volume
    control, we ignore the first volume setting, which restores the volume
    setting from gconf.  The reason we do this is that pulseaudio remembers
    our volume for itself and applies it correctly, without modifying the
    reference output volume.
    
    This change also fixes the volume change notification in the playbin2
    backend, which was using the wrong signal.

 backends/gstreamer/rb-player-gst-xfade.c |   21 +++++++++----
 backends/gstreamer/rb-player-gst.c       |   49 ++++++++++++++++++++++++++----
 2 files changed, 58 insertions(+), 12 deletions(-)
---
diff --git a/backends/gstreamer/rb-player-gst-xfade.c b/backends/gstreamer/rb-player-gst-xfade.c
index c6ba5e8..44638e7 100644
--- a/backends/gstreamer/rb-player-gst-xfade.c
+++ b/backends/gstreamer/rb-player-gst-xfade.c
@@ -275,11 +275,8 @@ struct _RBPlayerGstXFadePrivate
 	GList *streams;
 	gint linked_streams;
 
-	gboolean can_signal_direct_error;
-	GError *error;
-
-	gboolean playing;
-
+	int volume_changed;
+	int volume_applied;
 	float cur_volume;
 	guint buffer_size;	/* kB */
 
@@ -2802,9 +2799,19 @@ start_sink_locked (RBPlayerGstXFade *player, GList **messages, GError **error)
 	if (player->priv->volume_handler == NULL) {
 		rb_debug ("sink doesn't provide volume control, using volume element");
 		player->priv->volume_handler = g_object_ref (player->priv->volume);
+	} else if (player->priv->volume_applied == 0) {
+		/* ignore the initial volume setting, allowing the
+		 * sink to restore its own volume.
+		 */
+		player->priv->volume_applied = 1;
+	}
+
+	/* if there has been a volume change that we haven't applied, apply it now */
+	if (player->priv->volume_applied < player->priv->volume_changed) {
+		g_object_set (player->priv->volume_handler, "volume", player->priv->cur_volume, NULL);
+		player->priv->volume_applied = player->priv->volume_changed;
 	}
 
-	g_object_set (player->priv->volume_handler, "volume", player->priv->cur_volume, NULL);
 	g_signal_connect_object (player->priv->volume_handler,
 				 "notify::volume",
 				 G_CALLBACK (stream_volume_changed),
@@ -3688,11 +3695,13 @@ rb_player_gst_xfade_set_volume (RBPlayer *iplayer, float volume)
 {
 	RBPlayerGstXFade *player = RB_PLAYER_GST_XFADE (iplayer);
 
+	player->priv->volume_changed++;
 	if (player->priv->volume_handler != NULL) {
 		gdouble v = (gdouble)volume;
 
 		/* maybe use a controller here for smoother changes? */
 		g_object_set (player->priv->volume_handler, "volume", v, NULL);
+		player->priv->volume_applied = player->priv->volume_changed;
 	}
 	player->priv->cur_volume = volume;
 }
diff --git a/backends/gstreamer/rb-player-gst.c b/backends/gstreamer/rb-player-gst.c
index 20dd373..fb94f1d 100644
--- a/backends/gstreamer/rb-player-gst.c
+++ b/backends/gstreamer/rb-player-gst.c
@@ -102,6 +102,8 @@ struct _RBPlayerGstPrivate
 
 	gboolean emitted_error;
 
+	gint volume_changed;
+	gint volume_applied;
 	float cur_volume;
 	float replaygain_scale;
 
@@ -158,7 +160,7 @@ emit_volume_changed_idle (RBPlayerGst *player)
 }
 
 static void
-volume_notify_cb (GObject *element, GParamSpec *pspec, RBPlayerGst *player)
+volume_notify_cb (GObject *element, GstObject *prop_object, GParamSpec *pspec, RBPlayerGst *player)
 {
 	gdouble v;
 	g_object_get (element, "volume", &v, NULL);
@@ -372,7 +374,7 @@ construct_pipeline (RBPlayerGst *mp, GError **error)
 				 G_CALLBACK (about_to_finish_cb),
 				 mp, 0);
 	g_signal_connect_object (G_OBJECT (mp->priv->playbin),
-				 "notify::volume",
+				 "deep-notify::volume",
 				 G_CALLBACK (volume_notify_cb),
 				 mp, 0);
 	g_signal_connect_object (G_OBJECT (mp->priv->playbin),
@@ -486,8 +488,6 @@ construct_pipeline (RBPlayerGst *mp, GError **error)
 		mp->priv->cur_volume = 0;
 	mp->priv->replaygain_scale = 1.0f;
 
-	rb_player_set_volume (RB_PLAYER (mp), mp->priv->cur_volume);
-
 	rb_debug ("pipeline construction complete");
 	return TRUE;
 }
@@ -714,6 +714,17 @@ impl_opened (RBPlayer *player)
 	return mp->priv->uri != NULL;
 }
 
+static void
+set_playbin_volume (RBPlayerGst *player, float volume)
+{
+	/* ignore the deep-notify we get directly from the sink, as it causes deadlock.
+	 * we still get another one anyway.
+	 */
+	g_signal_handlers_block_by_func (player->priv->playbin, volume_notify_cb, player);
+	g_object_set (player->priv->playbin, "volume", volume, NULL);
+	g_signal_handlers_unblock_by_func (player->priv->playbin, volume_notify_cb, player);
+}
+
 static gboolean
 impl_play (RBPlayer *player, RBPlayerPlayType play_type, gint64 crossfade, GError **error)
 {
@@ -783,6 +794,28 @@ impl_play (RBPlayer *player, RBPlayerPlayType play_type, gint64 crossfade, GErro
 					       (GSourceFunc) tick_timeout,
 					       mp);
 		}
+
+		if (mp->priv->volume_applied == 0) {
+			GstElement *e;
+
+			/* if the sink provides volume control, ignore the first
+			 * volume setting, allowing the sink to restore its own
+			 * volume.
+			 */
+			e = rb_player_gst_find_element_with_property (mp->priv->audio_sink, "volume");
+			if (e != NULL) {
+				mp->priv->volume_applied = 1;
+			}
+			gst_object_unref (e);
+
+			if (mp->priv->volume_applied < mp->priv->volume_changed) {
+				float volume = mp->priv->cur_volume * mp->priv->replaygain_scale;
+				rb_debug ("applying initial volume: %f", volume);
+				set_playbin_volume (mp, volume);
+			}
+
+			mp->priv->volume_applied = mp->priv->volume_changed;
+		}
 	}
 
 	return result;
@@ -871,8 +904,12 @@ impl_set_volume (RBPlayer *player,
 	RBPlayerGst *mp = RB_PLAYER_GST (player);
 	g_return_if_fail (volume >= 0.0 && volume <= 1.0);
 
-	if (mp->priv->playbin != NULL) {
-		g_object_set (mp->priv->playbin, "volume", volume * mp->priv->replaygain_scale, NULL);
+	mp->priv->volume_changed++;
+	if (mp->priv->volume_applied > 0) {
+		set_playbin_volume (mp, volume * mp->priv->replaygain_scale);
+		mp->priv->volume_applied = mp->priv->volume_changed;
+	} else {
+		/* volume will be applied in the first call to impl_play */
 	}
 
 	mp->priv->cur_volume = volume;