From 850b023758458593b2d7edc09904b5eb4551d622 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 25 Jan 2014 14:47:23 +0100 Subject: [PATCH 01/14] audio: Add debug logging of stream switching --- mopidy/audio/actor.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index ca023125..068981fc 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -115,6 +115,8 @@ class Audio(pykka.ThreadingActor): self._disconnect(source, 'enough-data') self._disconnect(source, 'seek-data') + logger.debug('Ready to switch to new stream') + def _on_new_source(self, element, pad): uri = element.get_property('uri') if not uri or not uri.startswith('appsrc://'): @@ -292,6 +294,9 @@ class Audio(pykka.ThreadingActor): logger.warning( '%s Debug message: %s', str(error).decode('utf-8'), debug.decode('utf-8') or 'None') + elif message.type == gst.MESSAGE_ELEMENT: + if message.structure.has_name('playbin2-stream-changed'): + logger.debug('Playback of new stream started') def _on_playbin_state_changed(self, old_state, new_state, pending_state): if new_state == gst.STATE_READY and pending_state == gst.STATE_NULL: From 0cfd69432e8ba9781e1ff1aa40c22ef7d922c4e1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 25 Jan 2014 15:13:10 +0100 Subject: [PATCH 02/14] audio: Queue audio sink data to give us some headroom for about to finish events --- mopidy/audio/actor.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 068981fc..0f510a73 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -160,15 +160,35 @@ class Audio(pykka.ThreadingActor): def _setup_output(self): output_desc = self._config['audio']['output'] try: - output = gst.parse_bin_from_description( + user_output = gst.parse_bin_from_description( output_desc, ghost_unconnected_pads=True) - self._playbin.set_property('audio-sink', output) - logger.info('Audio output set to "%s"', output_desc) except gobject.GError as ex: logger.error( 'Failed to create audio output "%s": %s', output_desc, ex) process.exit_process() + output = gst.Bin('output') + + # Queue element to buy use time between the about to finish event and + # the actual switch, i.e. about to switch can block for longer thanks + # to this queue. + # TODO: make the min-max values a setting? + queue = gst.element_factory_make('queue') + queue.set_property('max-size-buffers', 0) + queue.set_property('max-size-bytes', 0) + queue.set_property('max-size-time', 5 * gst.SECOND) + queue.set_property('min-threshold-time', 3 * gst.SECOND) + + output.add(user_output) + output.add(queue) + + queue.link(user_output) + ghost_pad = gst.GhostPad('sink', queue.get_pad('sink')) + output.add_pad(ghost_pad) + + logger.info('Audio output set to "%s"', output_desc) + self._playbin.set_property('audio-sink', output) + def _setup_visualizer(self): visualizer_element = self._config['audio']['visualizer'] if not visualizer_element: From 100b32af98e9a03555b2ac9864f2a22d65141ac7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 25 Jan 2014 16:29:09 +0100 Subject: [PATCH 03/14] core: Add a bunch of TODOs --- mopidy/audio/actor.py | 2 ++ mopidy/core/playback.py | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 0f510a73..f2003ca0 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -42,6 +42,7 @@ PLAYBIN_FLAGS = (1 << 1) | (1 << 4) | (1 << 7) PLAYBIN_VIS_FLAGS = PLAYBIN_FLAGS | (1 << 3) +# TODO: split out mixer as these are to intertwined right now class Audio(pykka.ThreadingActor): """ Audio output through `GStreamer `_. @@ -412,6 +413,7 @@ class Audio(pykka.ThreadingActor): We will get a GStreamer message when the stream playback reaches the token, and can then do any end-of-stream related tasks. """ + # TODO: replace this with emit_data(None)? self._playbin.get_property('source').emit('end-of-stream') def get_position(self): diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index b2acb35a..5f296c89 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -11,6 +11,7 @@ from . import listener logger = logging.getLogger(__name__) +# TODO: split mixing out from playback? class PlaybackController(object): pykka_traversable = True @@ -24,6 +25,7 @@ class PlaybackController(object): self._mute = False def _get_backend(self): + # TODO: take in track instead if self.current_tl_track is None: return None uri = self.current_tl_track.track.uri @@ -129,6 +131,7 @@ class PlaybackController(object): ### Methods + # TODO: remove this. def change_track(self, tl_track, on_error_step=1): """ Change to the given track, keeping the current playback state. @@ -147,6 +150,7 @@ class PlaybackController(object): elif old_state == PlaybackState.PAUSED: self.pause() + # TODO: this is not really end of track, this is on_need_next_track def on_end_of_track(self): """ Tell the playback controller that end of track is reached. @@ -184,6 +188,9 @@ class PlaybackController(object): """ tl_track = self.core.tracklist.next_track(self.current_tl_track) if tl_track: + # TODO: switch to: + # backend.play(track) + # wait for state change? self.change_track(tl_track) else: self.stop(clear_current_track=True) @@ -192,6 +199,9 @@ class PlaybackController(object): """Pause playback.""" backend = self._get_backend() if not backend or backend.playback.pause().get(): + # TODO: switch to: + # backend.track(pause) + # wait for state change? self.state = PlaybackState.PAUSED self._trigger_track_playback_paused() @@ -226,6 +236,10 @@ class PlaybackController(object): assert tl_track in self.core.tracklist.tl_tracks + # TODO: switch to: + # backend.play(track) + # wait for state change? + if self.state == PlaybackState.PLAYING: self.stop() @@ -236,6 +250,7 @@ class PlaybackController(object): if success: self.core.tracklist.mark_playing(tl_track) + # TODO: replace with stream-changed self._trigger_track_playback_started() else: self.core.tracklist.mark_unplayable(tl_track) @@ -253,6 +268,9 @@ class PlaybackController(object): will continue. If it was paused, it will still be paused, etc. """ tl_track = self.current_tl_track + # TODO: switch to: + # self.play(....) + # wait for state change? self.change_track( self.core.tracklist.previous_track(tl_track), on_error_step=-1) @@ -263,7 +281,11 @@ class PlaybackController(object): backend = self._get_backend() if backend and backend.playback.resume().get(): self.state = PlaybackState.PLAYING + # TODO: trigger via gst messages self._trigger_track_playback_resumed() + # TODO: switch to: + # backend.resume() + # wait for state change? def seek(self, time_position): """ From d962277bc9fa937d6965964e89fd45987a5f25a0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 26 Jan 2014 23:27:30 +0100 Subject: [PATCH 04/14] audio: Add stream_changed event plus tests --- mopidy/audio/actor.py | 25 +++++++++++++++++++++++-- mopidy/audio/listener.py | 10 ++++++++++ tests/audio/test_actor.py | 13 +++++++++++++ tests/audio/test_listener.py | 3 +++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index f2003ca0..44794201 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -317,7 +317,12 @@ class Audio(pykka.ThreadingActor): str(error).decode('utf-8'), debug.decode('utf-8') or 'None') elif message.type == gst.MESSAGE_ELEMENT: if message.structure.has_name('playbin2-stream-changed'): - logger.debug('Playback of new stream started') + self._on_stream_changed(message) + + def _on_stream_changed(self, message): + uri = message.structure['uri'] + logger.debug('Triggering event: stream_changed(uri=%s)', uri) + AudioListener.send('stream_changed', uri=uri) def _on_playbin_state_changed(self, old_state, new_state, pending_state): if new_state == gst.STATE_READY and pending_state == gst.STATE_NULL: @@ -349,7 +354,7 @@ class Audio(pykka.ThreadingActor): 'state_changed', old_state=old_state, new_state=new_state) def _on_end_of_stream(self): - logger.debug('Triggering reached_end_of_stream event') + logger.debug('Triggering event: reached_end_of_stream event') AudioListener.send('reached_end_of_stream') def set_uri(self, uri): @@ -476,6 +481,22 @@ class Audio(pykka.ThreadingActor): """ return self._set_state(gst.STATE_NULL) + def wait_for_state_change(self): + """Block until any pending state changes are complete. + + Should only be used by test. + """ + self._playbin.get_state() + + def process_messages(self): + """Manually process messages from bus. + + Should only be used by test. + """ + bus = self._playbin.get_bus() + while bus.have_pending(): + self._on_message(bus, bus.pop()) + def _set_state(self, state): """ Internal method for setting the raw GStreamer state. diff --git a/mopidy/audio/listener.py b/mopidy/audio/listener.py index 537a81dd..d2690031 100644 --- a/mopidy/audio/listener.py +++ b/mopidy/audio/listener.py @@ -27,6 +27,16 @@ class AudioListener(listener.Listener): """ pass + def stream_changed(self, uri): + """ + Called whenever the end of the audio stream changes. + + *MAY* be implemented by actor. + + :param string uri: URI the stream has started playing. + """ + pass + def state_changed(self, old_state, new_state): """ Called after the playback state have changed. diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 3f7e56ce..b0ee4429 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import mock import unittest import pygst @@ -114,6 +115,18 @@ class AudioTest(unittest.TestCase): def test_invalid_output_raises_error(self): pass # TODO + @mock.patch.object(audio.AudioListener, 'send') + def test_stream_changed_event(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + + self.audio.wait_for_state_change() + self.audio.process_messages().get() + + call = mock.call('stream_changed', uri=self.song_uri) + self.assertIn(call, send_mock.call_args_list) + class AudioStateTest(unittest.TestCase): def setUp(self): diff --git a/tests/audio/test_listener.py b/tests/audio/test_listener.py index 08286cf9..c579fd55 100644 --- a/tests/audio/test_listener.py +++ b/tests/audio/test_listener.py @@ -24,3 +24,6 @@ class AudioListenerTest(unittest.TestCase): def test_listener_has_default_impl_for_state_changed(self): self.listener.state_changed(None, None) + + def test_listener_has_default_impl_for_stream_changed(self): + self.listener.stream_changed(None) From a4315251ca8582a725ff1e86c1875d4155e6f01e Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 26 Jan 2014 23:30:12 +0100 Subject: [PATCH 05/14] audio: Slight test refactor to not hide what is going on --- tests/audio/test_actor.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index b0ee4429..598ff3d1 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -35,25 +35,25 @@ class AudioTest(unittest.TestCase): def tearDown(self): pykka.ActorRegistry.stop_all() - def prepare_uri(self, uri): - self.audio.prepare_change() - self.audio.set_uri(uri) - def test_start_playback_existing_file(self): - self.prepare_uri(self.song_uri) + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) self.assertTrue(self.audio.start_playback().get()) def test_start_playback_non_existing_file(self): - self.prepare_uri(self.song_uri + 'bogus') + self.audio.prepare_change() + self.audio.set_uri(self.song_uri + 'bogus') self.assertFalse(self.audio.start_playback().get()) def test_pause_playback_while_playing(self): - self.prepare_uri(self.song_uri) + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) self.audio.start_playback() self.assertTrue(self.audio.pause_playback().get()) def test_stop_playback_while_playing(self): - self.prepare_uri(self.song_uri) + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) self.audio.start_playback() self.assertTrue(self.audio.stop_playback().get()) From 6490d5bd2dbd3f949f0c84c30846056e0814e99b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 27 Jan 2014 22:34:42 +0100 Subject: [PATCH 06/14] audio: Add more tests for audio events --- mopidy/audio/actor.py | 17 +++-- tests/audio/test_actor.py | 147 +++++++++++++++++++++++++++++++++++++- 2 files changed, 155 insertions(+), 9 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 44794201..cbec0b4c 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -352,6 +352,8 @@ class Audio(pykka.ThreadingActor): old_state, new_state) AudioListener.send( 'state_changed', old_state=old_state, new_state=new_state) + if new_state == PlaybackState.STOPPED: + AudioListener.send('stream_changed', uri=None) def _on_end_of_stream(self): logger.debug('Triggering event: reached_end_of_stream event') @@ -484,18 +486,21 @@ class Audio(pykka.ThreadingActor): def wait_for_state_change(self): """Block until any pending state changes are complete. - Should only be used by test. + Should only be used by tests. """ self._playbin.get_state() - def process_messages(self): - """Manually process messages from bus. + def enable_sync_handler(self): + """Enable manual processing of messages from bus. - Should only be used by test. + Should only be used by tests. """ + def sync_handler(bus, message): + self._on_message(bus, message) + return gst.BUS_DROP + bus = self._playbin.get_bus() - while bus.have_pending(): - self._on_message(bus, bus.pop()) + bus.set_sync_handler(sync_handler) def _set_state(self, state): """ diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 598ff3d1..8c956ab3 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import mock +import threading import unittest import pygst @@ -13,6 +14,7 @@ gobject.threads_init() import pykka from mopidy import audio +from mopidy.audio.constants import PlaybackState from mopidy.utils.path import path_to_uri from tests import path_to_data_dir @@ -115,18 +117,157 @@ class AudioTest(unittest.TestCase): def test_invalid_output_raises_error(self): pass # TODO - @mock.patch.object(audio.AudioListener, 'send') - def test_stream_changed_event(self, send_mock): + +@mock.patch.object(audio.AudioListener, 'send') +class AudioEventTest(unittest.TestCase): + def setUp(self): + config = { + 'audio': { + 'mixer': 'fakemixer track_max_volume=65536', + 'mixer_track': None, + 'mixer_volume': None, + 'output': 'fakesink', + 'visualizer': None, + } + } + self.song_uri = path_to_uri(path_to_data_dir('song1.wav')) + self.audio = audio.Audio.start(config=config).proxy() + self.audio.enable_sync_handler().get() + + def tearDown(self): + pykka.ActorRegistry.stop_all() + + # TODO: test wihtout uri set, with bad uri and gapless... + + def test_state_change_stopped_to_playing_event(self, send_mock): self.audio.prepare_change() self.audio.set_uri(self.song_uri) self.audio.start_playback() + self.audio.wait_for_state_change().get() + call = mock.call('state_changed', old_state=PlaybackState.STOPPED, + new_state=PlaybackState.PLAYING) + self.assertIn(call, send_mock.call_args_list) + + def test_state_change_stopped_to_paused_event(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.pause_playback() + + self.audio.wait_for_state_change().get() + call = mock.call('state_changed', old_state=PlaybackState.STOPPED, + new_state=PlaybackState.PAUSED) + self.assertIn(call, send_mock.call_args_list) + + def test_state_change_paused_to_playing_event(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.pause_playback() self.audio.wait_for_state_change() - self.audio.process_messages().get() + self.audio.start_playback() + + self.audio.wait_for_state_change().get() + call = mock.call('state_changed', old_state=PlaybackState.PAUSED, + new_state=PlaybackState.PLAYING) + self.assertIn(call, send_mock.call_args_list) + + def test_state_change_paused_to_stopped_event(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.pause_playback() + self.audio.wait_for_state_change() + self.audio.stop_playback() + + self.audio.wait_for_state_change().get() + call = mock.call('state_changed', old_state=PlaybackState.PAUSED, + new_state=PlaybackState.STOPPED) + self.assertIn(call, send_mock.call_args_list) + + def test_state_change_playing_to_paused_event(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + self.audio.wait_for_state_change() + self.audio.pause_playback() + + self.audio.wait_for_state_change().get() + call = mock.call('state_changed', old_state=PlaybackState.PLAYING, + new_state=PlaybackState.PAUSED) + self.assertIn(call, send_mock.call_args_list) + + def test_state_change_playing_to_stopped_event(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + self.audio.wait_for_state_change() + self.audio.stop_playback() + + self.audio.wait_for_state_change().get() + call = mock.call('state_changed', old_state=PlaybackState.PLAYING, + new_state=PlaybackState.STOPPED) + self.assertIn(call, send_mock.call_args_list) + + def test_stream_changed_event_on_playing(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + + # Since we are going from stopped to playing, the state change is + # enough to ensure the stream changed. + self.audio.wait_for_state_change().get() call = mock.call('stream_changed', uri=self.song_uri) self.assertIn(call, send_mock.call_args_list) + def test_stream_changed_event_on_paused_to_stopped(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.pause_playback() + self.audio.wait_for_state_change() + self.audio.stop_playback() + + self.audio.wait_for_state_change().get() + + call = mock.call('stream_changed', uri=None) + self.assertIn(call, send_mock.call_args_list) + + # Unlike the other events, having the state changed done is not + # enough to ensure our event is called. So we setup a threading + # event that we can wait for with a timeout while the track playback + # completes. + + def test_stream_changed_event_on_paused(self, send_mock): + event = threading.Event() + + def send(name, **kwargs): + if name == 'stream_changed': + event.set() + send_mock.side_effect = send + + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.pause_playback().get() + self.audio.wait_for_state_change().get() + + if not event.wait(timeout=5.0): + self.fail('Stream changed not reached within deadline') + + def test_reached_end_of_stream_event(self, send_mock): + event = threading.Event() + + def send(name, **kwargs): + if name == 'reached_end_of_stream': + event.set() + send_mock.side_effect = send + + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + self.audio.wait_for_state_change().get() + + if not event.wait(timeout=5.0): + self.fail('End of stream not reached within deadline') + class AudioStateTest(unittest.TestCase): def setUp(self): From 99d581f7fc12e1db9633d97d849ccb39d940d44b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 27 Jan 2014 23:04:15 +0100 Subject: [PATCH 07/14] audio: Add position_changed event to know when seeks happen --- mopidy/audio/actor.py | 9 ++++++ mopidy/audio/listener.py | 10 +++++++ tests/audio/test_actor.py | 56 ++++++++++++++++++++++++++++++++++++ tests/audio/test_listener.py | 3 ++ 4 files changed, 78 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index cbec0b4c..5fc84411 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -180,6 +180,8 @@ class Audio(pykka.ThreadingActor): queue.set_property('max-size-time', 5 * gst.SECOND) queue.set_property('min-threshold-time', 3 * gst.SECOND) + queue.get_pad('src').add_event_probe(self._on_pad_event) + output.add(user_output) output.add(queue) @@ -294,6 +296,13 @@ class Audio(pykka.ThreadingActor): self._disconnect(bus, 'message') bus.remove_signal_watch() + def _on_pad_event(self, pad, event): + if event.type == gst.EVENT_NEWSEGMENT: + # update, rate, format, start, stop, position + position = event.parse_new_segment()[5] // gst.MSECOND + AudioListener.send('position_changed', position=position) + return True + def _on_message(self, bus, message): if (message.type == gst.MESSAGE_STATE_CHANGED and message.src == self._playbin): diff --git a/mopidy/audio/listener.py b/mopidy/audio/listener.py index d2690031..5b33ffe6 100644 --- a/mopidy/audio/listener.py +++ b/mopidy/audio/listener.py @@ -37,6 +37,16 @@ class AudioListener(listener.Listener): """ pass + def position_changed(self, position_changed): + """ + Called whenever the position of the stream changes. + + *MAY* be implemented by actor. + + :param int position: Position in milliseconds. + """ + pass + def state_changed(self, old_state, new_state): """ Called after the playback state have changed. diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 8c956ab3..ad88b92a 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -231,6 +231,62 @@ class AudioEventTest(unittest.TestCase): call = mock.call('stream_changed', uri=None) self.assertIn(call, send_mock.call_args_list) + def test_position_changed_on_pause(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.pause_playback() + self.audio.wait_for_state_change() + + self.audio.wait_for_state_change().get() + + call = mock.call('position_changed', position=0) + self.assertIn(call, send_mock.call_args_list) + + def test_position_changed_on_play(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + self.audio.wait_for_state_change() + + self.audio.wait_for_state_change().get() + + call = mock.call('position_changed', position=0) + self.assertIn(call, send_mock.call_args_list) + + def test_position_changed_on_seek(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.set_position(2000) + + self.audio.wait_for_state_change().get() + + call = mock.call('position_changed', position=0) + self.assertNotIn(call, send_mock.call_args_list) + + def test_position_changed_on_seek_after_play(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + self.audio.wait_for_state_change() + self.audio.set_position(2000) + + self.audio.wait_for_state_change().get() + + call = mock.call('position_changed', position=2000) + self.assertIn(call, send_mock.call_args_list) + + def test_position_changed_on_seek_after_pause(self, send_mock): + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.pause_playback() + self.audio.wait_for_state_change() + self.audio.set_position(2000) + + self.audio.wait_for_state_change().get() + + call = mock.call('position_changed', position=2000) + self.assertIn(call, send_mock.call_args_list) + # Unlike the other events, having the state changed done is not # enough to ensure our event is called. So we setup a threading # event that we can wait for with a timeout while the track playback diff --git a/tests/audio/test_listener.py b/tests/audio/test_listener.py index c579fd55..84f5b59e 100644 --- a/tests/audio/test_listener.py +++ b/tests/audio/test_listener.py @@ -27,3 +27,6 @@ class AudioListenerTest(unittest.TestCase): def test_listener_has_default_impl_for_stream_changed(self): self.listener.stream_changed(None) + + def test_listener_has_default_impl_for_position_changed(self): + self.listener.position_changed(None) From 0d18bdea79fb36c7edebed1c3650f5efc360310a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 27 Jan 2014 23:43:11 +0100 Subject: [PATCH 08/14] audio: Add about to finish callback support --- mopidy/audio/actor.py | 30 ++++++++++++++++++++++-------- tests/audio/test_actor.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 5fc84411..64874938 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -58,6 +58,7 @@ class Audio(pykka.ThreadingActor): self._playbin = None self._signal_ids = {} # {(element, event): signal_id} + self._about_to_finish_callback = None self._mixer = None self._mixer_track = None @@ -108,15 +109,15 @@ class Audio(pykka.ThreadingActor): def _on_about_to_finish(self, element): source, self._appsrc = self._appsrc, None - if source is None: - return - self._appsrc_caps = None + if source is not None: + self._appsrc_caps = None + self._disconnect(source, 'need-data') + self._disconnect(source, 'enough-data') + self._disconnect(source, 'seek-data') - self._disconnect(source, 'need-data') - self._disconnect(source, 'enough-data') - self._disconnect(source, 'seek-data') - - logger.debug('Ready to switch to new stream') + if self._about_to_finish_callback: + logger.debug('Calling about to finish callback.') + self._about_to_finish_callback() def _on_new_source(self, element, pad): uri = element.get_property('uri') @@ -432,6 +433,19 @@ class Audio(pykka.ThreadingActor): # TODO: replace this with emit_data(None)? self._playbin.get_property('source').emit('end-of-stream') + def set_about_to_finish_callback(self, callback): + """ + Configure audio to use an about to finish callback. + + This should be used to achieve gapless playback. For this to work the + callback *MUST* call :meth:`set_uri` with the new URI to play and + block until this call has been made. :meth:`prepare_change` is not + needed before :meth:`set_uri` in this one special case. + + :param callable callback: Callback to run when we need the next URI. + """ + self._about_to_finish_callback = callback + def get_position(self): """ Get position in milliseconds. diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index ad88b92a..756c4d62 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -324,6 +324,45 @@ class AudioEventTest(unittest.TestCase): if not event.wait(timeout=5.0): self.fail('End of stream not reached within deadline') + # Make sure that gapless really works: + + def test_gapless(self, send_mock): + song2_uri = path_to_uri(path_to_data_dir('song2.wav')) + + uris = [song2_uri] + events = [] + done = threading.Event() + + def callback(): + if uris: + self.audio.set_uri(uris.pop()).get() + + def send(name, **kwargs): + events.append((name, kwargs)) + if name == 'reached_end_of_stream': + done.set() + + send_mock.side_effect = send + self.audio.set_about_to_finish_callback(callback).get() + + self.audio.prepare_change() + self.audio.set_uri(self.song_uri) + self.audio.start_playback() + self.audio.wait_for_state_change().get() + + if not done.wait(timeout=5.0): + self.fail('EOS not received') + + excepted = [ + ('position_changed', {'position': 0}), + ('stream_changed', {'uri': self.song_uri}), + ('state_changed', {'old_state': PlaybackState.STOPPED, + 'new_state': PlaybackState.PLAYING}), + ('position_changed', {'position': 0}), + ('stream_changed', {'uri': song2_uri}), + ('reached_end_of_stream', {})] + self.assertEqual(excepted, events) + class AudioStateTest(unittest.TestCase): def setUp(self): From a660524113e87a21b081fd402eef60dbade74aff Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 28 Jan 2014 00:03:57 +0100 Subject: [PATCH 09/14] audio: Debug log position_changed event triggering --- mopidy/audio/actor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 64874938..41206c75 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -301,6 +301,8 @@ class Audio(pykka.ThreadingActor): if event.type == gst.EVENT_NEWSEGMENT: # update, rate, format, start, stop, position position = event.parse_new_segment()[5] // gst.MSECOND + logger.debug('Triggering event: position_changed(position=%s)', + position) AudioListener.send('position_changed', position=position) return True From d1b91117b43fbcaa6d67e8589bb83f14e52c50c0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 28 Jan 2014 21:34:31 +0100 Subject: [PATCH 10/14] audio: Update dummy audio used for testing --- mopidy/audio/dummy.py | 31 +++++++++++++++++++++++++++---- tests/audio/test_actor.py | 1 + 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/mopidy/audio/dummy.py b/mopidy/audio/dummy.py index ad14390f..272fe346 100644 --- a/mopidy/audio/dummy.py +++ b/mopidy/audio/dummy.py @@ -17,12 +17,12 @@ class DummyAudio(pykka.ThreadingActor): super(DummyAudio, self).__init__() self.state = PlaybackState.STOPPED self._position = 0 - - def set_on_end_of_track(self, callback): - pass + self._callback = None + self._uri = None def set_uri(self, uri): - pass + assert self._uri is None, 'prepare change not called before set' + self._uri = uri def set_appsrc(self, *args, **kwargs): pass @@ -38,6 +38,7 @@ class DummyAudio(pykka.ThreadingActor): def set_position(self, position): self._position = position + AudioListener.send('position_changed', position=position) return True def start_playback(self): @@ -47,6 +48,7 @@ class DummyAudio(pykka.ThreadingActor): return self._change_state(PlaybackState.PAUSED) def prepare_change(self): + self._uri = None return True def stop_playback(self): @@ -61,8 +63,29 @@ class DummyAudio(pykka.ThreadingActor): def set_metadata(self, track): pass + def set_about_to_finish_callback(self, callback): + self._callback = callback + def _change_state(self, new_state): + if not self._uri: + return False + + if self.state == PlaybackState.STOPPED and self._uri: + AudioListener.send('stream_changed', uri=self._uri) + old_state, self.state = self.state, new_state AudioListener.send( 'state_changed', old_state=old_state, new_state=new_state) + return True + + def trigger_fake_end_of_stream(self): + AudioListener.send('reached_end_of_stream') + + def trigger_fake_about_to_finish(self): + if not self._callback: + return + self.prepare_change() + self._callback() + if not self._uri: + self.trigger_fake_end_of_stream() diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 756c4d62..55b79a97 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -138,6 +138,7 @@ class AudioEventTest(unittest.TestCase): pykka.ActorRegistry.stop_all() # TODO: test wihtout uri set, with bad uri and gapless... + # TODO: playing->playing triggered by seek should be removed def test_state_change_stopped_to_playing_event(self, send_mock): self.audio.prepare_change() From dd98ad83535da97649b1fcf5d0921d8b006a3431 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 28 Jan 2014 23:19:33 +0100 Subject: [PATCH 11/14] audio: Update tests and dummy audio actor to pass --- mopidy/audio/dummy.py | 47 +++++--- tests/audio/test_actor.py | 218 ++++++++++++++++++++++---------------- 2 files changed, 160 insertions(+), 105 deletions(-) diff --git a/mopidy/audio/dummy.py b/mopidy/audio/dummy.py index 272fe346..ee7e73b7 100644 --- a/mopidy/audio/dummy.py +++ b/mopidy/audio/dummy.py @@ -13,12 +13,14 @@ from .listener import AudioListener class DummyAudio(pykka.ThreadingActor): - def __init__(self): + def __init__(self, config=None): super(DummyAudio, self).__init__() self.state = PlaybackState.STOPPED + self._volume = 0 self._position = 0 self._callback = None self._uri = None + self._state_change_result = True def set_uri(self, uri): assert self._uri is None, 'prepare change not called before set' @@ -55,10 +57,11 @@ class DummyAudio(pykka.ThreadingActor): return self._change_state(PlaybackState.STOPPED) def get_volume(self): - return 0 + return self._volume def set_volume(self, volume): - pass + self._volume = volume + return True def set_metadata(self, track): pass @@ -66,26 +69,44 @@ class DummyAudio(pykka.ThreadingActor): def set_about_to_finish_callback(self, callback): self._callback = callback + def enable_sync_handler(self): + pass + + def wait_for_state_change(self): + pass + def _change_state(self, new_state): if not self._uri: return False if self.state == PlaybackState.STOPPED and self._uri: + AudioListener.send('position_changed', position=0) + AudioListener.send('stream_changed', uri=self._uri) + + if new_state == PlaybackState.STOPPED: + self._uri = None AudioListener.send('stream_changed', uri=self._uri) old_state, self.state = self.state, new_state AudioListener.send( 'state_changed', old_state=old_state, new_state=new_state) - return True + return self._state_change_result - def trigger_fake_end_of_stream(self): - AudioListener.send('reached_end_of_stream') + def trigger_fake_playback_failure(self): + self._state_change_result = False - def trigger_fake_about_to_finish(self): - if not self._callback: - return - self.prepare_change() - self._callback() - if not self._uri: - self.trigger_fake_end_of_stream() + def get_about_to_finish_callback(self): + # This needs to be called from outside the actor or we lock up. + def wrapper(): + if self._callback: + self.prepare_change() + self._callback() + + if not self._uri or not self._callback: + AudioListener.send('reached_end_of_stream') + else: + AudioListener.send('position_changed', position=0) + AudioListener.send('stream_changed', uri=self._uri) + + return wrapper diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 55b79a97..e5272efc 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -14,48 +14,82 @@ gobject.threads_init() import pykka from mopidy import audio +from mopidy.audio import dummy as dummy_audio from mopidy.audio.constants import PlaybackState from mopidy.utils.path import path_to_uri from tests import path_to_data_dir +""" +We want to make sure both our real audio class and the fake one behave +correctly. So each test is first run against the real class, then repeated +against our dummy. +""" -class AudioTest(unittest.TestCase): - def setUp(self): - config = { - 'audio': { - 'mixer': 'fakemixer track_max_volume=65536', - 'mixer_track': None, - 'mixer_volume': None, - 'output': 'fakesink', - 'visualizer': None, - } + +class BaseTest(unittest.TestCase): + config = { + 'audio': { + 'mixer': 'fakemixer track_max_volume=65536', + 'mixer_track': None, + 'mixer_volume': None, + 'output': 'fakesink', + 'visualizer': None, } - self.song_uri = path_to_uri(path_to_data_dir('song1.wav')) - self.audio = audio.Audio.start(config=config).proxy() + } + + uris = [path_to_uri(path_to_data_dir('song1.wav')), + path_to_uri(path_to_data_dir('song2.wav'))] + + audio_class = audio.Audio + + def setUp(self): + self.audio = self.audio_class.start(config=self.config).proxy() def tearDown(self): pykka.ActorRegistry.stop_all() + def possibly_trigger_fake_playback_error(self): + pass + + def possibly_trigger_fake_about_to_finish(self): + pass + + +class DummyMixin(object): + audio_class = dummy_audio.DummyAudio + + def possibly_trigger_fake_playback_error(self): + self.audio.trigger_fake_playback_failure() + + def possibly_trigger_fake_about_to_finish(self): + callback = self.audio.get_about_to_finish_callback().get() + if callback: + callback() + + +class AudioTest(BaseTest): def test_start_playback_existing_file(self): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.assertTrue(self.audio.start_playback().get()) def test_start_playback_non_existing_file(self): + self.possibly_trigger_fake_playback_error() + self.audio.prepare_change() - self.audio.set_uri(self.song_uri + 'bogus') + self.audio.set_uri(self.uris[0] + 'bogus') self.assertFalse(self.audio.start_playback().get()) def test_pause_playback_while_playing(self): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.assertTrue(self.audio.pause_playback().get()) def test_stop_playback_while_playing(self): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.assertTrue(self.audio.stop_playback().get()) @@ -67,40 +101,6 @@ class AudioTest(unittest.TestCase): def test_end_of_data_stream(self): pass # TODO - def test_set_volume(self): - for value in range(0, 101): - self.assertTrue(self.audio.set_volume(value).get()) - self.assertEqual(value, self.audio.get_volume().get()) - - def test_set_volume_with_mixer_max_below_100(self): - config = { - 'audio': { - 'mixer': 'fakemixer track_max_volume=40', - 'mixer_track': None, - 'mixer_volume': None, - 'output': 'fakesink', - 'visualizer': None, - } - } - self.audio = audio.Audio.start(config=config).proxy() - - for value in range(0, 101): - self.assertTrue(self.audio.set_volume(value).get()) - self.assertEqual(value, self.audio.get_volume().get()) - - def test_set_volume_with_mixer_min_equal_max(self): - config = { - 'audio': { - 'mixer': 'fakemixer track_max_volume=0', - 'mixer_track': None, - 'mixer_volume': None, - 'output': 'fakesink', - 'visualizer': None, - } - } - self.audio = audio.Audio.start(config=config).proxy() - self.assertEqual(0, self.audio.get_volume().get()) - @unittest.SkipTest def test_set_mute(self): pass # TODO Probably needs a fakemixer with a mixer track @@ -118,31 +118,22 @@ class AudioTest(unittest.TestCase): pass # TODO -@mock.patch.object(audio.AudioListener, 'send') -class AudioEventTest(unittest.TestCase): - def setUp(self): - config = { - 'audio': { - 'mixer': 'fakemixer track_max_volume=65536', - 'mixer_track': None, - 'mixer_volume': None, - 'output': 'fakesink', - 'visualizer': None, - } - } - self.song_uri = path_to_uri(path_to_data_dir('song1.wav')) - self.audio = audio.Audio.start(config=config).proxy() - self.audio.enable_sync_handler().get() +class AudioDummyTest(DummyMixin, AudioTest): + pass - def tearDown(self): - pykka.ActorRegistry.stop_all() + +@mock.patch.object(audio.AudioListener, 'send') +class AudioEventTest(BaseTest): + def setUp(self): + super(AudioEventTest, self).setUp() + self.audio.enable_sync_handler().get() # TODO: test wihtout uri set, with bad uri and gapless... # TODO: playing->playing triggered by seek should be removed def test_state_change_stopped_to_playing_event(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.audio.wait_for_state_change().get() @@ -152,7 +143,7 @@ class AudioEventTest(unittest.TestCase): def test_state_change_stopped_to_paused_event(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.pause_playback() self.audio.wait_for_state_change().get() @@ -162,7 +153,7 @@ class AudioEventTest(unittest.TestCase): def test_state_change_paused_to_playing_event(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.pause_playback() self.audio.wait_for_state_change() self.audio.start_playback() @@ -174,7 +165,7 @@ class AudioEventTest(unittest.TestCase): def test_state_change_paused_to_stopped_event(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.pause_playback() self.audio.wait_for_state_change() self.audio.stop_playback() @@ -186,7 +177,7 @@ class AudioEventTest(unittest.TestCase): def test_state_change_playing_to_paused_event(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.audio.wait_for_state_change() self.audio.pause_playback() @@ -198,7 +189,7 @@ class AudioEventTest(unittest.TestCase): def test_state_change_playing_to_stopped_event(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.audio.wait_for_state_change() self.audio.stop_playback() @@ -210,19 +201,19 @@ class AudioEventTest(unittest.TestCase): def test_stream_changed_event_on_playing(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() # Since we are going from stopped to playing, the state change is # enough to ensure the stream changed. self.audio.wait_for_state_change().get() - call = mock.call('stream_changed', uri=self.song_uri) + call = mock.call('stream_changed', uri=self.uris[0]) self.assertIn(call, send_mock.call_args_list) def test_stream_changed_event_on_paused_to_stopped(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.pause_playback() self.audio.wait_for_state_change() self.audio.stop_playback() @@ -234,7 +225,7 @@ class AudioEventTest(unittest.TestCase): def test_position_changed_on_pause(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.pause_playback() self.audio.wait_for_state_change() @@ -245,7 +236,7 @@ class AudioEventTest(unittest.TestCase): def test_position_changed_on_play(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.audio.wait_for_state_change() @@ -256,7 +247,7 @@ class AudioEventTest(unittest.TestCase): def test_position_changed_on_seek(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.set_position(2000) self.audio.wait_for_state_change().get() @@ -266,7 +257,7 @@ class AudioEventTest(unittest.TestCase): def test_position_changed_on_seek_after_play(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.audio.wait_for_state_change() self.audio.set_position(2000) @@ -278,7 +269,7 @@ class AudioEventTest(unittest.TestCase): def test_position_changed_on_seek_after_pause(self, send_mock): self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.pause_playback() self.audio.wait_for_state_change() self.audio.set_position(2000) @@ -302,11 +293,11 @@ class AudioEventTest(unittest.TestCase): send_mock.side_effect = send self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.pause_playback().get() self.audio.wait_for_state_change().get() - if not event.wait(timeout=5.0): + if not event.wait(timeout=1.0): self.fail('Stream changed not reached within deadline') def test_reached_end_of_stream_event(self, send_mock): @@ -318,19 +309,18 @@ class AudioEventTest(unittest.TestCase): send_mock.side_effect = send self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() self.audio.wait_for_state_change().get() - if not event.wait(timeout=5.0): + self.possibly_trigger_fake_about_to_finish() + if not event.wait(timeout=1.0): self.fail('End of stream not reached within deadline') # Make sure that gapless really works: def test_gapless(self, send_mock): - song2_uri = path_to_uri(path_to_data_dir('song2.wav')) - - uris = [song2_uri] + uris = self.uris[1:] events = [] done = threading.Event() @@ -347,24 +337,68 @@ class AudioEventTest(unittest.TestCase): self.audio.set_about_to_finish_callback(callback).get() self.audio.prepare_change() - self.audio.set_uri(self.song_uri) + self.audio.set_uri(self.uris[0]) self.audio.start_playback() + + self.possibly_trigger_fake_about_to_finish() self.audio.wait_for_state_change().get() - if not done.wait(timeout=5.0): + self.possibly_trigger_fake_about_to_finish() + if not done.wait(timeout=1.0): self.fail('EOS not received') excepted = [ ('position_changed', {'position': 0}), - ('stream_changed', {'uri': self.song_uri}), + ('stream_changed', {'uri': self.uris[0]}), ('state_changed', {'old_state': PlaybackState.STOPPED, 'new_state': PlaybackState.PLAYING}), ('position_changed', {'position': 0}), - ('stream_changed', {'uri': song2_uri}), + ('stream_changed', {'uri': self.uris[1]}), ('reached_end_of_stream', {})] self.assertEqual(excepted, events) +class AudioDummyEventTest(DummyMixin, AudioEventTest): + pass + + +# TODO: this is really a mixer scaling test, has nothing to do with audio +class MixerTest(BaseTest): + def test_set_volume(self): + for value in range(0, 101): + self.assertTrue(self.audio.set_volume(value).get()) + self.assertEqual(value, self.audio.get_volume().get()) + + def test_set_volume_with_mixer_max_below_100(self): + config = { + 'audio': { + 'mixer': 'fakemixer track_max_volume=40', + 'mixer_track': None, + 'mixer_volume': None, + 'output': 'fakesink', + 'visualizer': None, + } + } + self.audio = self.audio_class.start(config=config).proxy() + + for value in range(0, 101): + self.assertTrue(self.audio.set_volume(value).get()) + self.assertEqual(value, self.audio.get_volume().get()) + + def test_set_volume_with_mixer_min_equal_max(self): + config = { + 'audio': { + 'mixer': 'fakemixer track_max_volume=0', + 'mixer_track': None, + 'mixer_volume': None, + 'output': 'fakesink', + 'visualizer': None, + } + } + self.audio = self.audio_class.start(config=config).proxy() + self.assertEqual(0, self.audio.get_volume().get()) + + class AudioStateTest(unittest.TestCase): def setUp(self): self.audio = audio.Audio(config=None) From e4932c05b107171a06b27600d7b1f310e6b35d5f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 30 Jan 2014 22:21:59 +0100 Subject: [PATCH 12/14] audio: Add more TODOs --- tests/audio/test_actor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index e5272efc..bda38164 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -130,6 +130,8 @@ class AudioEventTest(BaseTest): # TODO: test wihtout uri set, with bad uri and gapless... # TODO: playing->playing triggered by seek should be removed + # TODO: codify expected state after EOS + # TODO: consider returning a future or a threading event? def test_state_change_stopped_to_playing_event(self, send_mock): self.audio.prepare_change() From bf752859dac7d4c6fe20b72fa227653df0cd5967 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 29 Jul 2014 23:12:53 +0200 Subject: [PATCH 13/14] flake8: Fix duplicate import and import order in audio actor test --- tests/audio/test_actor.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index ae1a7991..b27d9855 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -import mock import threading import unittest From 5d1dd1a35569f51a23296b1d286379a2cf92b850 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 2 Aug 2014 20:45:55 +0200 Subject: [PATCH 14/14] review-fixes: Mostly typos etc. --- mopidy/audio/actor.py | 9 +++++---- mopidy/audio/listener.py | 2 +- tests/audio/test_actor.py | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 08552b33..7dc9c2ba 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -45,7 +45,7 @@ PLAYBIN_FLAGS = (1 << 1) | (1 << 4) | (1 << 7) PLAYBIN_VIS_FLAGS = PLAYBIN_FLAGS | (1 << 3) -# TODO: split out mixer as these are to intertwined right now +# TODO: split out mixer as these are too intertwined right now class Audio(pykka.ThreadingActor): """ Audio output through `GStreamer `_. @@ -89,6 +89,7 @@ class Audio(pykka.ThreadingActor): self._teardown_mixer() self._teardown_playbin() + # TODO: split out signal tracking helper class. def _connect(self, element, event, *args): """Helper to keep track of signal ids based on element+event""" self._signal_ids[(element, event)] = element.connect(event, *args) @@ -201,7 +202,7 @@ class Audio(pykka.ThreadingActor): output = gst.Bin('output') - # Queue element to buy use time between the about to finish event and + # Queue element to buy us time between the about to finish event and # the actual switch, i.e. about to switch can block for longer thanks # to this queue. # TODO: make the min-max values a setting? @@ -337,7 +338,7 @@ class Audio(pykka.ThreadingActor): logger.debug('Buffer %d%% full', percent) def _on_end_of_stream(self): - logger.debug('Triggering event: reached_end_of_stream event') + logger.debug('Audio event: reached_end_of_stream') AudioListener.send('reached_end_of_stream') def _on_error(self, error, debug): @@ -421,7 +422,7 @@ class Audio(pykka.ThreadingActor): def set_about_to_finish_callback(self, callback): """ - Configure audio to use an about to finish callback. + Configure audio to use an about-to-finish callback. This should be used to achieve gapless playback. For this to work the callback *MUST* call :meth:`set_uri` with the new URI to play and diff --git a/mopidy/audio/listener.py b/mopidy/audio/listener.py index 11613149..b272d15a 100644 --- a/mopidy/audio/listener.py +++ b/mopidy/audio/listener.py @@ -29,7 +29,7 @@ class AudioListener(listener.Listener): def stream_changed(self, uri): """ - Called whenever the end of the audio stream changes. + Called whenever the audio stream changes. *MAY* be implemented by actor. diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index b27d9855..2426f54e 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -141,7 +141,7 @@ class AudioEventTest(BaseTest): super(AudioEventTest, self).setUp() self.audio.enable_sync_handler().get() - # TODO: test wihtout uri set, with bad uri and gapless... + # TODO: test without uri set, with bad uri and gapless... # TODO: playing->playing triggered by seek should be removed # TODO: codify expected state after EOS # TODO: consider returning a future or a threading event?