From a9ab571ac90cce21b94fef705855b9598c941a30 Mon Sep 17 00:00:00 2001 From: Christian Hoffmann Date: Fri, 12 Apr 2024 21:10:24 +0000 Subject: [PATCH] Fix PlayerMPD.prev/next() when stopped (#2326) * utils: Add get_config_action This abstracts away the functionality to resolve a given config option to an action in a pre-defined dict. Co-authored-by: Christian Hoffmann * 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 ignore the action by default (similar to v2). It can also be configured to rewind the playlist instead by setting the new config option `playermpd.end_of_playlist_next_action: rewind` or to stop playing. Fixes #2294 Fixes #2327 Co-authored-by: pabera <1260686+pabera@users.noreply.github.com> --------- Co-authored-by: pabera <1260686+pabera@users.noreply.github.com> --- .../default-settings/jukebox.default.yaml | 6 ++ src/jukebox/components/playermpd/__init__.py | 59 ++++++++++++++++++- src/jukebox/jukebox/utils.py | 13 ++++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/resources/default-settings/jukebox.default.yaml b/resources/default-settings/jukebox.default.yaml index c087cc024..b8e429333 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: none + # 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..772b8c654 100644 --- a/src/jukebox/components/playermpd/__init__.py +++ b/src/jukebox/components/playermpd/__init__.py @@ -156,6 +156,28 @@ def __init__(self): self.second_swipe_action = None self.decode_2nd_swipe_option() + self.end_of_playlist_next_action = utils.get_config_action(cfg, + 'playermpd', + 'end_of_playlist_next_action', + {'rewind': self.rewind, + 'stop': self.stop, + 'none': lambda: None}, + logger) + self.stopped_prev_action = utils.get_config_action(cfg, + 'playermpd', + 'stopped_prev_action', + {'rewind': self.rewind, + 'prev': self._prev_in_stopped_state, + 'none': lambda: None}, + logger) + self.stopped_next_action = utils.get_current_song(cfg, + 'playermpd', + 'stopped_next_action', + {'rewind': self.rewind, + 'next': self._next_in_stopped_state, + 'none': lambda: None}, + logger) + self.mpd_client = mpd.MPDClient() self.coverart_cache_manager = CoverartCacheManager() @@ -327,15 +349,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): diff --git a/src/jukebox/jukebox/utils.py b/src/jukebox/jukebox/utils.py index dbd647490..4cc0270ae 100644 --- a/src/jukebox/jukebox/utils.py +++ b/src/jukebox/jukebox/utils.py @@ -183,6 +183,19 @@ def rpc_call_to_str(cfg_rpc_call: Dict, with_args=True) -> str: return readable +def get_config_action(cfg, section, option, default, valid_actions_dict, logger): + """ + Looks up the given {section}.{option} config option and returns + the associated entry from valid_actions_dict, if valid. Falls back to the given + default otherwise. + """ + action = cfg.setndefault(section, option, value='').lower() + if action not in valid_actions_dict: + logger.error(f"Config {section}.{option} must be one of {valid_actions_dict.keys()}. Using default '{default}'") + action = default + return valid_actions_dict[action] + + def indent(doc, spaces=4): lines = doc.split('\n') for i in range(0, len(lines)):