From b3e92ff5731e4070b2a78712f76bfdd8268d8e14 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 12:35:52 +0100 Subject: [PATCH 01/19] audio: Log buffering messages --- mopidy/audio/actor.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index bd974bed..c524bae1 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -207,6 +207,9 @@ class Audio(pykka.ThreadingActor): 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() + logger.debug('Buffer %d%% full', percent) elif message.type == gst.MESSAGE_EOS: self._on_end_of_stream() elif message.type == gst.MESSAGE_ERROR: From a555e84225b4411a5acebac38124562f1bb19a43 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 12:36:37 +0100 Subject: [PATCH 02/19] audio: Add appsrc need-data and enough-data callbacks --- mopidy/audio/actor.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index c524bae1..9f96c161 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -47,6 +47,10 @@ class Audio(pykka.ThreadingActor): self._volume_set = None self._appsrc = None + self._appsrc_need_data_callback = None + self._appsrc_need_data_id = None + self._appsrc_enough_data_callback = None + self._appsrc_enough_data_id = None self._appsrc_seek_data_callback = None self._appsrc_seek_data_id = None @@ -84,6 +88,12 @@ class Audio(pykka.ThreadingActor): source, self._appsrc = self._appsrc, None if source is None: return + if self._appsrc_need_data_id is not None: + source.disconnect(self._appsrc_need_data_id) + self._appsrc_need_data_id = None + if self._appsrc_enough_data_id is not None: + source.disconnect(self._appsrc_enough_data_id) + self._appsrc_enough_data_id = None if self._appsrc_seek_data_id is not None: source.disconnect(self._appsrc_seek_data_id) self._appsrc_seek_data_id = None @@ -104,11 +114,26 @@ class Audio(pykka.ThreadingActor): source.set_property('format', b'time') source.set_property('stream-type', b'seekable') + self._appsrc_need_data_id = source.connect( + 'need-data', self._appsrc_on_need_data) + self._appsrc_enough_data_id = source.connect( + 'enough-data', self._appsrc_on_enough_data) self._appsrc_seek_data_id = source.connect( 'seek-data', self._appsrc_on_seek_data) self._appsrc = source + def _appsrc_on_need_data(self, appsrc, length_hint_in_ns): + length_hint_in_ms = length_hint_in_ns // gst.MSECOND + if self._appsrc_need_data_callback is not None: + self._appsrc_need_data_callback(length_hint_in_ms) + return True + + def _appsrc_on_enough_data(self, appsrc): + if self._appsrc_enough_data_callback is not None: + self._appsrc_enough_data_callback() + return True + def _appsrc_on_seek_data(self, appsrc, time_in_ns): time_in_ms = time_in_ns // gst.MSECOND if self._appsrc_seek_data_callback is not None: @@ -264,16 +289,22 @@ class Audio(pykka.ThreadingActor): """ self._playbin.set_property('uri', uri) - def set_appsrc(self, seek_data=None): + def set_appsrc(self, need_data=None, enough_data=None, seek_data=None): """ Switch to using appsrc for getting audio to be played. You *MUST* call :meth:`prepare_change` before calling this method. + :param need_data: callback for when appsrc needs data + :type need_data: callable which takes data length hint in ms + :param enough_data: callback for when appsrc has enough data + :type enough_data: callable :param seek_data: callback for when data from a new position is needed to continue playback :type seek_data: callable which takes time position in ms """ + self._appsrc_need_data_callback = need_data + self._appsrc_enough_data_callback = enough_data self._appsrc_seek_data_callback = seek_data self._playbin.set_property('uri', 'appsrc://') From a02f2a96020bc4ad0f7ef2cd6c597c325c879435 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 12:37:12 +0100 Subject: [PATCH 03/19] audio: Make appsrc buffer between 0.5 and 1 MB of data --- mopidy/audio/actor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 9f96c161..c3d11d98 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -110,9 +110,10 @@ class Audio(pykka.ThreadingActor): b'rate=(int)44100') source = element.get_property('source') source.set_property('caps', default_caps) - # GStreamer does not like unicode source.set_property('format', b'time') source.set_property('stream-type', b'seekable') + source.set_property('max-bytes', 1024 * 1024) # 1 MB + source.set_property('min-percent', 50) self._appsrc_need_data_id = source.connect( 'need-data', self._appsrc_on_need_data) From 50e8ff04b3fe6718e394567dd785fba58ec20768 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 12:38:37 +0100 Subject: [PATCH 04/19] spotify: Only push audio data when GStreamer wants more --- mopidy/backends/spotify/playback.py | 23 +++++++++++++++++++++- mopidy/backends/spotify/session_manager.py | 5 +++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index d80ef543..9855e124 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -11,6 +11,16 @@ from mopidy.backends import base logger = logging.getLogger('mopidy.backends.spotify') +def need_data_callback(spotify_backend, length_hint): + logger.debug('need_data_callback(%d) called', length_hint) + spotify_backend.playback.on_need_data(length_hint) + + +def enough_data_callback(spotify_backend): + logger.debug('enough_data_callback() called') + spotify_backend.playback.on_enough_data() + + def seek_data_callback(spotify_backend, time_position): logger.debug('seek_data_callback(%d) called', time_position) spotify_backend.playback.on_seek_data(time_position) @@ -31,7 +41,10 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): self.backend.spotify.session.play(1) self.audio.prepare_change() - self.audio.set_appsrc(seek_data=seek_data_callback_bound) + self.audio.set_appsrc( + need_data=None, + enough_data=None, + seek_data=seek_data_callback_bound) self.audio.start_playback() self.audio.set_metadata(track) @@ -44,6 +57,14 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): self.backend.spotify.session.play(0) return super(SpotifyPlaybackProvider, self).stop() + def on_need_data(self, length_hint): + logger.debug('playback.on_need_data(%d) called', length_hint) + self.backend.spotify.push_audio_data = True + + def on_enough_data(self): + logger.debug('playback.on_enough_data() called') + self.backend.spotify.push_audio_data = False + def on_seek_data(self, time_position): logger.debug('playback.on_seek_data(%d) called', time_position) self.backend.spotify.next_buffer_timestamp = time_position diff --git a/mopidy/backends/spotify/session_manager.py b/mopidy/backends/spotify/session_manager.py index 0eed9939..a9c0884e 100644 --- a/mopidy/backends/spotify/session_manager.py +++ b/mopidy/backends/spotify/session_manager.py @@ -46,6 +46,7 @@ class SpotifySessionManager(process.BaseThread, PyspotifySessionManager): self.backend_ref = backend_ref self.connected = threading.Event() + self.push_audio_data = True self.next_buffer_timestamp = None self.container_manager = None @@ -107,6 +108,10 @@ class SpotifySessionManager(process.BaseThread, PyspotifySessionManager): """Callback used by pyspotify""" # pylint: disable = R0913 # Too many arguments (8/5) + + if not self.push_audio_data: + return 0 + assert sample_type == 0, 'Expects 16-bit signed integer samples' capabilites = """ audio/x-raw-int, From aa0c6f6dc02a9ccf114bd95499256d9a973b7573 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 12:18:48 +0100 Subject: [PATCH 05/19] audio: Add MB constant to make code more readable --- mopidy/audio/actor.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 6aecbe8a..672aa540 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -21,6 +21,9 @@ logger = logging.getLogger('mopidy.audio') mixers.register_mixers() +MB = 1 << 20 + + class Audio(pykka.ThreadingActor): """ Audio output through `GStreamer `_. @@ -109,7 +112,7 @@ class Audio(pykka.ThreadingActor): source.set_property('caps', self._appsrc_caps) source.set_property('format', b'time') source.set_property('stream-type', b'seekable') - source.set_property('max-bytes', 1024 * 1024) # 1 MB + source.set_property('max-bytes', 1 * MB) source.set_property('min-percent', 50) self._appsrc_need_data_id = source.connect( From 7c790d61b2c046960a578725b546cec1555eb064 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 12:21:05 +0100 Subject: [PATCH 06/19] spotify: Hook need-data and enough-data callbacks onto appsrc --- mopidy/backends/spotify/playback.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index 6899ee47..45850107 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -38,6 +38,10 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): 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( + enough_data_callback, spotify_backend) seek_data_callback_bound = functools.partial( seek_data_callback, spotify_backend) @@ -49,8 +53,8 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): self.audio.prepare_change() self.audio.set_appsrc( self._caps, - need_data=None, - enough_data=None, + 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) From 5e94aa19ec5419a30c219746acd5f265dfaaf75c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 12:51:13 +0100 Subject: [PATCH 07/19] spotify: Reduce callback logging --- mopidy/backends/spotify/playback.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index 45850107..c148972c 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -12,17 +12,14 @@ logger = logging.getLogger('mopidy.backends.spotify') def need_data_callback(spotify_backend, length_hint): - logger.debug('need_data_callback(%d) called', length_hint) spotify_backend.playback.on_need_data(length_hint) def enough_data_callback(spotify_backend): - logger.debug('enough_data_callback() called') spotify_backend.playback.on_enough_data() def seek_data_callback(spotify_backend, time_position): - logger.debug('seek_data_callback(%d) called', time_position) spotify_backend.playback.on_seek_data(time_position) From a3446621f465ee3e0b8ef630c9239536ae9b264a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 24 Dec 2012 13:55:46 +0100 Subject: [PATCH 08/19] Fix use of threading.Event for Python 2.6 and clear connected state. threading.Event's wait method returns None on python pre 2.7, which means all searches would fail. This also corrects that fact that we weren't clearing the connected threading event on disconnects. I did not add any tests for this at this time as I just want to get the fix out. --- mopidy/backends/spotify/library.py | 4 +++- mopidy/backends/spotify/session_manager.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index a8a9bcd6..96e5f616 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -163,7 +163,9 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): translator.to_mopidy_track(t) for t in results.tracks()]) future.set(search_result) - if not self.backend.spotify.connected.wait(settings.SPOTIFY_TIMEOUT): + # Wait always returns None on python 2.6 :/ + self.backend.spotify.connected.wait(settings.SPOTIFY_TIMEOUT) + if not self.backend.spotify.connected.is_set(): logger.debug('Not connected: Spotify search cancelled') return SearchResult(uri='spotify:search') diff --git a/mopidy/backends/spotify/session_manager.py b/mopidy/backends/spotify/session_manager.py index f2631406..8326d7b4 100644 --- a/mopidy/backends/spotify/session_manager.py +++ b/mopidy/backends/spotify/session_manager.py @@ -83,6 +83,7 @@ class SpotifySessionManager(process.BaseThread, PyspotifySessionManager): def logged_out(self, session): """Callback used by pyspotify""" logger.info('Disconnected from Spotify') + self.connected.clear() def metadata_updated(self, session): """Callback used by pyspotify""" From c37ce8751e38c851ed7b71f7927c0e42132d7d8e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 20:23:51 +0100 Subject: [PATCH 09/19] Release v0.11.1 --- docs/changes.rst | 7 +++++++ mopidy/__init__.py | 2 +- tests/version_test.py | 5 +++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index e705444b..296e7e38 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -5,6 +5,13 @@ Changes This change log is used to track all major changes to Mopidy. +v0.11.1 (2012-12-24) +==================== + +Spotify search was broken in 0.11.0 for users of Python 2.6. This release fixes +it. If you're using Python 2.7, v0.11.0 and v0.11.1 should be equivalent. + + v0.11.0 (2012-12-24) ==================== diff --git a/mopidy/__init__.py b/mopidy/__init__.py index 2e5aeeba..9d66b722 100644 --- a/mopidy/__init__.py +++ b/mopidy/__init__.py @@ -23,7 +23,7 @@ if (isinstance(pykka.__version__, basestring) warnings.filterwarnings('ignore', 'could not open display') -__version__ = '0.11.0' +__version__ = '0.11.1' from mopidy import settings as default_settings_module diff --git a/tests/version_test.py b/tests/version_test.py index f353f201..1cb3967c 100644 --- a/tests/version_test.py +++ b/tests/version_test.py @@ -33,5 +33,6 @@ class VersionTest(unittest.TestCase): self.assertLess(SV('0.8.0'), SV('0.8.1')) self.assertLess(SV('0.8.1'), SV('0.9.0')) self.assertLess(SV('0.9.0'), SV('0.10.0')) - self.assertLess(SV('0.10.0'), SV(__version__)) - self.assertLess(SV(__version__), SV('0.11.1')) + self.assertLess(SV('0.10.0'), SV('0.11.0')) + self.assertLess(SV('0.11.0'), SV(__version__)) + self.assertLess(SV(__version__), SV('0.11.2')) From 64ae7865bb52cf5737595652fd8bf2a7629cc8d8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 3 Jan 2013 19:38:31 +0100 Subject: [PATCH 10/19] audio: Add clocktime_to_millisecond util function --- mopidy/audio/utils.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/mopidy/audio/utils.py b/mopidy/audio/utils.py index 9d0f46dd..5bb3c4ca 100644 --- a/mopidy/audio/utils.py +++ b/mopidy/audio/utils.py @@ -6,7 +6,8 @@ import gst def calculate_duration(num_samples, sample_rate): - """Determine duration of samples using GStreamer helper for precise math.""" + """Determine duration of samples using GStreamer helper for precise + math.""" return gst.util_uint64_scale(num_samples, gst.SECOND, sample_rate) @@ -28,10 +29,15 @@ def create_buffer(data, capabilites=None, timestamp=None, duration=None): def millisecond_to_clocktime(value): - """Convert a millisecond time to internal gstreamer time.""" + """Convert a millisecond time to internal GStreamer time.""" return value * gst.MSECOND +def clocktime_to_millisecond(value): + """Convert a millisecond time to internal GStreamer time.""" + return value // gst.MSECOND + + def supported_uri_schemes(uri_schemes): """Determine which URIs we can actually support from provided whitelist. From c76ac2d2126705d77cd4f556d5cf2b66cc3eefc9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 3 Jan 2013 19:40:53 +0100 Subject: [PATCH 11/19] audio: Use time conversion utils --- mopidy/audio/actor.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 672aa540..b021c35a 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -12,7 +12,7 @@ import pykka from mopidy import settings from mopidy.utils import process -from . import mixers +from . import mixers, utils from .constants import PlaybackState from .listener import AudioListener @@ -124,10 +124,10 @@ class Audio(pykka.ThreadingActor): self._appsrc = source - def _appsrc_on_need_data(self, appsrc, length_hint_in_ns): - length_hint_in_ms = length_hint_in_ns // gst.MSECOND + def _appsrc_on_need_data(self, appsrc, gst_length_hint): + length_hint = utils.clocktime_to_millisecond(gst_length_hint) if self._appsrc_need_data_callback is not None: - self._appsrc_need_data_callback(length_hint_in_ms) + self._appsrc_need_data_callback(length_hint) return True def _appsrc_on_enough_data(self, appsrc): @@ -135,10 +135,10 @@ class Audio(pykka.ThreadingActor): self._appsrc_enough_data_callback() return True - def _appsrc_on_seek_data(self, appsrc, time_in_ns): - time_in_ms = time_in_ns // gst.MSECOND + def _appsrc_on_seek_data(self, appsrc, gst_position): + position = utils.clocktime_to_millisecond(gst_position) if self._appsrc_seek_data_callback is not None: - self._appsrc_seek_data_callback(time_in_ms) + self._appsrc_seek_data_callback(position) return True def _teardown_playbin(self): @@ -349,8 +349,8 @@ class Audio(pykka.ThreadingActor): :rtype: int """ try: - position = self._playbin.query_position(gst.FORMAT_TIME)[0] - return position // gst.MSECOND + gst_position = self._playbin.query_position(gst.FORMAT_TIME)[0] + return utils.clocktime_to_millisecond(gst_position) except gst.QueryError: logger.debug('Position query failed') return 0 @@ -363,9 +363,9 @@ class Audio(pykka.ThreadingActor): :type position: int :rtype: :class:`True` if successful, else :class:`False` """ + gst_position = utils.millisecond_to_clocktime(position) return self._playbin.seek_simple( - gst.Format(gst.FORMAT_TIME), gst.SEEK_FLAG_FLUSH, - position * gst.MSECOND) + gst.Format(gst.FORMAT_TIME), gst.SEEK_FLAG_FLUSH, gst_position) def start_playback(self): """ From e07a6e151aa1efde9b1aadcf0022ce488e226514 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 3 Jan 2013 21:37:31 +0100 Subject: [PATCH 12/19] js: Add a package.json to simplify JS dev env setup --- js/README.rst | 39 +++++++++++++-------------------------- js/grunt.js | 4 +++- js/package.json | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 27 deletions(-) create mode 100644 js/package.json diff --git a/js/README.rst b/js/README.rst index a68dd9a0..e8782213 100644 --- a/js/README.rst +++ b/js/README.rst @@ -36,40 +36,27 @@ Building from source sudo apt-get update sudo apt-get install nodejs npm -2. Assuming you install from PPA, setup your ``NODE_PATH`` environment variable - to include ``/usr/lib/node_modules``. Add the following to your - ``~/.bashrc`` or equivalent:: - - export NODE_PATH=/usr/lib/node_modules:$NODE_PATH - -3. Install `Buster.js `_ and `Grunt - `_ globally (or locally, and make sure you get their - binaries on your ``PATH``):: - - sudo npm -g install buster grunt - -4. Install the grunt-buster Grunt plugin locally, when in the ``js/`` dir:: +2. Enter the ``js/`` dir and install development dependencies:: cd js/ - npm install grunt-buster + npm install -5. Install `PhantomJS `_ so that we can run the tests - without a browser:: +That's it. - sudo apt-get install phantomjs +You can now run the tests:: - It is packaged in Ubuntu since 12.04, but I haven't tested with versions - older than 1.6 which is the one packaged in Ubuntu 12.10. + npm test -6. Run Grunt to lint, test, concatenate, and minify the source:: +To run tests automatically when you save a file:: - grunt + npm run-script watch - The files in ``../mopidy/frontends/http/data/`` should now be up to date. +To run tests, concatenate, minify the source, and update the JavaScript files +in ``mopidy/frontends/http/data/``:: + npm run-script build -Development tips -================ +To run other `grunt `_ targets which isn't predefined in +``package.json`` and thus isn't available through ``npm run-script``:: -If you're coding on the JavaScript library, you should know about ``grunt -watch``. It lints and tests the code every time you save a file. + PATH=./node_modules/.bin:$PATH grunt foo diff --git a/js/grunt.js b/js/grunt.js index d835fd77..46afc8af 100644 --- a/js/grunt.js +++ b/js/grunt.js @@ -64,7 +64,9 @@ module.exports = function (grunt) { uglify: {} }); - grunt.registerTask("default", "lint buster concat min"); + grunt.registerTask("test", "lint buster"); + grunt.registerTask("build", "lint buster concat min"); + grunt.registerTask("default", "build"); grunt.loadNpmTasks("grunt-buster"); }; diff --git a/js/package.json b/js/package.json new file mode 100644 index 00000000..a8737cfb --- /dev/null +++ b/js/package.json @@ -0,0 +1,15 @@ +{ + "name": "mopidy", + "version": "0.0.0", + "devDependencies": { + "buster": "*", + "grunt": "*", + "grunt-buster": "*", + "phantomjs": "*" + }, + "scripts": { + "test": "grunt test", + "build": "grunt build", + "watch": "grunt watch" + } +} From edde1bc584aec31f1e2963d2197e3e78b9de0b45 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 3 Jan 2013 21:40:20 +0100 Subject: [PATCH 13/19] audio: Fix docstring --- mopidy/audio/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/audio/utils.py b/mopidy/audio/utils.py index 5bb3c4ca..15196b20 100644 --- a/mopidy/audio/utils.py +++ b/mopidy/audio/utils.py @@ -34,7 +34,7 @@ def millisecond_to_clocktime(value): def clocktime_to_millisecond(value): - """Convert a millisecond time to internal GStreamer time.""" + """Convert an internal GStreamer time to millisecond time.""" return value // gst.MSECOND From ea5bb18d5388119f399a834448d43906196c8dd1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 3 Jan 2013 22:24:58 +0100 Subject: [PATCH 14/19] audio: Update mixer track selection logic (fixes #307) We now ensure that the track we choose has one or more volume channels we can control. This change also fixes that fact the MIXER_TRACK setting would not work if we happened to find a track that was flaged as MASTER OUPUT before finding the right label, so far no one has reported this as an issue. --- mopidy/audio/actor.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 65edd037..537a6ad5 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -173,15 +173,23 @@ class Audio(pykka.ThreadingActor): mixer.get_factory().get_name(), track.label) def _select_mixer_track(self, mixer, track_label): - # Look for track with label == MIXER_TRACK, otherwise fallback to - # master track which is also an output. + # Ignore tracks without volumes, then look for track with + # label == settings.MIXER_TRACK, otherwise fallback to first usable + # track hoping the mixer gave them to us in a sensible order. + + usable_tracks = [] for track in mixer.list_tracks(): - if track_label: - if track.label == track_label: - return track + if not mixer.get_volume(track): + continue + + if track_label and track.label == track_label: + return track elif track.flags & (gst.interfaces.MIXER_TRACK_MASTER | gst.interfaces.MIXER_TRACK_OUTPUT): - return track + usable_tracks.append(track) + + if usable_tracks: + return usable_tracks[0] def _teardown_mixer(self): if self._mixer is not None: From c03d99e55b2b0b1944b1f679e82b98483184d902 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 3 Jan 2013 22:45:30 +0100 Subject: [PATCH 15/19] mpd: Use bytestring for **kwargs key (#302) --- mopidy/frontends/mpd/actor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 11e07aa7..33ccd077 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -19,10 +19,12 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): hostname = network.format_hostname(settings.MPD_SERVER_HOSTNAME) port = settings.MPD_SERVER_PORT + # NOTE: dict key must be bytestring to work on Python < 2.6.5 + # See https://github.com/mopidy/mopidy/issues/302 for details try: network.Server( hostname, port, - protocol=session.MpdSession, protocol_kwargs={'core': core}, + protocol=session.MpdSession, protocol_kwargs={b'core': core}, max_connections=settings.MPD_SERVER_MAX_CONNECTIONS, timeout=settings.MPD_SERVER_CONNECTION_TIMEOUT) except IOError as error: From 91297c1ba892df5bc09514598152bb7fb60dacfb Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 3 Jan 2013 22:54:13 +0100 Subject: [PATCH 16/19] Update changelog with mixer track selection change. --- docs/changes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index c2b076fc..b4a75f75 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -18,6 +18,8 @@ v0.12.0 (in development) - ``foo(**data)`` fails if the keys in ``data`` is unicode strings on Python < 2.6.5rc1. +- Improve selection of mixer tracks for volume control. (Fixes: :issuse:`307`) + **Spotify backend** - Let GStreamer handle time position tracking and seeks. (Fixes: :issue:`191`) From cbb68f9cbee36487e7be6122a23801f9c0699db1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 3 Jan 2013 23:13:06 +0100 Subject: [PATCH 17/19] docs: Bind console functions in JS examples For e.g. ``console.log`` to be used as a callback, it must be bound to the ``console`` object first: ``console.log.bind(console)``. If not, this will cause "Illegal invocation" errors in WebKit browsers. This change updates all our Mopidy.js examples to bind ``console`` functions before they are used as callbacks. Fixes #298. --- mopidy/frontends/http/__init__.py | 41 +++++++++++++++++-------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/mopidy/frontends/http/__init__.py b/mopidy/frontends/http/__init__.py index 94b8e58e..ab8dff42 100644 --- a/mopidy/frontends/http/__init__.py +++ b/mopidy/frontends/http/__init__.py @@ -198,7 +198,7 @@ them: .. code-block:: js - mopidy.on(console.log); + mopidy.on(console.log.bind(console)); Several types of events are emitted: @@ -289,7 +289,8 @@ Instead, typical usage will look like this: } }; - mopidy.playback.getCurrentTrack().then(printCurrentTrack, console.error); + mopidy.playback.getCurrentTrack().then( + printCurrentTrack, console.error.bind(console)); The first function passed to ``then()``, ``printCurrentTrack``, is the callback that will be called if the method call succeeds. The second function, @@ -303,7 +304,7 @@ callback: .. code-block:: js - mopidy.playback.next().then(null, console.error); + mopidy.playback.next().then(null, console.error.bind(console)); The promise objects returned by Mopidy.js adheres to the `CommonJS Promises/A `_ standard. We use the @@ -368,6 +369,8 @@ Example to get started with .. code-block:: js + var consoleError = console.error.bind(error); + var trackDesc = function (track) { return track.name + " by " + track.artists[0].name + " from " + track.album.name; @@ -381,14 +384,14 @@ Example to get started with mopidy.playback.play(tlTracks[0]).then(function () { mopidy.playback.getCurrentTrack().then(function (track) { console.log("Now playing:", trackDesc(track)); - }, console.error); - }, console.error); - }, console.error); - }, console.error); + }, consoleError); + }, consoleError); + }, consoleError); + }, consoleError); }; - var mopidy = new Mopidy(); // Connect to server - mopidy.on(console.log); // Log all events + var mopidy = new Mopidy(); // Connect to server + mopidy.on(console.log.bind(console)); // Log all events mopidy.on("state:online", queueAndPlayFirstPlaylist); Approximately the same behavior in a more functional style, using chaining @@ -396,6 +399,8 @@ Example to get started with .. code-block:: js + var consoleError = console.error.bind(error); + var getFirst = function (list) { return list[0]; }; @@ -429,23 +434,23 @@ Example to get started with var queueAndPlayFirstPlaylist = function () { mopidy.playlists.getPlaylists() // => list of Playlists - .then(getFirst, console.error) + .then(getFirst, consoleError) // => Playlist - .then(printTypeAndName, console.error) + .then(printTypeAndName, consoleError) // => Playlist - .then(extractTracks, console.error) + .then(extractTracks, consoleError) // => list of Tracks - .then(mopidy.tracklist.add, console.error) + .then(mopidy.tracklist.add, consoleError) // => list of TlTracks - .then(getFirst, console.error) + .then(getFirst, consoleError) // => TlTrack - .then(mopidy.playback.play, console.error) + .then(mopidy.playback.play, consoleError) // => null - .then(printNowPlaying, console.error); + .then(printNowPlaying, consoleError); }; - var mopidy = new Mopidy(); // Connect to server - mopidy.on(console.log); // Log all events + var mopidy = new Mopidy(); // Connect to server + mopidy.on(console.log.bind(console)); // Log all events mopidy.on("state:online", queueAndPlayFirstPlaylist); 9. The web page should now queue and play your first playlist every time your From 61dcb7d37da30a4e9af9e202f58a5da3a0f3ccb0 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 4 Jan 2013 10:26:30 +0100 Subject: [PATCH 18/19] mpd: Make request handler **kwargs keys bytestrings (#302) --- mopidy/frontends/mpd/protocol/__init__.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mopidy/frontends/mpd/protocol/__init__.py b/mopidy/frontends/mpd/protocol/__init__.py index 1827624b..55a1563b 100644 --- a/mopidy/frontends/mpd/protocol/__init__.py +++ b/mopidy/frontends/mpd/protocol/__init__.py @@ -56,7 +56,12 @@ def handle_request(pattern, auth_required=True): if match is not None: mpd_commands.add( MpdCommand(name=match.group(), auth_required=auth_required)) - compiled_pattern = re.compile(pattern, flags=re.UNICODE) + # NOTE: Make pattern a bytestring to get bytestring keys in the dict + # returned from matches.groupdict(), which is again used as a **kwargs + # dict. This is needed to work on Python < 2.6.5. See + # https://github.com/mopidy/mopidy/issues/302 for details. + bytestring_pattern = pattern.encode('utf-8') + compiled_pattern = re.compile(bytestring_pattern, flags=re.UNICODE) if compiled_pattern in request_handlers: raise ValueError('Tried to redefine handler for %s with %s' % ( pattern, func)) From 711dbe87384c7e7d025d541ae5f08cacb1733045 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 5 Jan 2013 18:35:43 +0100 Subject: [PATCH 19/19] audio: Create GStreaner connect/disconnect helper. The audio class had to many attributes for simply tracking connected signals in my opinion. This change adds a helper for adding and removing signals and storing the tracking data in a common dictionary instead of a ton of instance attributes. --- mopidy/audio/actor.py | 67 +++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index f81cbf6d..b154e21a 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -42,6 +42,7 @@ class Audio(pykka.ThreadingActor): super(Audio, self).__init__() self._playbin = None + self._signal_ids = {} # {(element, event): signal_id} self._mixer = None self._mixer_track = None @@ -52,15 +53,8 @@ class Audio(pykka.ThreadingActor): self._appsrc = None self._appsrc_caps = None self._appsrc_need_data_callback = None - self._appsrc_need_data_id = None self._appsrc_enough_data_callback = None - self._appsrc_enough_data_id = None self._appsrc_seek_data_callback = None - self._appsrc_seek_data_id = None - - self._notify_source_signal_id = None - self._about_to_finish_id = None - self._message_signal_id = None def on_start(self): try: @@ -77,31 +71,36 @@ class Audio(pykka.ThreadingActor): self._teardown_mixer() self._teardown_playbin() + 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) + + def _disconnect(self, element, event): + """Helper to disconnect signals created with _connect helper.""" + signal_id = self._signal_ids.pop((element, event), None) + if signal_id is not None: + element.disconnect(signal_id) + def _setup_playbin(self): - self._playbin = gst.element_factory_make('playbin2') + playbin = gst.element_factory_make('playbin2') fakesink = gst.element_factory_make('fakesink') - self._playbin.set_property('video-sink', fakesink) + playbin.set_property('video-sink', fakesink) - self._about_to_finish_id = self._playbin.connect( - 'about-to-finish', self._on_about_to_finish) - self._notify_source_signal_id = self._playbin.connect( - 'notify::source', self._on_new_source) + self._connect(playbin, 'about-to-finish', self._on_about_to_finish) + self._connect(playbin, 'notify::source', self._on_new_source) + + self._playbin = playbin def _on_about_to_finish(self, element): source, self._appsrc = self._appsrc, None if source is None: return self._appsrc_caps = None - if self._appsrc_need_data_id is not None: - source.disconnect(self._appsrc_need_data_id) - self._appsrc_need_data_id = None - if self._appsrc_enough_data_id is not None: - source.disconnect(self._appsrc_enough_data_id) - self._appsrc_enough_data_id = None - if self._appsrc_seek_data_id is not None: - source.disconnect(self._appsrc_seek_data_id) - self._appsrc_seek_data_id = None + + self._disconnect(source, 'need-data') + self._disconnect(source, 'enough-data') + self._disconnect(source, 'seek-data') def _on_new_source(self, element, pad): uri = element.get_property('uri') @@ -115,12 +114,9 @@ class Audio(pykka.ThreadingActor): source.set_property('max-bytes', 1 * MB) source.set_property('min-percent', 50) - self._appsrc_need_data_id = source.connect( - 'need-data', self._appsrc_on_need_data) - self._appsrc_enough_data_id = source.connect( - 'enough-data', self._appsrc_on_enough_data) - self._appsrc_seek_data_id = source.connect( - 'seek-data', self._appsrc_on_seek_data) + self._connect(source, 'need-data', self._appsrc_on_need_data) + self._connect(source, 'enough-data', self._appsrc_on_enough_data) + self._connect(source, 'seek-data', self._appsrc_on_seek_data) self._appsrc = source @@ -142,10 +138,8 @@ class Audio(pykka.ThreadingActor): return True def _teardown_playbin(self): - if self._about_to_finish_id: - self._playbin.disconnect(self._about_to_finish_id) - if self._notify_source_signal_id: - self._playbin.disconnect(self._notify_source_signal_id) + self._disconnect(self._playbin, 'about-to-finish') + self._disconnect(self._playbin, 'notify::source') self._playbin.set_state(gst.STATE_NULL) def _setup_output(self): @@ -228,13 +222,12 @@ class Audio(pykka.ThreadingActor): def _setup_message_processor(self): bus = self._playbin.get_bus() bus.add_signal_watch() - self._message_signal_id = bus.connect('message', self._on_message) + self._connect(bus, 'message', self._on_message) def _teardown_message_processor(self): - if self._message_signal_id: - bus = self._playbin.get_bus() - bus.disconnect(self._message_signal_id) - bus.remove_signal_watch() + bus = self._playbin.get_bus() + self._disconnect(bus, 'message') + bus.remove_signal_watch() def _on_message(self, bus, message): if (message.type == gst.MESSAGE_STATE_CHANGED