diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index ae6d280d..55aa45b8 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -21,6 +21,10 @@ logger = logging.getLogger(__name__) playlists.register_typefinders() playlists.register_elements() +_GST_STATE_MAPPING = { + gst.STATE_PLAYING: PlaybackState.PLAYING, + gst.STATE_PAUSED: PlaybackState.PAUSED, + gst.STATE_NULL: PlaybackState.STOPPED} MB = 1 << 20 @@ -48,13 +52,14 @@ class Audio(pykka.ThreadingActor): #: The GStreamer state mapped to :class:`mopidy.audio.PlaybackState` state = PlaybackState.STOPPED - _target_state = gst.STATE_NULL def __init__(self, config, mixer): super(Audio, self).__init__() self._config = config self._mixer = mixer + self._target_state = gst.STATE_NULL + self._buffering = False self._playbin = None self._signal_ids = {} # {(element, event): signal_id} @@ -224,31 +229,17 @@ class Audio(pykka.ThreadingActor): self._disconnect(bus, 'message') bus.remove_signal_watch() - def _on_message(self, bus, message): - if (message.type == gst.MESSAGE_STATE_CHANGED - and message.src == self._playbin): - old_state, new_state, pending_state = message.parse_state_changed() - self._on_playbin_state_changed(old_state, new_state, pending_state) - elif message.type == gst.MESSAGE_BUFFERING: - percent = message.parse_buffering() - if percent < 10: - self._playbin.set_state(gst.STATE_PAUSED) - if percent == 100 and self._target_state == gst.STATE_PLAYING: - self._playbin.set_state(gst.STATE_PLAYING) - logger.debug('Buffer %d%% full', percent) - elif message.type == gst.MESSAGE_EOS: + def _on_message(self, bus, msg): + if msg.type == gst.MESSAGE_STATE_CHANGED and msg.src == self._playbin: + self._on_playbin_state_changed(*msg.parse_state_changed()) + elif msg.type == gst.MESSAGE_BUFFERING: + self._on_buffering(msg.parse_buffering()) + elif msg.type == gst.MESSAGE_EOS: self._on_end_of_stream() - elif message.type == gst.MESSAGE_ERROR: - error, debug = message.parse_error() - logger.error( - '%s Debug message: %s', - str(error).decode('utf-8'), debug.decode('utf-8') or 'None') - self.stop_playback() - elif message.type == gst.MESSAGE_WARNING: - error, debug = message.parse_warning() - logger.warning( - '%s Debug message: %s', - str(error).decode('utf-8'), debug.decode('utf-8') or 'None') + elif msg.type == gst.MESSAGE_ERROR: + self._on_error(*msg.parse_error()) + elif msg.type == gst.MESSAGE_WARNING: + self._on_warning(*msg.parse_warning()) def _on_playbin_state_changed(self, old_state, new_state, pending_state): if new_state == gst.STATE_READY and pending_state == gst.STATE_NULL: @@ -264,25 +255,45 @@ class Audio(pykka.ThreadingActor): if new_state == gst.STATE_READY: return # Ignore READY state as it's GStreamer specific - if new_state == gst.STATE_PLAYING: - new_state = PlaybackState.PLAYING - elif new_state == gst.STATE_PAUSED: - new_state = PlaybackState.PAUSED - elif new_state == gst.STATE_NULL: - new_state = PlaybackState.STOPPED - + new_state = _GST_STATE_MAPPING[new_state] old_state, self.state = self.state, new_state + target_state = _GST_STATE_MAPPING[self._target_state] + if target_state == new_state: + target_state = None + logger.debug( - 'Triggering event: state_changed(old_state=%s, new_state=%s)', - old_state, new_state) - AudioListener.send( - 'state_changed', old_state=old_state, new_state=new_state) + 'Triggering event: state_changed(old_state=%s, new_state=%s, ' + 'target_state=%s)', old_state, new_state, target_state) + AudioListener.send('state_changed', old_state=old_state, + new_state=new_state, target_state=target_state) + + def _on_buffering(self, percent): + if percent < 10 and not self._buffering: + self._playbin.set_state(gst.STATE_PAUSED) + self._buffering = True + if percent == 100: + self._buffering = False + if self._target_state == gst.STATE_PLAYING: + self._playbin.set_state(gst.STATE_PLAYING) + + logger.debug('Buffer %d%% full', percent) def _on_end_of_stream(self): logger.debug('Triggering reached_end_of_stream event') AudioListener.send('reached_end_of_stream') + def _on_error(self, error, debug): + logger.error( + '%s Debug message: %s', + str(error).decode('utf-8'), debug.decode('utf-8') or 'None') + self.stop_playback() + + def _on_warning(self, error, debug): + logger.warning( + '%s Debug message: %s', + str(error).decode('utf-8'), debug.decode('utf-8') or 'None') + def set_uri(self, uri): """ Set URI of audio to be played. @@ -404,6 +415,7 @@ class Audio(pykka.ThreadingActor): :rtype: :class:`True` if successfull, else :class:`False` """ + self._buffering = False return self._set_state(gst.STATE_NULL) def _set_state(self, state): diff --git a/mopidy/audio/listener.py b/mopidy/audio/listener.py index 537a81dd..d5203ab9 100644 --- a/mopidy/audio/listener.py +++ b/mopidy/audio/listener.py @@ -27,17 +27,31 @@ class AudioListener(listener.Listener): """ pass - def state_changed(self, old_state, new_state): + def state_changed(self, old_state, new_state, target_state): """ Called after the playback state have changed. Will be called for both immediate and async state changes in GStreamer. + Target state is used to when we should be in the target state, but + temporarily need to switch to an other state. A typical example of this + is buffering. When this happens an event with + `old=PLAYING, new=PAUSED, target=PLAYING` will be emitted. Once we have + caught up a `old=PAUSED, new=PLAYING, target=None` event will be + be generated. + + Regular state changes will not have target state set as they are final + states which should be stable. + *MAY* be implemented by actor. :param old_state: the state before the change :type old_state: string from :class:`mopidy.core.PlaybackState` field :param new_state: the state after the change + :type new_state: A :class:`mopidy.core.PlaybackState` field :type new_state: string from :class:`mopidy.core.PlaybackState` field + :param target_state: the intended state + :type target_state: string from :class:`mopidy.core.PlaybackState` + field or :class:`None` if this is a final state. """ pass diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 8e4408f7..66f2aa82 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -68,14 +68,17 @@ class Core( def reached_end_of_stream(self): self.playback.on_end_of_track() - def state_changed(self, old_state, new_state): + 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 # Spotify play token is lost, the Spotify backend pauses audio # playback, but mopidy.core doesn't know this, so we need to update # mopidy.core's state to match the actual state in mopidy.audio. If we # don't do this, clients will think that we're still playing. - if (new_state == PlaybackState.PAUSED + + # We ignore cases when target state is set as this is buffering + # updates (at least for now) and we need to get #234 fixed... + if (new_state == PlaybackState.PAUSED and not target_state and self.playback.state != PlaybackState.PAUSED): self.playback.state = new_state self.playback._trigger_track_playback_paused() diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 869d8ac6..3b9fcad5 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -142,22 +142,15 @@ class AudioBufferingTest(unittest.TestCase): self.audio = audio.Audio(config=None, mixer=None) self.audio._playbin = mock.Mock(spec=['set_state']) - self.buffer_full_message = mock.Mock() - self.buffer_full_message.type = gst.MESSAGE_BUFFERING - self.buffer_full_message.parse_buffering = mock.Mock(return_value=100) - - self.buffer_empty_message = mock.Mock() - self.buffer_empty_message.type = gst.MESSAGE_BUFFERING - self.buffer_empty_message.parse_buffering = mock.Mock(return_value=0) - def test_pause_when_buffer_empty(self): playbin = self.audio._playbin self.audio.start_playback() playbin.set_state.assert_called_with(gst.STATE_PLAYING) playbin.set_state.reset_mock() - self.audio._on_message(None, self.buffer_empty_message) + self.audio._on_buffering(0) playbin.set_state.assert_called_with(gst.STATE_PAUSED) + self.assertTrue(self.audio._buffering) def test_stay_paused_when_buffering_finished(self): playbin = self.audio._playbin @@ -165,5 +158,35 @@ class AudioBufferingTest(unittest.TestCase): playbin.set_state.assert_called_with(gst.STATE_PAUSED) playbin.set_state.reset_mock() - self.audio._on_message(None, self.buffer_full_message) + self.audio._on_buffering(100) self.assertEqual(playbin.set_state.call_count, 0) + self.assertFalse(self.audio._buffering) + + def test_change_to_paused_while_buffering(self): + playbin = self.audio._playbin + self.audio.start_playback() + playbin.set_state.assert_called_with(gst.STATE_PLAYING) + playbin.set_state.reset_mock() + + self.audio._on_buffering(0) + playbin.set_state.assert_called_with(gst.STATE_PAUSED) + self.audio.pause_playback() + playbin.set_state.reset_mock() + + self.audio._on_buffering(100) + self.assertEqual(playbin.set_state.call_count, 0) + self.assertFalse(self.audio._buffering) + + def test_change_to_stopped_while_buffering(self): + playbin = self.audio._playbin + self.audio.start_playback() + playbin.set_state.assert_called_with(gst.STATE_PLAYING) + playbin.set_state.reset_mock() + + self.audio._on_buffering(0) + playbin.set_state.assert_called_with(gst.STATE_PAUSED) + playbin.set_state.reset_mock() + + self.audio.stop_playback() + playbin.set_state.assert_called_with(gst.STATE_NULL) + self.assertFalse(self.audio._buffering) diff --git a/tests/audio/test_listener.py b/tests/audio/test_listener.py index 1690ab80..6e0366cf 100644 --- a/tests/audio/test_listener.py +++ b/tests/audio/test_listener.py @@ -15,13 +15,14 @@ class AudioListenerTest(unittest.TestCase): self.listener.state_changed = mock.Mock() self.listener.on_event( - 'state_changed', old_state='stopped', new_state='playing') + 'state_changed', old_state='stopped', new_state='playing', + target_state=None) self.listener.state_changed.assert_called_with( - old_state='stopped', new_state='playing') + old_state='stopped', new_state='playing', target_state=None) def test_listener_has_default_impl_for_reached_end_of_stream(self): self.listener.reached_end_of_stream() def test_listener_has_default_impl_for_state_changed(self): - self.listener.state_changed(None, None) + self.listener.state_changed(None, None, None)