diff --git a/docs/changelog.rst b/docs/changelog.rst index 600b67eb..1cac30e3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,16 +5,64 @@ Changelog This changelog is used to track all major changes to Mopidy. -v2.0.0 (UNRELEASED) +v2.1.0 (UNRELEASED) =================== Feature release. +- Nothing yet. + + +v2.0.1 (UNRELEASED) +=================== + +Bug fix release. + +- Nothing yet. + + +v2.0.0 (2016-02-15) +=================== + +Mopidy 2.0 is here! + +Since the release of 1.1, we've closed or merged approximately 80 issues and +pull requests through about 350 commits by 14 extraordinary people, including +10 newcomers. That's about the same amount of issues and commits as between 1.0 +and 1.1. The number of contributors is a bit lower but we didn't have a real +life sprint during this development cycle. Thanks to :ref:`everyone ` +who has :ref:`contributed `! + +With the release of Mopidy 1.0 we promised that any extension working with +Mopidy 1.0 should continue working with all Mopidy 1.x releases. Mopidy 2.0 is +quite a friendly major release and will only break a single extension that we +know of: Mopidy-Spotify. To ensure that everything continues working, please +upgrade to Mopidy 2.0 and Mopidy-Spotify 3.0 at the same time. + +No deprecated functionality has been removed in Mopidy 2.0. + +The major features of Mopidy 2.0 are: + +- Gapless playback has been mostly implemented. It works as long as you don't + change tracks in the middle of a track or use previous and next. In a future + release, previous and next will also become gapless. It is now quite easy to + have Mopidy streaming audio over the network using Icecast. See the updated + :ref:`streaming` docs for details of how to set it up and workarounds for the + remaining issues. + +- Mopidy has upgraded from GStreamer 0.10 to 1.x. This has been in our backlog + for more than three years. With this upgrade we're ridding ourselves of + years of GStreamer bugs that have been fixed in newer releases, we can get + into Debian testing again, and we've removed the last major roadblock for + running Mopidy on Python 3. + Dependencies ------------ - Mopidy now requires GStreamer >= 1.2.3, as we've finally ported from - GStreamer 0.10. + GStreamer 0.10. Since we're requiring a new major version of our major + dependency, we're upping the major version of Mopidy too. (Fixes: + :issue:`225`) Core API -------- @@ -58,6 +106,9 @@ Local backend M3U backend ----------- +- Add :confval:`m3u/base_dir` for resolving relative paths in M3U + files. (Fixes: :issue:`1428`, PR: :issue:`1442`) + - Derive track name from file name for non-extended M3U playlists. (Fixes: :issue:`1364`, PR: :issue:`1369`) @@ -78,6 +129,12 @@ M3U backend - Improve reliability of playlist updates using the core playlist API by applying the write-replace pattern for file updates. +Stream backend +-------------- + +- Make sure both lookup and playback correctly handle playlists and our + blacklist support. (Fixes: :issue:`1445`, PR: :issue:`1447`) + MPD frontend ------------ @@ -97,14 +154,14 @@ MPD frontend - Idle events are now emitted on ``seeked`` events. This fix means that clients relying on ``idle`` events now get notified about seeks. - (Fixes: :issue:`1331` :issue:`1347`) + (Fixes: :issue:`1331`, PR: :issue:`1347`) - Idle events are now emitted on ``playlists_loaded`` events. This fix means that clients relying on ``idle`` events now get notified about playlist loads. - (Fixes: :issue:`1331` PR: :issue:`1347`) + (Fixes: :issue:`1331`, PR: :issue:`1347`) - Event handler for ``playlist_deleted`` has been unbroken. This unreported bug - would cause the MPD Frontend to crash preventing any further communication + would cause the MPD frontend to crash preventing any further communication via the MPD protocol. (PR: :issue:`1347`) Zeroconf @@ -132,6 +189,11 @@ Cleanups - Catch errors when loading :confval:`logging/config_file`. (Fixes: :issue:`1320`) +- **Breaking:** Removed unused internal + :class:`mopidy.internal.process.BaseThread`. This breaks Mopidy-Spotify + 1.4.0. Versions < 1.4.0 was already broken by Mopidy 1.1, while versions >= + 2.0 doesn't use this class. + Audio ----- @@ -155,13 +217,25 @@ Audio If your Mopidy backend uses ``set_appsrc()``, please refer to GStreamer documentation for details on the new caps string format. -- **Deprecated:** :func:`mopidy.audio.utils.create_buffer`'s ``capabilities`` - argument is no longer in use and will be removed in the future. As far as we - know, this is only used by Mopidy-Spotify. +- **Breaking:** :func:`mopidy.audio.utils.create_buffer`'s ``capabilities`` + argument is no longer in use and has been removed. As far as we know, this + was only used by Mopidy-Spotify. -- Duplicate seek events getting to AppSrc based backends is now fixed. This - should prevent seeking in Mopidy-Spotify from glitching. - (Fixes: :issue:`1404`) +- Duplicate seek events getting to ``appsrc`` based backends is now fixed. This + should prevent seeking in Mopidy-Spotify from glitching. (Fixes: + :issue:`1404`) + +- Workaround crash caused by a race that does not seem to affect functionality. + This should be fixed properly together with :issue:`1222`. (Fixes: + :issue:`1430`, PR: :issue:`1438`) + +- Add a new config option, :confval:`audio/buffer_time`, for setting the buffer + time of the GStreamer queue. If you experience buffering before track + changes, it may help to increase this. (Workaround for :issue:`1409`) + +- ``tags_changed`` events are only emitted for fields that have changed. + Previous behavior was to emit this for all fields received from GStreamer. + (PR: :issue:`1439`) Gapless ------- @@ -169,6 +243,9 @@ Gapless - Add partial support for gapless playback. Gapless now works as long as you don't change tracks or use next/previous. (PR: :issue:`1288`) + The :ref:`streaming` docs has been updated with the workarounds still needed + to properly stream Mopidy audio through Icecast. + - Core playback has been refactored to better handle gapless, and async state changes. diff --git a/docs/clients/mpd.rst b/docs/clients/mpd.rst index ee1b1903..e1cb6019 100644 --- a/docs/clients/mpd.rst +++ b/docs/clients/mpd.rst @@ -30,14 +30,17 @@ supported)" mode because the client tries to fetch all known metadata and do the search on the client side. The two other search modes works nicely, so this is not a problem. -The library view is very slow when used together with Mopidy-Spotify. A -workaround is to edit the ncmpcpp configuration file +With ncmpcpp <= 0.5, the library view is very slow when used together with +Mopidy-Spotify. A workaround is to edit the ncmpcpp configuration file (:file:`~/.ncmpcpp/config`) and set:: media_library_display_date = "no" With this change ncmpcpp's library view will still be a bit slow, but usable. +Note that this option was removed in ncmpcpp 0.6, but with this version, the +library view works well without it. + ncmpc ----- diff --git a/docs/config.rst b/docs/config.rst index b7874f83..08381aaa 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -161,6 +161,17 @@ These are the available audio configurations. For specific use cases, see ``gst-inspect-1.0`` to see what output properties can be set on the sink. For example: ``gst-inspect-1.0 shout2send`` +.. confval:: audio/buffer_time + + Buffer size in milliseconds. + + Expects an integer above 0. + + Sets the buffer size of the GStreamer queue. If you experience buffering + before track changes, it may help to increase this, possibly by at least a + few seconds. The default is letting GStreamer decide the size, which at the + time of this writing is 1000. + Logging configuration ===================== diff --git a/docs/ext/local.rst b/docs/ext/local.rst index ef9df5d7..1512524e 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -89,15 +89,6 @@ See :ref:`config` for general help on configuring Mopidy. Path to directory with local media files. -.. confval:: local/data_dir - - Path to directory to store local metadata such as libraries and playlists - in. - -.. confval:: local/playlists_dir - - Path to playlists directory with m3u files for local media. - .. confval:: local/scan_timeout Number of milliseconds before giving up scanning a file and moving on to diff --git a/docs/ext/m3u.rst b/docs/ext/m3u.rst index 37dc60be..35bd2036 100644 --- a/docs/ext/m3u.rst +++ b/docs/ext/m3u.rst @@ -55,6 +55,12 @@ See :ref:`config` for general help on configuring Mopidy. Path to directory with M3U files. Unset by default, in which case the extension's data dir is used to store playlists. +.. confval:: m3u/base_dir + + Path to base directory for resolving relative paths in M3U files. + If not set, relative paths are resolved based on the M3U file's + location. + .. confval:: m3u/default_encoding Text encoding used for files with extension ``.m3u``. Default is diff --git a/docs/installation/raspberrypi.rst b/docs/installation/raspberrypi.rst index 6d2dd3cd..f62bb124 100644 --- a/docs/installation/raspberrypi.rst +++ b/docs/installation/raspberrypi.rst @@ -68,7 +68,7 @@ How to for Raspbian Jessie #. Finally, you need to set a couple of :doc:`config values `, and then you're ready to :doc:`run Mopidy `. Alternatively you may - want to have Mopidy run as a :doc:`system service `, automatically + want to have Mopidy run as a :ref:`system service `, automatically starting at boot. diff --git a/docs/installation/source.rst b/docs/installation/source.rst index b5bd7b95..ee2ffad5 100644 --- a/docs/installation/source.rst +++ b/docs/installation/source.rst @@ -44,8 +44,10 @@ please follow the directions :ref:`here `. If you use Debian/Ubuntu you can install GStreamer like this:: - sudo apt-get install python-gst-1.0 gstreamer1.0-plugins-good \ - gstreamer1.0-plugins-ugly gstreamer1.0-tools + sudo apt-get install python-gst-1.0 \ + gir1.2-gstreamer-1.0 gir1.2-gst-plugins-base-1.0 \ + gstreamer1.0-plugins-good gstreamer1.0-plugins-ugly \ + gstreamer1.0-tools If you use Arch Linux, install the following packages from the official repository:: diff --git a/mopidy/__main__.py b/mopidy/__main__.py index ee87b82d..86a0c19c 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -27,7 +27,7 @@ def main(): log.bootstrap_delayed_logging() logger.info('Starting Mopidy %s', versioning.get_version()) - signal.signal(signal.SIGTERM, process.exit_handler) + signal.signal(signal.SIGTERM, process.sigterm_handler) # Windows does not have signal.SIGUSR1 if hasattr(signal, 'SIGUSR1'): signal.signal(signal.SIGUSR1, pykka.debug.log_thread_tracebacks) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 501a9d45..64300ff9 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -257,7 +257,11 @@ class _Handler(object): new_state = _GST_STATE_MAPPING[new_state] old_state, self._audio.state = self._audio.state, new_state - target_state = _GST_STATE_MAPPING[self._audio._target_state] + target_state = _GST_STATE_MAPPING.get(self._audio._target_state) + if target_state is None: + # XXX: Workaround for #1430, to be fixed properly by #1222. + logger.debug('Race condition happened. See #1222 and #1430.') + return if target_state == new_state: target_state = None @@ -322,9 +326,24 @@ class _Handler(object): def on_tag(self, taglist): tags = tags_lib.convert_taglist(taglist) gst_logger.debug('Got TAG bus message: tags=%r', dict(tags)) - self._audio._tags.update(tags) - logger.debug('Audio event: tags_changed(tags=%r)', tags.keys()) - AudioListener.send('tags_changed', tags=tags.keys()) + + # Postpone emitting tags until stream start. + if self._audio._pending_tags is not None: + self._audio._pending_tags.update(tags) + return + + # TODO: Add proper tests for only emitting changed tags. + unique = object() + changed = [] + for key, value in tags.items(): + # Update any tags that changed, and store changed keys. + if self._audio._tags.get(key, unique) != value: + self._audio._tags[key] = value + changed.append(key) + + if changed: + logger.debug('Audio event: tags_changed(tags=%r)', changed) + AudioListener.send('tags_changed', tags=changed) def on_missing_plugin(self, msg): desc = GstPbutils.missing_plugin_message_get_description(msg) @@ -345,6 +364,14 @@ class _Handler(object): logger.debug('Audio event: stream_changed(uri=%r)', uri) AudioListener.send('stream_changed', uri=uri) + # Emit any postponed tags that we got after about-to-finish. + tags, self._audio._pending_tags = self._audio._pending_tags, None + self._audio._tags = tags + + if tags: + logger.debug('Audio event: tags_changed(tags=%r)', tags.keys()) + AudioListener.send('tags_changed', tags=tags.keys()) + def on_segment(self, segment): gst_logger.debug( 'Got SEGMENT pad event: ' @@ -382,6 +409,7 @@ class Audio(pykka.ThreadingActor): self._buffering = False self._tags = {} self._pending_uri = None + self._pending_tags = None self._playbin = None self._outputs = None @@ -466,6 +494,11 @@ class Audio(pykka.ThreadingActor): # systems. So leave the default to play it safe. queue = Gst.ElementFactory.make('queue') + if self._config['audio']['buffer_time'] > 0: + queue.set_property( + 'max-size-time', + self._config['audio']['buffer_time'] * Gst.MSECOND) + audio_sink.add(queue) audio_sink.add(self._outputs) @@ -527,8 +560,8 @@ class Audio(pykka.ThreadingActor): else: current_volume = None - self._tags = {} # TODO: add test for this somehow self._pending_uri = uri + self._pending_tags = {} self._playbin.set_property('uri', uri) if self.mixer is not None and current_volume is not None: diff --git a/mopidy/audio/tags.py b/mopidy/audio/tags.py index 62784bc0..38a0bac9 100644 --- a/mopidy/audio/tags.py +++ b/mopidy/audio/tags.py @@ -58,6 +58,7 @@ gstreamer-GstTagList.html log.TRACE_LOG_LEVEL, 'Ignoring unknown tag data: %r = %r', tag, value) + # TODO: dict(result) to not leak the defaultdict, or just use setdefault? return result diff --git a/mopidy/audio/utils.py b/mopidy/audio/utils.py index 8bc5279d..2027485a 100644 --- a/mopidy/audio/utils.py +++ b/mopidy/audio/utils.py @@ -10,13 +10,13 @@ def calculate_duration(num_samples, sample_rate): return Gst.util_uint64_scale(num_samples, Gst.SECOND, sample_rate) -def create_buffer(data, capabilites=None, timestamp=None, duration=None): +def create_buffer(data, timestamp=None, duration=None): """Create a new GStreamer buffer based on provided data. Mainly intended to keep gst imports out of non-audio modules. - .. versionchanged:: 1.2 - ``capabilites`` argument is no longer in use + .. versionchanged:: 2.0 + ``capabilites`` argument was removed. """ if not data: raise ValueError('Cannot create buffer without data') diff --git a/mopidy/commands.py b/mopidy/commands.py index 76a6fd6e..fef2d5f8 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -5,6 +5,7 @@ import collections import contextlib import logging import os +import signal import sys import pykka @@ -13,7 +14,7 @@ from mopidy import config as config_lib, exceptions from mopidy.audio import Audio from mopidy.core import Core from mopidy.internal import deps, process, timer, versioning -from mopidy.internal.gi import GLib, GObject +from mopidy.internal.gi import GLib logger = logging.getLogger(__name__) @@ -283,7 +284,13 @@ class RootCommand(Command): help='`section/key=value` values to override config options') def run(self, args, config): - loop = GObject.MainLoop() + def on_sigterm(loop): + logger.info('GLib mainloop got SIGTERM. Exiting...') + loop.quit() + + loop = GLib.MainLoop() + GLib.unix_signal_add( + GLib.PRIORITY_DEFAULT, signal.SIGTERM, on_sigterm, loop) mixer_class = self.get_mixer_class(config, args.registry['mixer']) backend_classes = args.registry['backend'] @@ -301,6 +308,7 @@ class RootCommand(Command): backends = self.start_backends(config, backend_classes, audio) core = self.start_core(config, mixer, backends, audio) self.start_frontends(config, frontend_classes, core) + logger.info('Starting GLib mainloop') loop.run() except (exceptions.BackendError, exceptions.FrontendError, diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index fac89ac9..7827e52d 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -39,6 +39,7 @@ _audio_schema['mixer_track'] = Deprecated() _audio_schema['mixer_volume'] = Integer(optional=True, minimum=0, maximum=100) _audio_schema['output'] = String() _audio_schema['visualizer'] = Deprecated() +_audio_schema['buffer_time'] = Integer(optional=True, minimum=1) _proxy_schema = ConfigSchema('proxy') _proxy_schema['scheme'] = String(optional=True, diff --git a/mopidy/config/default.conf b/mopidy/config/default.conf index 8076a0f4..7b99d86a 100644 --- a/mopidy/config/default.conf +++ b/mopidy/config/default.conf @@ -16,6 +16,7 @@ config_file = mixer = software mixer_volume = output = autoaudiosink +buffer_time = [proxy] scheme = diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index aa735262..af59b3d3 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -1,9 +1,7 @@ from __future__ import absolute_import, unicode_literals -import contextlib import logging -from mopidy import exceptions from mopidy.audio import PlaybackState from mopidy.compat import urllib from mopidy.core import listener @@ -12,20 +10,6 @@ from mopidy.internal import deprecation, models, validation logger = logging.getLogger(__name__) -@contextlib.contextmanager -def _backend_error_handling(backend, reraise=None): - try: - yield - except exceptions.ValidationError as e: - logger.error('%s backend returned bad data: %s', - backend.actor_ref.actor_class.__name__, e) - except Exception as e: - if reraise and isinstance(e, reraise): - raise - logger.exception('%s backend caused an exception.', - backend.actor_ref.actor_class.__name__) - - class PlaybackController(object): pykka_traversable = True @@ -153,6 +137,7 @@ class PlaybackController(object): return self._pending_position backend = self._get_backend(self.get_current_tl_track()) if backend: + # TODO: Wrap backend call in error handling. return backend.playback.get_time_position().get() else: return 0 @@ -278,18 +263,23 @@ class PlaybackController(object): if self._state == PlaybackState.STOPPED: return - # TODO: check that we always have a current track - original_tl_track = self.get_current_tl_track() - next_tl_track = self.core.tracklist.eot_track(original_tl_track) + pending = self.core.tracklist.eot_track(self._current_tl_track) + while pending: + # TODO: Avoid infinite loops if all tracks are unplayable. + backend = self._get_backend(pending) + if not backend: + continue - # TODO: only set pending if we have a backend that can play it? - # TODO: skip tracks that don't have a backend? - self._pending_tl_track = next_tl_track - backend = self._get_backend(next_tl_track) + try: + if backend.playback.change_track(pending.track).get(): + self._pending_tl_track = pending + break + except Exception: + logger.exception('%s backend caused an exception.', + backend.actor_ref.actor_class.__name__) - if backend: - with _backend_error_handling(backend): - backend.playback.change_track(next_tl_track.track).get() + self.core.tracklist._mark_unplayable(pending) + pending = self.core.tracklist.eot_track(pending) def _on_tracklist_change(self): """ @@ -329,6 +319,7 @@ class PlaybackController(object): def pause(self): """Pause playback.""" backend = self._get_backend(self.get_current_tl_track()) + # TODO: Wrap backend call in error handling. if not backend or backend.playback.pause().get(): # TODO: switch to: # backend.track(pause) @@ -373,22 +364,14 @@ class PlaybackController(object): current = self._pending_tl_track or self._current_tl_track pending = tl_track or current or self.core.tracklist.next_track(None) - # avoid endless loop if 'repeat' is 'true' and no track is playable - # * 2 -> second run to get all playable track in a shuffled playlist - count = self.core.tracklist.get_length() * 2 while pending: - # TODO: should we consume unplayable tracks in this loop? if self._change(pending, PlaybackState.PLAYING): break else: self.core.tracklist._mark_unplayable(pending) current = pending pending = self.core.tracklist.next_track(current) - count -= 1 - if not count: - logger.info('No playable track in the list.') - break # TODO return result? @@ -404,14 +387,18 @@ class PlaybackController(object): if not backend: return False + # TODO: Wrap backend call in error handling. backend.playback.prepare_change() - track_change_result = False - with _backend_error_handling(backend): - track_change_result = backend.playback.change_track( - pending_tl_track.track).get() - if not track_change_result: - return False # TODO: test for this path + try: + if not backend.playback.change_track(pending_tl_track.track).get(): + return False + except Exception: + logger.exception('%s backend caused an exception.', + backend.actor_ref.actor_class.__name__) + return False + + # TODO: Wrap backend calls in error handling. if state == PlaybackState.PLAYING: try: return backend.playback.play().get() @@ -460,6 +447,7 @@ class PlaybackController(object): if self.get_state() != PlaybackState.PAUSED: return backend = self._get_backend(self.get_current_tl_track()) + # TODO: Wrap backend call in error handling. if backend and backend.playback.resume().get(): self.set_state(PlaybackState.PLAYING) # TODO: trigger via gst messages @@ -517,6 +505,7 @@ class PlaybackController(object): backend = self._get_backend(self.get_current_tl_track()) if not backend: return False + # TODO: Wrap backend call in error handling. return backend.playback.seek(time_position).get() def stop(self): @@ -524,6 +513,7 @@ class PlaybackController(object): if self.get_state() != PlaybackState.STOPPED: self._last_position = self.get_time_position() backend = self._get_backend(self.get_current_tl_track()) + # TODO: Wrap backend call in error handling. if not backend or backend.playback.stop().get(): self.set_state(PlaybackState.STOPPED) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 87790c25..3c17a898 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -39,7 +39,7 @@ class PlaylistsController(object): :rtype: list of string - .. versionadded:: 1.2 + .. versionadded:: 2.0 """ return list(sorted(self.backends.with_playlists.keys())) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 1c5a24cc..c6dbda6a 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -622,6 +622,8 @@ class TracklistController(object): def _mark_unplayable(self, tl_track): """Internal method for :class:`mopidy.core.PlaybackController`.""" logger.warning('Track is not playable: %s', tl_track.track.uri) + if self.get_consume() and tl_track is not None: + self.remove({'tlid': [tl_track.tlid]}) if self.get_random() and tl_track in self._shuffled: self._shuffled.remove(tl_track) diff --git a/mopidy/internal/process.py b/mopidy/internal/process.py index 0710a82f..4bf681dd 100644 --- a/mopidy/internal/process.py +++ b/mopidy/internal/process.py @@ -1,7 +1,6 @@ from __future__ import absolute_import, unicode_literals import logging -import signal import threading import pykka @@ -12,20 +11,23 @@ from mopidy.compat import thread logger = logging.getLogger(__name__) -SIGNALS = dict( - (k, v) for v, k in signal.__dict__.items() - if v.startswith('SIG') and not v.startswith('SIG_')) - - def exit_process(): logger.debug('Interrupting main...') thread.interrupt_main() logger.debug('Interrupted main') -def exit_handler(signum, frame): - """A :mod:`signal` handler which will exit the program on signal.""" - logger.info('Got %s signal', SIGNALS[signum]) +def sigterm_handler(signum, frame): + """A :mod:`signal` handler which will exit the program on signal. + + This function is not called when the process' main thread is running a GLib + mainloop. In that case, the GLib mainloop must listen for SIGTERM signals + and quit itself. + + For Mopidy subcommands that does not run the GLib mainloop, this handler + ensures a proper shutdown of the process on SIGTERM. + """ + logger.info('Got SIGTERM signal. Exiting...') exit_process() @@ -49,28 +51,3 @@ def stop_remaining_actors(): pykka.ActorRegistry.stop_all() num_actors = len(pykka.ActorRegistry.get_all()) logger.debug('All actors stopped.') - - -class BaseThread(threading.Thread): - - def __init__(self): - super(BaseThread, self).__init__() - # No thread should block process from exiting - self.daemon = True - - def run(self): - logger.debug('%s: Starting thread', self.name) - try: - self.run_inside_try() - except KeyboardInterrupt: - logger.info('Interrupted by user') - except ImportError as e: - logger.error(e) - except pykka.ActorDeadError as e: - logger.warning(e) - except Exception as e: - logger.exception(e) - logger.debug('%s: Exiting thread', self.name) - - def run_inside_try(self): - raise NotImplementedError diff --git a/mopidy/m3u/__init__.py b/mopidy/m3u/__init__.py index df769c88..6a7fad9a 100644 --- a/mopidy/m3u/__init__.py +++ b/mopidy/m3u/__init__.py @@ -21,6 +21,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() + schema['base_dir'] = config.Path(optional=True) schema['default_encoding'] = config.String() schema['default_extension'] = config.String(choices=['.m3u', '.m3u8']) schema['playlists_dir'] = config.Path(optional=True) diff --git a/mopidy/m3u/ext.conf b/mopidy/m3u/ext.conf index 862bc6f7..16291c83 100644 --- a/mopidy/m3u/ext.conf +++ b/mopidy/m3u/ext.conf @@ -1,5 +1,6 @@ [m3u] enabled = true playlists_dir = +base_dir = $XDG_MUSIC_DIR default_encoding = latin-1 default_extension = .m3u8 diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index 7e4e39ff..28be28d9 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -60,6 +60,7 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): self._playlists_dir = Extension.get_data_dir(config) else: self._playlists_dir = ext_config['playlists_dir'] + self._base_dir = ext_config['base_dir'] or self._playlists_dir self._default_encoding = ext_config['default_encoding'] self._default_extension = ext_config['default_extension'] @@ -97,7 +98,7 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): path = translator.uri_to_path(uri) try: with self._open(path, 'r') as fp: - items = translator.load_items(fp, self._playlists_dir) + items = translator.load_items(fp, self._base_dir) except EnvironmentError as e: log_environment_error('Error reading playlist %s' % uri, e) else: @@ -107,7 +108,7 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): path = translator.uri_to_path(uri) try: with self._open(path, 'r') as fp: - items = translator.load_items(fp, self._playlists_dir) + items = translator.load_items(fp, self._base_dir) mtime = os.path.getmtime(self._abspath(path)) except EnvironmentError as e: log_environment_error('Error reading playlist %s' % uri, e) diff --git a/mopidy/stream/actor.py b/mopidy/stream/actor.py index c2e39652..0861b5b0 100644 --- a/mopidy/stream/actor.py +++ b/mopidy/stream/actor.py @@ -25,10 +25,19 @@ class StreamBackend(pykka.ThreadingActor, backend.Backend): timeout=config['stream']['timeout'], proxy_config=config['proxy']) - self.library = StreamLibraryProvider( - backend=self, blacklist=config['stream']['metadata_blacklist']) - self.playback = StreamPlaybackProvider( - audio=audio, backend=self, config=config) + self._session = http.get_requests_session( + proxy_config=config['proxy'], + user_agent='%s/%s' % ( + stream.Extension.dist_name, stream.Extension.version)) + + blacklist = config['stream']['metadata_blacklist'] + self._blacklist_re = re.compile( + r'^(%s)$' % '|'.join(fnmatch.translate(u) for u in blacklist)) + + self._timeout = config['stream']['timeout'] + + self.library = StreamLibraryProvider(backend=self) + self.playback = StreamPlaybackProvider(audio=audio, backend=self) self.playlists = None self.uri_schemes = audio_lib.supported_uri_schemes( @@ -43,27 +52,23 @@ class StreamBackend(pykka.ThreadingActor, backend.Backend): class StreamLibraryProvider(backend.LibraryProvider): - - def __init__(self, backend, blacklist): - super(StreamLibraryProvider, self).__init__(backend) - self._scanner = backend._scanner - self._blacklist_re = re.compile( - r'^(%s)$' % '|'.join(fnmatch.translate(u) for u in blacklist)) - def lookup(self, uri): if urllib.parse.urlsplit(uri).scheme not in self.backend.uri_schemes: return [] - if self._blacklist_re.match(uri): + if self.backend._blacklist_re.match(uri): logger.debug('URI matched metadata lookup blacklist: %s', uri) return [Track(uri=uri)] - try: - result = self._scanner.scan(uri) - track = tags.convert_tags_to_track(result.tags).replace( - uri=uri, length=result.duration) - except exceptions.ScannerError as e: - logger.warning('Problem looking up %s: %s', uri, e) + _, scan_result = _unwrap_stream( + uri, timeout=self.backend._timeout, scanner=self.backend._scanner, + requests_session=self.backend._session) + + if scan_result: + track = tags.convert_tags_to_track(scan_result.tags).replace( + uri=uri, length=scan_result.duration) + else: + logger.warning('Problem looking up %s: %s', uri) track = Track(uri=uri) return [track] @@ -71,23 +76,21 @@ class StreamLibraryProvider(backend.LibraryProvider): class StreamPlaybackProvider(backend.PlaybackProvider): - def __init__(self, audio, backend, config): - super(StreamPlaybackProvider, self).__init__(audio, backend) - self._config = config - self._scanner = backend._scanner - self._session = http.get_requests_session( - proxy_config=config['proxy'], - user_agent='%s/%s' % ( - stream.Extension.dist_name, stream.Extension.version)) - def translate_uri(self, uri): - return _unwrap_stream( - uri, - timeout=self._config['stream']['timeout'], - scanner=self._scanner, - requests_session=self._session) + if urllib.parse.urlsplit(uri).scheme not in self.backend.uri_schemes: + return None + + if self.backend._blacklist_re.match(uri): + logger.debug('URI matched metadata lookup blacklist: %s', uri) + return uri + + unwrapped_uri, _ = _unwrap_stream( + uri, timeout=self.backend._timeout, scanner=self.backend._scanner, + requests_session=self.backend._session) + return unwrapped_uri +# TODO: cleanup the return value of this. def _unwrap_stream(uri, timeout, scanner, requests_session): """ Get a stream URI from a playlist URI, ``uri``. @@ -105,7 +108,7 @@ def _unwrap_stream(uri, timeout, scanner, requests_session): logger.info( 'Unwrapping stream from URI (%s) failed: ' 'playlist referenced itself', uri) - return None + return None, None else: seen_uris.add(uri) @@ -117,7 +120,7 @@ def _unwrap_stream(uri, timeout, scanner, requests_session): logger.info( 'Unwrapping stream from URI (%s) failed: ' 'timed out in %sms', uri, timeout) - return None + return None, None scan_result = scanner.scan(uri, timeout=scan_timeout) except exceptions.ScannerError as exc: logger.debug('GStreamer failed scanning URI (%s): %s', uri, exc) @@ -130,14 +133,14 @@ def _unwrap_stream(uri, timeout, scanner, requests_session): ): logger.debug( 'Unwrapped potential %s stream: %s', scan_result.mime, uri) - return uri + return uri, scan_result download_timeout = deadline - time.time() if download_timeout < 0: logger.info( 'Unwrapping stream from URI (%s) failed: timed out in %sms', uri, timeout) - return None + return None, None content = http.download( requests_session, uri, timeout=download_timeout) @@ -145,14 +148,14 @@ def _unwrap_stream(uri, timeout, scanner, requests_session): logger.info( 'Unwrapping stream from URI (%s) failed: ' 'error downloading URI %s', original_uri, uri) - return None + return None, None uris = playlists.parse(content) if not uris: logger.debug( 'Failed parsing URI (%s) as playlist; found potential stream.', uri) - return uri + return uri, None # TODO Test streams and return first that seems to be playable logger.debug( diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 2bcc792a..b6ec6170 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -22,6 +22,7 @@ from tests import dummy_audio, path_to_data_dir class BaseTest(unittest.TestCase): config = { 'audio': { + 'buffer_time': None, 'mixer': 'fakemixer track_max_volume=65536', 'mixer_track': None, 'mixer_volume': None, @@ -38,6 +39,7 @@ class BaseTest(unittest.TestCase): def setUp(self): # noqa: N802 config = { 'audio': { + 'buffer_time': None, 'mixer': 'foomixer', 'mixer_volume': None, 'output': 'testoutput', diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index de6626ba..3625893d 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -14,6 +14,16 @@ from mopidy.models import Track from tests import dummy_audio +class TestPlaybackProvider(backend.PlaybackProvider): + def translate_uri(self, uri): + if 'error' in uri: + raise Exception(uri) + elif 'unplayable' in uri: + return None + else: + return uri + + # TODO: Replace this with dummy_backend now that it uses a real # playbackprovider Since we rely on our DummyAudio to actually emit events we # need a "real" backend and not a mock so the right calls make it through to @@ -23,7 +33,7 @@ class TestBackend(pykka.ThreadingActor, backend.Backend): def __init__(self, config, audio): super(TestBackend, self).__init__() - self.playback = backend.PlaybackProvider(audio=audio, backend=self) + self.playback = TestPlaybackProvider(audio=audio, backend=self) class BaseTest(unittest.TestCase): @@ -186,6 +196,47 @@ class TestNextHandling(BaseTest): self.assertIn(tl_track, self.core.tracklist.tl_tracks) + def test_next_skips_over_unplayable_track(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + self.audio.trigger_fake_playback_failure(tl_tracks[1].track.uri) + self.core.playback.play(tl_tracks[0]) + self.replay_events() + + self.core.playback.next() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[2] + + def test_next_skips_over_change_track_error(self): + # Trigger an exception in translate_uri. + track = Track(uri='dummy:error', length=1234) + self.core.tracklist.add(tracks=[track], at_position=1) + + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play() + self.replay_events() + + self.core.playback.next() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[2] + + def test_next_skips_over_change_track_unplayable(self): + # Make translate_uri return None. + track = Track(uri='dummy:unplayable', length=1234) + self.core.tracklist.add(tracks=[track], at_position=1) + + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play() + self.replay_events() + + self.core.playback.next() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[2] + class TestPreviousHandling(BaseTest): # TODO Test previous() more @@ -231,8 +282,49 @@ class TestPreviousHandling(BaseTest): self.assertIn(tl_tracks[1], self.core.tracklist.tl_tracks) + def test_previous_skips_over_unplayable_track(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + self.audio.trigger_fake_playback_failure(tl_tracks[1].track.uri) + self.core.playback.play(tl_tracks[2]) + self.replay_events() -class OnAboutToFinishTest(BaseTest): + self.core.playback.previous() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[0] + + def test_previous_skips_over_change_track_error(self): + # Trigger an exception in translate_uri. + track = Track(uri='dummy:error', length=1234) + self.core.tracklist.add(tracks=[track], at_position=1) + + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[2]) + self.replay_events() + + self.core.playback.previous() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[0] + + def test_previous_skips_over_change_track_unplayable(self): + # Makes translate_uri return None. + track = Track(uri='dummy:unplayable', length=1234) + self.core.tracklist.add(tracks=[track], at_position=1) + + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[2]) + self.replay_events() + + self.core.playback.previous() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[0] + + +class TestOnAboutToFinish(BaseTest): def test_on_about_to_finish_keeps_finished_track_in_tracklist(self): tl_track = self.core.tracklist.get_tl_tracks()[0] @@ -242,6 +334,34 @@ class OnAboutToFinishTest(BaseTest): self.assertIn(tl_track, self.core.tracklist.tl_tracks) + def test_on_about_to_finish_skips_over_change_track_error(self): + # Trigger an exception in translate_uri. + track = Track(uri='dummy:error', length=1234) + self.core.tracklist.add(tracks=[track], at_position=1) + + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[0]) + self.replay_events() + + self.trigger_about_to_finish() + + assert self.core.playback.get_current_tl_track() == tl_tracks[2] + + def test_on_about_to_finish_skips_over_change_track_unplayable(self): + # Makes translate_uri return None. + track = Track(uri='dummy:unplayable', length=1234) + self.core.tracklist.add(tracks=[track], at_position=1) + + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[0]) + self.replay_events() + + self.trigger_about_to_finish() + + assert self.core.playback.get_current_tl_track() == tl_tracks[2] + class TestConsumeHandling(BaseTest): @@ -257,6 +377,20 @@ class TestConsumeHandling(BaseTest): self.assertNotIn(tl_track, self.core.tracklist.get_tl_tracks()) + def test_next_in_consume_mode_removes_unplayable_track(self): + last_playable_tl_track = self.core.tracklist.get_tl_tracks()[-2] + unplayable_tl_track = self.core.tracklist.get_tl_tracks()[-1] + self.audio.trigger_fake_playback_failure(unplayable_tl_track.track.uri) + + self.core.playback.play(last_playable_tl_track) + self.core.tracklist.set_consume(True) + + self.core.playback.next() + self.replay_events() + + self.assertNotIn( + unplayable_tl_track, self.core.tracklist.get_tl_tracks()) + def test_on_about_to_finish_in_consume_mode_removes_finished_track(self): tl_track = self.core.tracklist.get_tl_tracks()[0] @@ -967,7 +1101,7 @@ class TestBug1177Regression(unittest.TestCase): b.playback.change_track.assert_called_once_with(track2) -class CorePlaybackExportRestoreTest(BaseTest): +class TesetCorePlaybackExportRestore(BaseTest): def test_export(self): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -1021,3 +1155,30 @@ class CorePlaybackExportRestoreTest(BaseTest): def test_import_none(self): self.core.playback._restore_state(None, None) + + +class TestBug1352Regression(BaseTest): + tracks = [ + Track(uri='dummy:a', length=40000), + Track(uri='dummy:b', length=40000), + ] + + def test_next_when_paused_updates_history(self): + self.core.history._add_track = mock.Mock() + self.core.tracklist._mark_playing = mock.Mock() + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.playback.play() + self.replay_events() + + self.core.history._add_track.assert_called_once_with(self.tracks[0]) + self.core.tracklist._mark_playing.assert_called_once_with(tl_tracks[0]) + self.core.history._add_track.reset_mock() + self.core.tracklist._mark_playing.reset_mock() + + self.playback.pause() + self.playback.next() + self.replay_events() + + self.core.history._add_track.assert_called_once_with(self.tracks[1]) + self.core.tracklist._mark_playing.assert_called_once_with(tl_tracks[1]) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index 664da9e9..e0ea1ce4 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -24,6 +24,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): config = { 'm3u': { 'enabled': True, + 'base_dir': None, 'default_encoding': 'latin-1', 'default_extension': '.m3u', 'playlists_dir': path_to_data_dir(''), @@ -33,6 +34,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def setUp(self): # noqa: N802 self.config['m3u']['playlists_dir'] = tempfile.mkdtemp() self.playlists_dir = self.config['m3u']['playlists_dir'] + self.base_dir = self.config['m3u']['base_dir'] or self.playlists_dir audio = dummy_audio.create_proxy() backend = M3UBackend.start( @@ -261,6 +263,32 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assertEqual(playlist.name, result.name) self.assertEqual(track.uri, result.tracks[0].uri) + def test_playlist_with_absolute_path(self): + track = Track(uri='/tmp/test.mp3') + filepath = b'/tmp/test.mp3' + playlist = self.core.playlists.create('test') + playlist = playlist.replace(tracks=[track]) + playlist = self.core.playlists.save(playlist) + + self.assertEqual(len(self.core.playlists.as_list()), 1) + result = self.core.playlists.lookup('m3u:test.m3u') + self.assertEqual('m3u:test.m3u', result.uri) + self.assertEqual(playlist.name, result.name) + self.assertEqual('file://' + filepath, result.tracks[0].uri) + + def test_playlist_with_relative_path(self): + track = Track(uri='test.mp3') + filepath = os.path.join(self.base_dir, b'test.mp3') + playlist = self.core.playlists.create('test') + playlist = playlist.replace(tracks=[track]) + playlist = self.core.playlists.save(playlist) + + self.assertEqual(len(self.core.playlists.as_list()), 1) + result = self.core.playlists.lookup('m3u:test.m3u') + self.assertEqual('m3u:test.m3u', result.uri) + self.assertEqual(playlist.name, result.name) + self.assertEqual('file://' + filepath, result.tracks[0].uri) + def test_playlist_sort_order(self): def check_order(playlists, names): self.assertEqual(names, [playlist.name for playlist in playlists]) @@ -303,6 +331,13 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assertIsNone(item_refs) +class M3UPlaylistsProviderBaseDirectoryTest(M3UPlaylistsProviderTest): + + def setUp(self): # noqa: N802 + self.config['m3u']['base_dir'] = tempfile.mkdtemp() + super(M3UPlaylistsProviderBaseDirectoryTest, self).setUp() + + class DeprecatedM3UPlaylistsProviderTest(M3UPlaylistsProviderTest): def run(self, result=None): diff --git a/tests/stream/test_library.py b/tests/stream/test_library.py index 67053924..29348a6c 100644 --- a/tests/stream/test_library.py +++ b/tests/stream/test_library.py @@ -4,7 +4,6 @@ import mock import pytest -from mopidy.audio import scan from mopidy.internal import path from mopidy.models import Track from mopidy.stream import actor @@ -13,16 +12,23 @@ from tests import path_to_data_dir @pytest.fixture -def scanner(): - return scan.Scanner(timeout=100, proxy_config={}) +def config(): + return { + 'proxy': {}, + 'stream': { + 'timeout': 1000, + 'metadata_blacklist': [], + 'protocols': ['file'], + }, + 'file': { + 'enabled': False + }, + } @pytest.fixture -def backend(scanner): - backend = mock.Mock() - backend.uri_schemes = ['file'] - backend._scanner = scanner - return backend +def audio(): + return mock.Mock() @pytest.fixture @@ -30,26 +36,28 @@ def track_uri(): return path.path_to_uri(path_to_data_dir('song1.wav')) -def test_lookup_ignores_unknown_scheme(backend): - library = actor.StreamLibraryProvider(backend, []) - - assert library.lookup('http://example.com') == [] +def test_lookup_ignores_unknown_scheme(audio, config): + backend = actor.StreamBackend(audio=audio, config=config) + assert backend.library.lookup('http://example.com') == [] -def test_lookup_respects_blacklist(backend, track_uri): - library = actor.StreamLibraryProvider(backend, [track_uri]) +def test_lookup_respects_blacklist(audio, config, track_uri): + config['stream']['metadata_blacklist'].append(track_uri) + backend = actor.StreamBackend(audio=audio, config=config) - assert library.lookup(track_uri) == [Track(uri=track_uri)] + assert backend.library.lookup(track_uri) == [Track(uri=track_uri)] -def test_lookup_respects_blacklist_globbing(backend, track_uri): - blacklist = [path.path_to_uri(path_to_data_dir('')) + '*'] - library = actor.StreamLibraryProvider(backend, blacklist) +def test_lookup_respects_blacklist_globbing(audio, config, track_uri): + blacklist_glob = path.path_to_uri(path_to_data_dir('')) + '*' + config['stream']['metadata_blacklist'].append(blacklist_glob) + backend = actor.StreamBackend(audio=audio, config=config) - assert library.lookup(track_uri) == [Track(uri=track_uri)] + assert backend.library.lookup(track_uri) == [Track(uri=track_uri)] -def test_lookup_converts_uri_metadata_to_track(backend, track_uri): - library = actor.StreamLibraryProvider(backend, []) +def test_lookup_converts_uri_metadata_to_track(audio, config, track_uri): + backend = actor.StreamBackend(audio=audio, config=config) - assert library.lookup(track_uri) == [Track(length=4406, uri=track_uri)] + result = backend.library.lookup(track_uri) + assert result == [Track(length=4406, uri=track_uri)] diff --git a/tests/stream/test_playback.py b/tests/stream/test_playback.py index ef7da0bf..1816f73e 100644 --- a/tests/stream/test_playback.py +++ b/tests/stream/test_playback.py @@ -4,6 +4,8 @@ import mock import pytest +import requests.exceptions + import responses from mopidy import exceptions @@ -27,6 +29,11 @@ def config(): 'proxy': {}, 'stream': { 'timeout': TIMEOUT, + 'metadata_blacklist': [], + 'protocols': ['http'], + }, + 'file': { + 'enabled': False }, } @@ -36,24 +43,21 @@ def audio(): return mock.Mock() -@pytest.fixture +@pytest.yield_fixture def scanner(): - scan_mock = mock.Mock(spec=scan.Scanner) - scan_mock.scan.return_value = None - return scan_mock + patcher = mock.patch.object(scan, 'Scanner') + yield patcher.start()() + patcher.stop() @pytest.fixture -def backend(scanner): - backend = mock.Mock() - backend.uri_schemes = ['file'] - backend._scanner = scanner - return backend +def backend(audio, config, scanner): + return actor.StreamBackend(audio=audio, config=config) @pytest.fixture -def provider(audio, backend, config): - return actor.StreamPlaybackProvider(audio, backend, config) +def provider(backend): + return backend.playback class TestTranslateURI(object): @@ -184,14 +188,24 @@ class TestTranslateURI(object): % STREAM_URI in caplog.text()) assert result == STREAM_URI - def test_failed_download_returns_none(self, provider, caplog): - with mock.patch.object(actor, 'http') as http_mock: - http_mock.download.return_value = None + @responses.activate + def test_failed_download_returns_none(self, scanner, provider, caplog): + scanner.scan.side_effect = [ + mock.Mock(mime='text/foo', playable=False) + ] - result = provider.translate_uri(PLAYLIST_URI) + responses.add( + responses.GET, PLAYLIST_URI, + body=requests.exceptions.HTTPError('Kaboom')) + + result = provider.translate_uri(PLAYLIST_URI) assert result is None + assert ( + 'Unwrapping stream from URI (%s) failed: ' + 'error downloading URI' % PLAYLIST_URI) in caplog.text() + @responses.activate def test_playlist_references_itself(self, scanner, provider, caplog): scanner.scan.side_effect = [