Skip to content

Commit

Permalink
Fix PlayerMPD.prev/next() when stopped
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
hoffie and pabera committed Apr 11, 2024
1 parent 3865c5a commit 3554511
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 2 deletions.
6 changes: 6 additions & 0 deletions resources/default-settings/jukebox.default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
76 changes: 74 additions & 2 deletions src/jukebox/components/playermpd/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 3554511

Please sign in to comment.