diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 04b0ab06..b154e21a 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -20,6 +20,7 @@ logger = logging.getLogger('mopidy.audio') mixers.register_mixers() + MB = 1 << 20 @@ -49,8 +50,6 @@ class Audio(pykka.ThreadingActor): self._software_mixing = False self._volume_set = None - self._end_of_track_callback = None - self._appsrc = None self._appsrc_caps = None self._appsrc_need_data_callback = None @@ -94,28 +93,21 @@ class Audio(pykka.ThreadingActor): self._playbin = playbin def _on_about_to_finish(self, element): - # Cleanup appsrc related stuff. - old_appsrc, self._appsrc = self._appsrc, None - - self._disconnect(old_appsrc, 'need-data') - self._disconnect(old_appsrc, 'enough-data') - self._disconnect(old_appsrc, 'seek-data') - + source, self._appsrc = self._appsrc, None + if source is None: + return self._appsrc_caps = None - # Note that we can not let this function return until we have the next - # URI set for gapless / EOS free playback. This means all the - # subsequent remote calls to backends etc. down this code path need to - # block. - if self._end_of_track_callback: - self._end_of_track_callback() + self._disconnect(source, 'need-data') + self._disconnect(source, 'enough-data') + self._disconnect(source, 'seek-data') def _on_new_source(self, element, pad): - source = element.get_property('source') - - if source.get_factory().get_name() != 'appsrc': + uri = element.get_property('uri') + if not uri or not uri.startswith('appsrc://'): return + source = element.get_property('source') source.set_property('caps', self._appsrc_caps) source.set_property('format', b'time') source.set_property('stream-type', b'seekable') @@ -288,10 +280,6 @@ class Audio(pykka.ThreadingActor): logger.debug('Triggering reached_end_of_stream event') AudioListener.send('reached_end_of_stream') - def set_on_end_of_track(self, callback): - """Set callback to be called on end of track.""" - self._end_of_track_callback = callback - def set_uri(self, uri): """ Set URI of audio to be played. diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py index 2f077e49..f49aa89b 100644 --- a/mopidy/backends/base.py +++ b/mopidy/backends/base.py @@ -88,8 +88,8 @@ class BaseLibraryProvider(object): class BasePlaybackProvider(object): """ - :param audio: audio sub-system - :type audio: actor proxy to a :class:`mopidy.audio.Audio` actor. + :param audio: the audio actor + :type audio: actor proxy to an instance of :class:`mopidy.audio.Audio` :param backend: the backend :type backend: :class:`mopidy.backends.base.Backend` """ @@ -121,23 +121,8 @@ class BasePlaybackProvider(object): :rtype: :class:`True` if successful, else :class:`False` """ self.audio.prepare_change() - self.change_track(track) - return self.audio.start_playback().get() - - def change_track(self, track): - """ - Swith to provided track. - - Used for handling of EOT and and in :meth:`play`. - - *MAY be reimplemented by subclass.* - - :param track: the track to play - :type track: :class:`mopidy.models.Track` - :rtype: :class:`True` if successful, else :class:`False` - """ self.audio.set_uri(track.uri).get() - return True + return self.audio.start_playback().get() def resume(self): """ diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index f8de2e57..bda17634 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -35,9 +35,11 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): super(SpotifyPlaybackProvider, self).__init__(*args, **kwargs) self._first_seek = False - def change_track(self, track): - spotify_backend = self.backend.actor_ref.proxy() + def play(self, track): + if track.uri is None: + return False + spotify_backend = self.backend.actor_ref.proxy() need_data_callback_bound = functools.partial( need_data_callback, spotify_backend) enough_data_callback_bound = functools.partial( @@ -47,18 +49,21 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): self._first_seek = True - self.audio.set_appsrc( - self._caps, - need_data=need_data_callback_bound, - enough_data=enough_data_callback_bound, - seek_data=seek_data_callback_bound) - self.audio.set_metadata(track) - try: self.backend.spotify.session.load( Link.from_string(track.uri).as_track()) - self.backend.spotify.buffer_timestamp = 0 self.backend.spotify.session.play(1) + self.backend.spotify.buffer_timestamp = 0 + + self.audio.prepare_change() + self.audio.set_appsrc( + self._caps, + need_data=need_data_callback_bound, + enough_data=enough_data_callback_bound, + seek_data=seek_data_callback_bound) + self.audio.start_playback() + self.audio.set_metadata(track) + return True except SpotifyError as e: logger.info('Playback of %s failed: %s', track.uri, e) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 03ad2712..cd4ba180 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -46,11 +46,6 @@ class Core(pykka.ThreadingActor, AudioListener, BackendListener): self.tracklist = TracklistController(core=self) - if audio: - # Hook up blocking on end of track handler to audio sub-system. - audio.set_on_end_of_track( - lambda: self.actor_ref.proxy().playback.on_end_of_track().get()) - def get_uri_schemes(self): futures = [b.uri_schemes for b in self.backends] results = pykka.get_all(futures) @@ -61,7 +56,7 @@ class Core(pykka.ThreadingActor, AudioListener, BackendListener): """List of URI schemes we can handle""" def reached_end_of_stream(self): - self.playback.on_end_of_stream() + self.playback.on_end_of_track() def state_changed(self, old_state, new_state): # XXX: This is a temporary fix for issue #232 while we wait for a more diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 1f2e0384..21f09ad2 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -314,11 +314,6 @@ class PlaybackController(object): elif old_state == PlaybackState.PAUSED: self.pause() - def on_end_of_stream(self): - self._trigger_track_playback_ended() - self.state = PlaybackState.STOPPED - self.current_tl_track = None - def on_end_of_track(self): """ Tell the playback controller that end of track is reached. @@ -330,14 +325,11 @@ class PlaybackController(object): original_tl_track = self.current_tl_track - # As noted in mopidy.audio which calls this code, we need to make sure - # the calls to the backend are blocking or gapless / EOS free playback - # will break. if self.tl_track_at_eot: self._trigger_track_playback_ended() - self.current_tl_track = self.tl_track_at_eot - self._get_backend().playback.change_track(self.current_track).get() - self._trigger_track_playback_started() + self.play(self.tl_track_at_eot) + else: + self.stop(clear_current_track=True) if self.consume: self.core.tracklist.remove(tlid=original_tl_track.tlid) diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index d7979018..e12d54a5 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -115,7 +115,6 @@ class PlaybackControllerTest(object): def test_current_track_after_completed_playlist(self): self.playback.play(self.tracklist.tl_tracks[-1]) self.playback.on_end_of_track() - self.playback.on_end_of_stream() self.assertEqual(self.playback.state, PlaybackState.STOPPED) self.assertEqual(self.playback.current_track, None) @@ -344,8 +343,6 @@ class PlaybackControllerTest(object): self.playback.on_end_of_track() - self.playback.on_end_of_stream() - self.assertEqual(self.playback.state, PlaybackState.STOPPED) @populate_tracklist @@ -354,7 +351,6 @@ class PlaybackControllerTest(object): for _ in self.tracks: self.playback.on_end_of_track() - self.playback.on_end_of_stream() self.assertEqual(self.playback.current_track, None) self.assertEqual(self.playback.state, PlaybackState.STOPPED) @@ -367,6 +363,16 @@ class PlaybackControllerTest(object): self.playback.on_end_of_track() self.assertEqual(self.playback.state, PlaybackState.STOPPED) + @populate_tracklist + def test_end_of_track_skips_to_next_track_on_failure(self): + # If backend's play() returns False, it is a failure. + self.backend.playback.play = lambda track: track != self.tracks[1] + self.playback.play() + self.assertEqual(self.playback.current_track, self.tracks[0]) + self.playback.on_end_of_track() + self.assertNotEqual(self.playback.current_track, self.tracks[1]) + self.assertEqual(self.playback.current_track, self.tracks[2]) + @populate_tracklist def test_end_of_track_track_before_play(self): self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[0]) @@ -509,7 +515,6 @@ class PlaybackControllerTest(object): def test_tracklist_position_at_end_of_playlist(self): self.playback.play(self.tracklist.tl_tracks[-1]) self.playback.on_end_of_track() - self.playback.on_end_of_stream() self.assertEqual(self.playback.tracklist_position, None) def test_on_tracklist_change_gets_called(self): @@ -815,7 +820,6 @@ class PlaybackControllerTest(object): def test_end_of_playlist_stops(self): self.playback.play(self.tracklist.tl_tracks[-1]) self.playback.on_end_of_track() - self.playback.on_end_of_stream() self.assertEqual(self.playback.state, PlaybackState.STOPPED) def test_repeat_off_by_default(self): diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index 39fb020d..bbde4bcc 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -17,7 +17,7 @@ class TracklistControllerTest(object): def setUp(self): self.audio = audio.DummyAudio.start().proxy() self.backend = self.backend_class.start(audio=self.audio).proxy() - self.core = core.Core(audio=self.audio, backends=[self.backend]) + self.core = core.Core(audio=audio, backends=[self.backend]) self.controller = self.core.tracklist self.playback = self.core.playback