diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index b154e21a..04b0ab06 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -20,7 +20,6 @@ logger = logging.getLogger('mopidy.audio') mixers.register_mixers() - MB = 1 << 20 @@ -50,6 +49,8 @@ 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 @@ -93,21 +94,28 @@ class Audio(pykka.ThreadingActor): self._playbin = playbin def _on_about_to_finish(self, element): - source, self._appsrc = self._appsrc, None - if source is None: - return + # 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') + self._appsrc_caps = None - self._disconnect(source, 'need-data') - self._disconnect(source, 'enough-data') - self._disconnect(source, 'seek-data') + # 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() def _on_new_source(self, element, pad): - uri = element.get_property('uri') - if not uri or not uri.startswith('appsrc://'): + source = element.get_property('source') + + if source.get_factory().get_name() != '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') @@ -280,6 +288,10 @@ 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 f49aa89b..2f077e49 100644 --- a/mopidy/backends/base.py +++ b/mopidy/backends/base.py @@ -88,8 +88,8 @@ class BaseLibraryProvider(object): class BasePlaybackProvider(object): """ - :param audio: the audio actor - :type audio: actor proxy to an instance of :class:`mopidy.audio.Audio` + :param audio: audio sub-system + :type audio: actor proxy to a :class:`mopidy.audio.Audio` actor. :param backend: the backend :type backend: :class:`mopidy.backends.base.Backend` """ @@ -121,9 +121,24 @@ class BasePlaybackProvider(object): :rtype: :class:`True` if successful, else :class:`False` """ self.audio.prepare_change() - self.audio.set_uri(track.uri).get() + 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 + def resume(self): """ Resume playback at the same time position playback was paused. diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index bda17634..f8de2e57 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -35,11 +35,9 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): super(SpotifyPlaybackProvider, self).__init__(*args, **kwargs) self._first_seek = False - def play(self, track): - if track.uri is None: - return False - + def change_track(self, track): spotify_backend = self.backend.actor_ref.proxy() + need_data_callback_bound = functools.partial( need_data_callback, spotify_backend) enough_data_callback_bound = functools.partial( @@ -49,21 +47,18 @@ 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.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) - + self.backend.spotify.session.play(1) 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 cd4ba180..03ad2712 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -46,6 +46,11 @@ 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) @@ -56,7 +61,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_track() + self.playback.on_end_of_stream() 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 21f09ad2..1f2e0384 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -314,6 +314,11 @@ 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. @@ -325,11 +330,14 @@ 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.play(self.tl_track_at_eot) - else: - self.stop(clear_current_track=True) + self.current_tl_track = self.tl_track_at_eot + self._get_backend().playback.change_track(self.current_track).get() + self._trigger_track_playback_started() 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 09dffbab..155fa661 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -110,6 +110,7 @@ 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) @@ -338,6 +339,8 @@ class PlaybackControllerTest(object): self.playback.on_end_of_track() + self.playback.on_end_of_stream() + self.assertEqual(self.playback.state, PlaybackState.STOPPED) @populate_tracklist @@ -346,6 +349,7 @@ 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) @@ -358,16 +362,6 @@ 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]) @@ -510,6 +504,7 @@ 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,6 +810,7 @@ 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 71f44018..39536edc 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -18,7 +18,7 @@ class TracklistControllerTest(object): def setUp(self): self.audio = mock.Mock(spec=audio.Audio) self.backend = self.backend_class.start(audio=self.audio).proxy() - self.core = core.Core(audio=audio, backends=[self.backend]) + self.core = core.Core(audio=self.audio, backends=[self.backend]) self.controller = self.core.tracklist self.playback = self.core.playback