From 35545113c4f25a5b2cf774d985e89d78be54aab3 Mon Sep 17 00:00:00 2001 From: Christian Hoffmann Date: Fri, 12 Apr 2024 00:28:28 +0200 Subject: [PATCH] Fix PlayerMPD.prev/next() when stopped * Avoid MPD-related crashes during all prev/next() calls. * Explicitly handle prev() in stopped state, configurable via `playermpd.stopped_prev_action`. * Explicitly handle next() in stopped state, configurable via `playermpd.stopped_next_action`. * Explicitly handle next() when reaching the end of the playlist: jukebox-daemon will now stop playing by default (similar to v2). It can also be configured to restart the playlist instead by setting the new config option `playermpd.end_of_playlist_next_action: rewind` or to ignore the action completely. Fixes #2294 Fixes #2327 Co-authored-by: pabera <1260686+pabera@users.noreply.github.com> --- .../default-settings/jukebox.default.yaml | 6 ++ src/jukebox/components/playermpd/__init__.py | 76 ++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/resources/default-settings/jukebox.default.yaml b/resources/default-settings/jukebox.default.yaml index c087cc024..86a6c7218 100644 --- a/resources/default-settings/jukebox.default.yaml +++ b/resources/default-settings/jukebox.default.yaml @@ -87,6 +87,12 @@ playermpd: update_on_startup: true check_user_rights: true mpd_conf: ~/.config/mpd/mpd.conf + # Must be one of: 'none', 'stop', 'rewind': + end_of_playlist_next_action: stop + # Must be one of: 'none', 'prev', 'rewind': + stopped_prev_action: prev + # Must be one of: 'none', 'next', 'rewind': + stopped_next_action: next rpc: tcp_port: 5555 websocket_port: 5556 diff --git a/src/jukebox/components/playermpd/__init__.py b/src/jukebox/components/playermpd/__init__.py index dcbef2ea8..d9f0e15a9 100644 --- a/src/jukebox/components/playermpd/__init__.py +++ b/src/jukebox/components/playermpd/__init__.py @@ -156,6 +156,20 @@ def __init__(self): self.second_swipe_action = None self.decode_2nd_swipe_option() + self.end_of_playlist_next_action_dict = {'rewind': self.rewind, + 'stop': self.stop, + 'none': lambda: None} + self.end_of_playlist_next_action = None + self.stopped_prev_action_dict = {'rewind': self.rewind, + 'prev': self._prev_in_stopped_state, + 'none': lambda: None} + self.stopped_prev_action = None + self.stopped_next_action_dict = {'rewind': self.rewind, + 'next': self._next_in_stopped_state, + 'none': lambda: None} + self.stopped_next_action = None + self.decode_prev_next_actions() + self.mpd_client = mpd.MPDClient() self.coverart_cache_manager = CoverartCacheManager() @@ -231,6 +245,31 @@ def decode_2nd_swipe_option(self): custom_action['args'], custom_action['kwargs']) + def decode_prev_next_actions(self): + end_of_playlist_next_action = cfg.setndefault('playermpd', 'end_of_playlist_next_action', value='').lower() + if end_of_playlist_next_action not in self.end_of_playlist_next_action_dict: + end_of_playlist_next_action = 'stop' + logger.error(f"Config playermpd.end_of_playlist_next_action must be one of " + f"{self.end_of_playlist_next_action.keys()}. " + f"Using default 'stop'") + self.end_of_playlist_next_action = self.end_of_playlist_next_action_dict[end_of_playlist_next_action] + + stopped_prev_action = cfg.setndefault('playermpd', 'stopped_prev_action', value='').lower() + if stopped_prev_action not in self.stopped_prev_action_dict: + stopped_prev_action = 'prev' + logger.error(f"Config playermpd.stopped_prev_action must be one of " + f"{self.stopped_prev_action.keys()}. " + f"Using default 'prev'") + self.stopped_prev_action = self.stopped_prev_action_dict[stopped_prev_action] + + stopped_next_action = cfg.setndefault('playermpd', 'stopped_prev_action', value='').lower() + if stopped_next_action not in self.stopped_prev_action_dict: + stopped_next_action = 'prev' + logger.error(f"Config playermpd.stopped_next_action must be one of " + f"{self.stopped_next_action.keys()}. " + f"Using default 'next'") + self.stopped_next_action = self.stopped_prev_action_dict[stopped_prev_action] + def mpd_retry_with_mutex(self, mpd_cmd, *args): """ This method adds thread saftey for acceses to mpd via a mutex lock, @@ -327,15 +366,48 @@ def pause(self, state: int = 1): @plugs.tag def prev(self): logger.debug("Prev") + if self.mpd_status['state'] == 'stop': + logger.debug('Player is stopped, calling stopped_prev_action') + return self.stopped_prev_action() + try: + with self.mpd_lock: + self.mpd_client.previous() + except mpd.base.CommandError: + # This shouldn't happen in reality, but we still catch + # this error to avoid crashing the player thread: + logger.warning('Failed to go to previous song, ignoring') + + def _prev_in_stopped_state(self): with self.mpd_lock: - self.mpd_client.previous() + self.mpd_client.play(max(0, int(self.mpd_status['pos']) - 1)) @plugs.tag def next(self): """Play next track in current playlist""" logger.debug("Next") + if self.mpd_status['state'] == 'stop': + logger.debug('Player is stopped, calling stopped_next_action') + return self.stopped_next_action() + playlist_len = int(self.mpd_status.get('playlistlength', -1)) + current_pos = int(self.mpd_status.get('pos', 0)) + if current_pos == playlist_len - 1: + logger.debug(f'next() called during last song ({current_pos}) of ' + f'playlist (len={playlist_len}), running end_of_playlist_next_action.') + return self.end_of_playlist_next_action() + try: + with self.mpd_lock: + self.mpd_client.next() + except mpd.base.CommandError: + # This shouldn't happen in reality, but we still catch + # this error to avoid crashing the player thread: + logger.warning('Failed to go to next song, ignoring') + + def _next_in_stopped_state(self): + pos = int(self.mpd_status['pos']) + 1 + if pos > int(self.mpd_status['playlistlength']) - 1: + return self.end_of_playlist_next_action() with self.mpd_lock: - self.mpd_client.next() + self.mpd_client.play(pos) @plugs.tag def seek(self, new_time):