Merge pull request #1346 from adamcik/feature/eot-seek-handling
Gapless aware seek handling
This commit is contained in:
commit
23cdeb208b
@ -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)
|
||||
===================
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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."""
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user