refine playlist.py code

feeluown/gui/components/song_tag.py

@@ -93,7 +93,7 @@ async def _switch_provider(self, provider_id):
             standby, media = songs[0]
             assert standby != song
             self._app.show_msg(f'使用 {standby} 替换当前歌曲')
-            self._app.playlist.pure_set_current_song(standby, media)
+            self._app.playlist.set_current_song_with_media(standby, media)
             self._app.playlist.remove(song)
         else:
             self._app.show_msg(f'提供方 “{provider_id}” 没有找到可用的相似歌曲')

feeluown/player/playlist.py

@@ -12,6 +12,7 @@
 from feeluown.utils.utils import DedupList
 from feeluown.library import (
     MediaNotFound, SongModel, ModelType, VideoModel, ModelNotFound,
+    BriefSongModel,
 )
 from feeluown.media import Media
 from .metadata_assembler import MetadataAssembler
@@ -23,6 +24,7 @@
 
 TASK_SET_CURRENT_MODEL = 'playlist.set_current_model'
 TASK_PLAY_MODEL = 'playlist.play_model'
+TASK_PREPARE_MEDIA = 'playlist.prepare_media'
 
 
 class PlaybackMode(IntEnum):
@@ -311,7 +313,7 @@ def init_from(self, songs):
     def clear(self):
         """remove all songs from playlists"""
         if self.current_song is not None:
-            self.current_song = None
+            self.set_current_song_none()
         length = len(self._songs)
         self._songs.clear()
         if length > 0:
@@ -476,7 +478,7 @@ async def a_set_current_song(self, song):
         If the song is bad, then this will try to use a standby in Playlist.normal mode.
         """
         if song is None:
-            self.pure_set_current_song(None, None, None)
+            self.set_current_song_none()
             return None
 
         if self.mode is PlaylistMode.fm and song not in self._songs:
@@ -486,7 +488,11 @@ async def a_set_current_song(self, song):
         media = None        # The corresponding media to be set.
         try:
             self.play_model_stage_changed.emit(PlaylistPlayModelStage.prepare_media)
-            media = await self._prepare_media(song)
+            media = await self._app.task_mgr.run_afn_preemptive(
+                self._prepare_media,
+                song,
+                name=TASK_PREPARE_MEDIA,
+            )
         except MediaNotFound as e:
             if e.reason is MediaNotFound.Reason.check_children:
                 await self.a_set_current_song_children(song)
@@ -530,7 +536,7 @@ async def a_set_current_song(self, song):
             self.play_model_stage_changed.emit(PlaylistPlayModelStage.prepare_metadata)
             metadata = await self._metadata_mgr.prepare_for_song(target_song)
         self.play_model_stage_changed.emit(PlaylistPlayModelStage.load_media)
-        self.pure_set_current_song(target_song, media, metadata)
+        self.set_current_song_with_media(target_song, media, metadata)
 
     async def a_set_current_song_children(self, song):
         # TODO: maybe we can just add children to playlist?
@@ -567,43 +573,42 @@ async def find_and_use_standby(self, song):
         self._app.show_msg(f'未找到 {song} 的备用歌曲')
         return song, None
 
-    def pure_set_current_song(self, song, media, metadata=None):
+    def set_current_song_with_media(self, song, media, metadata=None):
         if song is None:
-            self._current_song = None
-        else:
-            # add it to playlist if song not in playlist
-            if song in self._songs:
-                self._current_song = song
-            else:
-                self.insert(song)
-                self._current_song = song
+            self.set_current_song_none()
+            return
+        # Add it to playlist if song not in playlist.
+        if song not in self._songs:
+            self.insert(song)
+        self._current_song = song
         self.song_changed.emit(song)
         self.song_changed_v2.emit(song, media)
-
-        if song is not None:
-            if media is None:
-                self._app.show_msg("没找到可用的播放链接，播放下一首...")
-                run_afn(self.a_next)
-            else:
-                # Note that the value of model v1 {}_display may be None.
-                kwargs = {}
-                if not self._app.has_gui:
-                    kwargs['video'] = False
-                # TODO: set artwork field
-                self._app.player.play(media, metadata=metadata, **kwargs)
+        if media is None:
+            self._app.show_msg("没找到可用的播放链接，播放下一首...")
+            run_afn(self.a_next)
         else:
-            self._app.player.stop()
+            kwargs = {}
+            if not self._app.has_gui:
+                kwargs['video'] = False
+            # TODO: set artwork field
+            self._app.player.play(media, metadata=metadata, **kwargs)
+
+    def set_current_song_none(self):
+        """A special case of `set_current_song_with_media`."""
+        self._current_song = None
+        self.song_changed.emit(None)
+        self.song_changed_v2.emit(None, None)
+        self._app.player.stop()
 
     async def _prepare_media(self, song):
-        task_spec = self._app.task_mgr.get_or_create('prepare-media')
-        # task_spec.disable_default_cb()
         if self.watch_mode is True:
-            mv_media = await task_spec.bind_coro(self._prepare_mv_media(song))
+            mv_media = await self._prepare_mv_media(song)
             if mv_media:
                 return mv_media
             self._app.show_msg('未找到可用的歌曲视频资源')
-        return await task_spec.bind_blocking_io(
-            self._app.library.song_prepare_media, song, self.audio_select_policy)
+        return await aio.run_fn(
+            self._app.library.song_prepare_media, song, self.audio_select_policy,
+        )
 
     async def _prepare_mv_media(self, song) -> Optional[Media]:
         try:
@@ -624,8 +629,11 @@ async def a_set_current_model(self, model):
 
         .. versionadded: 3.7.13
         """
-        assert ModelType(model.meta.model_type) is ModelType.video, \
-            "{model.meta.model_type} is not supported, expecting a video model, "
+        if model is None:
+            self._app.player.stop()
+            return
+        if isinstance(model, BriefSongModel):
+            return await self.a_set_current_song(model)
 
         video = model
         try:
@@ -649,38 +657,38 @@ async def a_play_model(self, model):
         """
         # Stop the player so that user know the action is working.
         self._app.player.stop()
-        self.play_model_handling.emit()
         if model is None:
-            self._app.player.stop()
+            return
+        self.play_model_handling.emit()
+        if ModelType(model.meta.model_type) is ModelType.song:
+            fn = self.a_set_current_song
+            upgrade_fn = self._app.library.song_upgrade
         else:
-            if ModelType(model.meta.model_type) is ModelType.song:
-                fn = self.a_set_current_song
-                upgrade_fn = self._app.library.song_upgrade
-            else:
-                fn = self.a_set_current_model
-                upgrade_fn = self._app.library.video_upgrade
-            try:
-                # Try to upgrade the model.
-                model = await aio.run_fn(upgrade_fn, model)
-            except ModelNotFound:
-                pass
-            except:  # noqa
-                logger.exception(f'upgrade model:{model} failed')
-            try:
-                await self._app.task_mgr.run_afn_preemptive(
-                    fn, model, name=TASK_SET_CURRENT_MODEL
-                )
-            except:  # noqa
-                logger.exception('play model failed')
-            else:
-                self._app.player.resume()
-                logger.info(f'play a model ({model}) succeed')
+            fn = self.a_set_current_model
+            upgrade_fn = self._app.library.video_upgrade
+        try:
+            # Try to upgrade the model.
+            model = await aio.run_fn(upgrade_fn, model)
+        except ModelNotFound:
+            pass
+        except:  # noqa
+            logger.exception(f'upgrade model:{model} failed')
+        try:
+            await self._app.task_mgr.run_afn_preemptive(
+                fn, model, name=TASK_SET_CURRENT_MODEL
+            )
+        except:  # noqa
+            logger.exception('play model failed')
+        else:
+            self._app.player.resume()
+            logger.info(f'play a model ({model}) succeed')
 
     """
     Sync methods.
 
     Currently, playlist has both async and sync methods to keep backward
     compatibility. Sync methods will be replaced by async methods in the end.
+    Sync methods just wrap the async method.
     """
     def play_model(self, model):
         """Set current model and play it
@@ -691,15 +699,10 @@ def play_model(self, model):
             self.a_play_model, model, name=TASK_PLAY_MODEL
         )
 
-    def set_current_model(self, model):
+    def set_current_model(self, model) -> asyncio.Task:
         """
         .. versionadded: 3.7.13
         """
-        if model is None:
-            self._app.player.stop()
-            return
-        if ModelType(model.meta.model_type) is ModelType.song:
-            return self.set_current_song(model)
         return self._app.task_mgr.run_afn_preemptive(
             self.a_set_current_model, model, name=TASK_SET_CURRENT_MODEL,
         )
@@ -709,14 +712,10 @@ def current_song(self, song: Optional[SongModel]):
         self.set_current_song(song)
 
     def set_current_song(self, song):
-        """设置当前歌曲，将歌曲加入到播放列表，并发出 song_changed 信号
-
+        """
         .. versionadded:: 3.7.11
            The method is added to replace current_song.setter.
         """
-        if song is None:
-            self.pure_set_current_song(None, None, None)
-            return None
         return self._app.task_mgr.run_afn_preemptive(
             self.a_set_current_song, song, name=TASK_SET_CURRENT_MODEL
         )

feeluown/task.py

@@ -44,7 +44,7 @@ def _before_bind(self):
             self._mgr.loop.call_soon_threadsafe(self._task.cancel)
         self._task = None
 
-    def bind_coro(self, coro):
+    def bind_coro(self, coro) -> asyncio.Task:
         """run the coroutine and bind the task
 
         it will cancel the previous task if exists
@@ -60,7 +60,7 @@ def bind_coro(self, coro):
             self._task.add_done_callback(self._cb)
         return self._task
 
-    def bind_blocking_io(self, func, *args):
+    def bind_blocking_io(self, func, *args) -> asyncio.Task:
         """run blocking io func in a thread executor, and bind the task
 
         it will cancel the previous task if exists

tests/player/test_playlist.py

@@ -80,10 +80,12 @@ def test_remove_song(mocker, pl, song, song1, song2):
         assert len(pl) == 1
 
 
-def test_set_current_song(pl, song2):
-    # Set a nonexisting song as current song
-    # The song should be inserted after current_song
-    pl.pure_set_current_song(song2, None)
+def test_set_current_song_with_media(pl, song2):
+    """
+    Set a non-existing song as current song,
+    and the song should be inserted after current_song.
+    """
+    pl.set_current_song_with_media(song2, None)
     assert pl.current_song == song2
     assert pl.list()[1] == song2
 
@@ -111,39 +113,42 @@ async def test_set_current_song_with_bad_song_1(
         mocker, song2, pl,
         pl_prepare_media_none,
         pl_list_standby_return_empty):
-    mock_pure_set_current_song = mocker.patch.object(Playlist, 'pure_set_current_song')
+    mock_set_current_song_with_media = mocker.patch.object(
+        Playlist, 'set_current_song_with_media')
     mock_mark_as_bad = mocker.patch.object(Playlist, 'mark_as_bad')
     await pl.a_set_current_song(song2)
     # A song that has no valid media should be marked as bad
     assert mock_mark_as_bad.called
     # Since there is no standby song, the media should be None
-    mock_pure_set_current_song.assert_called_once_with(song2, None, None)
+    mock_set_current_song_with_media.assert_called_once_with(song2, None, None)
 
 
 @pytest.mark.asyncio
 async def test_set_current_song_with_bad_song_2(
         mocker, song2, pl,
         pl_prepare_media_none,
         pl_list_standby_return_song2):
-    mock_pure_set_current_song = mocker.patch.object(Playlist, 'pure_set_current_song')
+    mock_set_current_song_with_media = mocker.patch.object(
+        Playlist, 'set_current_song_with_media')
     mock_mark_as_bad = mocker.patch.object(Playlist, 'mark_as_bad')
     sentinal = object()
     mocker.patch.object(MetadataAssembler, 'prepare_for_song', return_value=sentinal)
     await pl.a_set_current_song(song2)
     # A song that has no valid media should be marked as bad
     assert mock_mark_as_bad.called
-    mock_pure_set_current_song.assert_called_once_with(song2, SONG2_URL, sentinal)
+    mock_set_current_song_with_media.assert_called_once_with(song2, SONG2_URL, sentinal)
 
 
 @pytest.mark.asyncio
 async def test_set_current_song_with_bad_song_3(
         mocker, song2, app_mock,
-        pl_prepare_media_none,):
+        pl_prepare_media_none, ):
     """song has mv and the mv has valid media, should use mv's media as the media"""
     media = object()
     metadata = object()
 
-    mock_pure_set_current_song = mocker.patch.object(Playlist, 'pure_set_current_song')
+    mock_set_current_song_with_media = mocker.patch.object(
+        Playlist, 'set_current_song_with_media')
     mock_prepare_mv_media = mocker.patch.object(Playlist, '_prepare_mv_media',
                                                 return_value=media)
     mocker.patch.object(MetadataAssembler, 'prepare_for_song', return_value=metadata)
@@ -152,19 +157,7 @@ async def test_set_current_song_with_bad_song_3(
     pl = Playlist(app_mock)
     await pl.a_set_current_song(song2)
     mock_prepare_mv_media.assert_called_once_with(song2)
-    mock_pure_set_current_song.assert_called_once_with(song2, media, metadata)
-
-
-def test_pure_set_current_song(
-        mocker, song, song2, pl):
-    # Current song index is 0
-    assert pl.list().index(song) == 0
-    # song2 is not in playlist before
-    pl.pure_set_current_song(song2, SONG2_URL)
-    assert pl.current_song == song2
-    # The song should be inserted after the current song,
-    # so the index should be 1
-    assert pl.list().index(song2) == 1
+    mock_set_current_song_with_media.assert_called_once_with(song2, media, metadata)
 
 
 @pytest.mark.asyncio
@@ -224,7 +217,6 @@ async def test_playlist_exit_fm_mode(app_mock, song, mock_prepare_metadata):
     pl.mode = PlaylistMode.fm
     await pl.a_set_current_song(song)
     assert pl.mode is PlaylistMode.normal
-    assert app_mock.task_mgr.get_or_create.called
 
 
 @pytest.mark.asyncio
@@ -296,7 +288,8 @@ async def test_play_next_bad_song(app_mock, song, song1, mocker):
     Prepare media for song raises unknown error, the song should
     be marked as bad. Besides, it should try to find standby.
     """
-    mock_pure_set_current_song = mocker.patch.object(Playlist, 'pure_set_current_song')
+    mock_set_current_song_with_media = mocker.patch.object(
+        Playlist, 'set_current_song_with_media')
     mocker.patch.object(MetadataAssembler, 'prepare_for_song', return_value=object())
     mock_standby = mocker.patch.object(Playlist,
                                        'find_and_use_standby',
@@ -312,7 +305,7 @@ async def test_play_next_bad_song(app_mock, song, song1, mocker):
     await pl.a_set_current_song(pl.next_song)
     assert mock_mark_as_bad.called
     await asyncio.sleep(0.1)
-    assert mock_pure_set_current_song.called
+    assert mock_set_current_song_with_media.called
     assert mock_standby.called