fix/playlist_handling

ovos_plugin_common_play/ocp/media.py

@@ -194,7 +194,7 @@ def update(self, entry: Union[dict, MediaEntry], skipkeys: list = None, newonly:
 
     def extract_stream(self):
         """
-        Get metadata from ocp_plugins and add it to this MediaEntry
+        DEPRECATED: Get metadata from ocp_plugins and add it to this MediaEntry
         """
         uri = self.uri
         if not uri:

ovos_plugin_common_play/ocp/player.py

@@ -250,37 +250,55 @@ def set_now_playing(self, track: Union[dict, MediaEntry, Playlist, PluginStream]
         """
         LOG.debug(f"Playing: {track}")
         if isinstance(track, dict):
-            LOG.debug("Handling dict track")
-            if "uri" not in track:
-                track["uri"] = ""  # when syncing from MPRIS uri is missing
-            track = MediaEntry.from_dict(track)
+            LOG.debug(f"Handling dict track: {track}")
+            if "uri" not in track:  # TODO handle this better
+                track["uri"] = "external:"  # when syncing from MPRIS uri is missing
+            track = dict2entry(track)
         if not isinstance(track, (MediaEntry, Playlist, PluginStream)):
             raise ValueError(f"Expected MediaEntry/Playlist, but got: {track}")
-        self.now_playing.reset()  # reset now_playing to remove old metadata
+
+        try:
+            idx = self.playlist.index(track)  # find the entry in "now playing"
+        except ValueError:
+            idx = -1
         if isinstance(track, PluginStream):
-            track = track.as_media_entry
+            track = track.extract_media_entry(video=track.playback == PlaybackType.VIDEO)
+            LOG.info(f"PluginStream extracted: {track}")
+            if idx >= 0:
+                self.playlist[idx] = track  # update extracted plugin stream
+
         if isinstance(track, MediaEntry):
             # single track entry (MediaEntry)
             self.now_playing.update(track)
-            # copy now_playing (without event handlers) to playlist
-            # entry = self.now_playing.as_entry()
-            if track not in self.playlist:  # compared by uri
+
+            # update playlist position
+            if idx > -1:
+                self.playlist.set_position(idx)
+            # add to "now playing" if it's a new track
+            elif track not in self.playlist:  # compared by uri
                 self.playlist.add_entry(track)
+                self.playlist.set_position(len(self.playlist) - 1)
+            # find equivalent track position in playlist
+            else:
+                self.playlist.goto_track(track)
+
         elif isinstance(track, Playlist):
             # this is a playlist result (list of dicts)
             self.playlist.clear()
             for entry in track:
                 self.playlist.add_entry(entry)
 
+            # mew playlist -> reset playlist position to the start
+            self.playlist.set_position(0)
+
+            # update self.now_playing
             if len(self.playlist):
-                self.now_playing.update(self.playlist[0])
-            else:
-                # If there's no URI, the skill might be handling playback so
-                # now_playing should still be updated
-                self.now_playing.update(self.playlist.as_dict)
+                track = self.playlist[0]
+                return self.set_now_playing(track)
 
-        # sync playlist position
-        self.playlist.goto_track(self.now_playing)
+            # If there's no URI, the skill might be handling playback so
+            # now_playing should still be updated
+            self.now_playing.update(self.playlist.as_dict)
 
         # update gui values
         self.gui.update_current_track()
@@ -302,16 +320,14 @@ def validate_stream(self) -> bool:
         if self.active_backend not in [PlaybackType.SKILL,
                                        PlaybackType.UNDEFINED,
                                        PlaybackType.MPRIS]:
-            try:
-                self.now_playing.extract_stream()
-            except Exception as e:
-                LOG.exception(e)
-                return False
             has_gui = is_gui_running() or is_gui_connected(self.bus)
             if not has_gui or self.settings.get("force_audioservice", False) or \
                     self.settings.get("playback_mode") == PlaybackMode.FORCE_AUDIOSERVICE:
                 # No gui, so lets force playback to use audio only
+                LOG.debug("Casting to PlaybackType.AUDIO_SERVICE")
                 self.now_playing.playback = PlaybackType.AUDIO_SERVICE
+            if not self.now_playing.uri:
+                return False
             self.gui["stream"] = self.now_playing.uri
 
         self.gui.update_current_track()

requirements/requirements.txt

@@ -1,6 +1,6 @@
 ovos-plugin-manager >=0.0.26a5, < 0.1.0
 ovos-bus-client>=0.0.7, < 0.1.0
 ovos-utils>=0.0.38, < 1.0.0
-ovos-workshop>=0.0.16a39, < 0.1.0
+ovos-workshop>=0.0.16a48, < 0.1.0
 padacioso~=0.1, >=0.1.1
 dbus-next

test/unittests/test_audio_backends.py

@@ -51,26 +51,6 @@ def get_msg(msg):
 
         self.audio = AudioService(self.bus)
 
-    def test_native_ocp(self):
-        # assert that OCP is the selected default backend
-        self.assertTrue(isinstance(self.audio.default, OCPAudioBackend))
-
-        # assert that OCP is in "auto" mode
-        self.assertEqual(self.audio.default.config["mode"], "auto")
-
-        # assert that OCP is loaded
-        self.assertTrue(self.audio.default.ocp is not None)
-        self.assertTrue(isinstance(self.audio.default.ocp, OCP))
-
-        # assert that test backends also loaded
-        # NOTE: "service" is a list, should be named "services"
-        # not renamed for backwards compat but its a typo!
-        loaded_services = [s.name for s in self.audio.service]
-        self.assertIn("OCP", loaded_services)
-        # TODO fix me, add dummy plugins
-        #self.assertIn("mycroft_test", loaded_services)
-        #self.assertIn("ovos_test", loaded_services)
-
     def tearDown(self) -> None:
         self.audio.shutdown()
 

test/unittests/test_external_ocp.py

@@ -47,17 +47,6 @@ def get_msg(msg):
 
         cls.bus.on("message", get_msg)
 
-    @patch.object(Configuration, 'load_all_configs')
-    def test_external_ocp(self, mock):
-        mock.return_value = BASE_CONF
-        audio = AudioService(self.bus)
-        self.assertEqual(audio.config, BASE_CONF["Audio"])
-        # assert that ocp is in external mode
-        self.assertEqual(audio.default.config["mode"], "external")
-        # assert that OCP is not loaded
-        self.assertTrue(audio.default.ocp is None)
-        audio.shutdown()
-
 
 if __name__ == '__main__':
     unittest.main()

test/unittests/test_ocp_player.py

@@ -215,12 +215,10 @@ def test_set_now_playing(self):
         real_update_props = self.player.mpris.update_props
         real_update_track = self.player.gui.update_current_track
         real_update_plist = self.player.gui.update_playlist
-        real_nowplaying_reset = self.player.now_playing.reset
 
         self.player.mpris.update_props = Mock()
         self.player.gui.update_current_track = Mock()
         self.player.gui.update_playlist = Mock()
-        self.player.now_playing.reset = Mock()
 
         valid_dict = valid_search_results[0]
         valid_track = MediaEntry.from_dict(valid_search_results[1])
@@ -241,8 +239,6 @@ def test_set_now_playing(self):
         self.player.mpris.update_props.assert_called_once_with(
             {"Metadata": self.player.now_playing.mpris_metadata}
         )
-        self.player.now_playing.reset.assert_called_once()
-        self.player.now_playing.reset.reset_mock()
         self.player.gui.update_current_track.reset_mock()
         self.player.gui.update_playlist.reset_mock()
         self.player.mpris.update_props.reset_mock()
@@ -256,16 +252,13 @@ def test_set_now_playing(self):
         self.player.gui.update_playlist.assert_called_once()
         self.player.mpris.update_props.assert_called_once_with(
             {"Metadata": self.player.now_playing.mpris_metadata})
-        self.player.now_playing.reset.assert_called_once()
-        self.player.now_playing.reset.reset_mock()
         self.player.gui.update_current_track.reset_mock()
         self.player.gui.update_playlist.reset_mock()
         self.player.mpris.update_props.reset_mock()
 
         # Play invalid string result
         with self.assertRaises(ValueError):
             self.player.set_now_playing(invalid_str)
-        self.player.now_playing.reset.assert_not_called()
         self.player.gui.update_current_track.assert_not_called()
         self.player.gui.update_playlist.assert_not_called()
         self.player.mpris.update_props.assert_not_called()
@@ -276,12 +269,10 @@ def test_set_now_playing(self):
         self.player.gui.update_playlist.assert_called_once()
         self.player.mpris.update_props.assert_called_once_with(
             {"Metadata": self.player.now_playing.mpris_metadata})
-        self.player.now_playing.reset.assert_called_once()
 
         self.player.mpris.update_props = real_update_props
         self.player.gui.update_current_track = real_update_track
         self.player.gui.update_playlist = real_update_plist
-        self.player.now_playing.reset = real_nowplaying_reset
 
     @patch("ovos_plugin_common_play.ocp.player.is_gui_running")
     def test_validate_stream(self, gui_running):
@@ -423,7 +414,7 @@ def test_get_preferred_audio_backend(self):
         preferred = self.player._get_preferred_audio_backend()
         self.assertIsInstance(preferred, str)
         self.assertIn(preferred,
-                      ["ovos_common_play", "vlc", "mplayer", "simple"])
+                      ["mpv", "ovos_common_play", "vlc", "mplayer", "simple"])
 
     @patch("ovos_plugin_common_play.ocp.player.is_gui_running")
     def test_play(self, gui_running):