From 1e9745b5127ebb935b0ab672be321d374fcb97b7 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Tue, 14 May 2024 20:48:50 +0200 Subject: [PATCH] A collection of small bugfixes and tweaks (#1305) * Fix paged listings total * Precache album tracks * Speedup playlist track listings * Fix random sort for builtin playlists * Fix local images on Jellfyin they are considered as not remotely accessible * catch parse errors in jellyfin * Fix typo * Fix browse feature --- music_assistant/common/models/media_items.py | 5 ++-- .../server/controllers/media/albums.py | 7 +++++ .../server/controllers/media/artists.py | 2 +- .../server/controllers/media/base.py | 1 + music_assistant/server/controllers/music.py | 8 +++-- .../server/models/music_provider.py | 4 +-- .../server/providers/builtin/__init__.py | 8 ++--- .../server/providers/jellyfin/__init__.py | 29 +++++++++---------- .../server/providers/radiobrowser/__init__.py | 2 +- 9 files changed, 37 insertions(+), 29 deletions(-) diff --git a/music_assistant/common/models/media_items.py b/music_assistant/common/models/media_items.py index 366eab78..4fa8dd0a 100644 --- a/music_assistant/common/models/media_items.py +++ b/music_assistant/common/models/media_items.py @@ -577,9 +577,8 @@ class PagedItems(Generic[_T]): self.limit = limit self.offset = offset self.total = total - if total is None and offset == 0 and count > limit: - self.total = count - if total is None and offset and count < limit: + if total is None and count != limit: + # total is important so always calculate it from count if omitted self.total = offset + count def to_dict(self, *args, **kwargs) -> dict[str, Any]: diff --git a/music_assistant/server/controllers/media/albums.py b/music_assistant/server/controllers/media/albums.py index 566fa7c2..21e6d455 100644 --- a/music_assistant/server/controllers/media/albums.py +++ b/music_assistant/server/controllers/media/albums.py @@ -302,6 +302,13 @@ class AlbumsController(MediaControllerBase[Album]): # store (serializable items) in cache if prov.is_streaming_provider: self.mass.create_task(self.mass.cache.set(cache_key, [x.to_dict() for x in items])) + for item in items: + # if this is a complete track object, pre-cache it as + # that will save us an (expensive) lookup later + if item.image and item.artist_str and item.album and prov.domain != "builtin": + await self.mass.cache.set( + f"provider_item.track.{prov.lookup_key}.{item_id}", item.to_dict() + ) return items async def _get_provider_dynamic_tracks( diff --git a/music_assistant/server/controllers/media/artists.py b/music_assistant/server/controllers/media/artists.py index 20ae7a08..a0396182 100644 --- a/music_assistant/server/controllers/media/artists.py +++ b/music_assistant/server/controllers/media/artists.py @@ -326,7 +326,7 @@ class ArtistsController(MediaControllerBase[Artist]): if isinstance(update, ItemMapping): # NOTE that artist is the only mediatype where its accepted we # receive an itemmapping from streaming providers - update = Artist.from_item_mapping(update) + update = self._artist_from_item_mapping(update) metadata = cur_item.metadata else: metadata = update.metadata if overwrite else cur_item.metadata.update(update.metadata) diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index c2920b76..c9ce75cf 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -62,6 +62,7 @@ SORT_KEYS = { "position": "position ASC", "position_desc": "position DESC", "random": "RANDOM()", + "random_play_count": "RANDOM(), play_count", } diff --git a/music_assistant/server/controllers/music.py b/music_assistant/server/controllers/music.py index d6b455de..b2c9ba46 100644 --- a/music_assistant/server/controllers/music.py +++ b/music_assistant/server/controllers/music.py @@ -348,12 +348,13 @@ class MusicController(CoreController): ) if not prov: return PagedItems(items=prepend_items, limit=limit, offset=offset) - else: + elif offset == 0: back_path = f"{provider_instance}://" + "/".join(sub_path.split("/")[:-1]) prepend_items.append( BrowseFolder(item_id="back", provider=provider_instance, path=back_path, name="..") ) - prov_items = await prov.browse(path, offset, limit) + # limit -1 to account for the prepended items + prov_items = await prov.browse(path, offset=offset, limit=limit) prov_items.items = prepend_items + prov_items.items return prov_items @@ -376,7 +377,8 @@ class MusicController(CoreController): continue with suppress(MediaNotFoundError, ProviderUnavailableError): media_type = MediaType(db_row["media_type"]) - item = await self.get_item(media_type, db_row["item_id"], db_row["provider"]) + ctrl = self.get_controller(media_type) + item = await ctrl.get_provider_item(db_row["item_id"], db_row["provider"]) result.append(item) return result diff --git a/music_assistant/server/models/music_provider.py b/music_assistant/server/models/music_provider.py index e99ec7b0..ede8f9d7 100644 --- a/music_assistant/server/models/music_provider.py +++ b/music_assistant/server/models/music_provider.py @@ -292,7 +292,7 @@ class MusicProvider(Provider): # we may NOT use the default implementation if the provider does not support browse raise NotImplementedError items: list[MediaItemType] = [] - index = 0 + index = -1 subpath = path.split("://", 1)[1] # this reference implementation can be overridden with a provider specific approach generator: AsyncGenerator[MediaItemType, None] | None = None @@ -314,10 +314,10 @@ class MusicProvider(Provider): if generator: # grab items from library generator async for item in generator: + index += 1 if index < offset: continue items.append(item) - index += 1 if len(items) >= limit: break else: diff --git a/music_assistant/server/providers/builtin/__init__.py b/music_assistant/server/providers/builtin/__init__.py index 3735afdb..13f719dd 100644 --- a/music_assistant/server/providers/builtin/__init__.py +++ b/music_assistant/server/providers/builtin/__init__.py @@ -509,7 +509,7 @@ class BuiltinProvider(MusicProvider): result: list[Track] = [] if builtin_playlist_id == ALL_FAVORITE_TRACKS: res = await self.mass.music.tracks.library_items( - favorite=True, limit=2500, order_by="RANDOM(), play_count" + favorite=True, limit=2500, order_by="random_play_count" ) for idx, item in enumerate(res.items, 1): item.position = idx @@ -517,7 +517,7 @@ class BuiltinProvider(MusicProvider): return result if builtin_playlist_id == RANDOM_TRACKS: res = await self.mass.music.tracks.library_items( - limit=500, order_by="RANDOM(), play_count" + limit=500, order_by="random_play_count" ) for idx, item in enumerate(res.items, 1): item.position = idx @@ -525,7 +525,7 @@ class BuiltinProvider(MusicProvider): return result if builtin_playlist_id == RANDOM_ALBUM: for random_album in ( - await self.mass.music.albums.library_items(limit=1, order_by="RANDOM()") + await self.mass.music.albums.library_items(limit=1, order_by="random") ).items: # use the function specified in the queue controller as that # already handles unwrapping an album by user preference @@ -538,7 +538,7 @@ class BuiltinProvider(MusicProvider): return result if builtin_playlist_id == RANDOM_ARTIST: for random_artist in ( - await self.mass.music.artists.library_items(limit=1, order_by="RANDOM()") + await self.mass.music.artists.library_items(limit=1, order_by="random") ).items: # use the function specified in the queue controller as that # already handles unwrapping an artist by user preference diff --git a/music_assistant/server/providers/jellyfin/__init__.py b/music_assistant/server/providers/jellyfin/__init__.py index 3e66ec88..ad6932bb 100644 --- a/music_assistant/server/providers/jellyfin/__init__.py +++ b/music_assistant/server/providers/jellyfin/__init__.py @@ -310,7 +310,7 @@ class JellyfinProvider(MusicProvider): type=ImageType.THUMB, path=thumb, provider=self.instance_id, - remotely_accessible=True, + remotely_accessible=False, ) ] if ITEM_KEY_OVERVIEW in current_jellyfin_album: @@ -374,7 +374,7 @@ class JellyfinProvider(MusicProvider): type=ImageType.THUMB, path=thumb, provider=self.instance_id, - remotely_accessible=True, + remotely_accessible=False, ) ] return artist @@ -421,7 +421,7 @@ class JellyfinProvider(MusicProvider): type=ImageType.THUMB, path=thumb, provider=self.instance_id, - remotely_accessible=True, + remotely_accessible=False, ) ] if len(current_jellyfin_track[ITEM_KEY_ARTIST_ITEMS]) >= 1: @@ -452,13 +452,6 @@ class JellyfinProvider(MusicProvider): parent_album[ITEM_KEY_ALBUM_ARTIST], ) ) - track.artists.append( - self._get_item_mapping( - MediaType.ARTIST, - parent_album[ITEM_KEY_PARENT_ID], - parent_album[ITEM_KEY_ALBUM_ARTIST], - ) - ) else: track.artists.append(await self._parse_artist(name=VARIOUS_ARTISTS_NAME)) if ( @@ -513,7 +506,7 @@ class JellyfinProvider(MusicProvider): type=ImageType.THUMB, path=thumb, provider=self.instance_id, - remotely_accessible=True, + remotely_accessible=False, ) ] playlist.is_editable = False @@ -681,6 +674,7 @@ class JellyfinProvider(MusicProvider): ) -> list[Track]: """Get playlist tracks.""" result: list[Track] = [] + # TODO: Does Jellyfin support paging here? jellyfin_playlist = API.get_item(self._jellyfin_server.jellyfin, prov_playlist_id) playlist_items = await self._get_children( self._jellyfin_server, jellyfin_playlist[ITEM_KEY_ID], ITEM_TYPE_AUDIO @@ -688,10 +682,15 @@ class JellyfinProvider(MusicProvider): if not playlist_items: return result for index, jellyfin_track in enumerate(playlist_items): - if track := await self._parse_track(jellyfin_track): - if not track.position: - track.position = index - result.append(track) + try: + if track := await self._parse_track(jellyfin_track): + if not track.position: + track.position = index + result.append(track) + except (KeyError, ValueError) as err: + self.logger.error( + "Skipping track %s: %s", jellyfin_track.get(ITEM_KEY_NAME, index), str(err) + ) return result async def get_artist_albums(self, prov_artist_id) -> list[Album]: diff --git a/music_assistant/server/providers/radiobrowser/__init__.py b/music_assistant/server/providers/radiobrowser/__init__.py index 711b3313..a4f5d9da 100644 --- a/music_assistant/server/providers/radiobrowser/__init__.py +++ b/music_assistant/server/providers/radiobrowser/__init__.py @@ -102,7 +102,7 @@ class RadioBrowserProvider(MusicProvider): return result - async def browse(self, path: str, limit: int, offset: int) -> PagedItems[MediaItemType]: + async def browse(self, path: str, offset: int, limit: int) -> PagedItems[MediaItemType]: """Browse this provider's items. :param path: The path to browse, (e.g. provid://artists). -- 2.34.1