From b60e6806ced7a5f3059dc6ea256b2e5b80895b1e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 20 Sep 2012 13:38:40 +0200 Subject: [PATCH 1/6] Add get_time_position() to playback provider interface --- mopidy/backends/base/playback.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/mopidy/backends/base/playback.py b/mopidy/backends/base/playback.py index ae5a4383..197ba90e 100644 --- a/mopidy/backends/base/playback.py +++ b/mopidy/backends/base/playback.py @@ -65,6 +65,16 @@ class BasePlaybackProvider(object): """ return self.backend.audio.stop_playback().get() + def get_time_position(self): + """ + Get the current time position in milliseconds. + + *MAY be reimplemented by subclass.* + + :rtype: int + """ + return self.backend.audio.get_position().get() + def get_volume(self): """ Get current volume From f0613753160ad985a37ec054e7dfaee9729070d6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 20 Sep 2012 13:39:12 +0200 Subject: [PATCH 2/6] Override get_time_position() in the dummy backend --- mopidy/backends/dummy/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mopidy/backends/dummy/__init__.py b/mopidy/backends/dummy/__init__.py index 3ada0052..9c4a0e69 100644 --- a/mopidy/backends/dummy/__init__.py +++ b/mopidy/backends/dummy/__init__.py @@ -56,6 +56,7 @@ class DummyLibraryProvider(base.BaseLibraryProvider): class DummyPlaybackProvider(base.BasePlaybackProvider): def __init__(self, *args, **kwargs): super(DummyPlaybackProvider, self).__init__(*args, **kwargs) + self._time_position = 0 self._volume = None def pause(self): @@ -63,17 +64,22 @@ class DummyPlaybackProvider(base.BasePlaybackProvider): def play(self, track): """Pass None as track to force failure""" + self._time_position = 0 return track is not None def resume(self): return True def seek(self, time_position): + self._time_position = time_position return True def stop(self): return True + def get_time_position(self): + return self._time_position + def get_volume(self): return self._volume From 81fca7d68674c4784c35cd770423549cd429934b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 20 Sep 2012 19:33:34 +0200 Subject: [PATCH 3/6] Switch to time position from provider --- mopidy/core/playback.py | 3 +++ tests/frontends/mpd/status_test.py | 13 ++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 9f6030c1..1bebd270 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -307,6 +307,9 @@ class PlaybackController(object): @property def time_position(self): """Time position in milliseconds.""" + return self.provider.get_time_position() + + def _wall_clock_based_time_position(): if self.state == PlaybackState.PLAYING: time_since_started = (self._current_wall_time - self.play_time_started) diff --git a/tests/frontends/mpd/status_test.py b/tests/frontends/mpd/status_test.py index 59418a3b..2397b96f 100644 --- a/tests/frontends/mpd/status_test.py +++ b/tests/frontends/mpd/status_test.py @@ -159,18 +159,21 @@ class StatusHandlerTest(unittest.TestCase): self.assertLessEqual(position, total) def test_status_method_when_playing_contains_elapsed(self): - self.backend.playback.state = PAUSED - self.backend.playback.play_time_accumulated = 59123 + self.backend.current_playlist.append([Track(length=60000)]) + self.backend.playback.play() + self.backend.playback.pause() + self.backend.playback.seek(59123) result = dict(status.status(self.context)) self.assertIn('elapsed', result) self.assertEqual(result['elapsed'], '59.123') def test_status_method_when_starting_playing_contains_elapsed_zero(self): - self.backend.playback.state = PAUSED - self.backend.playback.play_time_accumulated = 123 # Less than 1000ms + self.backend.current_playlist.append([Track(length=10000)]) + self.backend.playback.play() + self.backend.playback.pause() result = dict(status.status(self.context)) self.assertIn('elapsed', result) - self.assertEqual(result['elapsed'], '0.123') + self.assertEqual(result['elapsed'], '0.000') def test_status_method_when_playing_contains_bitrate(self): self.backend.current_playlist.append([Track(bitrate=320)]) From ef17e36a1a641edc5605ba8520a4fd5700776f0e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 25 Sep 2012 11:11:59 +0200 Subject: [PATCH 4/6] Remove LocalPlaybackController --- mopidy/backends/local/__init__.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index db86e56f..c321c6e9 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -41,7 +41,7 @@ class LocalBackend(ThreadingActor, base.Backend): provider=library_provider) playback_provider = base.BasePlaybackProvider(backend=self) - self.playback = LocalPlaybackController(backend=self, + self.playback = core.PlaybackController(backend=self, provider=playback_provider) stored_playlists_provider = LocalStoredPlaylistsProvider(backend=self) @@ -59,18 +59,6 @@ class LocalBackend(ThreadingActor, base.Backend): self.audio = audio_refs[0].proxy() -class LocalPlaybackController(core.PlaybackController): - def __init__(self, *args, **kwargs): - super(LocalPlaybackController, self).__init__(*args, **kwargs) - - # XXX Why do we call stop()? Is it to set GStreamer state to 'READY'? - self.stop() - - @property - def time_position(self): - return self.backend.audio.get_position().get() - - class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): def __init__(self, *args, **kwargs): super(LocalStoredPlaylistsProvider, self).__init__(*args, **kwargs) From 2237e4f5a1e2e79d50485f0d1b97a5774af0ed40 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 25 Sep 2012 12:10:25 +0200 Subject: [PATCH 5/6] Move optional wall clock-based position tracking down to the playback provider --- mopidy/backends/base/playback.py | 45 +++++++++++++++++++++++++++ mopidy/backends/spotify/playback.py | 9 ++++++ mopidy/core/playback.py | 48 +++-------------------------- 3 files changed, 58 insertions(+), 44 deletions(-) diff --git a/mopidy/backends/base/playback.py b/mopidy/backends/base/playback.py index 197ba90e..b3b9959e 100644 --- a/mopidy/backends/base/playback.py +++ b/mopidy/backends/base/playback.py @@ -1,3 +1,8 @@ +import time + +from mopidy.core.playback import PlaybackState + + class BasePlaybackProvider(object): """ :param backend: the backend @@ -9,6 +14,9 @@ class BasePlaybackProvider(object): def __init__(self, backend): self.backend = backend + self._play_time_accumulated = 0 + self._play_time_started = 0 + def pause(self): """ Pause playback. @@ -95,3 +103,40 @@ class BasePlaybackProvider(object): :type volume: int [0..100] """ self.backend.audio.set_volume(volume) + + def wall_clock_based_time_position(self): + """ + Helper method that tracks track time position using the wall clock. + + To use this helper you must call the helper from your implementation of + :meth:`get_time_position` and return its return value. + + :rtype: int + """ + state = self.backend.playback.state + if state == PlaybackState.PLAYING: + time_since_started = (self._wall_time() - + self._play_time_started) + return self._play_time_accumulated + time_since_started + elif state == PlaybackState.PAUSED: + return self._play_time_accumulated + elif state == PlaybackState.STOPPED: + return 0 + + def update_play_time_on_play(self): + self._play_time_accumulated = 0 + self._play_time_started = self._wall_time() + + def update_play_time_on_pause(self): + time_since_started = self._wall_time() - self._play_time_started + self._play_time_accumulated += time_since_started + + def update_play_time_on_resume(self): + self._play_time_started = self._wall_time() + + def update_play_time_on_seek(self, time_position): + self._play_time_started = self._wall_time() + self._play_time_accumulated = time_position + + def _wall_time(self): + return int(time.time() * 1000) diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index 1c20da87..cd5b0689 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -5,8 +5,10 @@ from spotify import Link, SpotifyError from mopidy.backends.base import BasePlaybackProvider from mopidy.core import PlaybackState + logger = logging.getLogger('mopidy.backends.spotify.playback') + class SpotifyPlaybackProvider(BasePlaybackProvider): def play(self, track): if self.backend.playback.state == PlaybackState.PLAYING: @@ -38,3 +40,10 @@ class SpotifyPlaybackProvider(BasePlaybackProvider): def stop(self): self.backend.spotify.session.play(0) return super(SpotifyPlaybackProvider, self).stop() + + def get_time_position(self): + # XXX: The default implementation of get_time_position hangs/times out + # when used with the Spotify backend and GStreamer appsrc. If this can + # be resolved, we no longer need to use a wall clock based time + # position for Spotify playback. + return self.wall_clock_based_time_position() diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 1bebd270..b32f5b62 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -1,6 +1,5 @@ import logging import random -import time from mopidy.listeners import BackendListener @@ -20,7 +19,6 @@ def option_wrapper(name, default): return property(get_option, set_option) - class PlaybackState(object): """ Enum of playback states. @@ -87,8 +85,6 @@ class PlaybackController(object): self._state = PlaybackState.STOPPED self._shuffled = [] self._first_shuffle = True - self.play_time_accumulated = 0 - self.play_time_started = 0 def _get_cpid(self, cp_track): if cp_track is None: @@ -292,48 +288,11 @@ class PlaybackController(object): self._trigger_playback_state_changed(old_state, new_state) - # FIXME play_time stuff assumes backend does not have a better way of - # handeling this stuff :/ - if (old_state in (PlaybackState.PLAYING, PlaybackState.STOPPED) - and new_state == PlaybackState.PLAYING): - self._play_time_start() - elif (old_state == PlaybackState.PLAYING - and new_state == PlaybackState.PAUSED): - self._play_time_pause() - elif (old_state == PlaybackState.PAUSED - and new_state == PlaybackState.PLAYING): - self._play_time_resume() - @property def time_position(self): """Time position in milliseconds.""" return self.provider.get_time_position() - def _wall_clock_based_time_position(): - if self.state == PlaybackState.PLAYING: - time_since_started = (self._current_wall_time - - self.play_time_started) - return self.play_time_accumulated + time_since_started - elif self.state == PlaybackState.PAUSED: - return self.play_time_accumulated - elif self.state == PlaybackState.STOPPED: - return 0 - - def _play_time_start(self): - self.play_time_accumulated = 0 - self.play_time_started = self._current_wall_time - - def _play_time_pause(self): - time_since_started = self._current_wall_time - self.play_time_started - self.play_time_accumulated += time_since_started - - def _play_time_resume(self): - self.play_time_started = self._current_wall_time - - @property - def _current_wall_time(self): - return int(time.time() * 1000) - @property def volume(self): return self.provider.get_volume() @@ -411,6 +370,7 @@ class PlaybackController(object): """Pause playback.""" if self.provider.pause(): self.state = PlaybackState.PAUSED + self.provider.update_play_time_on_pause() self._trigger_track_playback_paused() def play(self, cp_track=None, on_error_step=1): @@ -453,6 +413,7 @@ class PlaybackController(object): if self.random and self.current_cp_track in self._shuffled: self._shuffled.remove(self.current_cp_track) + self.provider.update_play_time_on_play() self._trigger_track_playback_started() def previous(self): @@ -469,6 +430,7 @@ class PlaybackController(object): """If paused, resume playing the current track.""" if self.state == PlaybackState.PAUSED and self.provider.resume(): self.state = PlaybackState.PLAYING + self.provider.update_play_time_on_resume() self._trigger_track_playback_resumed() def seek(self, time_position): @@ -493,11 +455,9 @@ class PlaybackController(object): self.next() return True - self.play_time_started = self._current_wall_time - self.play_time_accumulated = time_position - success = self.provider.seek(time_position) if success: + self.provider.update_play_time_on_seek(time_position) self._trigger_seeked(time_position) return success From 90a538c5954e6845080d93a6aeb970a5d8078e89 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 25 Sep 2012 15:43:08 +0200 Subject: [PATCH 6/6] Move wall clock-based time position into Spotify backend --- mopidy/backends/base/playback.py | 45 ----------------------------- mopidy/backends/spotify/playback.py | 37 ++++++++++++++++++++++-- mopidy/core/playback.py | 4 --- 3 files changed, 34 insertions(+), 52 deletions(-) diff --git a/mopidy/backends/base/playback.py b/mopidy/backends/base/playback.py index b3b9959e..197ba90e 100644 --- a/mopidy/backends/base/playback.py +++ b/mopidy/backends/base/playback.py @@ -1,8 +1,3 @@ -import time - -from mopidy.core.playback import PlaybackState - - class BasePlaybackProvider(object): """ :param backend: the backend @@ -14,9 +9,6 @@ class BasePlaybackProvider(object): def __init__(self, backend): self.backend = backend - self._play_time_accumulated = 0 - self._play_time_started = 0 - def pause(self): """ Pause playback. @@ -103,40 +95,3 @@ class BasePlaybackProvider(object): :type volume: int [0..100] """ self.backend.audio.set_volume(volume) - - def wall_clock_based_time_position(self): - """ - Helper method that tracks track time position using the wall clock. - - To use this helper you must call the helper from your implementation of - :meth:`get_time_position` and return its return value. - - :rtype: int - """ - state = self.backend.playback.state - if state == PlaybackState.PLAYING: - time_since_started = (self._wall_time() - - self._play_time_started) - return self._play_time_accumulated + time_since_started - elif state == PlaybackState.PAUSED: - return self._play_time_accumulated - elif state == PlaybackState.STOPPED: - return 0 - - def update_play_time_on_play(self): - self._play_time_accumulated = 0 - self._play_time_started = self._wall_time() - - def update_play_time_on_pause(self): - time_since_started = self._wall_time() - self._play_time_started - self._play_time_accumulated += time_since_started - - def update_play_time_on_resume(self): - self._play_time_started = self._wall_time() - - def update_play_time_on_seek(self, time_position): - self._play_time_started = self._wall_time() - self._play_time_accumulated = time_position - - def _wall_time(self): - return int(time.time() * 1000) diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index cd5b0689..61696bd8 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -1,4 +1,5 @@ import logging +import time from spotify import Link, SpotifyError @@ -10,11 +11,25 @@ logger = logging.getLogger('mopidy.backends.spotify.playback') class SpotifyPlaybackProvider(BasePlaybackProvider): + def __init__(self, *args, **kwargs): + super(SpotifyPlaybackProvider, self).__init__(*args, **kwargs) + + self._play_time_accumulated = 0 + self._play_time_started = 0 + + def pause(self): + time_since_started = self._wall_time() - self._play_time_started + self._play_time_accumulated += time_since_started + + return super(SpotifyPlaybackProvider, self).pause() + def play(self, track): - if self.backend.playback.state == PlaybackState.PLAYING: - self.backend.spotify.session.play(0) if track.uri is None: return False + + self._play_time_accumulated = 0 + self._play_time_started = self._wall_time() + try: self.backend.spotify.session.load( Link.from_string(track.uri).as_track()) @@ -29,12 +44,17 @@ class SpotifyPlaybackProvider(BasePlaybackProvider): return False def resume(self): + self._play_time_started = self._wall_time() return self.seek(self.backend.playback.time_position) def seek(self, time_position): + self._play_time_started = self._wall_time() + self._play_time_accumulated = time_position + self.backend.audio.prepare_change() self.backend.spotify.session.seek(time_position) self.backend.audio.start_playback() + return True def stop(self): @@ -46,4 +66,15 @@ class SpotifyPlaybackProvider(BasePlaybackProvider): # when used with the Spotify backend and GStreamer appsrc. If this can # be resolved, we no longer need to use a wall clock based time # position for Spotify playback. - return self.wall_clock_based_time_position() + state = self.backend.playback.state + if state == PlaybackState.PLAYING: + time_since_started = (self._wall_time() - + self._play_time_started) + return self._play_time_accumulated + time_since_started + elif state == PlaybackState.PAUSED: + return self._play_time_accumulated + elif state == PlaybackState.STOPPED: + return 0 + + def _wall_time(self): + return int(time.time() * 1000) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index b32f5b62..82a11064 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -370,7 +370,6 @@ class PlaybackController(object): """Pause playback.""" if self.provider.pause(): self.state = PlaybackState.PAUSED - self.provider.update_play_time_on_pause() self._trigger_track_playback_paused() def play(self, cp_track=None, on_error_step=1): @@ -413,7 +412,6 @@ class PlaybackController(object): if self.random and self.current_cp_track in self._shuffled: self._shuffled.remove(self.current_cp_track) - self.provider.update_play_time_on_play() self._trigger_track_playback_started() def previous(self): @@ -430,7 +428,6 @@ class PlaybackController(object): """If paused, resume playing the current track.""" if self.state == PlaybackState.PAUSED and self.provider.resume(): self.state = PlaybackState.PLAYING - self.provider.update_play_time_on_resume() self._trigger_track_playback_resumed() def seek(self, time_position): @@ -457,7 +454,6 @@ class PlaybackController(object): success = self.provider.seek(time_position) if success: - self.provider.update_play_time_on_seek(time_position) self._trigger_seeked(time_position) return success