From: Marcel van der Veldt Date: Sat, 8 Jul 2023 11:03:22 +0000 (+0200) Subject: Improve Universal Group Player mute feature (#751) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=1ed01275424ce6e200f13fb0fe2d45a109e835a1;p=music-assistant-server.git Improve Universal Group Player mute feature (#751) * Handle last player turn off is a muted player * move group power on to ugp * restore mute if needed * Do not start playing if no members powered * (re)set mute_as_power feature for group members * dump playerstate off as it is redundant with the powered bool * do not crash on invalid cache data * prevent state flipflop --- diff --git a/music_assistant/common/models/config_entries.py b/music_assistant/common/models/config_entries.py index 82669913..54769491 100644 --- a/music_assistant/common/models/config_entries.py +++ b/music_assistant/common/models/config_entries.py @@ -17,7 +17,6 @@ from music_assistant.constants import ( CONF_EQ_MID, CONF_EQ_TREBLE, CONF_FLOW_MODE, - CONF_GROUPED_POWER_ON, CONF_HIDE_GROUP_CHILDS, CONF_LOG_LEVEL, CONF_OUTPUT_CHANNELS, @@ -423,20 +422,6 @@ CONF_ENTRY_CROSSFADE_DURATION = ConfigEntry( ) -CONF_ENTRY_GROUPED_POWER_ON = ConfigEntry( - key=CONF_GROUPED_POWER_ON, - type=ConfigEntryType.BOOLEAN, - default_value=True, - label="Forced Power ON of all group members", - description="Power ON all child players when the group player is powered on " - "(or playback started). \n" - "If this setting is disabled, playback will only start on players that " - "are already powered ON at the time of playback start.\n" - "When turning OFF the group player, all group members are turned off, " - "regardless of this setting.", - advanced=False, -) - DEFAULT_PLAYER_CONFIG_ENTRIES = ( CONF_ENTRY_VOLUME_NORMALIZATION, CONF_ENTRY_FLOW_MODE, diff --git a/music_assistant/common/models/enums.py b/music_assistant/common/models/enums.py index f94fd499..ae9803b6 100644 --- a/music_assistant/common/models/enums.py +++ b/music_assistant/common/models/enums.py @@ -212,7 +212,6 @@ class PlayerState(StrEnum): IDLE = "idle" PAUSED = "paused" PLAYING = "playing" - OFF = "off" class PlayerType(StrEnum): diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index 59d73934..2530f010 100755 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -327,7 +327,7 @@ class PlayerQueuesController(CoreController): """Clear all items in the queue.""" queue = self._queues[queue_id] queue.radio_source = [] - if queue.state not in (PlayerState.IDLE, PlayerState.OFF): + if queue.state != PlayerState.IDLE: self.mass.create_task(self.stop(queue_id)) queue.current_index = None queue.current_item = None @@ -539,12 +539,18 @@ class PlayerQueuesController(CoreController): async def on_player_register(self, player: Player) -> None: """Register PlayerQueue for given player/queue id.""" queue_id = player.player_id + queue = None # try to restore previous state if prev_state := await self.mass.cache.get(f"queue.state.{queue_id}"): - queue = PlayerQueue.from_dict(prev_state) - prev_items = await self.mass.cache.get(f"queue.items.{queue_id}", default=[]) - queue_items = [QueueItem.from_dict(x) for x in prev_items] - else: + try: + queue = PlayerQueue.from_dict(prev_state) + prev_items = await self.mass.cache.get(f"queue.items.{queue_id}", default=[]) + queue_items = [QueueItem.from_dict(x) for x in prev_items] + except Exception as err: + self.logger.warning( + "Failed to restore the queue(items) for %s - %s", player.display_name, str(err) + ) + if queue is None: queue = PlayerQueue( queue_id=queue_id, active=False, diff --git a/music_assistant/server/controllers/players.py b/music_assistant/server/controllers/players.py index 605e6d8e..e34376f2 100755 --- a/music_assistant/server/controllers/players.py +++ b/music_assistant/server/controllers/players.py @@ -195,11 +195,7 @@ class PlayerController(CoreController): # handle special mute_as_power feature if player.mute_as_power: player.powered = player.powered and not player.volume_muted - # set player state to off if player is not powered - if player.powered and player.state == PlayerState.OFF: - player.state = PlayerState.IDLE - elif not player.powered: - player.state = PlayerState.OFF + # basic throttle: do not send state changed events if player did not actually change prev_state = self._prev_states.get(player_id, {}) new_state = self._players[player_id].to_dict() @@ -357,6 +353,15 @@ class PlayerController(CoreController): # handle mute as power feature await self.cmd_volume_mute(player_id, not powered) + # restore mute if needed on poweroff + if ( + not powered + and player.volume_muted + and not player.mute_as_power + and PlayerFeature.VOLUME_MUTE not in player.supported_features + ): + await self.cmd_volume_mute(player_id, False) + if PlayerFeature.POWER not in player.supported_features: # player does not support power, use fake state instead player.powered = powered @@ -558,7 +563,9 @@ class PlayerController(CoreController): if group_players := self._get_player_groups(player.player_id): # prefer the first playing (or paused) group parent for group_player in group_players: - if group_player.state in (PlayerState.PLAYING, PlayerState.PAUSED): + # if the group player's playerid is within the curtrent url, + # this group is definitely active + if player.current_url and group_player.player_id in player.current_url: return group_player.player_id # fallback to the first powered group player for group_player in group_players: diff --git a/music_assistant/server/providers/slimproto/models.py b/music_assistant/server/providers/slimproto/models.py index f2aaa04a..7c54942f 100644 --- a/music_assistant/server/providers/slimproto/models.py +++ b/music_assistant/server/providers/slimproto/models.py @@ -16,7 +16,6 @@ if TYPE_CHECKING: PLAYMODE_MAP = { PlayerState.IDLE: "stop", PlayerState.PLAYING: "play", - PlayerState.OFF: "stop", PlayerState.PAUSED: "pause", } diff --git a/music_assistant/server/providers/ugp/__init__.py b/music_assistant/server/providers/ugp/__init__.py index d55b467e..80cef769 100644 --- a/music_assistant/server/providers/ugp/__init__.py +++ b/music_assistant/server/providers/ugp/__init__.py @@ -11,7 +11,6 @@ from typing import TYPE_CHECKING from music_assistant.common.models.config_entries import ( CONF_ENTRY_FLOW_MODE, - CONF_ENTRY_GROUPED_POWER_ON, CONF_ENTRY_HIDE_GROUP_MEMBERS, CONF_ENTRY_OUTPUT_CHANNELS, ConfigEntry, @@ -51,6 +50,19 @@ CONF_ENTRY_OUTPUT_CHANNELS_FORCED_STEREO = ConfigEntry.from_dict( CONF_ENTRY_FORCED_FLOW_MODE = ConfigEntry.from_dict( {**CONF_ENTRY_FLOW_MODE.to_dict(), "default_value": True, "value": True, "hidden": True} ) +CONF_ENTRY_GROUPED_POWER_ON = ConfigEntry( + key=CONF_GROUPED_POWER_ON, + type=ConfigEntryType.BOOLEAN, + default_value=False, + label="Forced Power ON of all group members", + description="Power ON all child players when the group player is powered on " + "(or playback started). \n" + "If this setting is disabled, playback will only start on players that " + "are already powered ON at the time of playback start.\n" + "When turning OFF the group player, all group members are turned off, " + "regardless of this setting.", + advanced=False, +) # ruff: noqa: ARG002 @@ -222,13 +234,23 @@ class UniversalGroupProvider(PlayerProvider): # power ON await self.cmd_power(player_id, True) group_player = self.mass.players.get(player_id) + + active_members = self._get_active_members( + player_id, only_powered=True, skip_sync_childs=True + ) + if len(active_members) == 0: + self.logger.warning( + "Play media requested for player %s but no member players are powered, " + "the request will be ignored", + group_player.display_name, + ) + return + group_player.extra_data["optimistic_state"] = PlayerState.PLAYING # forward command to all (powered) group child's async with asyncio.TaskGroup() as tg: - for member in self._get_active_members( - player_id, only_powered=True, skip_sync_childs=True - ): + for member in active_members: player_prov = self.mass.players.get_player_provider(member.player_id) tg.create_task( player_prov.cmd_play_url(member.player_id, url=url, queue_item=queue_item) @@ -236,17 +258,28 @@ class UniversalGroupProvider(PlayerProvider): async def cmd_handle_stream_job(self, player_id: str, stream_job: MultiClientStreamJob) -> None: """Handle StreamJob play command on given player.""" - # send stop first + # send stop first await self.cmd_stop(player_id) # power ON await self.cmd_power(player_id, True) group_player = self.mass.players.get(player_id) + + active_members = self._get_active_members( + player_id, only_powered=True, skip_sync_childs=True + ) + if len(active_members) == 0: + self.logger.warning( + "Play media requested for player %s but no member players are powered, " + "the request will be ignored", + group_player.display_name, + ) + return + group_player.extra_data["optimistic_state"] = PlayerState.PLAYING + # forward command to all (powered) group child's async with asyncio.TaskGroup() as tg: - for member in self._get_active_members( - player_id, only_powered=True, skip_sync_childs=True - ): + for member in active_members: player_prov = self.mass.players.get_player_provider(member.player_id) # we forward the stream_job to child to allow for nested groups etc tg.create_task( @@ -269,33 +302,45 @@ class UniversalGroupProvider(PlayerProvider): player_id, CONF_GROUPED_POWER_ON ) mute_childs = self.mass.config.get_raw_player_config_value(player_id, CONF_MUTE_CHILDS, []) - # set mute_as_power feature for group members - for child_player_id in mute_childs: - if child_player := self.mass.players.get(child_player_id): - child_player.mute_as_power = powered group_player = self.mass.players.get(player_id) async def set_child_power(child_player: Player) -> None: + # do not turn on the player if not explicitly requested + # so either the group player turns off OR + # it turns ON and we have the group_power_on config option enabled + if not (not powered or group_power_on): + return + # make sure to disable the mute as power workaround, + # otherwise the player keeps on playing "invisible" + if not powered and child_player.player_id in mute_childs: + child_player.mute_as_power = False + if child_player.volume_muted: + await self.mass.players.cmd_volume_mute(child_player.player_id, False) + # send actual power command to child player await self.mass.players.cmd_power(child_player.player_id, powered) + # set optimistic state on child player to prevent race conditions in other actions child_player.powered = powered - if not powered or group_power_on: - # turn on/off child players - async with asyncio.TaskGroup() as tg: - for member in self._get_active_members( - player_id, only_powered=not powered, skip_sync_childs=False - ): - tg.create_task(set_child_power(member)) + # turn on/off child players if needed + async with asyncio.TaskGroup() as tg: + for member in self._get_active_members( + player_id, only_powered=False, skip_sync_childs=False + ): + tg.create_task(set_child_power(member)) + + # (re)set mute_as_power feature for group members + for child_player_id in mute_childs: + if child_player := self.mass.players.get(child_player_id): + child_player.mute_as_power = powered group_player.powered = powered - group_player.extra_data["optimistic_state"] = PlayerState.IDLE + if not powered: + group_player.extra_data["optimistic_state"] = PlayerState.IDLE self.mass.players.update(player_id) if powered: # sync all players on power on await self._sync_players(player_id) - else: - group_player.extra_data["optimistic_state"] = PlayerState.OFF async def cmd_volume_set(self, player_id: str, volume_level: int) -> None: """Send VOLUME_SET command to given player.""" @@ -316,13 +361,11 @@ class UniversalGroupProvider(PlayerProvider): player_id, only_powered=False, skip_sync_childs=False ) group_player.group_childs = list(x.player_id for x in all_members) - # read the state from the first powered child player + # read the state from the first active group member for member in all_members: if member.synced_to: continue - if not member.powered: - continue - if member.state not in (PlayerState.PLAYING, PlayerState.PAUSED): + if not member.current_url or player_id not in member.current_url: continue group_player.current_url = member.current_url group_player.elapsed_time = member.elapsed_time @@ -330,8 +373,7 @@ class UniversalGroupProvider(PlayerProvider): group_player.state = member.state break else: - group_player.state = PlayerState.IDLE - group_player.current_url = None + group_player.state = group_player.extra_data["optimistic_state"] async def on_child_power(self, player_id: str, child_player: Player, new_power: bool) -> None: """ @@ -340,7 +382,6 @@ class UniversalGroupProvider(PlayerProvider): This is used to handle special actions such as muting as power or (re)syncing. """ group_player = self.mass.players.get(player_id) - mute_childs = self.mass.config.get_raw_player_config_value(player_id, CONF_MUTE_CHILDS, []) if not group_player.powered: # guard, this should be caught in the player controller but just in case... @@ -384,7 +425,7 @@ class UniversalGroupProvider(PlayerProvider): not new_power and group_player.extra_data["optimistic_state"] == PlayerState.PLAYING and child_player.player_id in self.prev_sync_leaders[player_id] - and child_player.player_id not in mute_childs + and not child_player.mute_as_power ): # a sync master player turned OFF while the group player # should still be playing - we need to resync/resume @@ -399,14 +440,14 @@ class UniversalGroupProvider(PlayerProvider): """Get (child) players attached to a grouped player.""" child_players: list[Player] = [] conf_members: list[str] = self.config.get_value(player_id) - mute_childs: list[str] = self.mass.config.get_raw_player_config_value( - player_id, CONF_MUTE_CHILDS, [] - ) ignore_ids = set() for child_id in conf_members: if child_player := self.mass.players.get(child_id, False): # work out power state - player_powered = True if child_id in mute_childs else child_player.powered + if child_player.mute_as_power: + player_powered = child_player.powered and not child_player.volume_muted + else: + player_powered = child_player.powered if not (not only_powered or player_powered): continue if child_player.synced_to and skip_sync_childs: