From 9a13ff87dd86e00ec27bdefedee85764079838bc Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Fri, 4 Aug 2023 02:18:22 +0200 Subject: [PATCH] A few small bugfixes (#818) * Make sure that file/library items never get cached * fix typo * fix end of queue reached in cast provider + title * Prefer album image for tracks * fix keyerror on non existent YTmusic track * Fix enqueueing logic * prevent enqueue next track if MA is not the active source * prevent unmount log at startup * cleanup only on coordinator * fix typo * fix typo * use flac as intermediate codec to airplay bridge --- music_assistant/common/models/media_items.py | 7 ++++ .../server/controllers/media/artists.py | 14 ++++---- .../server/controllers/media/base.py | 6 +--- .../server/controllers/media/tracks.py | 2 +- music_assistant/server/controllers/music.py | 9 ++--- .../server/controllers/player_queues.py | 35 ++++++++++++------- .../server/models/music_provider.py | 2 +- .../server/providers/airplay/__init__.py | 6 ++-- .../server/providers/chromecast/__init__.py | 16 +++++++-- .../server/providers/dlna/__init__.py | 4 +-- .../providers/filesystem_smb/__init__.py | 7 ++-- .../server/providers/plex/__init__.py | 6 ++-- .../server/providers/sonos/__init__.py | 10 +++--- .../server/providers/ytmusic/__init__.py | 5 +-- .../server/providers/ytmusic/helpers.py | 5 ++- 15 files changed, 81 insertions(+), 53 deletions(-) diff --git a/music_assistant/common/models/media_items.py b/music_assistant/common/models/media_items.py index 8cdd22bd..06f26a1a 100755 --- a/music_assistant/common/models/media_items.py +++ b/music_assistant/common/models/media_items.py @@ -330,6 +330,13 @@ class Track(MediaItem): """ return self.metadata and self.metadata.chapters and len(self.metadata.chapters) > 1 + @property + def image(self) -> MediaItemImage | None: + """Return (first) image from metadata (prefer album).""" + if isinstance(self.album, Album) and self.album.image: + return self.album.image + return super().image + @dataclass(kw_only=True) class AlbumTrack(Track): diff --git a/music_assistant/server/controllers/media/artists.py b/music_assistant/server/controllers/media/artists.py index dca0b5c7..0cee628d 100644 --- a/music_assistant/server/controllers/media/artists.py +++ b/music_assistant/server/controllers/media/artists.py @@ -62,7 +62,7 @@ class ArtistsController(MediaControllerBase[Artist]): ) -> Artist: """Add artist to library and return the database item.""" if isinstance(item, ItemMapping): - metadata_lookup = True + metadata_lookup = False # grab musicbrainz id and additional metadata if metadata_lookup: await self.mass.metadata.get_artist_metadata(item) @@ -232,6 +232,7 @@ class ArtistsController(MediaControllerBase[Artist]): provider_instance_id_or_domain: str, ) -> list[Track]: """Return top tracks for an artist on given provider.""" + items = [] assert provider_instance_id_or_domain != "library" artist = await self.get(item_id, provider_instance_id_or_domain, add_to_library=False) cache_checksum = artist.metadata.checksum @@ -247,7 +248,6 @@ class ArtistsController(MediaControllerBase[Artist]): items = await prov.get_artist_toptracks(item_id) else: # fallback implementation using the db - items = [] if db_artist := await self.mass.music.artists.get_library_item_by_prov_id( item_id, provider_instance_id_or_domain, @@ -257,8 +257,8 @@ class ArtistsController(MediaControllerBase[Artist]): query += ( f" AND tracks.provider_mappings LIKE '%\"{provider_instance_id_or_domain}\"%'" ) - if paged_list := await self.mass.music.tracks.library_items(extra_query=query): - items = paged_list.items + paged_list = await self.mass.music.tracks.library_items(extra_query=query) + return paged_list.items # store (serializable items) in cache self.mass.create_task( self.mass.cache.set(cache_key, [x.to_dict() for x in items], checksum=cache_checksum) @@ -281,6 +281,7 @@ class ArtistsController(MediaControllerBase[Artist]): provider_instance_id_or_domain: str, ) -> list[Album]: """Return albums for an artist on given provider.""" + items = [] assert provider_instance_id_or_domain != "library" artist = await self.get_provider_item(item_id, provider_instance_id_or_domain) cache_checksum = artist.metadata.checksum @@ -307,10 +308,7 @@ class ArtistsController(MediaControllerBase[Artist]): f" AND albums.provider_mappings LIKE '%\"{provider_instance_id_or_domain}\"%'" ) paged_list = await self.mass.music.albums.library_items(extra_query=query) - items = paged_list.items - else: - # edge case - items = [] + return paged_list.items # store (serializable items) in cache self.mass.create_task( self.mass.cache.set(cache_key, [x.to_dict() for x in items], checksum=cache_checksum) diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 54345cd3..32e82d13 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -253,7 +253,7 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): else: items = searchresult.radio # store (serializable items) in cache - if not prov.domain.startswith("filesystem"): # do not cache filesystem results + if prov.is_streaming_provider: # do not cache filesystem results self.mass.create_task( self.mass.cache.set(cache_key, [x.to_dict() for x in items], expiration=86400 * 7) ) @@ -717,8 +717,4 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): "version": db_row_dict["album_version"], } db_row_dict["album"] = ItemMapping.from_dict(db_row_dict["album"]) - if not db_row_dict["metadata"]["images"]: - # copy album image in case the track has no image - album_metadata = json_loads(db_row_dict["album_metadata"]) - db_row_dict["metadata"]["images"] = album_metadata["images"] return db_row_dict diff --git a/music_assistant/server/controllers/media/tracks.py b/music_assistant/server/controllers/media/tracks.py index b81f9301..fe4e8642 100644 --- a/music_assistant/server/controllers/media/tracks.py +++ b/music_assistant/server/controllers/media/tracks.py @@ -130,7 +130,7 @@ class TracksController(MediaControllerBase[Track]): # grab additional metadata if metadata_lookup: await self.mass.metadata.get_track_metadata(item) - # fallback track image from album (only if albumtype = single) + # allow track image from album (only if albumtype = single) if ( not item.image and isinstance(item.album, Album) diff --git a/music_assistant/server/controllers/music.py b/music_assistant/server/controllers/music.py index d636e132..0b2605f7 100755 --- a/music_assistant/server/controllers/music.py +++ b/music_assistant/server/controllers/music.py @@ -239,7 +239,7 @@ class MusicController(CoreController): cache_key = f"{prov.instance_id}.search.{search_query}.{limit}.{media_types_str}" cache_key += "".join(x for x in media_types) - if cache := await self.mass.cache.get(cache_key): + if prov.is_streaming_provider and (cache := await self.mass.cache.get(cache_key)): return SearchResults.from_dict(cache) # no items in cache - get listing from provider result = await prov.search( @@ -248,9 +248,10 @@ class MusicController(CoreController): limit, ) # store (serializable items) in cache - self.mass.create_task( - self.mass.cache.set(cache_key, result.to_dict(), expiration=86400 * 7) - ) + if prov.is_streaming_provider: + self.mass.create_task( + self.mass.cache.set(cache_key, result.to_dict(), expiration=86400 * 7) + ) return result @api_command("music/browse") diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index ac3703b6..e363d23c 100755 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -245,21 +245,29 @@ class PlayerQueuesController(CoreController): cur_index = queue.index_in_buffer or 0 else: cur_index = queue.current_index or 0 - shuffle = queue.shuffle_enabled and len(queue_items) >= 5 + shuffle = queue.shuffle_enabled and len(queue_items) > 1 # handle replace: clear all items and replace with the new items if option == QueueOption.REPLACE: - self.load(queue_id, queue_items=queue_items, shuffle=shuffle) + self.load( + queue_id, + queue_items=queue_items, + keep_remaining=False, + keep_played=False, + shuffle=shuffle, + ) await self.play_index(queue_id, 0) + return # handle next: add item(s) in the index next to the playing/loaded/buffered index - elif option == QueueOption.NEXT: + if option == QueueOption.NEXT: self.load( queue_id, queue_items=queue_items, insert_at_index=cur_index + 1, shuffle=shuffle, ) - elif option == QueueOption.REPLACE_NEXT: + return + if option == QueueOption.REPLACE_NEXT: self.load( queue_id, queue_items=queue_items, @@ -267,19 +275,20 @@ class PlayerQueuesController(CoreController): keep_remaining=False, shuffle=shuffle, ) + return # handle play: replace current loaded/playing index with new item(s) - elif option == QueueOption.PLAY: - if cur_index <= len(self._queue_items[queue_id]) - 1: - cur_index = 0 + if option == QueueOption.PLAY: self.load( queue_id, queue_items=queue_items, - insert_at_index=cur_index, + insert_at_index=cur_index + 1, shuffle=shuffle, ) - await self.play_index(queue_id, cur_index) + next_index = min(cur_index + 1, len(self._queue_items[queue_id]) - 1) + await self.play_index(queue_id, next_index) + return # handle add: add/append item(s) to the remaining queue items - elif option == QueueOption.ADD: + if option == QueueOption.ADD: if queue.shuffle_enabled: # shuffle the new items with remaining queue items insert_at_index = cur_index + 1 @@ -744,6 +753,7 @@ class PlayerQueuesController(CoreController): queue_items: list[QueueItem], insert_at_index: int = 0, keep_remaining: bool = True, + keep_played: bool = True, shuffle: bool = False, ) -> None: """Load new items at index. @@ -754,11 +764,10 @@ class PlayerQueuesController(CoreController): - keep_remaining: keep the remaining items after the insert - shuffle: (re)shuffle the items after insert index """ - # keep previous/played items, append the new ones - prev_items = self._queue_items[queue_id][:insert_at_index] + prev_items = self._queue_items[queue_id][:insert_at_index] if keep_played else [] next_items = queue_items - # if keep_remaining, append the old previous items + # if keep_remaining, append the old 'next' items if keep_remaining: next_items += self._queue_items[queue_id][insert_at_index:] diff --git a/music_assistant/server/models/music_provider.py b/music_assistant/server/models/music_provider.py index aa7e6177..944d119e 100644 --- a/music_assistant/server/models/music_provider.py +++ b/music_assistant/server/models/music_provider.py @@ -412,7 +412,7 @@ class MusicProvider(Provider): try: if not library_item and not prov_item.available: # skip unavailable tracks - self.logger.debg( + self.logger.debug( "Skipping sync of item %s because it is unavailable", prov_item.uri ) continue diff --git a/music_assistant/server/providers/airplay/__init__.py b/music_assistant/server/providers/airplay/__init__.py index 0fb82ed9..6030e3a1 100644 --- a/music_assistant/server/providers/airplay/__init__.py +++ b/music_assistant/server/providers/airplay/__init__.py @@ -75,7 +75,7 @@ PLAYER_CONFIG_ENTRIES = ( advanced=True, ), ConfigEntry.from_dict( - {**CONF_ENTRY_OUTPUT_CODEC.to_dict(), "default_value": "pcm", "hidden": True} + {**CONF_ENTRY_OUTPUT_CODEC.to_dict(), "default_value": "flac", "hidden": True} ), ) @@ -431,9 +431,9 @@ class AirplayProvider(PlayerProvider): # set codecs and sample rate to airplay default common_elem = xml_root.find("common") - common_elem.find("codecs").text = "pcm" + common_elem.find("codecs").text = "flc,pcm" common_elem.find("sample_rate").text = "44100" - common_elem.find("resample").text = "0" + common_elem.find("resample").text = "1" common_elem.find("player_volume").text = "20" # default values for players diff --git a/music_assistant/server/providers/chromecast/__init__.py b/music_assistant/server/providers/chromecast/__init__.py index 212c5d23..36c9b8c9 100644 --- a/music_assistant/server/providers/chromecast/__init__.py +++ b/music_assistant/server/providers/chromecast/__init__.py @@ -441,10 +441,20 @@ class ChromecastProvider(PlayerProvider): self.mass.loop.call_soon_threadsafe(self.mass.players.update, castplayer.player_id) # enqueue next item if needed - if castplayer.player.state == PlayerState.PLAYING and ( - not castplayer.next_url or castplayer.next_url == castplayer.player.current_url + if ( + castplayer.player.state == PlayerState.PLAYING + and castplayer.player.active_source == castplayer.player.player_id + and castplayer.next_url in (None, castplayer.player.current_url) ): asyncio.run_coroutine_threadsafe(self._enqueue_next_track(castplayer), self.mass.loop) + # handle end of MA queue - set current item to None + if ( + castplayer.player.state == PlayerState.IDLE + and castplayer.player.current_url + and (queue := self.mass.player_queues.get(castplayer.player_id)) + and queue.next_item is None + ): + castplayer.player.current_url = None def on_new_connection_status(self, castplayer: CastPlayer, status: ConnectionStatus) -> None: """Handle updated ConnectionStatus.""" @@ -583,7 +593,7 @@ class ChromecastProvider(PlayerProvider): "artist": queue_item.media_item.artists[0].name if queue_item.media_item.artists else "", - "title": queue_item.name, + "title": queue_item.media_item.name, "images": [{"url": image_url}] if image_url else None, } else: diff --git a/music_assistant/server/providers/dlna/__init__.py b/music_assistant/server/providers/dlna/__init__.py index e2c9cb26..7db38698 100644 --- a/music_assistant/server/providers/dlna/__init__.py +++ b/music_assistant/server/providers/dlna/__init__.py @@ -633,8 +633,8 @@ class DLNAPlayerProvider(PlayerProvider): # enqueue next item if needed if ( dlna_player.player.state == PlayerState.PLAYING - and dlna_player.player.player_id in current_url - and (not dlna_player.next_url or dlna_player.next_url == current_url) + and dlna_player.player.active_source == dlna_player.player.player_id + and dlna_player.next_url in (None, dlna_player.player.current_url) # prevent race conditions at start/stop by doing this check and (time.time() - dlna_player.last_command) > 4 ): diff --git a/music_assistant/server/providers/filesystem_smb/__init__.py b/music_assistant/server/providers/filesystem_smb/__init__.py index 9f0537b4..1641eebb 100644 --- a/music_assistant/server/providers/filesystem_smb/__init__.py +++ b/music_assistant/server/providers/filesystem_smb/__init__.py @@ -136,7 +136,8 @@ class SMBFileSystemProvider(LocalFileSystemProvider): await makedirs(self.base_path) try: - await self.unmount() + # do unmount first to cleanup any unexpected state + await self.unmount(ignore_error=True) await self.mount() except Exception as err: raise LoginFailed(f"Connection failed for the given details: {err}") from err @@ -193,7 +194,7 @@ class SMBFileSystemProvider(LocalFileSystemProvider): if proc.returncode != 0: raise LoginFailed(f"SMB mount failed with error: {stderr.decode()}") - async def unmount(self) -> None: + async def unmount(self, ignore_error: bool = False) -> None: """Unmount the remote share.""" proc = await asyncio.create_subprocess_shell( f"umount {self.base_path}", @@ -201,5 +202,5 @@ class SMBFileSystemProvider(LocalFileSystemProvider): stderr=asyncio.subprocess.PIPE, ) _, stderr = await proc.communicate() - if proc.returncode != 0: + if proc.returncode != 0 and not ignore_error: self.logger.warning("SMB unmount failed with error: %s", stderr.decode()) diff --git a/music_assistant/server/providers/plex/__init__.py b/music_assistant/server/providers/plex/__init__.py index eedc7cda..a5242a0e 100644 --- a/music_assistant/server/providers/plex/__init__.py +++ b/music_assistant/server/providers/plex/__init__.py @@ -326,9 +326,9 @@ class PlexProvider(MusicProvider): album.artists.append( self._get_item_mapping( - media_type=MediaType.ARTIST, - url=plex_album.parentKey, - provider=plex_album.parentTitle, + MediaType.ARTIST, + plex_album.parentKey, + plex_album.parentTitle, ) ) return album diff --git a/music_assistant/server/providers/sonos/__init__.py b/music_assistant/server/providers/sonos/__init__.py index d164d1e0..687d20c7 100644 --- a/music_assistant/server/providers/sonos/__init__.py +++ b/music_assistant/server/providers/sonos/__init__.py @@ -254,7 +254,8 @@ class SonosPlayerProvider(PlayerProvider): for player_id in list(self.sonosplayers): player = self.sonosplayers.pop(player_id) player.player.available = False - player.soco.end_direct_control_session() + if player.soco.is_coordinator: + player.soco.end_direct_control_session() self.sonosplayers = None def on_player_config_changed( @@ -620,9 +621,10 @@ class SonosPlayerProvider(PlayerProvider): self.mass.players.update(sonos_player.player_id) # enqueue next item if needed - if sonos_player.player.state == PlayerState.PLAYING and ( - sonos_player.next_url is None - or sonos_player.next_url == sonos_player.player.current_url + if ( + sonos_player.player.state == PlayerState.PLAYING + and sonos_player.player.active_source == sonos_player.player.player_id + and sonos_player.next_url in (None, sonos_player.player.current_url) ): self.mass.create_task(self._enqueue_next_track(sonos_player)) diff --git a/music_assistant/server/providers/ytmusic/__init__.py b/music_assistant/server/providers/ytmusic/__init__.py index f2385e0d..6b3f667e 100644 --- a/music_assistant/server/providers/ytmusic/__init__.py +++ b/music_assistant/server/providers/ytmusic/__init__.py @@ -479,8 +479,7 @@ class YoutubeMusicProvider(MusicProvider): if track: tracks.append(track) except InvalidDataError: - track = await self.get_track(track["videoId"]) - if track: + if track := await self.get_track(track["videoId"]): tracks.append(track) return tracks return [] @@ -493,6 +492,8 @@ class YoutubeMusicProvider(MusicProvider): headers=self._headers, signature_timestamp=self._signature_timestamp, ) + if not track_obj: + raise MediaNotFoundError(f"Item {item_id} not found") stream_format = await self._parse_stream_format(track_obj) url = await self._parse_stream_url(stream_format=stream_format, item_id=item_id) if not await self._is_valid_deciphered_url(url=url): diff --git a/music_assistant/server/providers/ytmusic/helpers.py b/music_assistant/server/providers/ytmusic/helpers.py index e994ad0f..f4203427 100644 --- a/music_assistant/server/providers/ytmusic/helpers.py +++ b/music_assistant/server/providers/ytmusic/helpers.py @@ -65,13 +65,16 @@ async def get_playlist(prov_playlist_id: str, headers: dict[str, str]) -> dict[s async def get_track( prov_track_id: str, headers: dict[str, str], signature_timestamp: str -) -> dict[str, str]: +) -> dict[str, str] | None: """Async wrapper around the ytmusicapi get_playlist function.""" def _get_song(): ytm = ytmusicapi.YTMusic(auth=json.dumps(headers)) track_obj = ytm.get_song(videoId=prov_track_id, signatureTimestamp=signature_timestamp) track = {} + if "videoDetails" not in track_obj: + # video that no longer exists + return None track["videoId"] = track_obj["videoDetails"]["videoId"] track["title"] = track_obj["videoDetails"]["title"] track["artists"] = [ -- 2.34.1