From 1ad2475f9e3c88b992f3fd3d6a77842287d3e4db Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Mon, 28 Oct 2013 10:09:04 +0100
Subject: [PATCH] DecoderControl: convert mutex and client_cond to a reference

Share the Mutex between the DecoderThread and the PlayerThread.  This
simplifies synchronization between the two threads and fixes a freeze
problem: while the PlayerThread waits for the DeocderThread, it cannot
answer requests from the main thread, and the main thread will block
until the DecoderThread finishes.
---
 NEWS                   |  1 +
 src/DecoderControl.cxx |  5 +++--
 src/DecoderControl.hxx | 17 ++++++++++++++---
 src/PlayerThread.cxx   | 31 ++++++++++++-------------------
 4 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/NEWS b/NEWS
index 6cccb5953..6e4bb5439 100644
--- a/NEWS
+++ b/NEWS
@@ -34,6 +34,7 @@ ver 0.18 (2012/??/??)
   - mvp: remove obsolete plugin
 * improved decoder/output error reporting
 * eliminate timer wakeup on idle MPD
+* fix unresponsive MPD while waiting for stream
 
 ver 0.17.6 (2013/10/14)
 * mixer:
diff --git a/src/DecoderControl.cxx b/src/DecoderControl.cxx
index 299acac8c..d76580cbb 100644
--- a/src/DecoderControl.cxx
+++ b/src/DecoderControl.cxx
@@ -26,8 +26,9 @@
 
 #include <assert.h>
 
-DecoderControl::DecoderControl()
-	:state(DecoderState::STOP),
+DecoderControl::DecoderControl(Mutex &_mutex, Cond &_client_cond)
+	:mutex(_mutex), client_cond(_client_cond),
+	 state(DecoderState::STOP),
 	 command(DecoderCommand::NONE),
 	 song(nullptr),
 	 replay_gain_db(0), replay_gain_prev_db(0) {}
diff --git a/src/DecoderControl.hxx b/src/DecoderControl.hxx
index 4b3137108..23cb394ca 100644
--- a/src/DecoderControl.hxx
+++ b/src/DecoderControl.hxx
@@ -62,8 +62,13 @@ struct DecoderControl {
 
 	/**
 	 * This lock protects #state and #command.
+	 *
+	 * This is usually a reference to PlayerControl::mutex, so
+	 * that both player thread and decoder thread share a mutex.
+	 * This simplifies synchronization with #cond and
+	 * #client_cond.
 	 */
-	mutable Mutex mutex;
+	Mutex &mutex;
 
 	/**
 	 * Trigger this object after you have modified #command.  This
@@ -75,8 +80,10 @@ struct DecoderControl {
 	/**
 	 * The trigger of this object's client.  It is signalled
 	 * whenever an event occurs.
+	 *
+	 * This is usually a reference to PlayerControl::cond.
 	 */
-	Cond client_cond;
+	Cond &client_cond;
 
 	DecoderState state;
 	DecoderCommand command;
@@ -143,7 +150,11 @@ struct DecoderControl {
 
 	MixRampInfo mix_ramp, previous_mix_ramp;
 
-	DecoderControl();
+	/**
+	 * @param _mutex see #mutex
+	 * @param _client_cond see #client_cond
+	 */
+	DecoderControl(Mutex &_mutex, Cond &_client_cond);
 	~DecoderControl();
 
 	/**
diff --git a/src/PlayerThread.cxx b/src/PlayerThread.cxx
index 79bfc4018..7273a3300 100644
--- a/src/PlayerThread.cxx
+++ b/src/PlayerThread.cxx
@@ -322,9 +322,9 @@ Player::WaitForDecoder()
 
 	queued = false;
 
-	Error error = dc.LockGetError();
+	pc.Lock();
+	Error error = dc.GetError();
 	if (error.IsDefined()) {
-		pc.Lock();
 		pc.SetError(PlayerError::DECODER, std::move(error));
 
 		pc.next_song->Free();
@@ -347,8 +347,6 @@ Player::WaitForDecoder()
 	   player_check_decoder_startup() */
 	decoder_starting = true;
 
-	pc.Lock();
-
 	/* update PlayerControl's song information */
 	pc.total_time = pc.next_song->GetDuration();
 	pc.bit_rate = 0;
@@ -429,14 +427,11 @@ Player::CheckDecoderStartup()
 {
 	assert(decoder_starting);
 
-	dc.Lock();
+	pc.Lock();
 
 	Error error = dc.GetError();
 	if (error.IsDefined()) {
 		/* the decoder failed */
-		dc.Unlock();
-
-		pc.Lock();
 		pc.SetError(PlayerError::DECODER, std::move(error));
 		pc.Unlock();
 
@@ -444,7 +439,7 @@ Player::CheckDecoderStartup()
 	} else if (!dc.IsStarting()) {
 		/* the decoder is ready and ok */
 
-		dc.Unlock();
+		pc.Unlock();
 
 		if (output_open &&
 		    !audio_output_all_wait(pc, 1))
@@ -475,7 +470,7 @@ Player::CheckDecoderStartup()
 		/* the decoder is not yet ready; wait
 		   some more */
 		dc.WaitForDecoder();
-		dc.Unlock();
+		pc.Unlock();
 
 		return true;
 	}
@@ -807,19 +802,19 @@ Player::PlayNextChunk()
 		} else {
 			/* there are not enough decoded chunks yet */
 
-			dc.Lock();
+			pc.Lock();
 
 			if (dc.IsIdle()) {
 				/* the decoder isn't running, abort
 				   cross fading */
-				dc.Unlock();
+				pc.Unlock();
 
 				xfade_state = CrossFadeState::DISABLED;
 			} else {
 				/* wait for the decoder */
 				dc.Signal();
 				dc.WaitForDecoder();
-				dc.Unlock();
+				pc.Unlock();
 
 				return true;
 			}
@@ -865,12 +860,12 @@ Player::PlayNextChunk()
 	/* this formula should prevent that the decoder gets woken up
 	   with each chunk; it is more efficient to make it decode a
 	   larger block at a time */
-	dc.Lock();
+	pc.Lock();
 	if (!dc.IsIdle() &&
 	    dc.pipe->GetSize() <= (pc.buffered_before_play +
 				   buffer.GetSize() * 3) / 4)
 		dc.Signal();
-	dc.Unlock();
+	pc.Unlock();
 
 	return true;
 }
@@ -957,11 +952,9 @@ Player::Run()
 				    !SendSilence())
 					break;
 
-				dc.Lock();
+				pc.Lock();
 				/* XXX race condition: check decoder again */
 				dc.WaitForDecoder();
-				dc.Unlock();
-				pc.Lock();
 				continue;
 			} else {
 				/* buffering is complete */
@@ -1107,7 +1100,7 @@ player_task(void *arg)
 {
 	PlayerControl &pc = *(PlayerControl *)arg;
 
-	DecoderControl dc;
+	DecoderControl dc(pc.mutex, pc.cond);
 	decoder_thread_start(dc);
 
 	MusicBuffer buffer(pc.buffer_chunks);