From b53ffd96e67f2c327c2f5571bd9efc68d93a35c1 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Sun, 26 Mar 2023 17:56:40 +0200 Subject: [PATCH] Various small fixes (#578) * Fix images for items in the queue * Fix queue item detection for sonos in flow mode * fix some issues in the matching logic * Fix refresh item * change to debug logging * typos * try to fix timeout --- music_assistant/common/models/queue_item.py | 3 +- .../server/controllers/media/artists.py | 2 +- .../server/controllers/media/tracks.py | 9 +++++- .../server/controllers/metadata.py | 4 +-- music_assistant/server/controllers/music.py | 19 ++++++----- .../server/controllers/player_queues.py | 27 ++++++++++------ music_assistant/server/controllers/players.py | 5 +-- music_assistant/server/controllers/streams.py | 18 +++++------ music_assistant/server/helpers/compare.py | 32 +++++++++++++------ music_assistant/server/helpers/process.py | 2 +- .../server/providers/sonos/__init__.py | 14 +++----- 11 files changed, 79 insertions(+), 56 deletions(-) diff --git a/music_assistant/common/models/queue_item.py b/music_assistant/common/models/queue_item.py index ce89d4e3..a5705d74 100644 --- a/music_assistant/common/models/queue_item.py +++ b/music_assistant/common/models/queue_item.py @@ -74,11 +74,12 @@ class QueueItem(DataClassDictMixin): name=name, duration=media_item.duration, media_item=media_item, + image_url=media_item.image.url if media_item.image else None, ) async def resolve_image_url(self, mass: MusicAssistant) -> None: """Resolve Image URL for the MediaItem.""" - if self.image_url: + if self.image_url and self.image_url.startswith("http"): return if not self.media_item: return diff --git a/music_assistant/server/controllers/media/artists.py b/music_assistant/server/controllers/media/artists.py index 1766a7b5..a558e478 100644 --- a/music_assistant/server/controllers/media/artists.py +++ b/music_assistant/server/controllers/media/artists.py @@ -404,7 +404,7 @@ class ArtistsController(MediaControllerBase[Artist]): # make sure we have a full track if isinstance(ref_track.album, ItemMapping): ref_track = await self.mass.music.tracks.get( # noqa: PLW2901 - ref_track.item_id, ref_track.provider + ref_track.item_id, ref_track.provider, add_to_db=False ) for search_str in ( f"{db_artist.name} - {ref_track.name}", diff --git a/music_assistant/server/controllers/media/tracks.py b/music_assistant/server/controllers/media/tracks.py index 54ddf48e..e8850cad 100644 --- a/music_assistant/server/controllers/media/tracks.py +++ b/music_assistant/server/controllers/media/tracks.py @@ -181,7 +181,14 @@ class TracksController(MediaControllerBase[Track]): for search_result_item in search_result: if not search_result_item.available: continue - if compare_track(search_result_item, db_track): + # do a basic compare first + if not compare_track(search_result_item, db_track): + continue + # we must fetch the full album version, search results are simplified objects + prov_track = await self.get_provider_item( + search_result_item.item_id, search_result_item.provider + ) + if compare_track(prov_track, db_track): # 100% match, we can simply update the db with additional provider ids match_found = True await self.update_db_item(db_track.item_id, search_result_item) diff --git a/music_assistant/server/controllers/metadata.py b/music_assistant/server/controllers/metadata.py index bdd18b7d..b9931d05 100755 --- a/music_assistant/server/controllers/metadata.py +++ b/music_assistant/server/controllers/metadata.py @@ -84,7 +84,7 @@ class MetaDataController: if self.scan_busy: return - LOGGER.info("Start scan for missing artist metadata") + LOGGER.debug("Start scan for missing artist metadata") self.scan_busy = True async for artist in self.mass.music.artists.iter_db_items(): if artist.metadata.last_refresh is not None: @@ -100,7 +100,7 @@ class MetaDataController: # this is slow on purpose to not cause stress on the metadata providers await asyncio.sleep(30) self.scan_busy = False - LOGGER.info("Finished scan for missing artist metadata") + LOGGER.debug("Finished scan for missing artist metadata") self.mass.create_task(scan_artist_metadata) diff --git a/music_assistant/server/controllers/music.py b/music_assistant/server/controllers/music.py index 673b07f7..735aeb8e 100755 --- a/music_assistant/server/controllers/music.py +++ b/music_assistant/server/controllers/music.py @@ -11,12 +11,7 @@ from music_assistant.common.helpers.datetime import utc_timestamp from music_assistant.common.helpers.uri import parse_uri from music_assistant.common.models.enums import EventType, MediaType, ProviderFeature, ProviderType from music_assistant.common.models.errors import MusicAssistantError -from music_assistant.common.models.media_items import ( - BrowseFolder, - MediaItem, - MediaItemType, - SearchResults, -) +from music_assistant.common.models.media_items import BrowseFolder, MediaItemType, SearchResults from music_assistant.common.models.provider import SyncTask from music_assistant.constants import ( CONF_DB_LIBRARY, @@ -282,6 +277,7 @@ class MusicController: provider_instance: str | None = None, force_refresh: bool = False, lazy: bool = True, + add_to_db: bool = False, ) -> MediaItemType: """Get single music item by id and media type.""" assert ( @@ -297,6 +293,7 @@ class MusicController: provider_instance=provider_instance, force_refresh=force_refresh, lazy=lazy, + add_to_db=add_to_db, ) @api_command("music/library/add") @@ -375,7 +372,7 @@ class MusicController: ctrl = self.get_controller(media_type) await ctrl.delete_db_item(db_item_id, recursive) - async def refresh_items(self, items: list[MediaItem]) -> None: + async def refresh_items(self, items: list[MediaItemType]) -> None: """Refresh MediaItems to force retrieval of full info and matches. Creates background tasks to process the action. @@ -383,9 +380,10 @@ class MusicController: for media_item in items: self.mass.create_task(self.refresh_item(media_item)) + @api_command("music/refresh_item") async def refresh_item( self, - media_item: MediaItem, + media_item: MediaItemType, ): """Try to refresh a mediaitem by requesting it's full object or search for substitutes.""" try: @@ -395,6 +393,7 @@ class MusicController: provider_domain=media_item.provider, force_refresh=True, lazy=False, + add_to_db=True, ) except MusicAssistantError: pass @@ -465,7 +464,7 @@ class MusicController: allow_replace=True, ) - async def library_add_items(self, items: list[MediaItem]) -> None: + async def library_add_items(self, items: list[MediaItemType]) -> None: """Add media item(s) to the library. Creates background tasks to process the action. @@ -475,7 +474,7 @@ class MusicController: self.add_to_library(media_item.media_type, media_item.item_id, media_item.provider) ) - async def library_remove_items(self, items: list[MediaItem]) -> None: + async def library_remove_items(self, items: list[MediaItemType]) -> None: """Remove media item(s) from the library. Creates background tasks to process the action. diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index 0ecd38ba..889b41be 100755 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -522,15 +522,16 @@ class PlayerQueuesController: if queue.active: # update current item from player report player_item_index = self.index_by_id(queue_id, player.current_item_id) - if player_item_index is not None: - if queue.flow_mode: - # flow mode active, calculate current item - ( - queue.current_index, - queue.elapsed_time, - ) = self.__get_queue_stream_index(queue, player, player_item_index) - else: - queue.current_index = player_item_index + if player_item_index is None: + player_item_index = self._get_player_item_index(queue_id, player.current_url) + if queue.flow_mode: + # flow mode active, calculate current item + ( + queue.current_index, + queue.elapsed_time, + ) = self.__get_queue_stream_index(queue, player, player_item_index) + else: + queue.current_index = player_item_index queue.current_item = self.get_item(queue_id, queue.current_index) queue.next_item = self.get_next_item(queue_id) @@ -799,3 +800,11 @@ class PlayerQueuesController: track_time = elapsed_time_queue + track_sec_skipped - total_time break return queue_index, track_time + + def _get_player_item_index(self, queue_id: str, url: str) -> str | None: + """Parse QueueItem ID from Player's current url.""" + if url and self.mass.webserver.base_url in url and "/stream/" in url: + # try to extract the item id from the uri + current_item_id = url.rsplit("/")[-2] + return self.index_by_id(queue_id, current_item_id) + return None diff --git a/music_assistant/server/controllers/players.py b/music_assistant/server/controllers/players.py index 6a3a431d..27f9044b 100755 --- a/music_assistant/server/controllers/players.py +++ b/music_assistant/server/controllers/players.py @@ -529,7 +529,7 @@ class PlayerController: player.active_queue == player.player_id and player.state == PlayerState.PLAYING ) if player_playing: - self.update(player_id) + self.mass.loop.call_soon(self.update, player_id) # Poll player; # - every 360 seconds if the player if not powered # - every 30 seconds if the player is powered @@ -546,7 +546,7 @@ class PlayerController: player.available = False player.state = PlayerState.IDLE player.powered = False - self.update(player_id) + self.mass.loop.call_soon(self.update, player_id) except Exception as err: # pylint: disable=broad-except LOGGER.warning( "Error while requesting latest state from player %s: %s", @@ -556,4 +556,5 @@ class PlayerController: ) if count >= 360: count = 0 + await asyncio.sleep(0) await asyncio.sleep(1) diff --git a/music_assistant/server/controllers/streams.py b/music_assistant/server/controllers/streams.py index fe77b8ac..cc157835 100644 --- a/music_assistant/server/controllers/streams.py +++ b/music_assistant/server/controllers/streams.py @@ -128,7 +128,7 @@ class StreamJob: if len(self.subscribers) == 0 and self._audio_task and not self.finished: self._audio_task.cancel() - async def _put_data(self, data: Any, timeout: float = 600) -> None: + async def _put_data(self, data: Any, timeout: float = 1200) -> None: """Put chunk of data to all subscribers.""" async with asyncio.timeout(timeout): async with asyncio.TaskGroup() as tg: @@ -378,6 +378,7 @@ class StreamsController: " please create an issue report on the Music Assistant issue tracker.", player.display_name, ) + self.mass.create_task(self.mass.players.queues.next(player_id)) raise web.HTTPBadRequest(reason="Stream is already running.") # all checks passed, start streaming! @@ -415,14 +416,13 @@ class StreamsController: await resp.write(chunk) bytes_streamed += len(chunk) - # DISABLE FOR NOW TO AVOID ISSUES WITH SONOS ICW YOUTUBE MUSIC - # do not allow the player to prebuffer more than 30 seconds - # seconds_streamed = int(bytes_streamed / stream_job.pcm_sample_size) - # if ( - # seconds_streamed > 30 - # and (seconds_streamed - player.corrected_elapsed_time) > 30 - # ): - # await asyncio.sleep(1) + # do not allow the player to prebuffer more than 60 seconds + seconds_streamed = int(bytes_streamed / stream_job.pcm_sample_size) + if ( + seconds_streamed > 120 + and (seconds_streamed - player.corrected_elapsed_time) > 30 + ): + await asyncio.sleep(1) if not enable_icy: continue diff --git a/music_assistant/server/helpers/compare.py b/music_assistant/server/helpers/compare.py index 8b9128c8..d107aca3 100644 --- a/music_assistant/server/helpers/compare.py +++ b/music_assistant/server/helpers/compare.py @@ -30,7 +30,7 @@ def loose_compare_strings(base: str, alt: str) -> bool: 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: + if not str1 or not str2: return False # return early if total length mismatch if abs(len(str1) - len(str2)) > 4: @@ -106,6 +106,10 @@ def compare_item_ids( left_item: MediaItem | ItemMapping, right_item: MediaItem | ItemMapping ) -> bool: """Compare item_id(s) of two media items.""" + if not left_item.provider or not right_item.provider: + return False + if not left_item.item_id or not right_item.item_id: + return False if left_item.provider == right_item.provider and left_item.item_id == right_item.item_id: return True @@ -153,7 +157,11 @@ def compare_barcode( ): """Compare two sets of barcodes and return True if a match was found.""" for left_barcode in left_barcodes: + if not left_barcode.strip(): + continue for right_barcode in right_barcodes: + if not right_barcode.strip(): + continue # convert EAN-13 to UPC-A by stripping off the leading zero left_upc = left_barcode[1:] if left_barcode.startswith("0") else left_barcode right_upc = right_barcode[1:] if right_barcode.startswith("0") else right_barcode @@ -168,7 +176,11 @@ def compare_isrc( ): """Compare two sets of isrc codes and return True if a match was found.""" for left_isrc in left_isrcs: + if not left_isrc.strip(): + continue for right_isrc in right_isrcs: + if not right_isrc.strip(): + continue if compare_strings(left_isrc, right_isrc): return True return False @@ -237,15 +249,6 @@ def compare_track(left_track: Track, right_track: Track): # track name must match if not compare_strings(left_track.name, right_track.name, False): return False - # exact albumtrack match = 100% match - if ( - compare_album(left_track.album, right_track.album) - and left_track.track_number - and right_track.track_number - and ((left_track.disc_number or 1) == (right_track.disc_number or 1)) - and left_track.track_number == right_track.track_number - ): - return True # track version must match if not compare_version(left_track.version, right_track.version): return False @@ -255,6 +258,15 @@ def compare_track(left_track: Track, right_track: Track): # track if both tracks are (not) explicit if not compare_explicit(left_track.metadata, right_track.metadata): return False + # exact albumtrack match = 100% match + if ( + compare_album(left_track.album, right_track.album) + and left_track.track_number + and right_track.track_number + and ((left_track.disc_number or 1) == (right_track.disc_number or 1)) + and left_track.track_number == right_track.track_number + ): + return True # exact album match = 100% match if left_track.albums and right_track.albums: for left_album in left_track.albums: diff --git a/music_assistant/server/helpers/process.py b/music_assistant/server/helpers/process.py index 6e9c60a8..0d584c34 100644 --- a/music_assistant/server/helpers/process.py +++ b/music_assistant/server/helpers/process.py @@ -12,7 +12,7 @@ from collections.abc import AsyncGenerator, Coroutine LOGGER = logging.getLogger(__name__) DEFAULT_CHUNKSIZE = 128000 -DEFAULT_TIMEOUT = 600 +DEFAULT_TIMEOUT = 30 * 60 # pylint: disable=invalid-name diff --git a/music_assistant/server/providers/sonos/__init__.py b/music_assistant/server/providers/sonos/__init__.py index 349ea02b..5dab9d33 100644 --- a/music_assistant/server/providers/sonos/__init__.py +++ b/music_assistant/server/providers/sonos/__init__.py @@ -109,24 +109,18 @@ class SonosPlayer: self.track_info_updated = time.time() self.elapsed_time = _timespan_secs(self.track_info["position"]) or 0 + current_item_id = None if track_metadata := self.track_info.get("metadata"): # extract queue_item_id from metadata xml try: xml_root = ET.XML(track_metadata) for match in xml_root.iter("{http://purl.org/dc/elements/1.1/}queueItemId"): item_id = match.text - self.current_item_id = item_id + current_item_id = item_id break except (ET.ParseError, AttributeError): - self.current_item_id = None - - if ( - self.current_item_id is None - and "/stream/" in self.track_info["uri"] - and self.player_id in self.track_info["uri"] - ): - # try to extract the item id from the uri - self.current_item_id = self.track_info["uri"].rsplit("/")[-2] + pass + self.current_item_id = current_item_id # speaker info if update_speaker_info: -- 2.34.1