From: Marcel van der Veldt Date: Fri, 22 Jul 2022 10:06:28 +0000 (+0200) Subject: A few small usability fixes (#433) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=9a0176aada5a38ae3e86db32d35e6abbaa036e51;p=music-assistant-server.git A few small usability fixes (#433) * make the album/track versions lookup a bit more strict * ignore MediaNotFoundError when recursively deleting items from the db * small typo * Fix: refresh overwrites info of local files * prefer local filesystem at lookups * allow overwrite of local metadata when filesystem * Allow resume for non-MA sources Allow resume after announce or pause even if the previous playing media was not initiated by MA * fix race condition --- diff --git a/music_assistant/controllers/music/albums.py b/music_assistant/controllers/music/albums.py index c969b11b..8bb1aa25 100644 --- a/music_assistant/controllers/music/albums.py +++ b/music_assistant/controllers/music/albums.py @@ -4,11 +4,12 @@ from __future__ import annotations import asyncio from typing import List, Optional, Union -from music_assistant.helpers.compare import compare_album, compare_artist +from music_assistant.helpers.compare import compare_album, loose_compare_strings from music_assistant.helpers.database import TABLE_ALBUMS, TABLE_TRACKS from music_assistant.helpers.json import json_serializer from music_assistant.helpers.tags import FALLBACK_ARTIST from music_assistant.models.enums import EventType, MusicProviderFeature, ProviderType +from music_assistant.models.errors import MediaNotFoundError from music_assistant.models.event import MassEvent from music_assistant.models.media_controller import MediaControllerBase from music_assistant.models.media_items import ( @@ -72,11 +73,7 @@ class AlbumsController(MediaControllerBase[Album]): *[self.search(search_query, prov_type) for prov_type in prov_types] ) for prov_item in prov_items - if ( - (prov_item.sort_name in album.sort_name) - or (album.sort_name in prov_item.sort_name) - ) - and compare_artist(prov_item.artist, album.artist) + if loose_compare_strings(album.name, prov_item.name) } # make sure that the 'base' version is included for prov_version in album.provider_ids: @@ -91,11 +88,11 @@ class AlbumsController(MediaControllerBase[Album]): # return the aggregated result return all_versions.values() - async def add(self, item: Album, overwrite_existing: bool = False) -> Album: + async def add(self, item: Album) -> Album: """Add album to local db and return the database item.""" # grab additional metadata await self.mass.metadata.get_album_metadata(item) - db_item = await self.add_db_item(item, overwrite_existing) + db_item = await self.add_db_item(item) # also fetch same album on all providers await self._match(db_item) db_item = await self.get_db_item(db_item.item_id) @@ -229,7 +226,7 @@ class AlbumsController(MediaControllerBase[Album]): provider_ids = item.provider_ids album_artists = await self._get_album_artists(item, overwrite=True) else: - metadata = cur_item.metadata.update(item.metadata) + metadata = cur_item.metadata.update(item.metadata, item.provider.is_file()) provider_ids = {*cur_item.provider_ids, *item.provider_ids} album_artists = await self._get_album_artists(item, cur_item) @@ -276,7 +273,12 @@ class AlbumsController(MediaControllerBase[Album]): ) assert not (db_rows and not recursive), "Tracks attached to album" for db_row in db_rows: - await self.mass.music.albums.delete_db_item(db_row["item_id"], recursive) + try: + await self.mass.music.albums.delete_db_item( + db_row["item_id"], recursive + ) + except MediaNotFoundError: + pass # delete the album itself from db await super().delete_db_item(item_id) diff --git a/music_assistant/controllers/music/artists.py b/music_assistant/controllers/music/artists.py index f5f42955..b112c28b 100644 --- a/music_assistant/controllers/music/artists.py +++ b/music_assistant/controllers/music/artists.py @@ -8,6 +8,7 @@ from typing import Any, Dict, List, Optional from music_assistant.helpers.database import TABLE_ALBUMS, TABLE_ARTISTS, TABLE_TRACKS from music_assistant.helpers.json import json_serializer from music_assistant.models.enums import EventType, MusicProviderFeature, ProviderType +from music_assistant.models.errors import MediaNotFoundError from music_assistant.models.event import MassEvent from music_assistant.models.media_controller import MediaControllerBase from music_assistant.models.media_items import ( @@ -89,11 +90,11 @@ class ArtistsController(MediaControllerBase[Artist]): final_items[key].in_library = True return list(final_items.values()) - async def add(self, item: Artist, overwrite_existing: bool = False) -> Artist: + async def add(self, item: Artist) -> Artist: """Add artist to local db and return the database item.""" # grab musicbrainz id and additional metadata await self.mass.metadata.get_artist_metadata(item) - db_item = await self.add_db_item(item, overwrite_existing) + db_item = await self.add_db_item(item) # also fetch same artist on all providers await self.match_artist(db_item) db_item = await self.get_db_item(db_item.item_id) @@ -249,7 +250,7 @@ class ArtistsController(MediaControllerBase[Artist]): metadata = item.metadata provider_ids = item.provider_ids else: - metadata = cur_item.metadata.update(item.metadata) + metadata = cur_item.metadata.update(item.metadata, item.provider.is_file()) provider_ids = {*cur_item.provider_ids, *item.provider_ids} await self.mass.database.update( @@ -279,7 +280,12 @@ class ArtistsController(MediaControllerBase[Artist]): ) assert not (db_rows and not recursive), "Albums attached to artist" for db_row in db_rows: - await self.mass.music.albums.delete_db_item(db_row["item_id"], recursive) + try: + await self.mass.music.albums.delete_db_item( + db_row["item_id"], recursive + ) + except MediaNotFoundError: + pass # check artist tracks db_rows = await self.mass.database.get_rows_from_query( @@ -288,7 +294,12 @@ class ArtistsController(MediaControllerBase[Artist]): ) assert not (db_rows and not recursive), "Tracks attached to artist" for db_row in db_rows: - await self.mass.music.albums.delete_db_item(db_row["item_id"], recursive) + try: + await self.mass.music.albums.delete_db_item( + db_row["item_id"], recursive + ) + except MediaNotFoundError: + pass # delete the artist itself from db await super().delete_db_item(item_id) diff --git a/music_assistant/controllers/music/playlists.py b/music_assistant/controllers/music/playlists.py index 58f576bd..018578e3 100644 --- a/music_assistant/controllers/music/playlists.py +++ b/music_assistant/controllers/music/playlists.py @@ -81,11 +81,11 @@ class PlaylistController(MediaControllerBase[Playlist]): ) return items - async def add(self, item: Playlist, overwrite_existing: bool = False) -> Playlist: + async def add(self, item: Playlist) -> Playlist: """Add playlist to local db and return the new database item.""" item.metadata.last_refresh = int(time()) await self.mass.metadata.get_playlist_metadata(item) - return await self.add_db_item(item, overwrite_existing) + return await self.add_db_item(item) async def create( self, name: str, prov_id: Union[ProviderType, str, None] = None diff --git a/music_assistant/controllers/music/radio.py b/music_assistant/controllers/music/radio.py index 31cf36d3..0791c6a0 100644 --- a/music_assistant/controllers/music/radio.py +++ b/music_assistant/controllers/music/radio.py @@ -5,6 +5,7 @@ import asyncio from time import time from typing import List, Optional +from music_assistant.helpers.compare import loose_compare_strings from music_assistant.helpers.database import TABLE_RADIOS from music_assistant.helpers.json import json_serializer from music_assistant.models.enums import EventType, MediaType, ProviderType @@ -41,12 +42,7 @@ class RadioController(MediaControllerBase[Radio]): *[self.search(radio.name, prov_type) for prov_type in prov_types] ) for prov_item in prov_items - if ( - (prov_item.name in radio.name) - or (radio.name in prov_item.name) - or (prov_item.sort_name in radio.sort_name) - or (radio.sort_name in prov_item.sort_name) - ) + if loose_compare_strings(radio.name, prov_item.name) } # make sure that the 'base' version is included for prov_version in radio.provider_ids: @@ -61,11 +57,11 @@ class RadioController(MediaControllerBase[Radio]): # return the aggregated result return all_versions.values() - async def add(self, item: Radio, overwrite_existing: bool = False) -> Radio: + async def add(self, item: Radio) -> Radio: """Add radio to local db and return the new database item.""" item.metadata.last_refresh = int(time()) await self.mass.metadata.get_radio_metadata(item) - return await self.add_db_item(item, overwrite_existing) + return await self.add_db_item(item) async def add_db_item(self, item: Radio, overwrite_existing: bool = False) -> Radio: """Add a new item record to the database.""" diff --git a/music_assistant/controllers/music/tracks.py b/music_assistant/controllers/music/tracks.py index efd65798..d8df400d 100644 --- a/music_assistant/controllers/music/tracks.py +++ b/music_assistant/controllers/music/tracks.py @@ -4,7 +4,11 @@ from __future__ import annotations import asyncio from typing import List, Optional, Union -from music_assistant.helpers.compare import compare_artists, compare_track +from music_assistant.helpers.compare import ( + compare_artists, + compare_track, + loose_compare_strings, +) from music_assistant.helpers.database import TABLE_TRACKS from music_assistant.helpers.json import json_serializer from music_assistant.models.enums import ( @@ -48,13 +52,13 @@ class TracksController(MediaControllerBase[Track]): track.artists = full_artists return track - async def add(self, item: Track, overwrite_existing: bool = False) -> Track: + async def add(self, item: Track) -> Track: """Add track to local db and return the new database item.""" # make sure we have artists assert item.artists # grab additional metadata await self.mass.metadata.get_track_metadata(item) - db_item = await self.add_db_item(item, overwrite_existing) + db_item = await self.add_db_item(item) # also fetch same track on all providers (will also get other quality versions) await self._match(db_item) return await self.get_db_item(db_item.item_id) @@ -77,10 +81,7 @@ class TracksController(MediaControllerBase[Track]): *[self.search(search_query, prov_type) for prov_type in prov_types] ) for prov_item in prov_items - if ( - (prov_item.sort_name in track.sort_name) - or (track.sort_name in prov_item.sort_name) - ) + if loose_compare_strings(track.name, prov_item.name) and compare_artists(prov_item.artists, track.artists, any_match=True) } # make sure that the 'base' version is included @@ -215,7 +216,7 @@ class TracksController(MediaControllerBase[Track]): track_artists = await self._get_track_artists(item, overwrite=True) track_albums = await self._get_track_albums(item, overwrite=True) else: - metadata = cur_item.metadata.update(item.metadata, overwrite) + metadata = cur_item.metadata.update(item.metadata, item.provider.is_file()) provider_ids = {*cur_item.provider_ids, *item.provider_ids} track_artists = await self._get_track_artists(cur_item, item) track_albums = await self._get_track_albums(cur_item, item) diff --git a/music_assistant/helpers/compare.py b/music_assistant/helpers/compare.py index 639d9e7d..0bca83cf 100644 --- a/music_assistant/helpers/compare.py +++ b/music_assistant/helpers/compare.py @@ -15,6 +15,22 @@ from music_assistant.models.media_items import ( ) +def loose_compare_strings(base: str, alt: str) -> bool: + """Compare strings and return True even on partial match.""" + # this is used to display 'versions' of the same track/album + # where we account for other spelling or some additional wording in the title + word_count = len(base.split(" ")) + if word_count == 1 and len(base) < 10: + return compare_strings(base, alt, False) + base_comp = create_safe_string(base) + alt_comp = create_safe_string(alt) + if base_comp in alt_comp: + return True + if alt_comp in base_comp: + return True + return False + + def compare_strings(str1: str, str2: str, strict: bool = True) -> bool: """Compare strings and return True if we have an (almost) perfect match.""" if str1 is None or str2 is None: diff --git a/music_assistant/models/media_controller.py b/music_assistant/models/media_controller.py index 5dce7f52..20609c20 100644 --- a/music_assistant/models/media_controller.py +++ b/music_assistant/models/media_controller.py @@ -43,7 +43,7 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): self._db_add_lock = asyncio.Lock() @abstractmethod - async def add(self, item: ItemCls, overwrite_existing: bool = False) -> ItemCls: + async def add(self, item: ItemCls) -> ItemCls: """Add item to local db and return the database item.""" raise NotImplementedError @@ -126,7 +126,6 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): force_refresh: bool = False, lazy: bool = True, details: ItemCls = None, - overwrite_existing: bool = None, ) -> ItemCls: """Return (full) details for a single media item.""" assert provider or provider_id, "provider or provider_id must be supplied" @@ -137,8 +136,6 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): provider=provider, provider_id=provider_id, ) - if overwrite_existing is None: - overwrite_existing = force_refresh if db_item and (time() - db_item.last_refresh) > REFRESH_INTERVAL: # it's been too long since the full metadata was last retrieved (or never at all) force_refresh = True @@ -175,14 +172,14 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): # only if we really need to wait for the result (e.g. to prevent race conditions), we # can set lazy to false and we await to job to complete. add_job = self.mass.add_job( - self.add(details, overwrite_existing=overwrite_existing), + self.add(details), f"Add {details.uri} to database", ) if not lazy: await add_job.wait() return add_job.result - return db_item if db_item else details + return details async def search( self, @@ -286,12 +283,15 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): if item.provider == ProviderType.DATABASE: # make sure we have a full object item = await self.get_db_item(item.item_id) - for prov in item.provider_ids: - # returns the first provider that is available - if not prov.available: - continue - if self.mass.music.get_provider(prov.prov_id): - return (prov.prov_id, prov.item_id) + for prefer_file in (True, False): + for prov in item.provider_ids: + # returns the first provider that is available + if not prov.available: + continue + if prefer_file and not prov.prov_type.is_file(): + continue + if self.mass.music.get_provider(prov.prov_id): + return (prov.prov_id, prov.item_id) return None, None async def get_db_items_by_query( diff --git a/music_assistant/models/media_items.py b/music_assistant/models/media_items.py index 8d36fbab..b5c753b6 100755 --- a/music_assistant/models/media_items.py +++ b/music_assistant/models/media_items.py @@ -95,7 +95,7 @@ class MediaItemMetadata(DataClassDictMixin): def update( self, new_values: "MediaItemMetadata", - allow_overwrite: bool = True, + allow_overwrite: bool = False, ) -> "MediaItemMetadata": """Update metadata (in-place) with new values.""" for fld in fields(self): diff --git a/music_assistant/models/player_queue.py b/music_assistant/models/player_queue.py index 0ee41734..b4b8f8fe 100644 --- a/music_assistant/models/player_queue.py +++ b/music_assistant/models/player_queue.py @@ -74,7 +74,7 @@ class PlayerQueue: self._current_index: Optional[int] = None self._current_item_elapsed_time: int = 0 self._prev_item: Optional[QueueItem] = None - self._last_state = str + self._last_player_state: Tuple[str, str] = ("", "") self._items: List[QueueItem] = [] self._save_task: TimerHandle = None self._last_player_update: int = 0 @@ -316,8 +316,8 @@ class PlayerQueue: PlayerState.PAUSED, ): await self.stop() - await self._wait_for_state((PlayerState.OFF, PlayerState.IDLE)) self._announcement_in_progress = True + await self._wait_for_state((PlayerState.OFF, PlayerState.IDLE)) # turn on player if needed if not self.player.powered: @@ -385,7 +385,7 @@ class PlayerQueue: if self._announcement_in_progress: self.logger.warning("Ignore queue command: An announcement is in progress") return - if self.active and self.player.state == PlayerState.PAUSED: + if self.player.state == PlayerState.PAUSED: await self.player.play() else: await self.resume() @@ -434,6 +434,11 @@ class PlayerQueue: async def resume(self) -> None: """Resume previous queue.""" + last_player_url = self._last_player_state[1] + if last_player_url and self.mass.streams.base_url not in last_player_url: + self.logger.info("Trying to resume non-MA content %s...", last_player_url) + await self.player.play_url(last_player_url) + return resume_item = self.current_item resume_pos = self._current_item_elapsed_time if ( @@ -641,10 +646,19 @@ class PlayerQueue: def on_player_update(self) -> None: """Call when player updates.""" - player_state_str = f"{self.player.state.value}.{self.player.current_url}" - if self._last_state != player_state_str: + cur_player_state = (self.player.state.value, self.player.current_url) + if self._last_player_state != cur_player_state: # playback state changed - self._last_state = player_state_str + if self._announcement_in_progress: + # while announcement in progress dont update the last url + # to allow us to resume from 3rd party sources + # https://github.com/music-assistant/hass-music-assistant/issues/697 + self._last_player_state = ( + cur_player_state[0], + self._last_player_state[1], + ) + else: + self._last_player_state = cur_player_state # always signal update if playback state changed self.signal_update()