Change volume to an object with an attached reason so we can make better choices up the stack

3 files changed, 90 insertions, 88 deletions

diff --git a/src/service.vala b/src/service.vala
index 14a49da..c19da9d 100644
--- a/src/service.vala
+++ b/src/service.vala
@@ -173,7 +173,7 @@ public class IndicatorSound.Service: Object {
 			}
 
 			/* Normalize volume, because the volume action's state is [0.0, 1.0], see create_volume_action() */
-			this.actions.change_action_state ("volume", this.volume_control.volume / this.max_volume);
+			this.actions.change_action_state ("volume", this.volume_control.volume.volume / this.max_volume);
 		}
 	}
 
@@ -210,8 +210,11 @@ public class IndicatorSound.Service: Object {
 	void activate_scroll_action (SimpleAction action, Variant? param) {
 		int delta = param.get_int32(); /* positive for up, negative for down */
 
-		double v = this.volume_control.volume + volume_step_percentage * delta;
-		this.volume_control.volume = v.clamp (0.0, this.max_volume);
+		var scrollvol = new VolumeControl.Volume();
+		double v = this.volume_control.volume.volume + volume_step_percentage * delta;
+		scrollvol.volume = v.clamp (0.0, this.max_volume);
+		scrollvol.reason = VolumeControl.VolumeReasons.USER_KEYPRESS;
+		this.volume_control.volume = scrollvol;
 	}
 
 	void activate_desktop_settings (SimpleAction action, Variant? param) {
@@ -246,7 +249,7 @@ public class IndicatorSound.Service: Object {
 	}
 
 	void update_root_icon () {
-		double volume = this.volume_control.volume;
+		double volume = this.volume_control.volume.volume;
 		string icon;
 		if (this.volume_control.mute)
 			icon = this.mute_blocks_sound ? "audio-volume-muted-blocking-panel" : "audio-volume-muted-panel";
@@ -283,8 +286,6 @@ public class IndicatorSound.Service: Object {
 
 	private bool check_sync_notification = false;
 	private bool support_sync_notification = false;
-	private string last_output_notification = "multimedia";
-	private double last_volume_notification = 0;
 
 	void update_sync_notification () {
 		if (!check_sync_notification) {
@@ -310,11 +311,11 @@ public class IndicatorSound.Service: Object {
 
 		/* Choose an icon */
 		string icon = "audio-volume-muted";
-		if (volume_control.volume <= 0.0)
+		if (volume_control.volume.volume <= 0.0)
 			icon = "audio-volume-muted";
-		else if (volume_control.volume <= 0.3)
+		else if (volume_control.volume.volume <= 0.3)
 			icon = "audio-volume-low";
-		else if (volume_control.volume <= 0.7)
+		else if (volume_control.volume.volume <= 0.7)
 			icon = "audio-volume-medium";
 		else
 			icon = "audio-volume-high";
@@ -327,7 +328,7 @@ public class IndicatorSound.Service: Object {
 		/* Put it all into the notification */
 		sync_notification.clear_hints ();
 		sync_notification.update (_("Volume"), volume_label, icon);
-		sync_notification.set_hint ("value", (int32)(volume_control.volume * 100.0));
+		sync_notification.set_hint ("value", (int32)(volume_control.volume.volume * 100.0));
 		sync_notification.set_hint ("x-canonical-value-bar-tint", tint);
 		sync_notification.set_hint ("x-canonical-private-synchronous", "true");
 		sync_notification.set_hint ("x-canonical-non-shaped-icon", "true");
@@ -425,44 +426,40 @@ public class IndicatorSound.Service: Object {
 		 * volume_control.set_volume().
 		 */
 
-		double volume = this.volume_control.volume / this.max_volume;
+		double volume = this.volume_control.volume.volume / this.max_volume;
 
 		volume_action = new SimpleAction.stateful ("volume", VariantType.INT32, new Variant.double (volume));
 
 		volume_action.change_state.connect ( (action, val) => {
 			double v = val.get_double () * this.max_volume;
-			volume_control.volume = v.clamp (0.0, this.max_volume);
+
+			var vol = new VolumeControl.Volume();
+			vol.volume = v.clamp (0.0, this.max_volume);
+			vol.reason = VolumeControl.VolumeReasons.USER_KEYPRESS;
+			volume_control.volume = vol;
 		});
 
 		/* activating this action changes the volume by the amount given in the parameter */
 		volume_action.activate.connect ( (action, param) => {
 			int delta = param.get_int32 ();
-			double v = volume_control.volume + volume_step_percentage * delta;
-			volume_control.volume = v.clamp (0.0, this.max_volume);
+			double v = volume_control.volume.volume + volume_step_percentage * delta;
+
+			var vol = new VolumeControl.Volume();
+			vol.volume = v.clamp (0.0, this.max_volume);
+			vol.reason = VolumeControl.VolumeReasons.USER_KEYPRESS;
+			volume_control.volume = vol;
 		});
 
 		this.volume_control.notify["volume"].connect (() => {
 			/* Normalize volume, because the volume action's state is [0.0, 1.0], see create_volume_action() */
-			volume_action.set_state (new Variant.double (this.volume_control.volume / this.max_volume));
+			volume_action.set_state (new Variant.double (this.volume_control.volume.volume / this.max_volume));
 
 			this.update_root_icon ();
 
-			/* Update our volume and output */
-			var oldoutput = this.last_output_notification;
-			this.last_output_notification = this.volume_control.stream;
-
-			var oldvolume = this.last_volume_notification;
-			this.last_volume_notification = volume_control.volume;
-
-			/* Suppress notifications of volume changes if it is because the 
-			   output stream changed. */
-			if (oldoutput != this.last_output_notification)
-				return;
-			/* Supress updates that don't change the value */
-			if (GLib.Math.fabs(oldvolume - this.last_volume_notification) < 0.01)
-				return;
-
-			this.update_sync_notification ();
+			var reason = volume_control.volume.reason;
+			if (reason == VolumeControl.VolumeReasons.USER_KEYPRESS ||
+					reason == VolumeControl.VolumeReasons.DEVICE_OUTPUT_CHANGE)
+				this.update_sync_notification ();
 		});
 
 		this.volume_control.bind_property ("ready", volume_action, "enabled", BindingFlags.SYNC_CREATE);
diff --git a/src/volume-control-pulse.vala b/src/volume-control-pulse.vala
index 1e81ce1..bcbd6ea 100644
--- a/src/volume-control-pulse.vala
+++ b/src/volume-control-pulse.vala
@@ -42,7 +42,7 @@ public class VolumeControlPulse : VolumeControl
 	private PulseAudio.Context context;
 	private bool   _mute = true;
 	private bool   _is_playing = false;
-	private double _volume = 0.0;
+	private VolumeControl.Volume _volume = new VolumeControl.Volume();
 	private double _mic_volume = 0.0;
 
 	/* Used by the pulseaudio stream restore extension */
@@ -87,12 +87,15 @@ public class VolumeControlPulse : VolumeControl
 	/** true when high volume warnings should be shown */
 	public override bool high_volume {
 		get {
-			return this._volume > 0.75 && _active_port_headphone;	
+			return this._volume.volume > 0.75 && _active_port_headphone;	
 		}
 	}
 
 	public VolumeControlPulse ()
 	{
+		_volume.volume = 0.0;
+		_volume.reason = VolumeControl.VolumeReasons.PULSE_CHANGE;
+
 		if (loop == null)
 			loop = new PulseAudio.GLibMainLoop ();
 
@@ -164,8 +167,6 @@ public class VolumeControlPulse : VolumeControl
 
 	private void sink_info_cb_for_props (Context c, SinkInfo? i, int eol)
 	{
-		bool old_high_volume = this.high_volume;
-
 		if (i == null)
 			return;
 
@@ -197,16 +198,13 @@ public class VolumeControlPulse : VolumeControl
 		}
 
 		if (_pulse_use_stream_restore == false &&
-				_volume != volume_to_double (i.volume.max ()))
+				_volume.volume != volume_to_double (i.volume.max ()))
 		{
-			_volume = volume_to_double (i.volume.max ());
-			this.notify_property("volume");
-			start_local_volume_timer();
+			var vol = new VolumeControl.Volume();
+			vol.volume = volume_to_double (i.volume.max ());
+			vol.reason = VolumeControl.VolumeReasons.PULSE_CHANGE;
+			this.volume = vol;
 		} 
-		
-		if (this.high_volume != old_high_volume) {
-			this.notify_property("high-volume");
-		}
 	}
 
 	private void source_info_cb (Context c, SourceInfo? i, int eol)
@@ -264,17 +262,18 @@ public class VolumeControlPulse : VolumeControl
 					Variant body = message.get_body ();
 					Variant varray = body.get_child_value (0);
 
-					uint32 type = 0, volume = 0;
+					uint32 type = 0, lvolume = 0;
 					VariantIter iter = varray.iterator ();
-					iter.next ("(uu)", &type, &volume);
+					iter.next ("(uu)", &type, &lvolume);
 					/* Here we need to compare integer values to avoid rounding issues, so just
 					 * using the volume values used by pulseaudio */
-					PulseAudio.Volume cvolume = double_to_volume (_volume);
-					if (volume != cvolume) {
+					PulseAudio.Volume cvolume = double_to_volume (_volume.volume);
+					if (lvolume != cvolume) {
 						/* Someone else changed the volume for this role, reflect on the indicator */
-						_volume = volume_to_double (volume);
-						this.notify_property("volume");
-						start_local_volume_timer();
+						var vol = new VolumeControl.Volume();
+						vol.volume = volume_to_double (lvolume);
+						vol.reason = VolumeControl.VolumeReasons.PULSE_CHANGE;
+						this.volume = vol;
 					}
 				}
 			}
@@ -315,9 +314,10 @@ public class VolumeControlPulse : VolumeControl
 				VariantIter iter = tmp.iterator ();
 				iter.next ("(uu)", &type, &volume);
 
-				_volume = volume_to_double (volume);
-				this.notify_property("volume");
-				start_local_volume_timer();
+				var vol = new VolumeControl.Volume();
+				vol.volume = volume_to_double (volume);
+				vol.reason = VolumeControl.VolumeReasons.VOLUME_STREAM_CHANGE;
+				this.volume = vol;
 			} catch (GLib.Error e) {
 				warning ("unable to get volume for active role %s (%s)", sink_input_objp, e.message);
 			}
@@ -542,7 +542,7 @@ public class VolumeControlPulse : VolumeControl
 			return;
 
 		unowned CVolume cvol = i.volume;
-		cvol.scale (double_to_volume (_volume));
+		cvol.scale (double_to_volume (_volume.volume));
 		c.set_sink_volume_by_index (i.index, cvol, set_volume_success_cb);
 	}
 
@@ -566,7 +566,7 @@ public class VolumeControlPulse : VolumeControl
 
 		try {
 			var builder = new VariantBuilder (new VariantType ("a(uu)"));
-			builder.add ("(uu)", 0, double_to_volume (_volume));
+			builder.add ("(uu)", 0, double_to_volume (_volume.volume));
 			Variant volume = builder.end ();
 
 			/* Increase the signal counter so we can handle the callback */
@@ -588,31 +588,6 @@ public class VolumeControlPulse : VolumeControl
 		}
 	}
 
-	bool set_volume_internal (double volume)
-	{
-		if (context.get_state () != Context.State.READY)
-			return false;
-
-		if (_volume != volume) {
-			var old_high_volume = this.high_volume;
-
-			_volume = volume;
-			if (_pulse_use_stream_restore)
-				set_volume_active_role.begin ();
-			else
-				context.get_server_info (server_info_cb_for_set_volume);
-
-			this.notify_property("volume");
-
-			if (this.high_volume != old_high_volume)
-				this.notify_property("high-volume");
-
-			return true;
-		} else {
-			return false;
-		}
-	}
-
 	void set_mic_volume_success_cb (Context c, int success)
 	{
 		if ((bool)success)
@@ -627,14 +602,26 @@ public class VolumeControlPulse : VolumeControl
 		}
 	}
 
-	public override double volume {
+	public override VolumeControl.Volume volume {
 		get {
 			return _volume;
 		}
 		set {
-			if (set_volume_internal (value)) {
-				start_local_volume_timer();
-			}
+			var old_high_volume = this.high_volume;
+			_volume = value;
+
+			/* Make sure we're connected to Pulse and pulse didn't give us the change */
+			if (context.get_state () != Context.State.READY &&
+					_volume.reason != VolumeControl.VolumeReasons.PULSE_CHANGE)
+				if (_pulse_use_stream_restore)
+					set_volume_active_role.begin ();
+				else
+					context.get_server_info (server_info_cb_for_set_volume);
+
+			if (this.high_volume != old_high_volume)
+				this.notify_property("high-volume");
+
+			start_local_volume_timer();
 		}
 	}
 
@@ -839,7 +826,7 @@ public class VolumeControlPulse : VolumeControl
 		}
 	}
 
-	private async void sync_volume_to_accountsservice (double volume)
+	private async void sync_volume_to_accountsservice (VolumeControl.Volume volume)
 	{
 		if (_user_proxy == null)
 			return;
@@ -848,7 +835,7 @@ public class VolumeControlPulse : VolumeControl
 		_volume_cancellable.reset ();
 
 		try {
-			yield _user_proxy.get_connection ().call (_user_proxy.get_name (), _user_proxy.get_object_path (), "org.freedesktop.DBus.Properties", "Set", new Variant ("(ssv)", _user_proxy.get_interface_name (), "Volume", new Variant ("d", volume)), null, DBusCallFlags.NONE, -1, _volume_cancellable);
+			yield _user_proxy.get_connection ().call (_user_proxy.get_name (), _user_proxy.get_object_path (), "org.freedesktop.DBus.Properties", "Set", new Variant ("(ssv)", _user_proxy.get_interface_name (), "Volume", new Variant ("d", volume.volume)), null, DBusCallFlags.NONE, -1, _volume_cancellable);
 		} catch (GLib.Error e) {
 			warning ("unable to sync volume to AccountsService: %s", e.message);
 		}
@@ -891,7 +878,11 @@ public class VolumeControlPulse : VolumeControl
 	{
 		if (_accountservice_volume_timer == 0) {
 			// If we haven't been messing with local volume recently, apply immediately.
-			if (_local_volume_timer == 0 && !set_volume_internal (_account_service_volume)) {
+			if (_local_volume_timer == 0) {
+				var vol = new VolumeControl.Volume();
+				vol.volume = _account_service_volume;
+				vol.reason = VolumeControl.VolumeReasons.ACCOUNTS_SERVICE_SET;
+				this.volume = vol;
 				return;
 			}
 			// Else check again in another second if needed.
diff --git a/src/volume-control.vala b/src/volume-control.vala
index b06ea56..3f1c799 100644
--- a/src/volume-control.vala
+++ b/src/volume-control.vala
@@ -20,13 +20,27 @@
 
 public abstract class VolumeControl : Object
 {
+	public enum VolumeReasons {
+		PULSE_CHANGE,
+		ACCOUNTS_SERVICE_SET,
+		DEVICE_OUTPUT_CHANGE,
+		USER_KEYPRESS,
+		VOLUME_STREAM_CHANGE
+	}
+
+	public class Volume : Object {
+		public double volume;
+		public VolumeReasons reason;
+	}
+
 	public virtual string stream { get { return ""; } }
 	public virtual bool ready { get { return false; } set { } }
 	public virtual bool active_mic { get { return false; } set { } }
 	public virtual bool high_volume { get { return false; } }
 	public virtual bool mute { get { return false; } }
 	public virtual bool is_playing { get { return false; } }
-	public virtual double volume { get { return 0.0; } set { } }
+	private Volume _volume;
+	public virtual Volume volume { get { return _volume; } set { } }
 	public virtual double mic_volume { get { return 0.0; } set { } }
 
 	public abstract void set_mute (bool mute);