From 1683ba59bb55e001fad8d989be52580502e32479 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Mon, 13 May 2024 14:23:45 +0200 Subject: [PATCH] Limit queries to all online (metadata) resources (#1295) --- .../server/controllers/media/base.py | 5 +- .../server/controllers/metadata.py | 55 +++--------------- music_assistant/server/controllers/music.py | 3 - .../server/providers/fanarttv/__init__.py | 4 +- .../server/providers/filesystem_local/base.py | 57 +++++-------------- .../server/providers/musicbrainz/__init__.py | 2 +- .../server/providers/qobuz/__init__.py | 2 +- .../server/providers/spotify/__init__.py | 2 +- .../server/providers/theaudiodb/__init__.py | 4 +- .../server/providers/tidal/__init__.py | 2 +- .../server/providers/tunein/__init__.py | 2 +- 11 files changed, 32 insertions(+), 106 deletions(-) diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index b5f5cc56..c2920b76 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -252,9 +252,8 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): ) if library_item and (time() - (library_item.metadata.last_refresh or 0)) > REFRESH_INTERVAL: # it's been too long since the full metadata was last retrieved (or never at all) - if library_item.available: - # do not attempts metadata refresh on unavailable items as it has side effects - metadata_lookup = True + # NOTE: do not attempt metadata refresh on unavailable items as it has side effects + metadata_lookup = library_item.available if library_item and not (force_refresh or metadata_lookup or add_to_library): # we have a library item and no refreshing is needed, return the results! diff --git a/music_assistant/server/controllers/metadata.py b/music_assistant/server/controllers/metadata.py index 6b16ac41..193db627 100644 --- a/music_assistant/server/controllers/metadata.py +++ b/music_assistant/server/controllers/metadata.py @@ -28,7 +28,7 @@ from music_assistant.common.models.enums import ( ProviderFeature, ProviderType, ) -from music_assistant.common.models.errors import MediaNotFoundError, ProviderUnavailableError +from music_assistant.common.models.errors import ProviderUnavailableError from music_assistant.common.models.media_items import ( Album, Artist, @@ -102,7 +102,6 @@ class MetaDataController(CoreController): super().__init__(*args, **kwargs) self.cache = self.mass.cache self._pref_lang: str | None = None - self.scan_busy: bool = False self.manifest.name = "Metadata controller" self.manifest.description = ( "Music Assistant's core controller which handles all metadata for music." @@ -198,34 +197,6 @@ class MetaDataController(CoreController): # if we reach this point, we couldn't match the language self.logger.warning("%s is not a valid language", lang) - def start_scan(self) -> None: - """Start background scan for missing metadata.""" - - async def scan_artist_metadata() -> None: - """Background task that scans for artists missing metadata on filesystem providers.""" - if self.scan_busy: - return - - self.logger.debug("Start scan for missing artist metadata") - self.scan_busy = True - async for artist in self.mass.music.artists.iter_library_items(): - if artist.metadata.last_refresh is not None: - continue - # most important is to see artist thumb in listings - # so if that is already present, move on - # full details can be grabbed later - if artist.image: - continue - # simply grabbing the full artist will trigger a full fetch - with suppress(MediaNotFoundError): - await self.mass.music.artists.get(artist.item_id, artist.provider, lazy=False) - # this is slow on purpose to not cause stress on the metadata providers - await asyncio.sleep(30) - self.scan_busy = False - self.logger.debug("Finished scan for missing artist metadata") - - self.mass.create_task(scan_artist_metadata) - async def get_artist_metadata(self, artist: Artist) -> None: """Get/update rich metadata for an artist.""" if not artist.mbid: @@ -368,24 +339,12 @@ class MetaDataController(CoreController): """Fetch musicbrainz id by performing search using the artist name, albums and tracks.""" if compare_strings(artist.name, VARIOUS_ARTISTS_NAME): return VARIOUS_ARTISTS_ID_MBID - ref_albums = await self.mass.music.artists.albums(artist.item_id, artist.provider) - if len(ref_albums) < 10: - # fetch reference albums from provider(s) attached to the artist - for provider_mapping in artist.provider_mappings: - if provider_mapping.provider_instance == artist.provider: - continue - ref_albums += await self.mass.music.artists.albums( - provider_mapping.item_id, provider_mapping.provider_instance - ) - ref_tracks = await self.mass.music.artists.tracks(artist.item_id, artist.provider) - if len(ref_tracks) < 10: - # fetch reference tracks from provider(s) attached to the artist - for provider_mapping in artist.provider_mappings: - if provider_mapping.provider_instance == artist.provider: - continue - ref_tracks += await self.mass.music.artists.tracks( - provider_mapping.item_id, provider_mapping.provider_instance - ) + ref_albums = await self.mass.music.artists.albums( + artist.item_id, artist.provider, in_library_only=False + ) + ref_tracks = await self.mass.music.artists.tracks( + artist.item_id, artist.provider, in_library_only=False + ) # start lookup of musicbrainz id musicbrainz: MusicbrainzProvider = self.mass.get_provider("musicbrainz") assert musicbrainz diff --git a/music_assistant/server/controllers/music.py b/music_assistant/server/controllers/music.py index 02292e9f..d6b455de 100644 --- a/music_assistant/server/controllers/music.py +++ b/music_assistant/server/controllers/music.py @@ -719,9 +719,6 @@ class MusicController(CoreController): else: self.logger.info("Sync task for %s completed", provider.name) self.mass.signal_event(EventType.SYNC_TASKS_UPDATED, data=self.in_progress_syncs) - # trigger metadata scan after all provider syncs completed - if len(self.in_progress_syncs) == 0: - self.mass.metadata.start_scan() task.add_done_callback(on_sync_task_done) diff --git a/music_assistant/server/providers/fanarttv/__init__.py b/music_assistant/server/providers/fanarttv/__init__.py index b0857e03..f39f38d5 100644 --- a/music_assistant/server/providers/fanarttv/__init__.py +++ b/music_assistant/server/providers/fanarttv/__init__.py @@ -75,7 +75,7 @@ class FanartTvMetadataProvider(MetadataProvider): async def handle_async_init(self) -> None: """Handle async initialization of the provider.""" self.cache = self.mass.cache - self.throttler = Throttler(rate_limit=2, period=1) + self.throttler = Throttler(rate_limit=1, period=30) @property def supported_features(self) -> tuple[ProviderFeature, ...]: @@ -132,7 +132,7 @@ class FanartTvMetadataProvider(MetadataProvider): return metadata return None - @use_cache(86400 * 14) + @use_cache(86400 * 30) async def _get_data(self, endpoint, **kwargs) -> dict | None: """Get data from api.""" url = f"http://webservice.fanart.tv/v3/{endpoint}" diff --git a/music_assistant/server/providers/filesystem_local/base.py b/music_assistant/server/providers/filesystem_local/base.py index d1ae90e8..09532d21 100644 --- a/music_assistant/server/providers/filesystem_local/base.py +++ b/music_assistant/server/providers/filesystem_local/base.py @@ -20,11 +20,7 @@ from music_assistant.common.models.config_entries import ( ConfigValueOption, ) from music_assistant.common.models.enums import ExternalID, ProviderFeature, StreamType -from music_assistant.common.models.errors import ( - InvalidDataError, - MediaNotFoundError, - MusicAssistantError, -) +from music_assistant.common.models.errors import MediaNotFoundError, MusicAssistantError from music_assistant.common.models.media_items import ( Album, Artist, @@ -56,7 +52,6 @@ from .helpers import get_parentdir if TYPE_CHECKING: from collections.abc import AsyncGenerator - from music_assistant.server.providers.musicbrainz import MusicbrainzProvider CONF_MISSING_ALBUM_ARTIST_ACTION = "missing_album_artist_action" @@ -64,14 +59,13 @@ CONF_ENTRY_MISSING_ALBUM_ARTIST = ConfigEntry( key=CONF_MISSING_ALBUM_ARTIST_ACTION, type=ConfigEntryType.STRING, label="Action when a track is missing the Albumartist ID3 tag", - default_value="folder_name", + default_value="various_artists", help_link="https://music-assistant.io/music-providers/filesystem/#tagging-files", required=False, options=( - ConfigValueOption("Skip track and log warning", "skip"), ConfigValueOption("Use Track artist(s)", "track_artist"), ConfigValueOption("Use Various Artists", "various_artists"), - ConfigValueOption("Use Folder name", "folder_name"), + ConfigValueOption("Use Folder name (if possible)", "folder_name"), ), ) @@ -754,32 +748,15 @@ class FileSystemProviderBase(MusicProvider): else: # album artist tag is missing, determine fallback fallback_action = self.config.get_value(CONF_MISSING_ALBUM_ARTIST_ACTION) - musicbrainz: MusicbrainzProvider = self.mass.get_provider("musicbrainz") - assert musicbrainz - # lookup track details on musicbrainz first - if mb_search_details := await musicbrainz.search( - tags.artists[0], tags.album, tags.title, tags.version - ): - # get full releasegroup details and get the releasegroup artist(s) - mb_details = await musicbrainz.get_releasegroup_details(mb_search_details[1].id) - for mb_artist in mb_details.artist_credit: - artist = await self._parse_artist( - mb_artist.artist.name, mb_artist.artist.sort_name - ) - artist.mbid = mb_artist.artist.id - album_artists.append(artist) - if not tags.musicbrainz_recordingid: - tags.tags["musicbrainzrecordingid"] = mb_search_details[2].id - if not tags.musicbrainz_releasegroupid: - tags.tags["musicbrainzreleasegroupid"] = mb_search_details[1].id - # fallback to various artists (if defined by user) - elif fallback_action == "various_artists": + if fallback_action == "folder_name" and album_dir: + possible_artist_folder = os.path.dirname(album_dir) self.logger.warning( - "%s is missing ID3 tag [albumartist], using %s as fallback", + "%s is missing ID3 tag [albumartist], using foldername %s as fallback", file_item.path, - VARIOUS_ARTISTS_NAME, + possible_artist_folder, ) - album_artists = [await self._parse_artist(name=VARIOUS_ARTISTS_NAME)] + album_artist_str = possible_artist_folder.rsplit(os.sep)[-1] + album_artists = [await self._parse_artist(name=album_artist_str)] # fallback to track artists (if defined by user) elif fallback_action == "track_artist": self.logger.warning( @@ -790,20 +767,14 @@ class FileSystemProviderBase(MusicProvider): await self._parse_artist(name=track_artist_str) for track_artist_str in tags.artists ] - elif fallback_action == "folder_name" and album_dir: - possible_artist_folder = os.path.dirname(album_dir) + # all other: fallback to various artists + else: self.logger.warning( - "%s is missing ID3 tag [albumartist], using foldername %s as fallback", + "%s is missing ID3 tag [albumartist], using %s as fallback", file_item.path, - possible_artist_folder, + VARIOUS_ARTISTS_NAME, ) - album_artist_str = possible_artist_folder.rsplit(os.sep)[-1] - album_artists = [await self._parse_artist(name=album_artist_str)] - # fallback to just log error and add track without album - else: - # default action is to skip the track - msg = "missing ID3 tag [albumartist]" - raise InvalidDataError(msg) + album_artists = [await self._parse_artist(name=VARIOUS_ARTISTS_NAME)] track.album = await self._parse_album( tags.album, diff --git a/music_assistant/server/providers/musicbrainz/__init__.py b/music_assistant/server/providers/musicbrainz/__init__.py index 469968f4..5c23e143 100644 --- a/music_assistant/server/providers/musicbrainz/__init__.py +++ b/music_assistant/server/providers/musicbrainz/__init__.py @@ -204,7 +204,7 @@ class MusicBrainzRecording(DataClassDictMixin): class MusicbrainzProvider(MetadataProvider): """The Musicbrainz Metadata provider.""" - throttler = ThrottlerManager(rate_limit=1, period=1) + throttler = ThrottlerManager(rate_limit=1, period=30) async def handle_async_init(self) -> None: """Handle async initialization of the provider.""" diff --git a/music_assistant/server/providers/qobuz/__init__.py b/music_assistant/server/providers/qobuz/__init__.py index c2a23d53..178cd4b6 100644 --- a/music_assistant/server/providers/qobuz/__init__.py +++ b/music_assistant/server/providers/qobuz/__init__.py @@ -124,7 +124,7 @@ class QobuzProvider(MusicProvider): _user_auth_info: str | None = None # rate limiter needs to be specified on provider-level, # so make it an instance attribute - throttler = ThrottlerManager(rate_limit=1, period=1) + throttler = ThrottlerManager(rate_limit=1, period=2) async def handle_async_init(self) -> None: """Handle async initialization of the provider.""" diff --git a/music_assistant/server/providers/spotify/__init__.py b/music_assistant/server/providers/spotify/__init__.py index c3e5a9e5..ddd203ca 100644 --- a/music_assistant/server/providers/spotify/__init__.py +++ b/music_assistant/server/providers/spotify/__init__.py @@ -129,7 +129,7 @@ class SpotifyProvider(MusicProvider): _librespot_bin: str | None = None # rate limiter needs to be specified on provider-level, # so make it an instance attribute - throttler = ThrottlerManager(rate_limit=1, period=1) + throttler = ThrottlerManager(rate_limit=1, period=2) async def handle_async_init(self) -> None: """Handle async initialization of the provider.""" diff --git a/music_assistant/server/providers/theaudiodb/__init__.py b/music_assistant/server/providers/theaudiodb/__init__.py index 52b0d69d..0957224b 100644 --- a/music_assistant/server/providers/theaudiodb/__init__.py +++ b/music_assistant/server/providers/theaudiodb/__init__.py @@ -109,7 +109,7 @@ class AudioDbMetadataProvider(MetadataProvider): async def handle_async_init(self) -> None: """Handle async initialization of the provider.""" self.cache = self.mass.cache - self.throttler = Throttler(rate_limit=2, period=1) + self.throttler = Throttler(rate_limit=1, period=30) @property def supported_features(self) -> tuple[ProviderFeature, ...]: @@ -295,7 +295,7 @@ class AudioDbMetadataProvider(MetadataProvider): break return metadata - @use_cache(86400 * 14) + @use_cache(86400 * 30) async def _get_data(self, endpoint, **kwargs) -> dict | None: """Get data from api.""" url = f"https://theaudiodb.com/api/v1/json/{app_var(3)}/{endpoint}" diff --git a/music_assistant/server/providers/tidal/__init__.py b/music_assistant/server/providers/tidal/__init__.py index 5e42a464..e2cf785b 100644 --- a/music_assistant/server/providers/tidal/__init__.py +++ b/music_assistant/server/providers/tidal/__init__.py @@ -214,7 +214,7 @@ class TidalProvider(MusicProvider): _tidal_user_id: str | None = None # rate limiter needs to be specified on provider-level, # so make it an instance attribute - throttler = ThrottlerManager(rate_limit=1, period=0.5) + throttler = ThrottlerManager(rate_limit=1, period=2) async def handle_async_init(self) -> None: """Handle async initialization of the provider.""" diff --git a/music_assistant/server/providers/tunein/__init__.py b/music_assistant/server/providers/tunein/__init__.py index 66c6ea8e..a489de9f 100644 --- a/music_assistant/server/providers/tunein/__init__.py +++ b/music_assistant/server/providers/tunein/__init__.py @@ -91,7 +91,7 @@ class TuneInProvider(MusicProvider): async def handle_async_init(self) -> None: """Handle async initialization of the provider.""" - self._throttler = Throttler(rate_limit=1, period=1) + self._throttler = Throttler(rate_limit=1, period=2) async def get_library_radios(self) -> AsyncGenerator[Radio, None]: """Retrieve library/subscribed radio stations from the provider.""" -- 2.34.1