Commit 1ad2475f authored by Max Kellermann's avatar Max Kellermann

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