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
* 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 MiczFlor#2294

Co-authored-by: pabera <[email protected]>
  • Loading branch information
hoffie and pabera committed Apr 11, 2024
1 parent 27b23cb commit 76b5c6d
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
2 changes: 2 additions & 0 deletions resources/default-settings/jukebox.default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 41 additions & 4 deletions src/jukebox/components/playermpd/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

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

0 comments on commit 76b5c6d

Please sign in to comment.