From b9c0a795487193721cd1b1aac45a5f4ce3530843 Mon Sep 17 00:00:00 2001 From: Shaowen Yin Date: Sun, 18 Aug 2024 14:28:29 +0800 Subject: [PATCH] player: implement async_METHOD for playlist (#866) --- feeluown/gui/components/song_tag.py | 2 +- feeluown/library/ytdl.py | 8 +- feeluown/player/playlist.py | 181 ++++++++++++++++------------ feeluown/task.py | 4 +- setup.cfg | 7 ++ tests/player/test_fm.py | 2 +- tests/player/test_playlist.py | 72 +++++------ 7 files changed, 150 insertions(+), 126 deletions(-) diff --git a/feeluown/gui/components/song_tag.py b/feeluown/gui/components/song_tag.py index ade527b93b..cceb70469e 100644 --- a/feeluown/gui/components/song_tag.py +++ b/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}” 没有找到可用的相似歌曲') diff --git a/feeluown/library/ytdl.py b/feeluown/library/ytdl.py index 4fb59308bb..a0fd799024 100644 --- a/feeluown/library/ytdl.py +++ b/feeluown/library/ytdl.py @@ -48,7 +48,7 @@ def match_rule(self, url, source=''): def select_audio(self, url, _: Optional[str] = None, source='') -> Optional[Media]: matched_rule = self.match_rule(url, source) if matched_rule is None: - return + return None http_proxy = matched_rule.get('http_proxy') ytdl_opts = {} ytdl_opts.update(self._default_audio_ytdl_opts) @@ -71,7 +71,7 @@ def select_audio(self, url, _: Optional[str] = None, source='') -> Optional[Medi def select_video(self, url, _: Optional[str] = None, source='') -> Optional[Media]: matched_rule = self.match_rule(url, source) if matched_rule is None: - return + return None http_proxy = matched_rule.get('http_proxy') ytdl_opts = {} ytdl_opts.update(self._default_video_ytdl_opts) @@ -91,7 +91,7 @@ def select_video(self, url, _: Optional[str] = None, source='') -> Optional[Medi ): video_candidates.append((f['url'], f['width'])) if not (audio_candidates and video_candidates): - return + return None audio_candidates = sorted( audio_candidates, key=lambda c: c[1] or 0, reverse=True ) @@ -123,5 +123,7 @@ def select_video(self, url, _: Optional[str] = None, source='') -> Optional[Medi # print() media = ytdl.select_video(url, None, 'ytmusic') + assert media is not None + assert isinstance(media.manifest, VideoAudioManifest) print(media.manifest.video_url) print(media.manifest.audio_url) diff --git a/feeluown/player/playlist.py b/feeluown/player/playlist.py index cfb7f341fb..f03e0ae53b 100644 --- a/feeluown/player/playlist.py +++ b/feeluown/player/playlist.py @@ -11,7 +11,8 @@ from feeluown.utils.dispatch import Signal from feeluown.utils.utils import DedupList from feeluown.library import ( - MediaNotFound, SongModel, ModelType, VideoModel, + MediaNotFound, SongModel, ModelType, VideoModel, ModelNotFound, + BriefSongModel, ) from feeluown.media import Media from .metadata_assembler import MetadataAssembler @@ -21,6 +22,10 @@ logger = logging.getLogger(__name__) +TASK_SET_CURRENT_MODEL = 'playlist.set_current_model' +TASK_PLAY_MODEL = 'playlist.play_model' +TASK_PREPARE_MEDIA = 'playlist.prepare_media' + class PlaybackMode(IntEnum): """ @@ -133,8 +138,6 @@ def __init__(self, app: 'App', songs=None, playback_mode=PlaybackMode.loop, #: When watch mode is on, playlist try to play the mv/video of the song self.watch_mode = False - self._t_scm = self._app.task_mgr.get_or_create('set-current-model') - # .. versionadded:: 3.7.11 # The *songs_removed* and *songs_added* signal. self.songs_removed = Signal() # (index, count) @@ -310,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: @@ -473,39 +476,27 @@ def current_song(self, song: Optional[SongModel]): def current_song_mv(self) -> Optional[VideoModel]: return self._current_song_mv - def set_current_song(self, song) -> Optional[asyncio.Task]: - """设置当前歌曲，将歌曲加入到播放列表，并发出 song_changed 信号 - - .. note:: - - 该方法理论上只应该被 Player 对象调用。 - - if song has not valid media, we find a replacement in other providers + async def a_set_current_song(self, song): + """Set the `song` as the current song. - .. versionadded:: 3.7.11 - The method is added to replace current_song.setter. + 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: self.mode = PlaylistMode.normal - # FIXME(cosven): `current_song.setter` depends on app.task_mgr and app.library, - # which make it hard to test. - return self._t_scm.bind_coro(self.a_set_current_song(song)) - - async def a_set_current_song(self, song): - """Set the `song` as the current song. - - If the song is bad, then this will try to use a standby in Playlist.normal mode. - """ target_song = song # The song to be set. 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) @@ -549,7 +540,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? @@ -586,43 +577,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: @@ -637,25 +627,17 @@ async def _prepare_mv_media(self, song) -> Optional[Media]: logger.exception(f'fail to get {song} mv: {e}') return mv_media - def set_current_model(self, model): - """ - .. 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._t_scm.bind_coro(self.a_set_current_model(model)) - async def a_set_current_model(self, model): """ TODO: handle when model is a song .. 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: @@ -673,28 +655,67 @@ async def a_set_current_model(self, model): kwargs['video'] = False self._app.player.play(media, metadata=metadata, **kwargs) + async def a_play_model(self, model): + """ + .. versionadded: 4.1.7 + """ + # Stop the player so that user know the action is working. + self._app.player.stop() + if model is None: + 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: + 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 .. versionadded: 3.7.14 """ - # Stop the player so that user know the action is working. - self._app.player.stop() - self.play_model_handling.emit() - task = self.set_current_model(model) - if task is not None: - def cb(future): - try: - future.result() - except: # noqa - logger.exception('play model failed') - else: - self._app.player.resume() - logger.info(f'play a model ({model}) succeed') - task.add_done_callback(cb) + self._app.task_mgr.run_afn_preemptive( + self.a_play_model, model, name=TASK_PLAY_MODEL + ) + + def set_current_model(self, model) -> asyncio.Task: + """ + .. versionadded: 3.7.13 + """ + return self._app.task_mgr.run_afn_preemptive( + self.a_set_current_model, model, name=TASK_SET_CURRENT_MODEL, + ) + + def set_current_song(self, song): + """ + .. versionadded:: 3.7.11 + The method is added to replace current_song.setter. + """ + return self._app.task_mgr.run_afn_preemptive( + self.a_set_current_song, song, name=TASK_SET_CURRENT_MODEL + ) diff --git a/feeluown/task.py b/feeluown/task.py index 5b20b5d7b4..eb44780d00 100644 --- a/feeluown/task.py +++ b/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 diff --git a/setup.cfg b/setup.cfg index faa5729ea8..00bb23f1ee 100644 --- a/setup.cfg +++ b/setup.cfg @@ -11,6 +11,9 @@ ignore = E402 max-line-length = 89 exclude = feeluown/mpv.py +[mypy] +disable_error_code = import-untyped + [yapf] column_limit = 89 based_on_style = pep8 @@ -39,3 +42,7 @@ addopts = -q --doctest-modules [mypy-feeluown.mpv] ignore_errors = True + +# I don't know how to type hint mixin class which is related to PyQt5. +[mypy-feeluown.gui.helpers] +ignore_errors = True diff --git a/tests/player/test_fm.py b/tests/player/test_fm.py index 90a3c23c9a..f5363ce816 100644 --- a/tests/player/test_fm.py +++ b/tests/player/test_fm.py @@ -91,7 +91,7 @@ def is_active(fm): return fm.is_active await asyncio.sleep(0.1) # wait for fm-fetch-song task finished # user trigger play next - app_mock.playlist.current_song = song + await app_mock.playlist.a_set_current_song(song) assert playlist.mode is PlaylistMode.normal assert is_active(fm) is False diff --git a/tests/player/test_playlist.py b/tests/player/test_playlist.py index 58d069b3ae..29f1cd03f0 100644 --- a/tests/player/test_playlist.py +++ b/tests/player/test_playlist.py @@ -80,22 +80,23 @@ 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 @pytest.mark.asyncio -async def test_play_model(pl, song, mocker): - mock_set = mocker.patch.object(pl, 'set_current_model') +async def test_play_model(pl, app_mock, song, mocker): f = asyncio.Future() f.set_result(None) - mock_set.return_value = f - pl.play_model(song) - await asyncio.sleep(0.1) + mocker.patch.object(pl, 'set_current_model', return_value=f) + app_mock.task_mgr.run_afn_preemptive.return_value = f + await pl.a_play_model(song) # The player.resume method must be called. assert pl._app.player.resume.called @@ -112,13 +113,14 @@ 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 @@ -126,25 +128,27 @@ 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) @@ -153,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 @@ -188,6 +180,11 @@ def mock_a_set_cursong(mocker): mocker.patch.object(Playlist, 'a_set_current_song', new=mock.MagicMock) +@pytest.fixture +def mock_prepare_metadata(mocker): + mocker.patch.object(MetadataAssembler, 'prepare_for_song', return_value=object()) + + @pytest.mark.asyncio async def test_playlist_change_mode(app_mock, mocker): # from normal to fm @@ -215,30 +212,26 @@ async def test_playlist_change_repeat_shuffle_mode(app_mock): @pytest.mark.asyncio -async def test_playlist_exit_fm_mode(app_mock, song, mocker, - mock_a_set_cursong): +async def test_playlist_exit_fm_mode(app_mock, song, mock_prepare_metadata): pl = Playlist(app_mock) pl.mode = PlaylistMode.fm - pl.current_song = song + 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 -async def test_playlist_fm_mode_play_next(app_mock, song, song1, - mock_a_set_cursong): +async def test_playlist_fm_mode_play_next(app_mock, song, song1, mock_prepare_metadata): pl = Playlist(app_mock) pl.mode = PlaylistMode.fm pl.fm_add(song1) pl.fm_add(song) pl._current_song = song1 - pl.current_song = song # should not exit fm mode + await pl.a_set_current_song(song) # should not exit fm mode assert pl.mode is PlaylistMode.fm @pytest.mark.asyncio -async def test_playlist_fm_mode_play_previous(app_mock, song, song1, - mock_a_set_cursong): +async def test_playlist_fm_mode_play_previous(app_mock, song, song1): pl = Playlist(app_mock) pl.mode = PlaylistMode.fm pl.fm_add(song1) @@ -295,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', @@ -311,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