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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: pabera <[email protected]>

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

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):

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)):