Merge branch 'develop' of https://github.com/mopidy/mopidy into fix/310-persist-mopidy-state-between-runs

This commit is contained in:
Jens Luetjen 2016-03-31 19:04:45 +02:00
commit 2bb9e39ba6
11 changed files with 249 additions and 51 deletions

3
.github/ISSUE_TEMPLATE.md vendored Normal file
View File

@ -0,0 +1,3 @@
Please use https://discuss.mopidy.com/ for support questions.
GitHub Issues should only be used for confirmed problems with Mopidy and well-defined feature requests.

View File

@ -18,7 +18,25 @@ v2.0.1 (UNRELEASED)
Bug fix release.
- Nothing yet.
- Audio: Set ``soft-volume`` flag on GStreamer's playbin element. This is the
playbin's default, but we managed to override it when configuring the playbin
to only process audio. This should fix the "Volume/mute is not available"
warning.
- Audio: Fix buffer conversion. This fixes image extraction.
(Fixes: :issue:`1469`, PR: :issue:`1472`)
- Audio: Update scan logic to workaround GStreamer issue where tags and
duration might only be available after we start playing.
(Fixes: :issue:`935`, :issue:`1453`, :issue:`1474` and :issue:`1480`, PR:
:issue:`1487`)
- Core: Avoid endless loop if all tracks in the tracklist are unplayable and
consume mode is off. (Fixes: :issue:`1221`, :issue:`1454`, PR: :issue:`1455`)
- File: Ensure path comparision is done between bytestrings only. Fixes crash
where a :confval:`file/media_dirs` path contained non-ASCII characters.
(Fixes: :issue:`1345`, PR: :issue:`1493`)
v2.0.0 (2016-02-15)

View File

@ -21,6 +21,9 @@ logger = logging.getLogger(__name__)
# set_state() on a pipeline.
gst_logger = logging.getLogger('mopidy.audio.gst')
_GST_PLAY_FLAGS_AUDIO = 0x02
_GST_PLAY_FLAGS_SOFT_VOLUME = 0x10
_GST_STATE_MAPPING = {
Gst.State.PLAYING: PlaybackState.PLAYING,
Gst.State.PAUSED: PlaybackState.PAUSED,
@ -448,7 +451,8 @@ class Audio(pykka.ThreadingActor):
def _setup_playbin(self):
playbin = Gst.ElementFactory.make('playbin')
playbin.set_property('flags', 2) # GST_PLAY_FLAG_AUDIO
playbin.set_property(
'flags', _GST_PLAY_FLAGS_AUDIO | _GST_PLAY_FLAGS_SOFT_VOLUME)
# TODO: turn into config values...
playbin.set_property('buffer-size', 5 << 20) # 5MB

View File

@ -61,8 +61,7 @@ class Scanner(object):
try:
_start_pipeline(pipeline)
tags, mime, have_audio = _process(pipeline, timeout)
duration = _query_duration(pipeline)
tags, mime, have_audio, duration = _process(pipeline, timeout)
seekable = _query_seekable(pipeline)
finally:
signals.clear()
@ -136,30 +135,6 @@ def _start_pipeline(pipeline):
pipeline.set_state(Gst.State.PLAYING)
def _query_duration(pipeline, timeout=100):
# 1. Try and get a duration, return if success.
# 2. Some formats need to play some buffers before duration is found.
# 3. Wait for a duration change event.
# 4. Try and get a duration again.
success, duration = pipeline.query_duration(Gst.Format.TIME)
if success and duration >= 0:
return duration // Gst.MSECOND
result = pipeline.set_state(Gst.State.PLAYING)
if result == Gst.StateChangeReturn.FAILURE:
return None
gst_timeout = timeout * Gst.MSECOND
bus = pipeline.get_bus()
bus.timed_pop_filtered(gst_timeout, Gst.MessageType.DURATION_CHANGED)
success, duration = pipeline.query_duration(Gst.Format.TIME)
if success and duration >= 0:
return duration // Gst.MSECOND
return None
def _query_seekable(pipeline):
query = Gst.Query.new_seeking(Gst.Format.TIME)
pipeline.query(query)
@ -172,6 +147,7 @@ def _process(pipeline, timeout_ms):
mime = None
have_audio = False
missing_message = None
duration = None
types = (
Gst.MessageType.ELEMENT |
@ -179,11 +155,12 @@ def _process(pipeline, timeout_ms):
Gst.MessageType.ERROR |
Gst.MessageType.EOS |
Gst.MessageType.ASYNC_DONE |
Gst.MessageType.DURATION_CHANGED |
Gst.MessageType.TAG
)
timeout = timeout_ms
previous = int(time.time() * 1000)
start = int(time.time() * 1000)
while timeout > 0:
message = bus.timed_pop_filtered(timeout * Gst.MSECOND, types)
@ -197,7 +174,7 @@ def _process(pipeline, timeout_ms):
mime = message.get_structure().get_value('caps').get_name()
if mime and (
mime.startswith('text/') or mime == 'application/xml'):
return tags, mime, have_audio
return tags, mime, have_audio, duration
elif message.get_structure().get_name() == 'have-audio':
have_audio = True
elif message.type == Gst.MessageType.ERROR:
@ -205,21 +182,43 @@ def _process(pipeline, timeout_ms):
if missing_message and not mime:
caps = missing_message.get_structure().get_value('detail')
mime = caps.get_structure(0).get_name()
return tags, mime, have_audio
return tags, mime, have_audio, duration
raise exceptions.ScannerError(error)
elif message.type == Gst.MessageType.EOS:
return tags, mime, have_audio
return tags, mime, have_audio, duration
elif message.type == Gst.MessageType.ASYNC_DONE:
if message.src == pipeline:
return tags, mime, have_audio
success, duration = pipeline.query_duration(Gst.Format.TIME)
if success:
duration = duration // Gst.MSECOND
else:
duration = None
if tags and duration is not None:
return tags, mime, have_audio, duration
# Workaround for upstream bug which causes tags/duration to arrive
# after pre-roll. We get around this by starting to play the track
# and then waiting for a duration change.
# https://bugzilla.gnome.org/show_bug.cgi?id=763553
result = pipeline.set_state(Gst.State.PLAYING)
if result == Gst.StateChangeReturn.FAILURE:
return tags, mime, have_audio, duration
elif message.type == Gst.MessageType.DURATION_CHANGED:
# duration will be read after ASYNC_DONE received; for now
# just give it a non-None value to flag that we have a duration:
duration = 0
elif message.type == Gst.MessageType.TAG:
taglist = message.parse_tag()
# Note that this will only keep the last tag.
tags.update(tags_lib.convert_taglist(taglist))
now = int(time.time() * 1000)
timeout -= now - previous
previous = now
timeout = timeout_ms - (int(time.time() * 1000) - start)
# workaround for https://bugzilla.gnome.org/show_bug.cgi?id=763553:
# if we got what we want then stop playing (and wait for ASYNC_DONE)
if tags and duration is not None:
pipeline.set_state(Gst.State.PAUSED)
raise exceptions.ScannerError('Timeout after %dms' % timeout_ms)

View File

@ -53,6 +53,10 @@ gstreamer-GstTagList.html
result[tag].append(value.decode('utf-8', 'replace'))
elif isinstance(value, (compat.text_type, bool, numbers.Number)):
result[tag].append(value)
elif isinstance(value, Gst.Sample):
data = _extract_sample_data(value)
if data:
result[tag].append(data)
else:
logger.log(
log.TRACE_LOG_LEVEL,
@ -62,6 +66,13 @@ gstreamer-GstTagList.html
return result
def _extract_sample_data(sample):
buf = sample.get_buffer()
if not buf:
return None
return buf.extract_dup(0, buf.get_size())
# TODO: split based on "stream" and "track" based conversion? i.e. handle data
# from radios in it's own helper instead?
def convert_tags_to_track(tags):

View File

@ -264,22 +264,27 @@ class PlaybackController(object):
return
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
# 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
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__)
while pending:
backend = self._get_backend(pending)
if backend:
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__)
self.core.tracklist._mark_unplayable(pending)
pending = self.core.tracklist.eot_track(pending)
count -= 1
if not count:
logger.info('No playable track in the list.')
break
def _on_tracklist_change(self):
"""
@ -302,6 +307,9 @@ class PlaybackController(object):
"""
state = self.get_state()
current = self._pending_tl_track or self._current_tl_track
# 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 current:
pending = self.core.tracklist.next_track(current)
@ -313,6 +321,10 @@ class PlaybackController(object):
# if current == pending:
# break
current = pending
count -= 1
if not count:
logger.info('No playable track in the list.')
break
# TODO return result?
@ -364,6 +376,9 @@ 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:
if self._change(pending, PlaybackState.PLAYING):
@ -372,6 +387,10 @@ class PlaybackController(object):
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?
@ -428,6 +447,9 @@ class PlaybackController(object):
self._previous = True
state = self.get_state()
current = self._pending_tl_track or self._current_tl_track
# 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 current:
pending = self.core.tracklist.previous_track(current)
@ -439,6 +461,10 @@ class PlaybackController(object):
# if current == pending:
# break
current = pending
count -= 1
if not count:
logger.info('No playable track in the list.')
break
# TODO: no return value?

View File

@ -132,5 +132,6 @@ class FileLibraryProvider(backend.LibraryProvider):
def _is_in_basedir(self, local_path):
return any(
path.is_path_inside_base_dir(local_path, media_dir['path'])
path.is_path_inside_base_dir(
local_path, media_dir['path'].encode('utf-8'))
for media_dir in self._media_dirs)

View File

@ -196,6 +196,11 @@ def find_mtimes(root, follow=False):
def is_path_inside_base_dir(path, base_path):
if not isinstance(path, bytes):
raise ValueError('path is not a bytestring')
if not isinstance(base_path, bytes):
raise ValueError('base_path is not a bytestring')
if path.endswith(os.sep):
raise ValueError('Path %s cannot end with a path separator'
% path)

View File

@ -46,7 +46,7 @@ def path_to_local_track_uri(relpath):
def path_to_local_directory_uri(relpath):
"""Convert path relative to :confval:`local/media_dir` directory URI."""
"""Convert path relative to :confval:`local/media_dir` to directory URI."""
if isinstance(relpath, compat.text_type):
relpath = relpath.encode('utf-8')
return 'local:directory:%s' % urllib.quote(relpath)

View File

@ -15,11 +15,42 @@ from tests import dummy_audio
class TestPlaybackProvider(backend.PlaybackProvider):
def __init__(self, audio, backend):
super(TestPlaybackProvider, self).__init__(audio, backend)
self._call_limit = 10
self._call_count = 0
self._call_onetime = False
def reset_call_limit(self):
self._call_count = 0
self._call_onetime = False
def is_call_limit_reached(self):
return self._call_count > self._call_limit
def _translate_uri_call_limit(self, uri):
self._call_count += 1
if self._call_count > self._call_limit:
# return any url (not 'None') to stop the endless loop
return 'assert: call limit reached'
if 'limit_never' in uri:
# unplayable
return None
elif 'limit_one' in uri:
# one time playable
if self._call_onetime:
return None
self._call_onetime = True
return uri
def translate_uri(self, uri):
if 'error' in uri:
raise Exception(uri)
elif 'unplayable' in uri:
return None
elif 'limit' in uri:
return self._translate_uri_call_limit(uri)
else:
return uri
@ -1182,3 +1213,77 @@ class TestBug1352Regression(BaseTest):
self.core.history._add_track.assert_called_once_with(self.tracks[1])
self.core.tracklist._mark_playing.assert_called_once_with(tl_tracks[1])
class TestEndlessLoop(BaseTest):
tracks_play = [
Track(uri='dummy:limit_never:a'),
Track(uri='dummy:limit_never:b')
]
tracks_other = [
Track(uri='dummy:limit_never:a'),
Track(uri='dummy:limit_one'),
Track(uri='dummy:limit_never:b')
]
def test_play(self):
self.core.tracklist.clear()
self.core.tracklist.add(self.tracks_play)
self.backend.playback.reset_call_limit().get()
self.core.tracklist.set_repeat(True)
tl_tracks = self.core.tracklist.get_tl_tracks()
self.core.playback.play(tl_tracks[0])
self.replay_events()
self.assertFalse(self.backend.playback.is_call_limit_reached().get())
def test_next(self):
self.core.tracklist.clear()
self.core.tracklist.add(self.tracks_other)
self.backend.playback.reset_call_limit().get()
self.core.tracklist.set_repeat(True)
tl_tracks = self.core.tracklist.get_tl_tracks()
self.core.playback.play(tl_tracks[1])
self.replay_events()
self.core.playback.next()
self.replay_events()
self.assertFalse(self.backend.playback.is_call_limit_reached().get())
def test_previous(self):
self.core.tracklist.clear()
self.core.tracklist.add(self.tracks_other)
self.backend.playback.reset_call_limit().get()
self.core.tracklist.set_repeat(True)
tl_tracks = self.core.tracklist.get_tl_tracks()
self.core.playback.play(tl_tracks[1])
self.replay_events()
self.core.playback.previous()
self.replay_events()
self.assertFalse(self.backend.playback.is_call_limit_reached().get())
def test_on_about_to_finish(self):
self.core.tracklist.clear()
self.core.tracklist.add(self.tracks_other)
self.backend.playback.reset_call_limit().get()
self.core.tracklist.set_repeat(True)
tl_tracks = self.core.tracklist.get_tl_tracks()
self.core.playback.play(tl_tracks[1])
self.replay_events()
self.trigger_about_to_finish()
self.assertFalse(self.backend.playback.is_call_limit_reached().get())

View File

@ -7,6 +7,8 @@ import shutil
import tempfile
import unittest
import pytest
from mopidy import compat, exceptions
from mopidy.internal import path
from mopidy.internal.gi import GLib
@ -392,6 +394,30 @@ class FindMTimesTest(unittest.TestCase):
self.assertEqual(errors, {})
class TestIsPathInsideBaseDir(object):
def test_when_inside(self):
assert path.is_path_inside_base_dir(
'/æ/øå'.encode('utf-8'),
''.encode('utf-8'))
def test_when_outside(self):
assert not path.is_path_inside_base_dir(
'/æ/øå'.encode('utf-8'),
''.encode('utf-8'))
def test_byte_inside_str_fails(self):
with pytest.raises(ValueError):
path.is_path_inside_base_dir('/æ/øå'.encode('utf-8'), '')
def test_str_inside_byte_fails(self):
with pytest.raises(ValueError):
path.is_path_inside_base_dir('/æ/øå', ''.encode('utf-8'))
def test_str_inside_str_fails(self):
with pytest.raises(ValueError):
path.is_path_inside_base_dir('/æ/øå', '')
# TODO: kill this in favour of just os.path.getmtime + mocks
class MtimeTest(unittest.TestCase):