diff --git a/docs/changelog.rst b/docs/changelog.rst index 3bda9237..8cc886ce 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,10 @@ Core API - Start ``tlid`` counting at 1 instead of 0 to keep in sync with MPD's ``songid``. +- :meth:`~mopidy.core.PlaybackController.get_time_position` now returns the + seek target while a seek is in progress. This gives better results than just + failing the position query. (Fixes: :issue:`312` PR: :issue:`1346`) + Models ------ @@ -83,6 +87,11 @@ Gapless - Tests have been updated to always use a core actor so async state changes don't trip us up. +- Seek events are now triggered when the seek completes. Previously the event + was emitted when the seek was requested, not when it completed. Further + changes have been made to make seek work correctly for gapless related corner + cases. (Fixes: :issue:`1305` PR: :issue:`1346`) + v1.1.2 (UNRELEASED) =================== diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index e365e4b7..93cb814e 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -90,6 +90,9 @@ class Core( def stream_changed(self, uri): self.playback._on_stream_changed(uri) + def position_changed(self, position): + self.playback._on_position_changed(position) + def state_changed(self, old_state, new_state, target_state): # XXX: This is a temporary fix for issue #232 while we wait for a more # permanent solution with the implementation of issue #234. When the diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 45e1b4ba..89bd92ee 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -26,6 +26,7 @@ class PlaybackController(object): self._current_tl_track = None self._pending_tl_track = None + self._pending_position = None self._last_position = None self._previous = False @@ -130,6 +131,8 @@ class PlaybackController(object): def get_time_position(self): """Get time position in milliseconds.""" + if self._pending_position is not None: + return self._pending_position backend = self._get_backend(self.get_current_tl_track()) if backend: return backend.playback.get_time_position().get() @@ -211,14 +214,24 @@ class PlaybackController(object): # This code path handles the stop() case, uri should be none. position, self._last_position = self._last_position, None - self._trigger_track_playback_ended(position) + if self._pending_position is None: + self._trigger_track_playback_ended(position) self._stream_title = None if self._pending_tl_track: self._set_current_tl_track(self._pending_tl_track) self._pending_tl_track = None - self.set_state(PlaybackState.PLAYING) - self._trigger_track_playback_started() + + if self._pending_position is None: + self.set_state(PlaybackState.PLAYING) + self._trigger_track_playback_started() + else: + self._seek(self._pending_position) + + def _on_position_changed(self, position): + if self._pending_position == position: + self._trigger_seeked(position) + self._pending_position = None def _on_about_to_finish_callback(self): """Callback that performs a blocking actor call to the real callback. @@ -437,11 +450,6 @@ class PlaybackController(object): if self.get_state() == PlaybackState.STOPPED: self.play() - # TODO: uncomment once we have tests for this. Should fix seek after - # about to finish doing wrong track. - # if self._current_tl_track and self._pending_tl_track: - # self.play(self._current_tl_track) - # We need to prefer the still playing track, but if nothing is playing # we fall back to the pending one. tl_track = self._current_tl_track or self._pending_tl_track @@ -455,14 +463,21 @@ class PlaybackController(object): self.next() return True + # Store our target position. + self._pending_position = time_position + + # Make sure we switch back to previous track if we get a seek while we + # have a pending track. + if self._current_tl_track and self._pending_tl_track: + self._change(self._current_tl_track, self.get_state()) + else: + return self._seek(time_position) + + def _seek(self, time_position): backend = self._get_backend(self.get_current_tl_track()) if not backend: return False - - success = backend.playback.seek(time_position).get() - if success: - self._trigger_seeked(time_position) - return success + return backend.playback.seek(time_position).get() def stop(self): """Stop playing.""" diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 4ae3b4ef..0da59b4d 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -432,6 +432,7 @@ class EventEmissionTest(BaseTest): self.core.playback.play(tl_tracks[0]) self.replay_events() self.core.playback.seek(1000) + self.replay_events() listener_mock.reset_mock() self.core.playback.stop() @@ -454,6 +455,7 @@ class EventEmissionTest(BaseTest): self.core.playback.play(tl_tracks[0]) self.replay_events() self.core.playback.seek(1000) + self.replay_events() listener_mock.reset_mock() self.core.playback.next() @@ -502,6 +504,7 @@ class EventEmissionTest(BaseTest): listener_mock.reset_mock() self.core.playback.seek(1000) + self.replay_events() listener_mock.send.assert_called_once_with( 'seeked', time_position=1000) @@ -529,6 +532,24 @@ class EventEmissionTest(BaseTest): ], listener_mock.send.mock_calls) + def test_seek_race_condition_emits_events(self, listener_mock): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[0]) + self.trigger_about_to_finish(replay_until='stream_changed') + listener_mock.reset_mock() + + self.core.playback.seek(1000) + self.replay_events() + + # When we trigger seek after an about to finish the other code that + # emits track stopped/started and playback state changed events gets + # triggered as we have to switch back to the previous track. + # The correct behavior would be to only emit seeked. + self.assertListEqual( + [mock.call('seeked', time_position=1000)], + listener_mock.send.mock_calls) + def test_previous_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -632,6 +653,19 @@ class SeekTest(BaseTest): self.core.playback.seek(1000) self.assertEqual(self.core.playback.state, core.PlaybackState.PAUSED) + def test_seek_race_condition_after_about_to_finish(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[0]) + self.replay_events() + + self.trigger_about_to_finish(replay_until='stream_changed') + self.core.playback.seek(1000) + self.replay_events() + + current_tl_track = self.core.playback.get_current_tl_track() + self.assertEqual(current_tl_track, tl_tracks[0]) + class TestStream(BaseTest): @@ -821,7 +855,6 @@ class BackendSelectionTest(unittest.TestCase): self.core.playback.play(self.tl_tracks[0]) self.trigger_stream_changed() - self.core.playback.seek(10000) self.core.playback.time_position self.playback1.get_time_position.assert_called_once_with() @@ -831,7 +864,6 @@ class BackendSelectionTest(unittest.TestCase): self.core.playback.play(self.tl_tracks[1]) self.trigger_stream_changed() - self.core.playback.seek(10000) self.core.playback.time_position self.assertFalse(self.playback1.get_time_position.called)