From: Maxim Raznatovski Date: Thu, 16 Jan 2025 18:15:47 +0000 (+0100) Subject: Fix: DSP edge cases around groups (#1879) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=3b4304f8a2cff55d1e6eaa77af7f91ef0b48c6fe;p=music-assistant-server.git Fix: DSP edge cases around groups (#1879) * fix: rework DSP reload triggers on group/ungroup * fix: DSP edge cases with single group members * fix: disabled DSPDetails of group members * chore: allow `Too many statements` for now on `update` --- diff --git a/music_assistant/controllers/players.py b/music_assistant/controllers/players.py index ff82ab57..9de73151 100644 --- a/music_assistant/controllers/players.py +++ b/music_assistant/controllers/players.py @@ -886,7 +886,7 @@ class PlayerController(CoreController): self._prev_states.pop(player_id, None) self.mass.signal_event(EventType.PLAYER_REMOVED, player_id) - def update( + def update( # noqa: PLR0915 self, player_id: str, skip_forward: bool = False, force_update: bool = False ) -> None: """Update player state.""" @@ -964,18 +964,45 @@ class PlayerController(CoreController): if len(changed_values) == 0 and not force_update: return - # handle DSP reload when player is grouped or ungrouped - prev_is_grouped = bool(prev_state.get("synced_to")) or bool(prev_state.get("group_childs")) - new_is_grouped = bool(new_state.get("synced_to")) or bool(new_state.get("group_childs")) + # handle DSP reload of the leader when on grouping and ungrouping + prev_child_count = len(prev_state.get("group_childs", [])) + new_child_count = len(new_state.get("group_childs", [])) + is_player_group = player.provider.startswith("player_group") - if prev_is_grouped != new_is_grouped: - dsp_config = self.mass.config.get_player_dsp_config(player_id) + # handle special case for PlayerGroups: since there are no leaders, + # DSP still always works with a single player in the group. + multi_device_dsp_threshold = 1 if is_player_group else 0 + + prev_is_multiple_devices = prev_child_count > multi_device_dsp_threshold + new_is_multiple_devices = new_child_count > multi_device_dsp_threshold + + if prev_is_multiple_devices != new_is_multiple_devices: supports_multi_device_dsp = PlayerFeature.MULTI_DEVICE_DSP in player.supported_features - if dsp_config.enabled and not supports_multi_device_dsp: - # We now know that that the player was grouped or ungrouped, - # the player has a custom DSP enabled, but the player provider does - # not support multi-device DSP. - # So we need to reload the DSP configuration. + dsp_enabled: bool + if is_player_group: + # Since player groups do not have leaders, we will use the only child + # that was in the group before and after the change + if prev_is_multiple_devices: + if childs := new_state.get("group_childs"): + # We shrank the group from multiple players to a single player + # So the now only child will control the DSP + dsp_enabled = self.mass.config.get_player_dsp_config(childs[0]).enabled + else: + dsp_enabled = False + elif childs := prev_state.get("group_childs"): + # We grew the group from a single player to multiple players, + # let's see if the previous single player had DSP enabled + dsp_enabled = self.mass.config.get_player_dsp_config(childs[0]).enabled + else: + dsp_enabled = False + else: + dsp_enabled = self.mass.config.get_player_dsp_config(player_id).enabled + if dsp_enabled and not supports_multi_device_dsp: + # We now know that that the group configuration has changed so: + # - multi-device DSP is not supported + # - we switched from a group with multiple players to a single player + # (or vice versa) + # - the leader has DSP enabled self.mass.create_task(self.mass.players.on_player_dsp_change(player_id)) if changed_values.keys() != {"elapsed_time"} or force_update: diff --git a/music_assistant/helpers/audio.py b/music_assistant/helpers/audio.py index 197436af..71e9c40f 100644 --- a/music_assistant/helpers/audio.py +++ b/music_assistant/helpers/audio.py @@ -185,11 +185,20 @@ async def strip_silence( return stripped_data -def get_player_dsp_details(mass: MusicAssistant, player: Player) -> DSPDetails: - """Return DSP details of single a player.""" +def get_player_dsp_details( + mass: MusicAssistant, player: Player, group_preventing_dsp=False +) -> DSPDetails: + """Return DSP details of single a player. + + This will however not check if the queried player is part of a group. + The caller is responsible for passing the result of is_grouping_preventing_dsp of + the leader/PlayerGroup as the group_preventing_dsp argument in such cases. + """ dsp_config = mass.config.get_player_dsp_config(player.player_id) dsp_state = DSPState.ENABLED if dsp_config.enabled else DSPState.DISABLED - if dsp_state == DSPState.ENABLED and is_grouping_preventing_dsp(player): + if dsp_state == DSPState.ENABLED and ( + group_preventing_dsp or is_grouping_preventing_dsp(player) + ): dsp_state = DSPState.DISABLED_BY_UNSUPPORTED_GROUP dsp_config = DSPConfig(enabled=False) @@ -212,6 +221,7 @@ def get_stream_dsp_details( """Return DSP details of all players playing this queue, keyed by player_id.""" player = mass.players.get(queue_id) dsp = {} + group_preventing_dsp: bool = False # We skip the PlayerGroups as they don't provide an audio output # by themselves, but only sync other players. @@ -219,12 +229,16 @@ def get_stream_dsp_details( details = get_player_dsp_details(mass, player) details.is_leader = True dsp[player.player_id] = details + else: + group_preventing_dsp = is_grouping_preventing_dsp(player) if player and player.group_childs: # grouped playback, get DSP details for each player in the group for child_id in player.group_childs: if child_player := mass.players.get(child_id): - dsp[child_id] = get_player_dsp_details(mass, child_player) + dsp[child_id] = get_player_dsp_details( + mass, child_player, group_preventing_dsp=group_preventing_dsp + ) return dsp @@ -917,13 +931,24 @@ def get_chunksize( def is_grouping_preventing_dsp(player: Player) -> bool: - """Check if grouping is preventing DSP from being applied. + """Check if grouping is preventing DSP from being applied to this leader/PlayerGroup. If this returns True, no DSP should be applied to the player. + This function will not check if the Player is in a group, the caller should do that first. """ - is_grouped = bool(player.synced_to) or bool(player.group_childs) + # We require the caller to handle non-leader cases themselves since player.synced_to + # can be unreliable in some edge cases multi_device_dsp_supported = PlayerFeature.MULTI_DEVICE_DSP in player.supported_features - return is_grouped and not multi_device_dsp_supported + child_count = len(player.group_childs) if player.group_childs else 0 + + is_multiple_devices: bool + if player.provider.startswith("player_group"): + # PlayerGroups have no leader, so having a child count of 1 means + # the group actually contains only a single player. + is_multiple_devices = child_count > 1 + else: + is_multiple_devices = child_count > 0 + return is_multiple_devices and not multi_device_dsp_supported def get_player_filter_params( @@ -941,6 +966,19 @@ def get_player_filter_params( # We can not correctly apply DSP to a grouped player without multi-device DSP support, # so we disable it. dsp.enabled = False + elif player.provider.startswith("player_group") and ( + PlayerFeature.MULTI_DEVICE_DSP not in player.supported_features + ): + # This is a special case! We have a player group where: + # - The group leader does not support MULTI_DEVICE_DSP + # - But only contains a single player (since nothing is preventing DSP) + # We can still apply the DSP of that single player. + if player.group_childs: + child_player = mass.players.get(player.group_childs[0]) + dsp = mass.config.get_player_dsp_config(mass, child_player) + else: + # This should normally never happen, but if it does, we disable DSP. + dsp.enabled = False if dsp.enabled: # Apply input gain