From 76b5c6d1fced32697632f4309d4e5847ea3e2417 Mon Sep 17 00:00:00 2001 From: Christian Hoffmann Date: Sat, 30 Mar 2024 20:47:01 +0100 Subject: [PATCH] Fix PlayerMPD.prev/next() when stopped * Avoid MPD-related crashes during all prev/next() calls. * Explicitly handle prev() in stopped state * Explicitly handle next() in stopped state * 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`. Fixes #2294 Co-authored-by: pabera <1260686+pabera@users.noreply.github.com> --- .../default-settings/jukebox.default.yaml | 2 + src/jukebox/components/playermpd/__init__.py | 45 +++++++++++++++++-- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/resources/default-settings/jukebox.default.yaml b/resources/default-settings/jukebox.default.yaml index c087cc024..7b0c882c7 100644 --- a/resources/default-settings/jukebox.default.yaml +++ b/resources/default-settings/jukebox.default.yaml @@ -87,6 +87,8 @@ playermpd: update_on_startup: true check_user_rights: true mpd_conf: ~/.config/mpd/mpd.conf + # Must be one of: 'stop', 'rewind': + end_of_playlist_next_action: stop rpc: tcp_port: 5555 websocket_port: 5556 diff --git a/src/jukebox/components/playermpd/__init__.py b/src/jukebox/components/playermpd/__init__.py index 49f630224..c48fa4936 100644 --- a/src/jukebox/components/playermpd/__init__.py +++ b/src/jukebox/components/playermpd/__init__.py @@ -156,6 +156,11 @@ 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} + self.end_of_playlist_next_action = None + self.decode_end_of_playlist_next_action() + self.mpd_client = mpd.MPDClient() self.coverart_cache_manager = CoverartCacheManager() @@ -231,6 +236,16 @@ def decode_2nd_swipe_option(self): custom_action['args'], custom_action['kwargs']) + def decode_end_of_playlist_next_action(self): + end_of_playlist_next_action = cfg.setndefault('playermpd', 'end_of_playlist_next_action', value='stop').lower() + if end_of_playlist_next_action in self.end_of_playlist_next_action_dict: + self.end_of_playlist_next_action = self.end_of_playlist_next_action_dict[end_of_playlist_next_action] + else: + self.end_of_playlist_next_action = self.end_of_playlist_next_action_dict['stop'] + logger.error(f"Config mpd.end_of_playlist_next_action must be one of " + f"{self.end_of_playlist_next_action.keys()}. " + f"Using default 'stop'") + def mpd_retry_with_mutex(self, mpd_cmd, *args): """ This method adds thread saftey for acceses to mpd via a mutex lock, @@ -327,15 +342,37 @@ def pause(self, state: int = 1): @plugs.tag def prev(self): logger.debug("Prev") - with self.mpd_lock: - self.mpd_client.previous() + if self.mpd_status['state'] == 'stop': + logger.debug('Player is stopped, rewinding instead') + return self.rewind() + 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') @plugs.tag def next(self): """Play next track in current playlist""" logger.debug("Next") - with self.mpd_lock: - self.mpd_client.next() + if self.mpd_status['state'] == 'stop': + logger.debug('Player is stopped, rewinding instead') + return self.rewind() + 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') @plugs.tag def seek(self, new_time):