Skip to content

Commit

Permalink
Fix: DSP edge cases around groups (#1879)
Browse files Browse the repository at this point in the history
* 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`
  • Loading branch information
maximmaxim345 authored Jan 16, 2025
1 parent 0f576aa commit 3b4304f
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 18 deletions.
49 changes: 38 additions & 11 deletions music_assistant/controllers/players.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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:
Expand Down
52 changes: 45 additions & 7 deletions music_assistant/helpers/audio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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


Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down

0 comments on commit 3b4304f

Please sign in to comment.