Fix volume nor being preserved after a song change on Window/OSX. Fixes

#1703

Only depend on the sink remembering the volume state if it's pulsesink.

quodlibet/quodlibet/player/gstbe/player.py

@@ -169,7 +169,7 @@ def __init__(self, librarian=None):
         self._last_position = 0
 
         self.bin = None
-        self._vol_element = None
+        self._int_vol_element = None
         self._ext_vol_element = None
         self._use_eq = False
         self._eq_element = None
@@ -199,7 +199,10 @@ def name(self):
 
     @property
     def has_external_volume(self):
-        return self._ext_vol_element is not None
+        ext = self._ext_vol_element
+        if ext is None or ext.get_factory().get_name() != "pulsesink":
+            return False
+        return True
 
     def _set_buffer_duration(self, duration):
         """Set the stream buffer duration in msecs"""
@@ -272,8 +275,8 @@ def __init_pipeline(self):
         # playbin2 has started to control the volume through pulseaudio,
         # which means the volume property can change without us noticing.
         # Use our own volume element for now until this works with PA.
-        self._vol_element = Gst.ElementFactory.make('volume', None)
-        pipeline.insert(0, self._vol_element)
+        self._int_vol_element = Gst.ElementFactory.make('volume', None)
+        pipeline.insert(0, self._int_vol_element)
 
         # Get all plugin elements and append audio converters.
         # playbin already includes one at the end
@@ -411,9 +414,9 @@ def _mute_element(self):
             # directsoundsink has a mute property but it doesn't work
             # https://bugzilla.gnome.org/show_bug.cgi?id=755106
             if ext.get_factory().get_name() == "directsoundsink":
-                return self._vol_element
+                return self._int_vol_element
             return ext
-        return self._vol_element
+        return self._int_vol_element
 
     def __destroy_pipeline(self):
         self._remove_plugin_elements()
@@ -440,7 +443,7 @@ def __destroy_pipeline(self):
         self._active_seeks = []
 
         self._ext_vol_element = None
-        self._vol_element = None
+        self._int_vol_element = None
         self._eq_element = None
 
     def _rebuild_pipeline(self):
@@ -636,7 +639,7 @@ def stop(self):
 
     def do_get_property(self, property):
         if property.name == 'volume':
-            if self.has_external_volume:
+            if self._ext_vol_element:
                 # pulsesink volume is only valid in PAUSED/PLAYING
                 current_state = self._ext_vol_element.get_state(0)[1]
                 if current_state >= Gst.State.PAUSED:
@@ -658,19 +661,19 @@ def _reset_replaygain(self):
         v = 1.0 if self.has_external_volume else self._volume
         v = self.calc_replaygain_volume(v)
         v = min(10.0, max(0.0, v))
-        self._vol_element.set_property('volume', v)
+        self._int_vol_element.set_property('volume', v)
 
     def do_set_property(self, property, v):
         if property.name == 'volume':
             self._volume = v
-            if self.has_external_volume:
+            if self._ext_vol_element:
                 v = min(10.0, max(0.0, v))
                 self._ext_vol_element.set_property("volume", v)
             else:
                 v = self.calc_replaygain_volume(v)
                 if self.bin:
                     v = min(10.0, max(0.0, v))
-                    self._vol_element.set_property('volume', v)
+                    self._int_vol_element.set_property('volume', v)
         elif property.name == 'mute':
             if self._mute_element is not None:
                 self._mute_element.props.mute = v

quodlibet/quodlibet/qltk/quodlibetwindow.py

@@ -160,7 +160,7 @@ def __init__(self, parent, player, library):
         if not player.has_external_volume:
             player.volume = config.getfloat("memory", "volume")
 
-        self.volume.connect("value-changed", self._on_volume_changed)
+        connect_destroy(player, "notify::volume", self._on_volume_changed)
         control_item.add(t)
 
         self.insert(Gtk.SeparatorToolItem(), 1)
@@ -201,8 +201,8 @@ def __init__(self, parent, player, library):
         context = self.get_style_context()
         context.add_class("primary-toolbar")
 
-    def _on_volume_changed(self, widget, value):
-        config.set("memory", "volume", str(value))
+    def _on_volume_changed(self, player, *args):
+        config.set("memory", "volume", str(player.volume))
 
     def __new_song(self, player, song):
         self.image.set_song(song)

quodlibet/tests/test_player.py

@@ -193,6 +193,12 @@ def test_mute(self):
         # backend don't have to support it, but shouldn't fail on set/get
         self.player.mute = not self.player.mute
 
+    def test_preserve_volume(self):
+        self.player.next()
+        self.player.volume = 0.5
+        self.player.next()
+        self.assertEqual(self.player.volume, 0.5)
+
 
 class TNullPlayer(TPlayer):
     NAME = "nullbe"