Track the reason a volume was set to filter volume changes more accurately Fixes: #1416520

Approved by: Jussi Pakkanen, PS Jenkins bot

4 files changed, 103 insertions, 96 deletions

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 479d01a..194dfc9 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -160,6 +160,7 @@ set(
 
 add_definitions(
   -w
+  -DG_LOG_DOMAIN="indicator-sound"
 )
 
 add_library(
@@ -170,6 +171,7 @@ add_library(
 target_link_libraries(
   indicator-sound-service-lib
   ${SOUNDSERVICE_LIBRARIES}
+  -lm
 )
 
 ###########################
diff --git a/src/service.vala b/src/service.vala
index 95f8f16..0c82538 100644
--- a/src/service.vala
+++ b/src/service.vala
@@ -168,7 +168,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);
 		}
 	}
 
@@ -204,8 +204,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) {
@@ -240,7 +243,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";
@@ -277,8 +280,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) {
@@ -304,11 +305,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";
@@ -321,7 +322,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)Math.round(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");
@@ -419,44 +420,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..403acec 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 */
@@ -52,7 +52,7 @@ public class VolumeControlPulse : VolumeControl
 	private Gee.ArrayList<uint32> _sink_input_list = new Gee.ArrayList<uint32> ();
 	private HashMap<uint32, string> _sink_input_hash = new HashMap<uint32, string> ();
 	private bool _pulse_use_stream_restore = false;
-	private uint32 _active_sink_input = -1;
+	private int32 _active_sink_input = -1;
 	private string[] _valid_roles = {"multimedia", "alert", "alarm", "phone"};
 	public override string stream {
 		get {
@@ -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;
 					}
 				}
 			}
@@ -283,7 +282,7 @@ public class VolumeControlPulse : VolumeControl
 		return message;
 	}
 
-	private async void update_active_sink_input (uint32 index)
+	private async void update_active_sink_input (int32 index)
 	{
 		if ((index == -1) || (index != _active_sink_input && index in _sink_input_list)) {
 			string sink_input_objp = _objp_role_alert;
@@ -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);
 			}
@@ -350,7 +350,7 @@ public class VolumeControlPulse : VolumeControl
 				/* Only switch the active sink input in case a phone one is not active */
 				if (_active_sink_input == -1 ||
 						_sink_input_hash.get (_active_sink_input) != _objp_role_phone)
-					update_active_sink_input.begin (sink_input.index);
+					update_active_sink_input.begin ((int32)sink_input.index);
 			}
 		}
 	}
@@ -362,7 +362,7 @@ public class VolumeControlPulse : VolumeControl
 			_sink_input_hash.unset (index);
 			if (index == _active_sink_input) {
 				if (_sink_input_list.size != 0)
-					update_active_sink_input.begin (_sink_input_list.get (0));
+					update_active_sink_input.begin ((int32)_sink_input_list.get (0));
 				else
 					update_active_sink_input.begin (-1);
 			}
@@ -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);
 	}
 
@@ -565,8 +565,9 @@ public class VolumeControlPulse : VolumeControl
 			active_role_objp = _sink_input_hash.get (_active_sink_input);
 
 		try {
+			double vol = _volume.volume;
 			var builder = new VariantBuilder (new VariantType ("a(uu)"));
-			builder.add ("(uu)", 0, double_to_volume (_volume));
+			builder.add ("(uu)", 0, double_to_volume (vol));
 			Variant volume = builder.end ();
 
 			/* Increase the signal counter so we can handle the callback */
@@ -579,7 +580,7 @@ public class VolumeControlPulse : VolumeControl
 					new Variant ("(ssv)", "org.PulseAudio.Ext.StreamRestore1.RestoreEntry", "Volume", volume),
 					null, DBusCallFlags.NONE, -1);
 
-			this.notify_property("volume");
+			debug ("Set volume to %f on path %s", vol, active_role_objp);
 		} catch (GLib.Error e) {
 			lock (_pa_volume_sig_count) {
 				_pa_volume_sig_count--;
@@ -588,31 +589,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 +603,28 @@ 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();
-			}
+			debug("Setting volume to %f for profile %d because %d", value.volume, _active_sink_input, value.reason);
+
+			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();
 		}
 	}
 
@@ -687,7 +677,7 @@ public class VolumeControlPulse : VolumeControl
 			}
 		}
 
-		stdout.printf ("PulseAudio dbus unix socket: %s\n", address);
+		debug ("PulseAudio dbus unix socket: %s", address);
 		try {
 			_pconn = new DBusConnection.for_address_sync (address, DBusConnectionFlags.AUTHENTICATION_CLIENT);
 		} catch (GLib.Error e) {
@@ -707,7 +697,7 @@ public class VolumeControlPulse : VolumeControl
 
 		/* Only use stream restore if every used role is available */
 		if (_objp_role_multimedia != null && _objp_role_alert != null && _objp_role_alarm != null && _objp_role_phone != null) {
-			stdout.printf ("Using PulseAudio DBUS Stream Restore module\n");
+			debug ("Using PulseAudio DBUS Stream Restore module");
 			/* Restore volume and update default entry */
 			update_active_sink_input.begin (-1);
 			_pulse_use_stream_restore = true;
@@ -723,7 +713,7 @@ public class VolumeControlPulse : VolumeControl
 			/* Workaround for older versions of vala that don't provide get_objv */
 			VariantIter iter = props_variant.iterator ();
 			iter.next ("o", &objp);
-			stdout.printf ("Found obj path %s for restore data named %s\n", objp, name);
+			debug ("Found obj path %s for restore data named %s\n", objp, name);
 		} catch (GLib.Error e) {
 			warning ("unable to find stream restore data for: %s", name);
 		}
@@ -839,7 +829,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 +838,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 +881,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);