decoder/Control: pass std::unique_lock<> to Cond::wait()

src/decoder/Bridge.cxx

@@ -74,10 +74,10 @@ DecoderBridge::CheckCancelRead() const noexcept
  * one.
  */
 static DecoderCommand
-need_chunks(DecoderControl &dc) noexcept
+NeedChunks(DecoderControl &dc, std::unique_lock<Mutex> &lock) noexcept
 {
 	if (dc.command == DecoderCommand::NONE)
-		dc.Wait();
+		dc.Wait(lock);
 
 	return dc.command;
 }
@@ -85,8 +85,8 @@ need_chunks(DecoderControl &dc) noexcept
 static DecoderCommand
 LockNeedChunks(DecoderControl &dc) noexcept
 {
-	const std::lock_guard<Mutex> protect(dc.mutex);
+	std::unique_lock<Mutex> lock(dc.mutex);
-	return need_chunks(dc);
+	return NeedChunks(dc, lock);
 }
 
 MusicChunk *

src/decoder/Control.cxx

@@ -39,12 +39,12 @@ DecoderControl::~DecoderControl() noexcept
 }
 
 void
-DecoderControl::WaitForDecoder() noexcept
+DecoderControl::WaitForDecoder(std::unique_lock<Mutex> &lock) noexcept
 {
 	assert(!client_is_waiting);
 	client_is_waiting = true;
 
-	client_cond.wait(mutex);
+	client_cond.wait(lock);
 
 	assert(client_is_waiting);
 	client_is_waiting = false;
@@ -88,7 +88,8 @@ DecoderControl::IsCurrentSong(const DetachedSong &_song) const noexcept
 }
 
 void
-DecoderControl::Start(std::unique_ptr<DetachedSong> _song,
+DecoderControl::Start(std::unique_lock<Mutex> &lock,
+		      std::unique_ptr<DetachedSong> _song,
 		      SongTime _start_time, SongTime _end_time,
 		      MusicBuffer &_buffer,
 		      std::shared_ptr<MusicPipe> _pipe) noexcept
@@ -103,25 +104,25 @@ DecoderControl::Start(std::unique_ptr<DetachedSong> _song,
 	pipe = std::move(_pipe);
 
 	ClearError();
-	SynchronousCommandLocked(DecoderCommand::START);
+	SynchronousCommandLocked(lock, DecoderCommand::START);
 }
 
 void
-DecoderControl::Stop() noexcept
+DecoderControl::Stop(std::unique_lock<Mutex> &lock) noexcept
 {
 	if (command != DecoderCommand::NONE)
 		/* Attempt to cancel the current command.  If it's too
 		   late and the decoder thread is already executing
 		   the old command, we'll call STOP again in this
 		   function (see below). */
-		SynchronousCommandLocked(DecoderCommand::STOP);
+		SynchronousCommandLocked(lock, DecoderCommand::STOP);
 
 	if (state != DecoderState::STOP && state != DecoderState::ERROR)
-		SynchronousCommandLocked(DecoderCommand::STOP);
+		SynchronousCommandLocked(lock, DecoderCommand::STOP);
 }
 
 void
-DecoderControl::Seek(SongTime t)
+DecoderControl::Seek(std::unique_lock<Mutex> &lock, SongTime t)
 {
 	assert(state != DecoderState::START);
 	assert(state != DecoderState::ERROR);
@@ -145,7 +146,7 @@ DecoderControl::Seek(SongTime t)
 
 	seek_time = t;
 	seek_error = false;
-	SynchronousCommandLocked(DecoderCommand::SEEK);
+	SynchronousCommandLocked(lock, DecoderCommand::SEEK);
 
 	if (seek_error)
 		throw std::runtime_error("Decoder failed to seek");

src/decoder/Control.hxx

@@ -207,8 +207,8 @@ public:
 	 * is only valid in the decoder thread.  The object must be locked
 	 * prior to calling this function.
 	 */
-	void Wait() noexcept {
+	void Wait(std::unique_lock<Mutex> &lock) noexcept {
-		cond.wait(mutex);
+		cond.wait(lock);
 	}
 
 	/**
@@ -218,7 +218,7 @@ public:
 	 *
 	 * Caller must hold the lock.
 	 */
-	void WaitForDecoder() noexcept;
+	void WaitForDecoder(std::unique_lock<Mutex> &lock) noexcept;
 
 	bool IsIdle() const noexcept {
 		return state == DecoderState::STOP ||
@@ -318,9 +318,9 @@ private:
 	 * To be called from the client thread.  Caller must lock the
 	 * object.
 	 */
-	void WaitCommandLocked() noexcept {
+	void WaitCommandLocked(std::unique_lock<Mutex> &lock) noexcept {
 		while (command != DecoderCommand::NONE)
-			WaitForDecoder();
+			WaitForDecoder(lock);
 	}
 
 	/**
@@ -330,10 +330,11 @@ private:
 	 * To be called from the client thread.  Caller must lock the
 	 * object.
 	 */
-	void SynchronousCommandLocked(DecoderCommand cmd) noexcept {
+	void SynchronousCommandLocked(std::unique_lock<Mutex> &lock,
+				      DecoderCommand cmd) noexcept {
 		command = cmd;
 		Signal();
-		WaitCommandLocked();
+		WaitCommandLocked(lock);
 	}
 
 	/**
@@ -344,9 +345,9 @@ private:
 	 * object.
 	 */
 	void LockSynchronousCommand(DecoderCommand cmd) noexcept {
-		const std::lock_guard<Mutex> protect(mutex);
+		std::unique_lock<Mutex> lock(mutex);
 		ClearError();
-		SynchronousCommandLocked(cmd);
+		SynchronousCommandLocked(lock, cmd);
 	}
 
 	void LockAsynchronousCommand(DecoderCommand cmd) noexcept {
@@ -382,7 +383,8 @@ public:
 	 * @param pipe the pipe which receives the decoded chunks (owned by
 	 * the caller)
 	 */
-	void Start(std::unique_ptr<DetachedSong> song,
+	void Start(std::unique_lock<Mutex> &lock,
+		   std::unique_ptr<DetachedSong> song,
 		   SongTime start_time, SongTime end_time,
 		   MusicBuffer &buffer,
 		   std::shared_ptr<MusicPipe> pipe) noexcept;
@@ -390,14 +392,14 @@ public:
 	/**
 	 * Caller must lock the object.
 	 */
-	void Stop() noexcept;
+	void Stop(std::unique_lock<Mutex> &lock) noexcept;
 
 	/**
 	 * Throws #std::runtime_error on error.
 	 *
 	 * Caller must lock the object.
 	 */
-	void Seek(SongTime t);
+	void Seek(std::unique_lock<Mutex> &lock, SongTime t);
 
 	void Quit() noexcept;
 

src/decoder/Thread.cxx

@@ -487,7 +487,7 @@ DecoderControl::RunThread() noexcept
 {
 	SetThreadName("decoder");
 
-	const std::lock_guard<Mutex> protect(mutex);
+	std::unique_lock<Mutex> lock(mutex);
 
 	do {
 		assert(state == DecoderState::STOP ||
@@ -528,7 +528,7 @@ DecoderControl::RunThread() noexcept
 			break;
 
 		case DecoderCommand::NONE:
-			Wait();
+			Wait(lock);
 			break;
 		}
 	} while (command != DecoderCommand::NONE || !quit);

src/player/Thread.cxx

@@ -222,7 +222,8 @@ private:
 	 *
 	 * Caller must lock the mutex.
 	 */
-	void StartDecoder(std::shared_ptr<MusicPipe> pipe) noexcept;
+	void StartDecoder(std::unique_lock<Mutex> &lock,
+			  std::shared_ptr<MusicPipe> pipe) noexcept;
 
 	/**
 	 * The decoder has acknowledged the "START" command (see
@@ -235,14 +236,14 @@ private:
 	 * @return false if the decoder has failed, true on success
 	 * (though the decoder startup may or may not yet be finished)
 	 */
-	bool CheckDecoderStartup() noexcept;
+	bool CheckDecoderStartup(std::unique_lock<Mutex> &lock) noexcept;
 
 	/**
 	 * Stop the decoder and clears (and frees) its music pipe.
 	 *
 	 * Caller must lock the mutex.
 	 */
-	void StopDecoder() noexcept;
+	void StopDecoder(std::unique_lock<Mutex> &lock) noexcept;
 
 	/**
 	 * Is the decoder still busy on the same song as the player?
@@ -275,7 +276,8 @@ private:
 	 *
 	 * @return false if the decoder has failed
 	 */
-	bool SeekDecoder(SongTime seek_time) noexcept;
+	bool SeekDecoder(std::unique_lock<Mutex> &lock,
+			 SongTime seek_time) noexcept;
 
 	/**
 	 * This is the handler for the #PlayerCommand::SEEK command.
@@ -284,7 +286,7 @@ private:
 	 *
 	 * @return false if the decoder has failed
 	 */
-	bool SeekDecoder() noexcept;
+	bool SeekDecoder(std::unique_lock<Mutex> &lock) noexcept;
 
 	void CancelPendingSeek() noexcept {
 		pending_seek = SongTime::zero();
@@ -342,7 +344,7 @@ private:
 	 *
 	 * @return false to stop playback
 	 */
-	bool ProcessCommand() noexcept;
+	bool ProcessCommand(std::unique_lock<Mutex> &lock) noexcept;
 
 	/**
 	 * This is called at the border between two songs: the audio output
@@ -363,7 +365,8 @@ public:
 };
 
 void
-Player::StartDecoder(std::shared_ptr<MusicPipe> _pipe) noexcept
+Player::StartDecoder(std::unique_lock<Mutex> &lock,
+		     std::shared_ptr<MusicPipe> _pipe) noexcept
 {
 	assert(queued || pc.command == PlayerCommand::SEEK);
 	assert(pc.next_song != nullptr);
@@ -373,17 +376,17 @@ Player::StartDecoder(std::shared_ptr<MusicPipe> _pipe) noexcept
 
 	SongTime start_time = pc.next_song->GetStartTime() + pc.seek_time;
 
-	dc.Start(std::make_unique<DetachedSong>(*pc.next_song),
+	dc.Start(lock, std::make_unique<DetachedSong>(*pc.next_song),
 		 start_time, pc.next_song->GetEndTime(),
 		 buffer, std::move(_pipe));
 }
 
 void
-Player::StopDecoder() noexcept
+Player::StopDecoder(std::unique_lock<Mutex> &lock) noexcept
 {
 	const PlayerControl::ScopeOccupied occupied(pc);
 
-	dc.Stop();
+	dc.Stop(lock);
 
 	if (dc.pipe != nullptr) {
 		/* clear and free the decoder pipe */
@@ -500,8 +503,8 @@ Player::OpenOutput() noexcept
 	return true;
 }
 
-bool
+inline bool
-Player::CheckDecoderStartup() noexcept
+Player::CheckDecoderStartup(std::unique_lock<Mutex> &lock) noexcept
 {
 	assert(decoder_starting);
 
@@ -534,7 +537,7 @@ Player::CheckDecoderStartup() noexcept
 		if (pending_seek > SongTime::zero()) {
 			assert(pc.seeking);
 
-			bool success = SeekDecoder(pending_seek);
+			bool success = SeekDecoder(lock, pending_seek);
 			pc.seeking = false;
 			pc.ClientSignal();
 			if (!success)
@@ -562,14 +565,14 @@ Player::CheckDecoderStartup() noexcept
 	} else {
 		/* the decoder is not yet ready; wait
 		   some more */
-		dc.WaitForDecoder();
+		dc.WaitForDecoder(lock);
 
 		return true;
 	}
 }
 
 bool
-Player::SeekDecoder(SongTime seek_time) noexcept
+Player::SeekDecoder(std::unique_lock<Mutex> &lock, SongTime seek_time) noexcept
 {
 	assert(song);
 	assert(!decoder_starting);
@@ -583,7 +586,7 @@ Player::SeekDecoder(SongTime seek_time) noexcept
 	try {
 		const PlayerControl::ScopeOccupied occupied(pc);
 
-		dc.Seek(song->GetStartTime() + seek_time);
+		dc.Seek(lock, song->GetStartTime() + seek_time);
 	} catch (...) {
 		/* decoder failure */
 		pc.SetError(PlayerError::DECODER, std::current_exception());
@@ -595,7 +598,7 @@ Player::SeekDecoder(SongTime seek_time) noexcept
 }
 
 inline bool
-Player::SeekDecoder() noexcept
+Player::SeekDecoder(std::unique_lock<Mutex> &lock) noexcept
 {
 	assert(pc.next_song != nullptr);
 
@@ -612,14 +615,14 @@ Player::SeekDecoder() noexcept
 		/* the decoder is already decoding the "next" song -
 		   stop it and start the previous song again */
 
-		StopDecoder();
+		StopDecoder(lock);
 
 		/* clear music chunks which might still reside in the
 		   pipe */
 		pipe->Clear();
 
 		/* re-start the decoder */
-		StartDecoder(pipe);
+		StartDecoder(lock, pipe);
 		ActivateDecoder();
 
 		pc.seeking = true;
@@ -650,7 +653,7 @@ Player::SeekDecoder() noexcept
 		} else {
 			/* send the SEEK command */
 
-			if (!SeekDecoder(pc.seek_time)) {
+			if (!SeekDecoder(lock, pc.seek_time)) {
 				pc.CommandFinished();
 				return false;
 			}
@@ -668,7 +671,7 @@ Player::SeekDecoder() noexcept
 }
 
 inline bool
-Player::ProcessCommand() noexcept
+Player::ProcessCommand(std::unique_lock<Mutex> &lock) noexcept
 {
 	switch (pc.command) {
 	case PlayerCommand::NONE:
@@ -697,7 +700,7 @@ Player::ProcessCommand() noexcept
 		pc.CommandFinished();
 
 		if (dc.IsIdle())
-			StartDecoder(std::make_shared<MusicPipe>());
+			StartDecoder(lock, std::make_shared<MusicPipe>());
 
 		break;
 
@@ -720,7 +723,7 @@ Player::ProcessCommand() noexcept
 		break;
 
 	case PlayerCommand::SEEK:
-		return SeekDecoder();
+		return SeekDecoder(lock);
 
 	case PlayerCommand::CANCEL:
 		if (pc.next_song == nullptr)
@@ -732,7 +735,7 @@ Player::ProcessCommand() noexcept
 		if (IsDecoderAtNextSong())
 			/* the decoder is already decoding the song -
 			   stop it and reset the position */
-			StopDecoder();
+			StopDecoder(lock);
 
 		pc.next_song.reset();
 		queued = false;
@@ -865,7 +868,7 @@ Player::PlayNextChunk() noexcept
 		} else {
 			/* there are not enough decoded chunks yet */
 
-			const std::lock_guard<Mutex> lock(pc.mutex);
+			std::unique_lock<Mutex> lock(pc.mutex);
 
 			if (dc.IsIdle()) {
 				/* the decoder isn't running, abort
@@ -874,7 +877,7 @@ Player::PlayNextChunk() noexcept
 			} else {
 				/* wait for the decoder */
 				dc.Signal();
-				dc.WaitForDecoder();
+				dc.WaitForDecoder(lock);
 
 				return true;
 			}
@@ -960,20 +963,20 @@ Player::Run() noexcept
 {
 	pipe = std::make_shared<MusicPipe>();
 
-	const std::lock_guard<Mutex> lock(pc.mutex);
+	std::unique_lock<Mutex> lock(pc.mutex);
 
-	StartDecoder(pipe);
+	StartDecoder(lock, pipe);
 	ActivateDecoder();
 
 	pc.state = PlayerState::PLAY;
 
 	pc.CommandFinished();
 
-	while (ProcessCommand()) {
+	while (ProcessCommand(lock)) {
 		if (decoder_starting) {
 			/* wait until the decoder is initialized completely */
 
-			if (!CheckDecoderStartup())
+			if (!CheckDecoderStartup(lock))
 				break;
 
 			continue;
@@ -988,7 +991,7 @@ Player::Run() noexcept
 			    !dc.IsIdle() && !buffer.IsFull()) {
 				/* not enough decoded buffer space yet */
 
-				dc.WaitForDecoder();
+				dc.WaitForDecoder(lock);
 				continue;
 			} else {
 				/* buffering is complete */
@@ -1002,7 +1005,7 @@ Player::Run() noexcept
 
 			assert(dc.pipe == nullptr || dc.pipe == pipe);
 
-			StartDecoder(std::make_shared<MusicPipe>());
+			StartDecoder(lock, std::make_shared<MusicPipe>());
 		}
 
 		if (/* no cross-fading if MPD is going to pause at the
@@ -1052,7 +1055,7 @@ Player::Run() noexcept
 			// TODO: eliminate this kludge
 			dc.Signal();
 
-			dc.WaitForDecoder();
+			dc.WaitForDecoder(lock);
 		} else if (IsDecoderAtNextSong()) {
 			/* at the beginning of a new song */
 
@@ -1079,12 +1082,12 @@ Player::Run() noexcept
 			// TODO: eliminate this kludge
 			dc.Signal();
 
-			dc.WaitForDecoder();
+			dc.WaitForDecoder(lock);
 		}
 	}
 
 	CancelPendingSeek();
-	StopDecoder();
+	StopDecoder(lock);
 
 	pipe.reset();