Added some tests for the MPV player

sublime/adapters/adapter_base.py

@@ -10,6 +10,7 @@ from typing import (
     cast,
     Dict,
     Iterable,
+    List,
     Optional,
     Sequence,
     Set,
@@ -186,7 +187,7 @@ class ConfigurationStore(dict):
         with secret storage yourself.
         """
         value = self.get(key)
-        if not isinstance(value, (tuple, list)) or len(value) != 2:
+        if not isinstance(value, list) or len(value) != 2:
             return None
 
         storage_type, storage_key = value
@@ -206,7 +207,7 @@ class ConfigurationStore(dict):
             try:
                 password_id = None
                 if password_type_and_id := self.get(key):
-                    if cast(Tuple[str, str], password_type_and_id[0]) == "keyring":
+                    if cast(List[str], password_type_and_id)[0] == "keyring":
                         password_id = password_type_and_id[1]
 
                 if password_id is None:

sublime/adapters/configure_server_form.py

@@ -157,7 +157,7 @@ class ConfigureServerForm(Gtk.Box):
             if cpd.default is not None:
                 config_store[key] = config_store.get(key, cpd.default)
 
-            label = Gtk.Label(cpd.description, halign=Gtk.Align.END)
+            label = Gtk.Label(label=cpd.description, halign=Gtk.Align.END)
 
             input_el_box = Gtk.Box()
             self.entries[key] = cast(

sublime/players/manager.py

@@ -1,3 +1,4 @@
+import logging
 import multiprocessing
 from dataclasses import dataclass
 from enum import Enum
@@ -20,7 +21,6 @@ from typing import (
 from .base import PlayerEvent
 from .chromecast import ChromecastPlayer  # noqa: F401
 from .mpv import MPVPlayer  # noqa: F401
-from ..config import AppConfiguration
 
 
 @dataclass
@@ -36,7 +36,7 @@ class PlayerDeviceEvent:
 
 
 class PlayerManager:
-    # Available Players
+    # Available Players. Order matters for UI display.
     available_player_types: List[Type] = [MPVPlayer, ChromecastPlayer]
 
     @staticmethod

sublime/players/mpv.py

@@ -94,7 +94,8 @@ class MPVPlayer(Player):
         return self._volume
 
     def set_volume(self, volume: float):
-        self.mpv.volume = volume
+        if not self._muted:
+            self.mpv.volume = volume
         self._volume = volume
 
     def get_is_muted(self) -> bool:
@@ -108,9 +109,10 @@ class MPVPlayer(Player):
         with self._progress_value_lock:
             self._progress_value_count = 0
 
-        options = {"force-seekable": "yes"}
-        if progress is not None:
-            options["start"] = str(progress.total_seconds())
+        options = {
+            "force-seekable": "yes",
+            "start": str(progress.total_seconds()),
+        }
         self.mpv.command(
             "loadfile", uri, "replace", ",".join(f"{k}={v}" for k, v in options.items())
         )
@@ -124,4 +126,6 @@ class MPVPlayer(Player):
         self.mpv.cycle("pause")
 
     def seek(self, position: timedelta):
+        print(position)
+        print(self.mpv.time_pos)
         self.mpv.seek(str(position.total_seconds()), "absolute")

sublime/players/players.py

@@ -37,21 +37,6 @@ from sublime.adapters.api_objects import Song
 from sublime.config import AppConfiguration
 
 
-@dataclass
-class PlayerEvent:
-    class Type(Enum):
-        PLAY_STATE_CHANGE = 0
-        VOLUME_CHANGE = 1
-        STREAM_CACHE_PROGRESS_CHANGE = 2
-        CONNECTING = 3
-        CONNECTED = 4
-
-    type: Type
-    playing: Optional[bool] = False
-    volume: Optional[float] = 0.0
-    stream_cache_duration: Optional[float] = 0.0
-
-
 class Player(abc.ABC):
     # TODO (#205): pull players out into different modules and actually document this
     # API because it's kinda a bit strange tbh.
@@ -62,7 +47,7 @@ class Player(abc.ABC):
         self,
         on_timepos_change: Callable[[Optional[float]], None],
         on_track_end: Callable[[], None],
-        on_player_event: Callable[[PlayerEvent], None],
+        on_player_event: Callable[[Any], None],
         config: AppConfiguration,
     ):
         self.on_timepos_change = on_timepos_change
@@ -154,95 +139,6 @@ class Player(abc.ABC):
         )
 
 
-class MPVPlayer(Player):
-    def __init__(
-        self,
-        on_timepos_change: Callable[[Optional[float]], None],
-        on_track_end: Callable[[], None],
-        on_player_event: Callable[[PlayerEvent], None],
-        config: AppConfiguration,
-    ):
-        super().__init__(on_timepos_change, on_track_end, on_player_event, config)
-
-        self.mpv = mpv.MPV()
-        self.mpv.audio_client_name = "sublime-music"
-        self.mpv.replaygain = config.replay_gain.as_string()
-        self.progress_value_lock = threading.Lock()
-        self.progress_value_count = 0
-        self._muted = False
-        self._volume = 100.0
-        self._can_hotswap_source = True
-
-        @self.mpv.property_observer("time-pos")
-        def time_observer(_, value: Optional[float]):
-            self.on_timepos_change(value)
-            if value is None and self.progress_value_count > 1:
-                self.on_track_end()
-                with self.progress_value_lock:
-                    self.progress_value_count = 0
-
-            if value:
-                with self.progress_value_lock:
-                    self.progress_value_count += 1
-
-        @self.mpv.property_observer("demuxer-cache-time")
-        def cache_size_observer(_, value: Optional[float]):
-            on_player_event(
-                PlayerEvent(
-                    PlayerEvent.Type.STREAM_CACHE_PROGRESS_CHANGE,
-                    stream_cache_duration=value,
-                )
-            )
-
-    def _is_playing(self) -> bool:
-        return not self.mpv.pause
-
-    def reset(self):
-        self._song_loaded = False
-        with self.progress_value_lock:
-            self.progress_value_count = 0
-
-    def play_media(self, file_or_url: str, progress: timedelta, song: Song):
-        self.had_progress_value = False
-        with self.progress_value_lock:
-            self.progress_value_count = 0
-
-        self.mpv.pause = False
-        self.mpv.command(
-            "loadfile",
-            file_or_url,
-            "replace",
-            f"force-seekable=yes,start={progress.total_seconds()}" if progress else "",
-        )
-        self._song_loaded = True
-
-    def pause(self):
-        self.mpv.pause = True
-
-    def toggle_play(self):
-        self.mpv.cycle("pause")
-
-    def seek(self, value: timedelta):
-        self.mpv.seek(str(value.total_seconds()), "absolute")
-
-    def _get_volume(self) -> float:
-        return self._volume
-
-    def _set_volume(self, value: float):
-        self._volume = value
-        self.mpv.volume = self._volume
-
-    def _get_is_muted(self) -> bool:
-        return self._muted
-
-    def _set_is_muted(self, value: bool):
-        self._muted = value
-        self.mpv.volume = 0 if value else self._volume
-
-    def shutdown(self):
-        pass
-
-
 class ChromecastPlayer(Player):
     chromecasts: List[Any] = []
     chromecast: pychromecast.Chromecast = None
@@ -352,7 +248,7 @@ class ChromecastPlayer(Player):
         self,
         on_timepos_change: Callable[[Optional[float]], None],
         on_track_end: Callable[[], None],
-        on_player_event: Callable[[PlayerEvent], None],
+        on_player_event: Callable[[Any], None],
         config: AppConfiguration,
     ):
         super().__init__(on_timepos_change, on_track_end, on_player_event, config)

tests/player_tests/mpv_tests.py

@@ -1,6 +1,66 @@
+from datetime import timedelta
+from pathlib import Path
+from time import sleep
+
 from sublime.players.mpv import MPVPlayer
 
 
 def test_init():
     empty_fn = lambda *a, **k: None
-    MPVPlayer(empty_fn, empty_fn, empty_fn, {"Replay Gain": "no"})
+    MPVPlayer(empty_fn, empty_fn, empty_fn, {"Replay Gain": "Disabled"})
+
+
+def is_close(expected: float, value: float, delta: float = 0.5) -> bool:
+    return abs(value - expected) < delta
+
+
+def test_play():
+    empty_fn = lambda *a, **k: None
+    mpv_player = MPVPlayer(empty_fn, empty_fn, empty_fn, {"Replay Gain": "Disabled"})
+
+    song_path = Path(__file__).parent.joinpath("mock_data/test-song.mp3")
+    mpv_player.play_media(str(song_path), timedelta(seconds=10), None)
+
+    # Test Mute and volume
+    # ==================================================================================
+    # Test normal volume change.
+    assert mpv_player.get_volume() == 100
+    mpv_player.set_volume(70)
+    assert mpv_player.get_volume() == 70
+
+    # Test mute
+    assert not mpv_player.get_is_muted()
+    mpv_player.set_muted(True)
+    assert mpv_player.get_is_muted()
+
+    # Test volume change when muted
+    mpv_player.set_volume(50)
+    assert mpv_player.get_volume() == 50
+    # The volume of the actual player should still be muted.
+    assert mpv_player.mpv.volume == 0
+
+    # Unmute and the volume of the actual player should be what we set (50)
+    mpv_player.set_muted(False)
+    assert mpv_player.mpv.volume == 50
+
+    # Test Play/Pause
+    # ==================================================================================
+    # Test Pause
+    assert mpv_player.playing
+    mpv_player.pause()
+    assert not mpv_player.playing
+
+    # Test toggle_play
+    mpv_player.toggle_play()
+    assert mpv_player.playing
+    mpv_player.toggle_play()
+    assert not mpv_player.playing
+
+    # Test seek
+    sleep(0.1)
+    assert is_close(10, mpv_player.mpv.time_pos)
+    mpv_player.seek(timedelta(seconds=20))
+    assert is_close(20, mpv_player.mpv.time_pos)
+
+    # Pause so that it doesn't keep playing while testing
+    mpv_player.pause()