Merge pull request #1030 from adamcik/feature/stream-reference

Rework current metadata track to something ref based

docs/changelog.rst

@@ -40,6 +40,10 @@ v0.20.0 (UNRELEASED)
 - Add :meth:`mopidy.core.LibraryController.get_distinct` for getting unique
   values for a given field. (Fixes: :issue:`913`, PR: :issue:`1022`)
 
+- Add :meth:`mopidy.core.Listener.stream_title_changed` and
+  :meth:`mopidy.core.PlaybackController.get_stream_title` for letting clients
+  know about the current song in streams.
+
 **Commands**
 
 - Make the ``mopidy`` command print a friendly error message if the
@@ -124,6 +128,9 @@ v0.20.0 (UNRELEASED)
   ``mixrampdelay`` commands are now supported but throw a NotImplemented
   exception.
 
+- Start setting the ``Name`` field which is used for radio streams.
+  (Fixes: :issue:`944`)
+
 **HTTP frontend**
 
 - Prevent race condition in webservice broadcast from breaking the server.

mopidy/core/actor.py

@@ -7,7 +7,6 @@ import pykka
 
 from mopidy import audio, backend, mixer
 from mopidy.audio import PlaybackState
-from mopidy.audio.utils import convert_tags_to_track
 from mopidy.core.history import HistoryController
 from mopidy.core.library import LibraryController
 from mopidy.core.listener import CoreListener
@@ -15,7 +14,6 @@ from mopidy.core.mixer import MixerController
 from mopidy.core.playback import PlaybackController
 from mopidy.core.playlists import PlaylistsController
 from mopidy.core.tracklist import TracklistController
-from mopidy.models import TlTrack, Track
 from mopidy.utils import versioning
 from mopidy.utils.deprecation import deprecated_property
 
@@ -88,6 +86,9 @@ class Core(
     def reached_end_of_stream(self):
         self.playback.on_end_of_track()
 
+    def stream_changed(self, uri):
+        self.playback.on_stream_changed(uri)
+
     def state_changed(self, old_state, new_state, target_state):
         # XXX: This is a temporary fix for issue #232 while we wait for a more
         # permanent solution with the implementation of issue #234. When the
@@ -116,30 +117,16 @@ class Core(
         CoreListener.send('mute_changed', mute=mute)
 
     def tags_changed(self, tags):
-        if not self.audio:
-            return
-
-        current_tl_track = self.playback.get_current_tl_track()
-        if current_tl_track is None:
+        if not self.audio or 'title' not in tags:
             return
 
         tags = self.audio.get_current_tags().get()
-        if not tags:
+        if not tags or 'title' not in tags or not tags['title']:
             return
 
-        current_track = current_tl_track.track
-        tags_track = convert_tags_to_track(tags)
-
-        track_kwargs = {k: v for k, v in current_track.__dict__.items() if v}
-        track_kwargs.update(
-            {k: v for k, v in tags_track.__dict__.items() if v})
-
-        self.playback._current_metadata_track = TlTrack(**{
-            'tlid': current_tl_track.tlid,
-            'track': Track(**track_kwargs)})
-
-        # TODO Move this into playback.current_metadata_track setter?
-        CoreListener.send('current_metadata_changed')
+        title = tags['title'][0]
+        self.playback._stream_title = title
+        CoreListener.send('stream_title_changed', title=title)
 
 
 class Backends(list):

mopidy/core/listener.py

@@ -164,9 +164,9 @@ class CoreListener(listener.Listener):
         """
         pass
 
-    def current_metadata_changed(self):
+    def stream_title_changed(self, title):
         """
-        Called whenever current track's metadata changed
+        Called whenever the currently playing stream title changes.
 
         *MAY* be implemented by actor.
         """

mopidy/core/playback.py

@@ -20,7 +20,7 @@ class PlaybackController(object):
         self.core = core
 
         self._current_tl_track = None
-        self._current_metadata_track = None
+        self._stream_title = None
         self._state = PlaybackState.STOPPED
 
     def _get_backend(self):
@@ -73,20 +73,9 @@ class PlaybackController(object):
         Use :meth:`get_current_track` instead.
     """
 
-    def get_current_metadata_track(self):
-        """
-        Get a :class:`mopidy.models.TlTrack` with updated metadata for the
-        currently playing track.
-
-        Returns :class:`None` if no track is currently playing.
-        """
-        return self._current_metadata_track
-
-    current_metadata_track = deprecated_property(get_current_metadata_track)
-    """
-    .. deprecated:: 0.20
-        Use :meth:`get_current_metadata_track` instead.
-    """
+    def get_stream_title(self):
+        """Get the current stream title or :class:`None`."""
+        return self._stream_title
 
     def get_state(self):
         """Get The playback state."""
@@ -244,6 +233,9 @@ class PlaybackController(object):
             self.stop()
             self.set_current_tl_track(None)
 
+    def on_stream_changed(self, uri):
+        self._stream_title = None
+
     def next(self):
         """
         Change to the next track.

mopidy/mpd/actor.py

@@ -74,5 +74,5 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener):
     def mute_changed(self, mute):
         self.send_idle('output')
 
-    def current_metadata_changed(self):
+    def stream_title_changed(self, title):
         self.send_idle('playlist')

mopidy/mpd/protocol/current_playlist.py

@@ -276,25 +276,20 @@ def plchanges(context, version):
     """
     # XXX Naive implementation that returns all tracks as changed
     tracklist_version = context.core.tracklist.version.get()
-    iversion = int(version)
-    if iversion < tracklist_version:
+    if version < tracklist_version:
         return translator.tracks_to_mpd_format(
             context.core.tracklist.tl_tracks.get())
-    elif iversion == tracklist_version:
-        # If version are equals, it is just a metadata update
-        # So we replace the updated track in playlist
-        current_md_track = context.core.playback.current_metadata_track.get()
-        if current_md_track is None:
+    elif version == tracklist_version:
+        # A version match could indicate this is just a metadata update, so
+        # check for a stream ref and let the client know about the change.
+        stream_title = context.core.playback.get_stream_title().get()
+        if stream_title is None:
             return None
 
-        ntl_tracks = []
-        tl_tracks = context.core.tracklist.tl_tracks.get()
-        for tl_track in tl_tracks:
-            if tl_track.tlid == current_md_track.tlid:
-                ntl_tracks.append(current_md_track)
-            else:
-                ntl_tracks.append(tl_track)
-        return translator.tracks_to_mpd_format(ntl_tracks)
+        tl_track = context.core.playback.current_tl_track.get()
+        position = context.core.tracklist.index(tl_track).get()
+        return translator.track_to_mpd_format(
+            tl_track, position=position, stream_title=stream_title)
 
 
 @protocol.commands.add('plchangesposid', version=protocol.INT)

mopidy/mpd/protocol/status.py

@@ -34,12 +34,12 @@ def currentsong(context):
         Displays the song info of the current song (same song that is
         identified in status).
     """
-    tl_track = context.core.playback.current_metadata_track.get()
-    if tl_track is None:
-        tl_track = context.core.playback.current_tl_track.get()
+    tl_track = context.core.playback.current_tl_track.get()
+    stream_title = context.core.playback.get_stream_title().get()
     if tl_track is not None:
         position = context.core.tracklist.index(tl_track).get()
-        return translator.track_to_mpd_format(tl_track, position=position)
+        return translator.track_to_mpd_format(
+            tl_track, position=position, stream_title=stream_title)
 
 
 @protocol.commands.add('idle', list_command=False)

mopidy/mpd/translator.py

@@ -15,7 +15,7 @@ def normalize_path(path, relative=False):
     return '/'.join(parts)
 
 
-def track_to_mpd_format(track, position=None):
+def track_to_mpd_format(track, position=None, stream_title=None):
     """
     Format track for output to MPD client.
 
@@ -23,16 +23,15 @@ def track_to_mpd_format(track, position=None):
     :type track: :class:`mopidy.models.Track` or :class:`mopidy.models.TlTrack`
     :param position: track's position in playlist
     :type position: integer
-    :param key: if we should set key
-    :type key: boolean
-    :param mtime: if we should set mtime
-    :type mtime: boolean
+    :param stream_title: the current streams title
+    :type position: string
     :rtype: list of two-tuples
     """
     if isinstance(track, TlTrack):
         (tlid, track) = track
     else:
         (tlid, track) = (None, track)
+
     result = [
         ('file', track.uri or ''),
         ('Time', track.length and (track.length // 1000) or 0),
@@ -41,6 +40,9 @@ def track_to_mpd_format(track, position=None):
         ('Album', track.album and track.album.name or ''),
     ]
 
+    if stream_title:
+        result.append(('Name', stream_title))
+
     if track.date:
         result.append(('Date', track.date))
 

tests/core/test_listener.py

@@ -58,5 +58,5 @@ class CoreListenerTest(unittest.TestCase):
     def test_listener_has_default_impl_for_seeked(self):
         self.listener.seeked(0)
 
-    def test_listener_has_default_impl_for_current_metadata_changed(self):
-        self.listener.current_metadata_changed()
+    def test_listener_has_default_impl_for_stream_title_changed(self):
+        self.listener.stream_title_changed('foobar')

tests/core/test_playback.py

@@ -4,9 +4,13 @@ import unittest
 
 import mock
 
+import pykka
+
 from mopidy import backend, core
 from mopidy.models import Track
 
+from tests import dummy_audio as audio
+
 
 class CorePlaybackTest(unittest.TestCase):
     def setUp(self):  # noqa: N802
@@ -525,3 +529,85 @@ class CorePlaybackTest(unittest.TestCase):
         self.assertFalse(self.playback2.get_time_position.called)
 
     # TODO Test on_tracklist_change
+
+
+# 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 audio.
+class TestBackend(pykka.ThreadingActor, backend.Backend):
+    uri_schemes = ['dummy']
+
+    def __init__(self, config, audio):
+        super(TestBackend, self).__init__()
+        self.playback = backend.PlaybackProvider(audio=audio, backend=self)
+
+
+class TestStream(unittest.TestCase):
+    def setUp(self):  # noqa: N802
+        self.audio = audio.DummyAudio.start().proxy()
+        self.backend = TestBackend.start(config={}, audio=self.audio).proxy()
+        self.core = core.Core(audio=self.audio, backends=[self.backend])
+        self.playback = self.core.playback
+
+        self.tracks = [Track(uri='dummy:a', length=1234),
+                       Track(uri='dummy:b', length=1234)]
+
+        self.core.tracklist.add(self.tracks)
+
+        self.events = []
+        self.patcher = mock.patch('mopidy.audio.listener.AudioListener.send')
+        self.send_mock = self.patcher.start()
+
+        def send(event, **kwargs):
+            self.events.append((event, kwargs))
+
+        self.send_mock.side_effect = send
+
+    def tearDown(self):  # noqa: N802
+        pykka.ActorRegistry.stop_all()
+        self.patcher.stop()
+
+    def replay_audio_events(self):
+        while self.events:
+            event, kwargs = self.events.pop(0)
+            self.core.on_event(event, **kwargs)
+
+    def test_get_stream_title_before_playback(self):
+        self.assertEqual(self.playback.get_stream_title(), None)
+
+    def test_get_stream_title_during_playback(self):
+        self.core.playback.play()
+
+        self.replay_audio_events()
+        self.assertEqual(self.playback.get_stream_title(), None)
+
+    def test_get_stream_title_during_playback_with_tags_change(self):
+        self.core.playback.play()
+        self.audio.trigger_fake_tags_changed({'title': ['foobar']}).get()
+
+        self.replay_audio_events()
+        self.assertEqual(self.playback.get_stream_title(), 'foobar')
+
+    def test_get_stream_title_after_next(self):
+        self.core.playback.play()
+        self.audio.trigger_fake_tags_changed({'title': ['foobar']}).get()
+        self.core.playback.next()
+
+        self.replay_audio_events()
+        self.assertEqual(self.playback.get_stream_title(), None)
+
+    def test_get_stream_title_after_next_with_tags_change(self):
+        self.core.playback.play()
+        self.audio.trigger_fake_tags_changed({'title': ['foo']}).get()
+        self.core.playback.next()
+        self.audio.trigger_fake_tags_changed({'title': ['bar']}).get()
+
+        self.replay_audio_events()
+        self.assertEqual(self.playback.get_stream_title(), 'bar')
+
+    def test_get_stream_title_after_stop(self):
+        self.core.playback.play()
+        self.audio.trigger_fake_tags_changed({'title': ['foobar']}).get()
+        self.core.playback.stop()
+
+        self.replay_audio_events()
+        self.assertEqual(self.playback.get_stream_title(), None)

tests/dummy_audio.py

@@ -109,6 +109,10 @@ class DummyAudio(pykka.ThreadingActor):
     def trigger_fake_playback_failure(self):
         self._state_change_result = False
 
+    def trigger_fake_tags_changed(self, tags):
+        self._tags = tags
+        audio.AudioListener.send('tags_changed', tags=self._tags.keys())
+
     def get_about_to_finish_callback(self):
         # This needs to be called from outside the actor or we lock up.
         def wrapper():