From dee378b77530a0c8ce5631a3d96dc542c467b723 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Sun, 26 Nov 2017 11:27:06 +0100
Subject: [PATCH] player/Thread: make SEEK (partially) non-blocking

When the decoder is still starting up while we handle a SEEK, finish
the "player SEEK" immediately and re-enter the player loop, being able
to handle commands (and even cancel the pending seek).

This is the first part in a series of patches to solve the "blocking
input blocks decoder, blocks player, blocks the main thread" problem.
There are many other blocking code locations left, and the main thread
isn't non-blocking either because it waits for "seeking" to become
false.
---
 src/player/Control.cxx |  5 ++++
 src/player/Control.hxx |  9 ++++++-
 src/player/Thread.cxx  | 56 ++++++++++++++++++++++++++++++++++--------
 3 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/src/player/Control.cxx b/src/player/Control.cxx
index 2522e2a41..4e4bdb00d 100644
--- a/src/player/Control.cxx
+++ b/src/player/Control.cxx
@@ -256,6 +256,11 @@ PlayerControl::SeekLocked(std::unique_ptr<DetachedSong> song, SongTime t)
 
 	assert(next_song == nullptr);
 
+	/* the SEEK command is asynchronous; until completion, the
+	   "seeking" flag is set */
+	while (seeking)
+		ClientWait();
+
 	if (error_type != PlayerError::NONE) {
 		assert(error);
 		std::rethrow_exception(error);
diff --git a/src/player/Control.hxx b/src/player/Control.hxx
index 4a2c84079..7310a0550 100644
--- a/src/player/Control.hxx
+++ b/src/player/Control.hxx
@@ -54,7 +54,9 @@ enum class PlayerCommand : uint8_t {
 	/**
 	 * Seek to a certain position in the specified song.  This
 	 * command can also be used to change the current song or
-	 * start playback.
+	 * start playback.  It "finishes" immediately, but
+	 * PlayerControl::seeking will be set until seeking really
+	 * completes (or fails).
 	 */
 	SEEK,
 
@@ -176,6 +178,11 @@ struct PlayerControl final : AudioOutputClient {
 
 	ReplayGainMode replay_gain_mode = ReplayGainMode::OFF;
 
+	/**
+	 * Is the player currently busy with the SEEK command?
+	 */
+	bool seeking = false;
+
 	/**
 	 * If this flag is set, then the player will be auto-paused at
 	 * the end of the song, before the next song starts to play.
diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx
index f9d0cd204..77eee0aa7 100644
--- a/src/player/Thread.cxx
+++ b/src/player/Thread.cxx
@@ -147,6 +147,16 @@ class Player {
 	 */
 	SongTime elapsed_time = SongTime::zero();
 
+	/**
+	 * If this is positive, then we need to ask the decoder to
+	 * seek after it has completed startup.  This is needed if the
+	 * decoder is in the middle of startup while the player
+	 * receives another seek command.
+	 *
+	 * This is only valid while #decoder_starting is true.
+	 */
+	SongTime pending_seek;
+
 	PeriodClock throttle_silence_log;
 
 public:
@@ -280,6 +290,11 @@ private:
 	 */
 	bool SeekDecoder() noexcept;
 
+	void CancelPendingSeek() noexcept {
+		pending_seek = SongTime::zero();
+		pc.seeking = false;
+	}
+
 	/**
 	 * Check if the decoder has reported an error, and forward it
 	 * to PlayerControl::SetError().
@@ -432,6 +447,7 @@ Player::ActivateDecoder() noexcept
 	/* set the "starting" flag, which will be cleared by
 	   CheckDecoderStartup() */
 	decoder_starting = true;
+	pending_seek = SongTime::zero();
 
 	/* update PlayerControl's song information */
 	pc.total_time = song->GetDuration();
@@ -528,6 +544,19 @@ Player::CheckDecoderStartup() noexcept
 
 		idle_add(IDLE_PLAYER);
 
+		if (pending_seek > SongTime::zero()) {
+			assert(pc.seeking);
+
+			bool success = SeekDecoder(pending_seek);
+			pc.seeking = false;
+			pc.ClientSignal();
+			if (!success)
+				return false;
+
+			/* re-fill the buffer after seeking */
+			buffering = true;
+		}
+
 		if (!paused && !OpenOutput()) {
 			FormatError(player_domain,
 				    "problems opening audio device "
@@ -619,6 +648,8 @@ Player::SeekDecoder() noexcept
 {
 	assert(pc.next_song != nullptr);
 
+	CancelPendingSeek();
+
 	{
 		const ScopeUnlock unlock(pc.mutex);
 		pc.outputs.Cancel();
@@ -650,18 +681,22 @@ Player::SeekDecoder() noexcept
 		pc.next_song.reset();
 		queued = false;
 
-		/* wait for the decoder to complete initialization
-		   (just in case that happens to be still in
-		   progress) */
+		if (decoder_starting) {
+			/* wait for the decoder to complete
+			   initialization; postpone the SEEK
+			   command */
 
-		if (!WaitDecoderStartup())
-			return false;
-
-		/* send the SEEK command */
-
-		if (!SeekDecoder(pc.seek_time)) {
+			pending_seek = pc.seek_time;
+			pc.seeking = true;
 			pc.CommandFinished();
-			return false;
+			return true;
+		} else {
+			/* send the SEEK command */
+
+			if (!SeekDecoder(pc.seek_time)) {
+				pc.CommandFinished();
+				return false;
+			}
 		}
 	}
 
@@ -1114,6 +1149,7 @@ Player::Run() noexcept
 		}
 	}
 
+	CancelPendingSeek();
 	StopDecoder();
 
 	ClearAndDeletePipe();