From 5b49d4e631328d31da8138a84c0d95525422dff9 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 27 Jul 2023 19:41:01 +0200 Subject: [PATCH] Various fixed for DLNA based players (#799) * prevent value changed on iteration error * some small fixes * fix power state * add guard * allow higher quality flow stream * make 24 bit for explicit defined * some finishing touches --- music_assistant/common/models/media_items.py | 2 + music_assistant/server/controllers/players.py | 4 + music_assistant/server/controllers/streams.py | 20 ++--- .../server/providers/chromecast/__init__.py | 1 + .../server/providers/dlna/__init__.py | 90 +++++++++++++------ .../server/providers/slimproto/__init__.py | 3 +- .../server/providers/sonos/__init__.py | 1 + .../server/providers/ugp/__init__.py | 3 + 8 files changed, 87 insertions(+), 37 deletions(-) diff --git a/music_assistant/common/models/media_items.py b/music_assistant/common/models/media_items.py index 93bf20c8..a1f4ab4b 100755 --- a/music_assistant/common/models/media_items.py +++ b/music_assistant/common/models/media_items.py @@ -329,6 +329,8 @@ class ItemMapping(DataClassDictMixin): def __eq__(self, other: ItemMapping) -> bool: """Check equality of two items.""" + if other is None: + return False # guard return self.uri == other.uri diff --git a/music_assistant/server/controllers/players.py b/music_assistant/server/controllers/players.py index 977c8967..eaf59ad7 100755 --- a/music_assistant/server/controllers/players.py +++ b/music_assistant/server/controllers/players.py @@ -202,6 +202,10 @@ class PlayerController(CoreController): # handle special mute_as_power feature if player.mute_as_power: player.powered = player.powered and not player.volume_muted + elif player.state == PlayerState.PLAYING and not player.powered: + # mark player as powered if its playing + # could happen for players that do not officially support power commands + player.powered = True # basic throttle: do not send state changed events if player did not actually change prev_state = self._prev_states.get(player_id, {}) diff --git a/music_assistant/server/controllers/streams.py b/music_assistant/server/controllers/streams.py index 3bbeff3a..a8d68619 100644 --- a/music_assistant/server/controllers/streams.py +++ b/music_assistant/server/controllers/streams.py @@ -63,7 +63,7 @@ DEFAULT_STREAM_HEADERS = { "icy-name": "Music Assistant", "icy-pub": "0", } -FLOW_MAX_SAMPLE_RATE = 96000 +FLOW_MAX_SAMPLE_RATE = 192000 FLOW_MAX_BIT_DEPTH = 24 @@ -144,7 +144,7 @@ class MultiClientStreamJob: # handle raw pcm if output_codec.is_pcm(): player = self.stream_controller.mass.players.get(child_player_id) - player_max_bit_depth = 32 if player.supports_24bit else 16 + player_max_bit_depth = 24 if player.supports_24bit else 16 output_sample_rate = min(self.pcm_format.sample_rate, player.max_sample_rate) output_bit_depth = min(self.pcm_format.bit_depth, player_max_bit_depth) output_channels = await self.stream_controller.mass.config.get_player_config_value( @@ -388,7 +388,7 @@ class StreamsController(CoreController): # handle raw pcm if output_codec.is_pcm(): player = self.mass.players.get(queue_id) - player_max_bit_depth = 32 if player.supports_24bit else 16 + player_max_bit_depth = 24 if player.supports_24bit else 16 if flow_mode: output_sample_rate = min(FLOW_MAX_SAMPLE_RATE, player.max_sample_rate) output_bit_depth = min(FLOW_MAX_BIT_DEPTH, player_max_bit_depth) @@ -433,16 +433,16 @@ class StreamsController(CoreController): # cleanup existing job first if not existing_job.finished: existing_job.stop() - + queue_player = self.mass.players.get(queue_id) + pcm_bit_depth = 24 if queue_player.supports_24bit else 16 + pcm_sample_rate = min(queue_player.max_sample_rate, 96000) self.multi_client_jobs[queue_id] = stream_job = MultiClientStreamJob( self, queue_id=queue_id, pcm_format=AudioFormat( - # hardcoded pcm quality of 48/24 for now - # TODO: change this to the highest quality supported by all child players ? - content_type=ContentType.from_bit_depth(24), - sample_rate=48000, - bit_depth=24, + content_type=ContentType.from_bit_depth(pcm_bit_depth), + sample_rate=pcm_sample_rate, + bit_depth=pcm_bit_depth, channels=2, ), start_queue_item=start_queue_item, @@ -999,7 +999,7 @@ class StreamsController(CoreController): else: output_sample_rate = min(default_sample_rate, queue_player.max_sample_rate) - player_max_bit_depth = 32 if queue_player.supports_24bit else 16 + player_max_bit_depth = 24 if queue_player.supports_24bit else 16 output_bit_depth = min(default_bit_depth, player_max_bit_depth) output_channels_str = await self.mass.config.get_player_config_value( queue_player.player_id, CONF_OUTPUT_CHANNELS diff --git a/music_assistant/server/providers/chromecast/__init__.py b/music_assistant/server/providers/chromecast/__init__.py index be7ad9f5..212c5d23 100644 --- a/music_assistant/server/providers/chromecast/__init__.py +++ b/music_assistant/server/providers/chromecast/__init__.py @@ -352,6 +352,7 @@ class ChromecastProvider(PlayerProvider): PlayerFeature.VOLUME_SET, ), max_sample_rate=96000, + supports_24bit=True, enabled_by_default=enabled_by_default, ), logger=self.logger.getChild(cast_info.friendly_name), diff --git a/music_assistant/server/providers/dlna/__init__.py b/music_assistant/server/providers/dlna/__init__.py index 1d467ece..d9fedf51 100644 --- a/music_assistant/server/providers/dlna/__init__.py +++ b/music_assistant/server/providers/dlna/__init__.py @@ -11,6 +11,7 @@ import asyncio import functools import time from collections.abc import Awaitable, Callable, Coroutine, Sequence +from contextlib import suppress from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Concatenate, ParamSpec, TypeVar @@ -87,12 +88,11 @@ def catch_request_errors( """Catch UpnpError errors and check availability before and after request.""" player_id = kwargs["player_id"] if "player_id" in kwargs else args[0] dlna_player = self.dlnaplayers[player_id] + dlna_player.last_command = time.time() self.logger.debug( - "Handling command %s for player %s - using args: %s %s", + "Handling command %s for player %s", func.__name__, dlna_player.player.display_name, - str(args), - str(kwargs), ) if not dlna_player.available: self.logger.warning("Device disappeared when trying to call %s", func.__name__) @@ -127,8 +127,9 @@ class DLNAPlayer: last_seen: float = field(default_factory=time.time) next_url: str | None = None next_item: QueueItem | None = None - supports_next_uri = True - end_of_track_reached = False + supports_next_uri: bool | None = None + end_of_track_reached: float | None = None + last_command: float = field(default_factory=time.time) def update_attributes(self): """Update attributes of the MA Player from DLNA state.""" @@ -147,10 +148,15 @@ class DLNAPlayer: self.player.elapsed_time_last_updated = ( self.device.media_position_updated_at.timestamp() ) - if self.device.media_duration and self.player.corrected_elapsed_time: - self.end_of_track_reached = ( - self.device.media_duration - self.player.corrected_elapsed_time - ) < 15 + # some dlna players get stuck at the end of the track and won't + # automatically play the next track, try to workaround that + if ( + self.device.media_duration + and self.player.corrected_elapsed_time + and self.player.state == PlayerState.PLAYING + and (self.device.media_duration - self.player.corrected_elapsed_time) <= 10 + ): + self.end_of_track_reached = time.time() else: # device is unavailable self.player.available = False @@ -242,7 +248,7 @@ class DLNAPlayerProvider(PlayerProvider): async def cmd_stop(self, player_id: str) -> None: """Send STOP command to given player.""" dlna_player = self.dlnaplayers[player_id] - dlna_player.end_of_track_reached = False + dlna_player.end_of_track_reached = None dlna_player.next_url = None assert dlna_player.device is not None await dlna_player.device.async_stop() @@ -276,6 +282,8 @@ class DLNAPlayerProvider(PlayerProvider): # always clear queue (by sending stop) first if dlna_player.device.can_stop: await self.cmd_stop(player_id) + dlna_player.next_url = None + dlna_player.end_of_track_reached = None didl_metadata = create_didl_metadata(self.mass, url, queue_item) title = queue_item.name if queue_item else "Music Assistant" @@ -284,7 +292,7 @@ class DLNAPlayerProvider(PlayerProvider): await dlna_player.device.async_wait_for_can_play(10) await dlna_player.device.async_play() # force poll the device - for sleep in (0, 1, 2): + for sleep in (1, 2): await asyncio.sleep(sleep) dlna_player.force_poll = True await self.poll_player(dlna_player.udn) @@ -344,7 +352,8 @@ class DLNAPlayerProvider(PlayerProvider): try: now = time.time() do_ping = dlna_player.force_poll or (now - dlna_player.last_seen) > 60 - await dlna_player.device.async_update(do_ping=do_ping) + with suppress(ValueError): + await dlna_player.device.async_update(do_ping=do_ping) dlna_player.last_seen = now if do_ping else dlna_player.last_seen except UpnpError as err: self.logger.debug("Device unavailable: %r", err) @@ -385,7 +394,7 @@ class DLNAPlayerProvider(PlayerProvider): await self._device_discovered(ssdp_udn, discovery_info["location"]) - await async_search(on_response, 60) + await async_search(on_response) finally: self._discovery_running = False @@ -394,7 +403,7 @@ class DLNAPlayerProvider(PlayerProvider): self.mass.create_task(self._run_discovery()) # reschedule self once finished - self.mass.loop.call_later(300, reschedule) + self.mass.loop.call_later(120, reschedule) async def _device_disconnect(self, dlna_player: DLNAPlayer) -> None: """ @@ -434,6 +443,8 @@ class DLNAPlayerProvider(PlayerProvider): self.logger.debug("Ignoring disabled player: %s", udn) return + is_sonos = "rincon" in udn.lower() + dlna_player = DLNAPlayer( udn=udn, player=Player( @@ -450,8 +461,10 @@ class DLNAPlayerProvider(PlayerProvider): address=description_url, manufacturer="unknown", ), + max_sample_rate=48000 if is_sonos else 192000, + supports_24bit=True, # disable sonos players by default in dlna - enabled_by_default="rincon" not in udn.lower(), + enabled_by_default=not is_sonos, ), description_url=description_url, ) @@ -507,13 +520,7 @@ class DLNAPlayerProvider(PlayerProvider): ) -> None: """Handle state variable(s) changed event from DLNA device.""" udn = service.device.udn - dlna_player = self.dlnaplayers[udn] - self.logger.debug( - "Received event for Player %s: %s", - dlna_player.player.display_name, - service, - ) if not state_variables: # Indicates a failure to resubscribe, check if device is still available @@ -530,6 +537,11 @@ class DLNAPlayerProvider(PlayerProvider): ): dlna_player.force_poll = True self.mass.create_task(self.poll_player(dlna_player.udn)) + self.logger.debug( + "Received new state from event for Player %s: %s", + dlna_player.player.display_name, + state_variable.value, + ) dlna_player.last_seen = time.time() self.mass.create_task(self._update_player(dlna_player)) @@ -551,7 +563,7 @@ class DLNAPlayerProvider(PlayerProvider): dlna_player.next_item = next_item # no need to try setting the next url if we already know the player does not support it - if not dlna_player.supports_next_uri: + if dlna_player.supports_next_uri is False: return # send queue item to dlna queue @@ -561,7 +573,15 @@ class DLNAPlayerProvider(PlayerProvider): await dlna_player.device.async_set_next_transport_uri(next_url, title, didl_metadata) except UpnpError: dlna_player.supports_next_uri = False - self.logger.info("Player does not support next uri") + self.logger.info( + "Player does not support next transport uri feature, " + "gapless playback is not possible." + ) + else: + # log once if we detected that the player supports the next transport uri + if dlna_player.supports_next_uri is None: + dlna_player.supports_next_uri = True + self.logger.debug("Player supports the next transport uri feature.") self.logger.debug( "Enqued next track (%s) to player %s", @@ -585,18 +605,36 @@ class DLNAPlayerProvider(PlayerProvider): self.mass.players.update(dlna_player.udn) # enqueue next item if needed - if dlna_player.player.state == PlayerState.PLAYING and ( - not dlna_player.next_url or dlna_player.next_url == current_url + if ( + dlna_player.player.state == PlayerState.PLAYING + and (not dlna_player.next_url or dlna_player.next_url == current_url) + # prevent race conditions at start/stop by doing this check + and (time.time() - dlna_player.last_command) > 10 ): self.mass.create_task(self._enqueue_next_track(dlna_player)) + # try to detect a player that gets stuck at the end of the track + if ( + dlna_player.end_of_track_reached + and dlna_player.next_url + and dlna_player.supports_next_uri + and time.time() - dlna_player.end_of_track_reached > 10 + ): + self.logger.warning( + "Detected that the player is stuck at the end of the track, " + "enabling workaround for this player." + ) + dlna_player.supports_next_uri = False # if player does not support next uri, manual play it if ( not dlna_player.supports_next_uri and prev_state == PlayerState.PLAYING and current_state == PlayerState.IDLE and dlna_player.next_url - and dlna_player.end_of_track_reached ): + self.logger.warning( + "Player does not support next_uri and end of track reached, " + "sending next url manually." + ) await self.cmd_play_url(dlna_player.udn, dlna_player.next_url, dlna_player.next_item) dlna_player.end_of_track_reached = False dlna_player.next_url = None diff --git a/music_assistant/server/providers/slimproto/__init__.py b/music_assistant/server/providers/slimproto/__init__.py index c273d1e1..2c90bd6c 100644 --- a/music_assistant/server/providers/slimproto/__init__.py +++ b/music_assistant/server/providers/slimproto/__init__.py @@ -583,6 +583,7 @@ class SlimprotoProvider(PlayerProvider): PlayerFeature.VOLUME_SET, ), max_sample_rate=int(client.max_sample_rate), + supports_24bit=int(client.max_sample_rate) > 44100, ) if virtual_provider_info: # if this player is part of a virtual provider run the callback @@ -776,7 +777,7 @@ class SlimprotoProvider(PlayerProvider): # update all attributes await self._handle_player_update(client) # update existing players so they can update their `can_sync_with` field - for item in self._socket_clients.values(): + for item in list(self._socket_clients.values()): if item.player_id == player_id: continue await self._handle_player_update(item) diff --git a/music_assistant/server/providers/sonos/__init__.py b/music_assistant/server/providers/sonos/__init__.py index dc4d3099..97da3d80 100644 --- a/music_assistant/server/providers/sonos/__init__.py +++ b/music_assistant/server/providers/sonos/__init__.py @@ -441,6 +441,7 @@ class SonosPlayerProvider(PlayerProvider): manufacturer=self.name, ), max_sample_rate=48000, + supports_24bit=True, ), speaker_info=speaker_info, speaker_info_updated=time.time(), diff --git a/music_assistant/server/providers/ugp/__init__.py b/music_assistant/server/providers/ugp/__init__.py index da670872..fdc5d933 100644 --- a/music_assistant/server/providers/ugp/__init__.py +++ b/music_assistant/server/providers/ugp/__init__.py @@ -169,6 +169,8 @@ class UniversalGroupProvider(PlayerProvider): PlayerFeature.VOLUME_MUTE, PlayerFeature.SET_MEMBERS, ), + max_sample_rate=96000, + supports_24bit=True, active_source=conf_key, group_childs=player_conf, ) @@ -382,6 +384,7 @@ class UniversalGroupProvider(PlayerProvider): all_members = self._get_active_members( player_id, only_powered=False, skip_sync_childs=False ) + group_player.max_sample_rate = max(x.max_sample_rate for x in all_members) group_player.group_childs = list(x.player_id for x in all_members) # read the state from the first active group member for member in all_members: -- 2.34.1