From: Marcel van der Veldt Date: Wed, 25 May 2022 19:36:13 +0000 (+0200) Subject: Support for album having multiple artists (#341) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=bc97ccd85aef8adb38e00bea185a906e4ef8a88b;p=music-assistant-server.git Support for album having multiple artists (#341) * Add support for multiple album artists --- diff --git a/music_assistant/controllers/music/albums.py b/music_assistant/controllers/music/albums.py index 712635a5..173777e5 100644 --- a/music_assistant/controllers/music/albums.py +++ b/music_assistant/controllers/music/albums.py @@ -3,7 +3,7 @@ from __future__ import annotations import asyncio import itertools -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union from databases import Database as Db @@ -16,6 +16,7 @@ from music_assistant.models.media_controller import MediaControllerBase from music_assistant.models.media_items import ( Album, AlbumType, + Artist, ItemMapping, MediaType, Track, @@ -136,12 +137,12 @@ class AlbumsController(MediaControllerBase[Album]): return await self.update_db_item(cur_item.item_id, album, db=db) # insert new album - album_artist = await self._get_album_artist(album, cur_item, db=db) + album_artists = await self._get_album_artists(album, cur_item, db=db) new_item = await self.mass.database.insert( self.db_table, { **album.to_db_row(), - "artist": json_serializer(album_artist) or None, + "artists": json_serializer(album_artists) or None, }, db=db, ) @@ -168,7 +169,7 @@ class AlbumsController(MediaControllerBase[Album]): assert album.artist, f"Album {album.name} is missing artist" async with self.mass.database.get_db(db) as db: cur_item = await self.get_db_item(item_id) - album_artist = await self._get_album_artist(album, cur_item, db=db) + album_artists = await self._get_album_artists(album, cur_item, db=db) if overwrite: metadata = album.metadata provider_ids = album.provider_ids @@ -191,7 +192,7 @@ class AlbumsController(MediaControllerBase[Album]): "year": album.year or cur_item.year, "upc": album.upc or cur_item.upc, "album_type": album_type.value, - "artist": json_serializer(album_artist) or None, + "artists": json_serializer(album_artists) or None, "metadata": json_serializer(metadata), "provider_ids": json_serializer(provider_ids), }, @@ -259,28 +260,35 @@ class AlbumsController(MediaControllerBase[Album]): provider.name, ) - async def _get_album_artist( + async def _get_album_artists( self, db_album: Album, updated_album: Optional[Album] = None, db: Optional[Db] = None, - ) -> ItemMapping | None: - """Extract (database) album artist as ItemMapping.""" + ) -> List[ItemMapping]: + """Extract (database) album artist(s) as ItemMapping.""" + album_artists = set() for album in (updated_album, db_album): - if not album or not album.artist: + if not album: continue + for artist in album.artists: + album_artists.add(await self._get_artist_mapping(artist, db=db)) + # use intermediate set to prevent duplicates + return list(album_artists) - if album.artist.provider == ProviderType.DATABASE: - if isinstance(album.artist, ItemMapping): - return album.artist - return ItemMapping.from_item(album.artist) + async def _get_artist_mapping( + self, artist: Union[Artist, ItemMapping], db: Optional[Db] = None + ) -> ItemMapping: + """Extract (database) track artist as ItemMapping.""" + if artist.provider == ProviderType.DATABASE: + if isinstance(artist, ItemMapping): + return artist + return ItemMapping.from_item(artist) - if db_artist := await self.mass.music.artists.get_db_item_by_prov_id( - album.artist.item_id, provider=album.artist.provider, db=db - ): - return ItemMapping.from_item(db_artist) - - db_artist = await self.mass.music.artists.add_db_item(album.artist, db=db) + if db_artist := await self.mass.music.artists.get_db_item_by_prov_id( + artist.item_id, provider=artist.provider, db=db + ): return ItemMapping.from_item(db_artist) - return None + db_artist = await self.mass.music.artists.add_db_item(artist, db=db) + return ItemMapping.from_item(db_artist) diff --git a/music_assistant/controllers/music/providers/filesystem.py b/music_assistant/controllers/music/providers/filesystem.py index 97bd135e..6cf3583d 100644 --- a/music_assistant/controllers/music/providers/filesystem.py +++ b/music_assistant/controllers/music/providers/filesystem.py @@ -14,6 +14,7 @@ from aiofiles.threadpool.binary import AsyncFileIO from tinytag.tinytag import TinyTag from music_assistant.helpers.compare import compare_strings +from music_assistant.helpers.database import SCHEMA_VERSION from music_assistant.helpers.util import ( create_clean_string, parse_title_and_version, @@ -40,6 +41,9 @@ from music_assistant.models.media_items import ( ) from music_assistant.models.provider import MusicProvider +FALLBACK_ARTIST = "Various Artists" +SPLITTERS = (";", ",", "Featuring", " Feat. ", " Feat ", "feat.", " & ", " / ") + async def scantree(path: str) -> AsyncGenerator[os.DirEntry, None]: """Recursively yield DirEntry objects for given directory.""" @@ -56,24 +60,18 @@ async def scantree(path: str) -> AsyncGenerator[os.DirEntry, None]: yield entry -def split_items(org_str: str, splitters: Tuple[str] = None) -> Tuple[str]: +def split_items(org_str: str) -> Tuple[str]: """Split up a tags string by common splitter.""" if isinstance(org_str, list): return org_str - if splitters is None: - splitters = ("/", ";", ",") if org_str is None: return tuple() - for splitter in splitters: + for splitter in SPLITTERS: if splitter in org_str: return tuple((x.strip() for x in org_str.split(splitter))) return (org_str,) -FALLBACK_ARTIST = "Various Artists" -ARTIST_SPLITTERS = (";", ",", "Featuring", " Feat. ", " Feat ", "feat.", " & ") - - class FileSystemProvider(MusicProvider): """ Implementation of a musicprovider for local files. @@ -150,7 +148,7 @@ class FileSystemProvider(MusicProvider): if checksum == prev_checksums.get(entry.path): continue try: - if track := await self._parse_track(entry.path, checksum): + if track := await self._parse_track(entry.path): # process album if track.album: db_album = await self.mass.music.albums.add_db_item( @@ -177,8 +175,9 @@ class FileSystemProvider(MusicProvider): await self.mass.music.tracks.set_db_library( db_track.item_id, True, db=db ) - elif playlist := await self._parse_playlist(entry.path, checksum): + elif playlist := await self._parse_playlist(entry.path): # add/update] playlist to db + playlist.metadata.checksum = checksum await self.mass.music.playlists.add_db_item(playlist, db=db) except Exception: # pylint: disable=broad-except # we don't want the whole sync to crash on one file so we catch all exceptions here @@ -267,11 +266,9 @@ class FileSystemProvider(MusicProvider): async def get_track(self, prov_track_id: str) -> Track: """Get full track details by id.""" itempath = await self.get_filepath(MediaType.TRACK, prov_track_id) - if await self.exists(itempath): - return await self._parse_track(itempath) - return await self.mass.music.tracks.get_db_item_by_prov_id( - provider_item_id=prov_track_id, provider_id=self.id - ) + if not await self.exists(itempath): + raise MediaNotFoundError(f"Track path does not exist: {itempath}") + return await self._parse_track(itempath) async def get_playlist(self, prov_playlist_id: str) -> Playlist: """Get full playlist details by id.""" @@ -306,8 +303,9 @@ class FileSystemProvider(MusicProvider): playlist_path = await self.get_filepath(MediaType.PLAYLIST, prov_playlist_id) if not await self.exists(playlist_path): raise MediaNotFoundError(f"Playlist path does not exist: {playlist_path}") - checksum = await self._get_checksum(playlist_path) - cache_key = f"{self.id}_playlist_tracks_{prov_playlist_id}" + mtime = await aiopath.getmtime(playlist_path) + checksum = f"{SCHEMA_VERSION}.{int(mtime)}" + cache_key = f"playlist_{self.id}_tracks_{prov_playlist_id}" if cache := await self.mass.cache.get(cache_key, checksum): return [Track.from_dict(x) for x in cache] index = 0 @@ -336,7 +334,7 @@ class FileSystemProvider(MusicProvider): if db_artist is None: raise MediaNotFoundError(f"Artist not found: {prov_artist_id}") # TODO: adjust to json query instead of text search - query = f"SELECT * FROM albums WHERE artist LIKE '%\"{db_artist.item_id}\"%'" + query = f"SELECT * FROM albums WHERE artists LIKE '%\"{db_artist.item_id}\"%'" query += f" AND provider_ids LIKE '%\"{self.type.value}\"%'" return await self.mass.music.albums.get_db_items(query) @@ -414,9 +412,7 @@ class FileSystemProvider(MusicProvider): bit_depth=16, # TODO: parse bitdepth ) - async def _parse_track( - self, track_path: str, checksum: Optional[int] = None - ) -> Track | None: + async def _parse_track(self, track_path: str) -> Track | None: """Try to parse a track from a filename by reading its tags.""" if not await self.exists(track_path): @@ -424,12 +420,6 @@ class FileSystemProvider(MusicProvider): track_item_id = self._get_item_id(track_path) - # reading file/tags is slow so we keep a cache and checksum - checksum = checksum or await self._get_checksum(track_path) - cache_key = f"{self.id}_tracks_{track_item_id}" - if cache := await self.mass.cache.get(cache_key, checksum): - return Track.from_dict(cache) - if not TinyTag.is_supported(track_path): return None @@ -439,9 +429,6 @@ class FileSystemProvider(MusicProvider): tags = await self.mass.loop.run_in_executor(None, parse_tags) - assert tags.title, "Required tag title is missing" - assert tags.artist, "Required tag artist is missing" - # prefer title from tag, fallback to filename if tags.title: track_title = tags.title @@ -450,7 +437,7 @@ class FileSystemProvider(MusicProvider): track_title = track_path.split(os.sep)[-1] track_title = track_title.replace(f".{ext}", "").replace("_", " ") self.logger.warning( - "%s is missing ID3 tags, use filename as fallback", track_path + "%s is missing ID3 tag [title], using filename as fallback", track_path ) name, version = parse_title_and_version(track_title) @@ -463,51 +450,65 @@ class FileSystemProvider(MusicProvider): in_library=True, ) - # work out if we have an artist/album/track.ext structure - track_parts = track_path.rsplit(os.sep) - album_folder = None - artist_folder = None - parentdir = os.path.dirname(track_path) - for _ in range(len(track_parts)): - dirname = parentdir.rsplit(os.sep)[-1] - if compare_strings(dirname, tags.albumartist): - artist_folder = parentdir - if compare_strings(dirname, tags.album): - album_folder = parentdir - parentdir = os.path.dirname(parentdir) - - # album artist - if tags.albumartist or artist_folder: - album_artist = await self._parse_artist( - name=tags.albumartist, artist_path=artist_folder, in_library=True - ) - else: - # always fallback to various artists as album artist if user did not tag album artist - # ID3 tag properly because we must have an album artist - album_artist = await self._parse_artist(name="Various Artists") - # album + # work out if we have an artist/album/track.ext structure if tags.album: + track_parts = track_path.rsplit(os.sep) + album_folder = None + artist_folder = None + parentdir = os.path.dirname(track_path) + for _ in range(len(track_parts)): + dirname = parentdir.rsplit(os.sep)[-1] + if compare_strings(dirname, tags.albumartist): + artist_folder = parentdir + if compare_strings(dirname, tags.album): + album_folder = parentdir + parentdir = os.path.dirname(parentdir) + + # album artist + if artist_folder: + album_artists = [ + await self._parse_artist( + name=tags.albumartist, + artist_path=artist_folder, + in_library=True, + ) + ] + elif tags.albumartist: + album_artists = [ + await self._parse_artist(name=item, in_library=True) + for item in split_items(tags.albumartist) + ] + + else: + # always fallback to various artists as album artist if user did not tag album artist + # ID3 tag properly because we must have an album artist + album_artists = [await self._parse_artist(name=FALLBACK_ARTIST)] + self.logger.warning( + "%s is missing ID3 tag [albumartist], using %s as fallback", + track_path, + FALLBACK_ARTIST, + ) + track.album = await self._parse_album( name=tags.album, album_path=album_folder, - artist=album_artist, + artists=album_artists, in_library=True, ) + else: + self.logger.warning("%s is missing ID3 tag [album]", track_path) - if ( - track.album - and track.album.artist - and track.album.artist.name == tags.artist - ): - track.artists = [track.album.artist] + # track artist(s) + if tags.artist == tags.albumartist: + track.artists = track.album.artists else: # Parse track artist(s) from artist string using common splitters used in ID3 tags # NOTE: do not use a '/' or '&' to prevent artists like AC/DC become messed up track_artists_str = tags.artist or FALLBACK_ARTIST track.artists = [ await self._parse_artist(item, in_library=False) - for item in split_items(track_artists_str, ARTIST_SPLITTERS) + for item in split_items(track_artists_str) ] # Check if track has embedded metadata @@ -530,8 +531,6 @@ class FileSystemProvider(MusicProvider): track.metadata.copyright = tags.extra["copyright"] if "lyrics" in tags.extra: track.metadata.lyrics = tags.extra["lyrics"] - # store last modified time as checksum - track.metadata.checksum = checksum quality_details = "" if track_path.endswith(".flac"): @@ -563,7 +562,6 @@ class FileSystemProvider(MusicProvider): url=track_path, ) ) - await self.mass.cache.set(cache_key, track.to_dict(), checksum, 86400 * 365 * 5) return track async def _parse_artist( @@ -571,7 +569,6 @@ class FileSystemProvider(MusicProvider): name: Optional[str] = None, artist_path: Optional[str] = None, in_library: bool = True, - skip_cache=False, ) -> Artist | None: """Lookup metadata in Artist folder.""" assert name or artist_path @@ -583,11 +580,6 @@ class FileSystemProvider(MusicProvider): if not name: name = artist_path.split(os.sep)[-1] - cache_key = f"{self.id}.artist.{artist_item_id}" - if not skip_cache: - if cache := await self.mass.cache.get(cache_key): - return Artist.from_dict(cache) - artist = Artist( artist_item_id, self.type, @@ -638,39 +630,29 @@ class FileSystemProvider(MusicProvider): if images: artist.metadata.images = images - await self.mass.cache.set(cache_key, artist.to_dict()) return artist async def _parse_album( self, name: Optional[str] = None, album_path: Optional[str] = None, - artist: Optional[Artist] = None, + artists: List[Artist] = None, in_library: bool = True, - skip_cache=False, ) -> Album | None: """Lookup metadata in Album folder.""" - assert name or album_path - if not album_path and artist: - # create fake path - album_path = os.path.join(self.config.path, artist.name, name) - elif not album_path: - album_path = os.path.join("Albums", name) + assert (name or album_path) and artists + # create fake path + album_path = os.path.join(self.config.path, artists[0].name, name) album_item_id = self._get_item_id(album_path) if not name: name = album_path.split(os.sep)[-1] - cache_key = f"{self.id}.album.{album_item_id}" - if not skip_cache: - if cache := await self.mass.cache.get(cache_key): - return Album.from_dict(cache) - album = Album( album_item_id, self.type, name, - artist=artist, + artists=artists, provider_ids={ MediaItemProviderId(album_item_id, self.type, self.id, url=album_path) }, @@ -713,7 +695,7 @@ class FileSystemProvider(MusicProvider): album_tracks = [ x async for x in scantree(album_path) if TinyTag.is_supported(x.path) ] - if artist and artist.sort_name == "variousartists": + if album.artist.sort_name == "variousartists": album.album_type = AlbumType.COMPILATION elif len(album_tracks) <= 5: album.album_type = AlbumType.SINGLE @@ -736,15 +718,11 @@ class FileSystemProvider(MusicProvider): if images: album.metadata.images = images - await self.mass.cache.set(cache_key, album.to_dict()) return album - async def _parse_playlist( - self, playlist_path: str, checksum: Optional[str] = None - ) -> Playlist | None: + async def _parse_playlist(self, playlist_path: str) -> Playlist | None: """Parse playlist from file.""" playlist_item_id = self._get_item_id(playlist_path) - checksum = checksum or await self._get_checksum(playlist_path) if not playlist_path.endswith(".m3u"): return None @@ -766,7 +744,6 @@ class FileSystemProvider(MusicProvider): ) ) playlist.owner = self._attr_name - playlist.metadata.checksum = checksum return playlist async def _parse_track_from_uri(self, uri: str): @@ -781,8 +758,10 @@ class FileSystemProvider(MusicProvider): ) return None # try to treat uri as filename + if self.config.path not in uri: + uri = os.path.join(self.config.path, uri) try: - return await self.get_track(uri) + return await self._parse_track(uri) except MediaNotFoundError: return None @@ -842,9 +821,3 @@ class FileSystemProvider(MusicProvider): def _get_item_id(self, file_path: str) -> str: """Create item id from filename.""" return create_clean_string(file_path.replace(self.config.path, "")) - - @staticmethod - async def _get_checksum(filename: str) -> int: - """Get checksum for file.""" - # use last modified time as checksum - return await aiopath.getmtime(filename) diff --git a/music_assistant/controllers/music/providers/qobuz.py b/music_assistant/controllers/music/providers/qobuz.py index 2bb04b4f..8ced8464 100644 --- a/music_assistant/controllers/music/providers/qobuz.py +++ b/music_assistant/controllers/music/providers/qobuz.py @@ -460,10 +460,7 @@ class QobuzProvider(MusicProvider): ) ) - if artist_obj: - album.artist = artist_obj - else: - album.artist = await self._parse_artist(album_obj["artist"]) + album.artist = await self._parse_artist(artist_obj or album_obj["artist"]) if ( album_obj.get("product_type", "") == "single" or album_obj.get("release_type", "") == "single" diff --git a/music_assistant/controllers/music/providers/spotify.py b/music_assistant/controllers/music/providers/spotify.py index ab5a9db2..9534833b 100644 --- a/music_assistant/controllers/music/providers/spotify.py +++ b/music_assistant/controllers/music/providers/spotify.py @@ -320,10 +320,8 @@ class SpotifyProvider(MusicProvider): album = Album( item_id=album_obj["id"], provider=self.type, name=name, version=version ) - for artist in album_obj["artists"]: - album.artist = await self._parse_artist(artist) - if album.artist: - break + for artist_obj in album_obj["artists"]: + album.artists.append(await self._parse_artist(artist_obj)) if album_obj["album_type"] == "single": album.album_type = AlbumType.SINGLE elif album_obj["album_type"] == "compilation": diff --git a/music_assistant/helpers/database.py b/music_assistant/helpers/database.py index 3fffd039..57e8ba35 100755 --- a/music_assistant/helpers/database.py +++ b/music_assistant/helpers/database.py @@ -10,7 +10,7 @@ if TYPE_CHECKING: from music_assistant.mass import MusicAssistant -SCHEMA_VERSION = 15 +SCHEMA_VERSION = 16 TABLE_TRACK_LOUDNESS = "track_loudness" TABLE_PLAYLOG = "playlog" @@ -198,8 +198,8 @@ class Database: # always create db tables if they don't exist to prevent errors trying to access them later await self.__create_database_tables(db) - if prev_version < 13: - # fixed nasty bugs in file provider, start clean just in case. + if prev_version < 15: + # too many changes, just recreate await db.execute(f"DROP TABLE IF EXISTS {TABLE_ARTISTS}") await db.execute(f"DROP TABLE IF EXISTS {TABLE_ALBUMS}") await db.execute(f"DROP TABLE IF EXISTS {TABLE_TRACKS}") @@ -207,20 +207,19 @@ class Database: await db.execute(f"DROP TABLE IF EXISTS {TABLE_RADIOS}") await db.execute(f"DROP TABLE IF EXISTS {TABLE_CACHE}") await db.execute(f"DROP TABLE IF EXISTS {TABLE_THUMBS}") - # recreate missing tables - await self.__create_database_tables(db) - - if prev_version < 14: - # no more need for prov_mappings table await db.execute("DROP TABLE IF EXISTS provider_mappings") - - if prev_version < 15: - # album --> albums on track entity - await db.execute(f"DROP TABLE IF EXISTS {TABLE_TRACKS}") - await db.execute(f"DROP TABLE IF EXISTS {TABLE_CACHE}") # recreate missing tables await self.__create_database_tables(db) + if prev_version and prev_version < 16: + # album artist --> album artists + await db.execute(f"ALTER TABLE {TABLE_ALBUMS} ADD artists json;") + for db_row in await self.get_rows(TABLE_ALBUMS, db=db): + match = {"item_id": db_row["item_id"]} + new_values = {"artists": f'[{ db_row["artist"]}]'} + await self.update(TABLE_ALBUMS, match, new_values, db=db) + await db.execute(f"ALTER TABLE {TABLE_ALBUMS} DROP COLUMN artist;") + # store current schema version await self.set_setting("version", str(SCHEMA_VERSION), db=db) @@ -254,7 +253,7 @@ class Database: in_library BOOLEAN DEFAULT 0, upc TEXT, musicbrainz_id TEXT, - artist json, + artists json, metadata json, provider_ids json );""" diff --git a/music_assistant/models/media_items.py b/music_assistant/models/media_items.py index b66a1984..4cb39d1f 100755 --- a/music_assistant/models/media_items.py +++ b/music_assistant/models/media_items.py @@ -245,11 +245,23 @@ class Album(MediaItem): media_type: MediaType = MediaType.ALBUM version: str = "" year: Optional[int] = None - artist: Union[Artist, ItemMapping, None] = None + artists: List[Union[Artist, ItemMapping]] = field(default_factory=list) album_type: AlbumType = AlbumType.UNKNOWN upc: Optional[str] = None musicbrainz_id: Optional[str] = None # release group id + @property + def artist(self) -> Artist | ItemMapping | None: + """Return (first) artist of album.""" + if self.artists: + return self.artists[0] + return None + + @artist.setter + def artist(self, artist: Union[Artist, ItemMapping]) -> None: + """Set (first/only) artist of album.""" + self.artists = [artist] + def __hash__(self): """Return custom hash.""" return hash((self.provider, self.item_id))