queue/Playlist: seek methods return bool/Error instead of PlaylistResult

5 files changed, 78 insertions, 48 deletions

diff --git a/src/Partition.hxx b/src/Partition.hxx
index cd7b3c66e..7a57f741e 100644
--- a/src/Partition.hxx
+++ b/src/Partition.hxx
@@ -142,17 +142,19 @@ struct Partition final : private PlayerListener, private MixerListener {
 		return playlist.PlayPrevious(pc);
 	}
 
-	PlaylistResult SeekSongPosition(unsigned song_position,
-					SongTime seek_time) {
-		return playlist.SeekSongPosition(pc, song_position, seek_time);
+	bool SeekSongPosition(unsigned song_position,
+			      SongTime seek_time, Error &error) {
+		return playlist.SeekSongPosition(pc, song_position, seek_time,
+						 error);
 	}
 
-	PlaylistResult SeekSongId(unsigned song_id, SongTime seek_time) {
-		return playlist.SeekSongId(pc, song_id, seek_time);
+	bool SeekSongId(unsigned song_id, SongTime seek_time, Error &error) {
+		return playlist.SeekSongId(pc, song_id, seek_time, error);
 	}
 
-	PlaylistResult SeekCurrent(SignedSongTime seek_time, bool relative) {
-		return playlist.SeekCurrent(pc, seek_time, relative);
+	bool SeekCurrent(SignedSongTime seek_time, bool relative,
+			 Error &error) {
+		return playlist.SeekCurrent(pc, seek_time, relative, error);
 	}
 
 	void SetRepeat(bool new_value) {
diff --git a/src/command/PlayerCommands.cxx b/src/command/PlayerCommands.cxx
index b9fc7d578..9cfd58a93 100644
--- a/src/command/PlayerCommands.cxx
+++ b/src/command/PlayerCommands.cxx
@@ -296,9 +296,10 @@ handle_seek(Client &client, Request args, Response &r)
 	if (!args.Parse(0, song, r) || !args.Parse(1, seek_time, r))
 		return CommandResult::ERROR;
 
-	PlaylistResult result =
-		client.partition.SeekSongPosition(song, seek_time);
-	return print_playlist_result(r, result);
+	Error error;
+	return client.partition.SeekSongPosition(song, seek_time, error)
+		? CommandResult::OK
+		: print_error(r, error);
 }
 
 CommandResult
@@ -311,9 +312,10 @@ handle_seekid(Client &client, Request args, Response &r)
 	if (!args.Parse(1, seek_time, r))
 		return CommandResult::ERROR;
 
-	PlaylistResult result =
-		client.partition.SeekSongId(id, seek_time);
-	return print_playlist_result(r, result);
+	Error error;
+	return client.partition.SeekSongId(id, seek_time, error)
+		? CommandResult::OK
+		: print_error(r, error);
 }
 
 CommandResult
@@ -325,9 +327,10 @@ handle_seekcur(Client &client, Request args, Response &r)
 	if (!ParseCommandArg(r, seek_time, p))
 		return CommandResult::ERROR;
 
-	PlaylistResult result =
-		client.partition.SeekCurrent(seek_time, relative);
-	return print_playlist_result(r, result);
+	Error error;
+	return client.partition.SeekCurrent(seek_time, relative, error)
+		? CommandResult::OK
+		: print_error(r, error);
 }
 
 CommandResult
diff --git a/src/queue/Playlist.hxx b/src/queue/Playlist.hxx
index a185a79c8..fb78e9373 100644
--- a/src/queue/Playlist.hxx
+++ b/src/queue/Playlist.hxx
@@ -280,16 +280,19 @@ public:
 
 	void PlayPrevious(PlayerControl &pc);
 
-	PlaylistResult SeekSongOrder(PlayerControl &pc,
-				     unsigned song_order,
-				     SongTime seek_time);
+	bool SeekSongOrder(PlayerControl &pc,
+			   unsigned song_order,
+			   SongTime seek_time,
+			   Error &error);
 
-	PlaylistResult SeekSongPosition(PlayerControl &pc,
-					unsigned song_position,
-					SongTime seek_time);
+	bool SeekSongPosition(PlayerControl &pc,
+			      unsigned sonag_position,
+			      SongTime seek_time,
+			      Error &error);
 
-	PlaylistResult SeekSongId(PlayerControl &pc,
-				  unsigned song_id, SongTime seek_time);
+	bool SeekSongId(PlayerControl &pc,
+			unsigned song_id, SongTime seek_time,
+			Error &error);
 
 	/**
 	 * Seek within the current song.  Fails if MPD is not currently
@@ -299,8 +302,9 @@ public:
 	 * @param relative if true, then the specified time is relative to the
 	 * current position
 	 */
-	PlaylistResult SeekCurrent(PlayerControl &pc,
-				   SignedSongTime seek_time, bool relative);
+	bool SeekCurrent(PlayerControl &pc,
+			 SignedSongTime seek_time, bool relative,
+			 Error &error);
 
 	bool GetRepeat() const {
 		return queue.repeat;
diff --git a/src/queue/PlaylistControl.cxx b/src/queue/PlaylistControl.cxx
index 43da2de62..f7b2b16a3 100644
--- a/src/queue/PlaylistControl.cxx
+++ b/src/queue/PlaylistControl.cxx
@@ -189,8 +189,9 @@ playlist::PlayPrevious(PlayerControl &pc)
 	PlayOrder(pc, order);
 }
 
-PlaylistResult
-playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time)
+bool
+playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time,
+			Error &error)
 {
 	assert(queue.IsValidOrder(i));
 
@@ -213,52 +214,71 @@ playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time)
 	if (!pc.LockSeek(new DetachedSong(queue.GetOrder(i)), seek_time)) {
 		UpdateQueuedSong(pc, queued_song);
 
-		return PlaylistResult::NOT_PLAYING;
+		// TODO: fix error code
+		error.Set(playlist_domain, int(PlaylistResult::NOT_PLAYING),
+			  "Decoder failed to seek");
+		return false;
 	}
 
 	queued = -1;
 	UpdateQueuedSong(pc, nullptr);
 
-	return PlaylistResult::SUCCESS;
+	return true;
 }
 
-PlaylistResult
+bool
 playlist::SeekSongPosition(PlayerControl &pc, unsigned song,
-			   SongTime seek_time)
+			   SongTime seek_time,
+			   Error &error)
 {
-	if (!queue.IsValidPosition(song))
-		return PlaylistResult::BAD_RANGE;
+	if (!queue.IsValidPosition(song)) {
+		error.Set(playlist_domain, int(PlaylistResult::BAD_RANGE),
+			  "Bad range");
+		return false;
+	}
 
 	unsigned i = queue.random
 		? queue.PositionToOrder(song)
 		: song;
 
-	return SeekSongOrder(pc, i, seek_time);
+	return SeekSongOrder(pc, i, seek_time, error);
 }
 
-PlaylistResult
-playlist::SeekSongId(PlayerControl &pc, unsigned id, SongTime seek_time)
+bool
+playlist::SeekSongId(PlayerControl &pc, unsigned id, SongTime seek_time,
+		     Error &error)
 {
 	int song = queue.IdToPosition(id);
-	if (song < 0)
-		return PlaylistResult::NO_SUCH_SONG;
+	if (song < 0) {
+		error.Set(playlist_domain, int(PlaylistResult::NO_SUCH_SONG),
+			  "No such song");
+		return false;
+	}
 
-	return SeekSongPosition(pc, song, seek_time);
+	return SeekSongPosition(pc, song, seek_time, error);
 }
 
-PlaylistResult
+bool
 playlist::SeekCurrent(PlayerControl &pc,
-		      SignedSongTime seek_time, bool relative)
+		      SignedSongTime seek_time, bool relative,
+		      Error &error)
 {
-	if (!playing)
-		return PlaylistResult::NOT_PLAYING;
+	if (!playing) {
+		error.Set(playlist_domain, int(PlaylistResult::NOT_PLAYING),
+			  "Not playing");
+		return false;
+	}
 
 	if (relative) {
 		const auto status = pc.LockGetStatus();
 
 		if (status.state != PlayerState::PLAY &&
-		    status.state != PlayerState::PAUSE)
-			return PlaylistResult::NOT_PLAYING;
+		    status.state != PlayerState::PAUSE) {
+			error.Set(playlist_domain,
+				  int(PlaylistResult::NOT_PLAYING),
+				  "Not playing");
+			return false;
+		}
 
 		seek_time += status.elapsed_time;
 		if (seek_time.IsNegative())
@@ -268,5 +288,5 @@ playlist::SeekCurrent(PlayerControl &pc,
 	if (seek_time.IsNegative())
 		seek_time = SignedSongTime::zero();
 
-	return SeekSongOrder(pc, current, SongTime(seek_time));
+	return SeekSongOrder(pc, current, SongTime(seek_time), error);
 }
diff --git a/src/queue/PlaylistState.cxx b/src/queue/PlaylistState.cxx
index 7cb1ef661..50d1eac60 100644
--- a/src/queue/PlaylistState.cxx
+++ b/src/queue/PlaylistState.cxx
@@ -198,7 +198,8 @@ playlist_state_restore(const char *line, TextFile &file,
 		else if (seek_time.count() == 0)
 			playlist.PlayPosition(pc, current);
 		else
-			playlist.SeekSongPosition(pc, current, seek_time);
+			playlist.SeekSongPosition(pc, current, seek_time,
+						  IgnoreError());
 
 		if (state == PlayerState::PAUSE)
 			pc.LockPause();