Carefully handle redirect from players to queue controller (#1427)
authorMarcel van der Veldt <m.vanderveldt@outlook.com>
Sat, 29 Jun 2024 22:29:20 +0000 (00:29 +0200)
committerGitHub <noreply@github.com>
Sat, 29 Jun 2024 22:29:20 +0000 (00:29 +0200)
music_assistant/server/controllers/player_queues.py
music_assistant/server/controllers/players.py

index 632f2e30cfdf092384f912f822155477852f4c5b..384a51ba943f9387730725aac9cc3312dc4e6d82 100644 (file)
@@ -540,9 +540,8 @@ class PlayerQueuesController(CoreController):
         if queue := self.get(queue_id):
             queue.stream_finished = None
             queue.end_of_track_reached = None
-        # forward the actual stop command to the player provider
-        if player_provider := self.mass.players.get_player_provider(queue_id):
-            await player_provider.cmd_stop(queue_id)
+        # forward the actual command to the player controller
+        await self.mass.players.cmd_stop(queue_id, skip_forward=True)
 
     @api_command("player_queues/play")
     async def play(self, queue_id: str) -> None:
@@ -551,15 +550,13 @@ class PlayerQueuesController(CoreController):
 
         - queue_id: queue_id of the playerqueue to handle the command.
         """
-        # always fetch the underlying player so we can raise early if its not available
-        player = self.mass.players.get(queue_id, True)
-        if player.announcement_in_progress:
+        queue_player: Player = self.mass.players.get(queue_id, True)
+        if queue_player.announcement_in_progress:
             self.logger.warning("Ignore queue command: An announcement is in progress")
             return
-        if self._queues[queue_id].state == PlayerState.PAUSED:
-            # forward the actual stop command to the player provider
-            if player_provider := self.mass.players.get_player_provider(queue_id):
-                await player_provider.cmd_play(queue_id)
+        if (queue := self._queues.get(queue_id)) and queue.state == PlayerState.PAUSED:
+            # forward the actual command to the player controller
+            await self.mass.players.cmd_play(queue_id, skip_forward=True)
         else:
             await self.resume(queue_id)
 
@@ -569,18 +566,8 @@ class PlayerQueuesController(CoreController):
 
         - queue_id: queue_id of the playerqueue to handle the command.
         """
-        # always fetch the underlying player so we can raise early if its not available
-        player = self.mass.players.get(queue_id, True)
-        if player.announcement_in_progress:
-            self.logger.warning("Ignore queue command: An announcement is in progress")
-            return
-        if PlayerFeature.PAUSE not in player.supported_features:
-            # if player does not support pause, we need to send stop
-            await self.stop(queue_id)
-            return
-        # forward the actual stop command to the player provider
-        if player_provider := self.mass.players.get_player_provider(queue_id):
-            await player_provider.cmd_pause(queue_id)
+        # forward the actual command to the player controller
+        await self.mass.players.cmd_pause(queue_id)
 
     @api_command("player_queues/play_pause")
     async def play_pause(self, queue_id: str) -> None:
@@ -588,7 +575,7 @@ class PlayerQueuesController(CoreController):
 
         - queue_id: queue_id of the queue to handle the command.
         """
-        if self._queues[queue_id].state == PlayerState.PLAYING:
+        if (queue := self._queues.get(queue_id)) and queue.state == PlayerState.PLAYING:
             await self.pause(queue_id)
             return
         await self.play(queue_id)
@@ -599,11 +586,13 @@ class PlayerQueuesController(CoreController):
 
         - queue_id: queue_id of the queue to handle the command.
         """
-        # always fetch the underlying player so we can raise early if its not available
-        player = self.mass.players.get(queue_id, True)
-        if player.announcement_in_progress:
+        queue_player: Player = self.mass.players.get(queue_id, True)
+        if queue_player.announcement_in_progress:
             self.logger.warning("Ignore queue command: An announcement is in progress")
             return
+        if (queue := self.get(queue_id)) is None or not queue.active:
+            # TODO: forward to underlying player if not active
+            return
         current_index = self._queues[queue_id].current_index
         if (next_index := self._get_next_index(queue_id, current_index, True)) is not None:
             await self.play_index(queue_id, next_index)
@@ -614,11 +603,13 @@ class PlayerQueuesController(CoreController):
 
         - queue_id: queue_id of the queue to handle the command.
         """
-        # always fetch the underlying player so we can raise early if its not available
-        player = self.mass.players.get(queue_id, True)
-        if player.announcement_in_progress:
+        queue_player: Player = self.mass.players.get(queue_id, True)
+        if queue_player.announcement_in_progress:
             self.logger.warning("Ignore queue command: An announcement is in progress")
             return
+        if (queue := self.get(queue_id)) is None or not queue.active:
+            # TODO: forward to underlying player if not active
+            return
         current_index = self._queues[queue_id].current_index
         if current_index is None:
             return
@@ -631,11 +622,13 @@ class PlayerQueuesController(CoreController):
         - queue_id: queue_id of the queue to handle the command.
         - seconds: number of seconds to skip in track. Use negative value to skip back.
         """
-        # always fetch the underlying player so we can raise early if its not available
-        player = self.mass.players.get(queue_id, True)
-        if player.announcement_in_progress:
+        queue_player: Player = self.mass.players.get(queue_id, True)
+        if queue_player.announcement_in_progress:
             self.logger.warning("Ignore queue command: An announcement is in progress")
             return
+        if (queue := self.get(queue_id)) is None or not queue.active:
+            # TODO: forward to underlying player if not active
+            return
         await self.seek(queue_id, self._queues[queue_id].elapsed_time + seconds)
 
     @api_command("player_queues/seek")
@@ -645,12 +638,13 @@ class PlayerQueuesController(CoreController):
         - queue_id: queue_id of the queue to handle the command.
         - position: position in seconds to seek to in the current playing item.
         """
-        # always fetch the underlying player so we can raise early if its not available
-        player = self.mass.players.get(queue_id, True)
-        if player.announcement_in_progress:
+        queue_player: Player = self.mass.players.get(queue_id, True)
+        if queue_player.announcement_in_progress:
             self.logger.warning("Ignore queue command: An announcement is in progress")
             return
-        queue = self._queues[queue_id]
+        if (queue := self.get(queue_id)) is None or not queue.active:
+            # TODO: forward to underlying player if not active
+            return
         assert queue.current_item, "No item loaded"
         assert queue.current_item.media_item.media_type == MediaType.TRACK
         assert queue.current_item.duration
index 50ebb939df7dbe12e0a7cc7050862434b9250a63..1e856917db1efca1edde533e062418a5c36fd769 100644 (file)
@@ -163,35 +163,40 @@ class PlayerController(CoreController):
 
     @api_command("players/cmd/stop")
     @handle_player_command
-    async def cmd_stop(self, player_id: str) -> None:
+    async def cmd_stop(self, player_id: str, skip_forward: bool = False) -> None:
         """Send STOP command to given player.
 
         - player_id: player_id of the player to handle the command.
         """
+        player_id = self._check_redirect(player_id)
         player = self.get(player_id, True)
-        # Always prefer queue controller's stop (as it also handles some other logic)
-        if player.active_source == player_id:
+        # Redirect to queue controller if active (as it also handles some other logic)
+        # Note that skip_forward will be set by the queue controller
+        # to prevent an endless loop.
+        if not skip_forward and player.active_source == player_id:
             await self.mass.player_queues.stop(player_id)
             return
-        # if the player doesn't have our queue controller active, forward the stop command
-        player_id = self._check_redirect(player_id)
         if player_provider := self.get_player_provider(player_id):
             await player_provider.cmd_stop(player_id)
 
     @api_command("players/cmd/play")
     @handle_player_command
-    async def cmd_play(self, player_id: str) -> None:
+    async def cmd_play(self, player_id: str, skip_forward: bool = False) -> None:
         """Send PLAY (unpause) command to given player.
 
         - player_id: player_id of the player to handle the command.
         """
+        player_id = self._check_redirect(player_id)
         player = self.get(player_id, True)
-        # Always prefer queue controller's play (as it also handles some other logic)
-        if player.active_source == player_id:
+        if player.announcement_in_progress:
+            self.logger.warning("Ignore queue command: An announcement is in progress")
+            return
+        # Redirect to queue controller if active (as it also handles some other logic)
+        # Note that skip_forward will be set by the queue controller
+        # to prevent an endless loop.
+        if not skip_forward and player.active_source == player_id:
             await self.mass.player_queues.play(player_id)
             return
-        # if the player doesn't have our queue controller active, forward the stop command
-        player_id = self._check_redirect(player_id)
         player_provider = self.get_player_provider(player_id)
         await player_provider.cmd_play(player_id)
 
@@ -202,13 +207,11 @@ class PlayerController(CoreController):
 
         - player_id: player_id of the player to handle the command.
         """
-        player = self.get(player_id, True)
-        # Always prefer queue controller's pause (as it also handles some other logic)
-        if player.active_source == player_id:
-            await self.mass.player_queues.pause(player_id)
-            return
         player_id = self._check_redirect(player_id)
         player = self.get(player_id, True)
+        if player.announcement_in_progress:
+            self.logger.warning("Ignore command: An announcement is in progress")
+            return
         if PlayerFeature.PAUSE not in player.supported_features:
             # if player does not support pause, we need to send stop
             await self.cmd_stop(player_id)