Fix: DSP edge cases around groups

music_assistant/controllers/players.py

@@ -886,7 +886,7 @@ def remove(self, player_id: str, cleanup_config: bool = True) -> None:
         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 @@ def update(
         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:

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,19 +221,24 @@ 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.
     if not player.provider.startswith("player_group"):
         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