From 27b14e762e1f9024e797208514ec883e25f9896e Mon Sep 17 00:00:00 2001 From: casey Date: Sun, 1 May 2016 00:52:10 -0700 Subject: [PATCH] Fixed some strange bugs and logic in Player/Stream interaction. Use the boost::condition properly to block waiting when the output device is full. --- src/contrib/oggdecoder/OggDecoder.cpp | 9 ++-- src/contrib/waveout/WaveOut.cpp | 2 +- src/contrib/waveout/WaveOutBuffer.cpp | 1 - src/core/audio/Player.cpp | 73 +++++++++++++-------------- src/core/audio/Player.h | 4 +- src/core/audio/Stream.cpp | 2 +- src/core/sdk/IDecoder.h | 6 --- src/core/sdk/IPlayer.h | 10 +--- 8 files changed, 43 insertions(+), 64 deletions(-) diff --git a/src/contrib/oggdecoder/OggDecoder.cpp b/src/contrib/oggdecoder/OggDecoder.cpp index ee2358ffb..9b44742f2 100644 --- a/src/contrib/oggdecoder/OggDecoder.cpp +++ b/src/contrib/oggdecoder/OggDecoder.cpp @@ -50,7 +50,7 @@ int OggDecoder::OggSeek(void *datasource, ogg_int64_t offset, int whence) { case SEEK_CUR: { long currentPosition = ((OggDecoder*)datasource)->fileStream->Position(); - if(((OggDecoder*)datasource)->fileStream->SetPosition(currentPosition+(long) offset)) { + if(((OggDecoder*) datasource)->fileStream->SetPosition(currentPosition + (long) offset)) { return 0; } } @@ -63,23 +63,24 @@ int OggDecoder::OggSeek(void *datasource, ogg_int64_t offset, int whence) { } } break; - default: + case SEEK_SET: { if(((OggDecoder*) datasource)->fileStream->SetPosition((long) offset)) { return 0; } } + break; } return -1; } long OggDecoder::OggTell(void *datasource) { - return ((OggDecoder*)datasource)->fileStream->Position(); + return ((OggDecoder*) datasource)->fileStream->Position(); } int OggDecoder::OggClose(void *datasource) { - if(((OggDecoder*)datasource)->fileStream->Close()) { + if(((OggDecoder*) datasource)->fileStream->Close()) { return 0; } return -1; diff --git a/src/contrib/waveout/WaveOut.cpp b/src/contrib/waveout/WaveOut.cpp index 6bf22954a..d6ab5aa63 100644 --- a/src/contrib/waveout/WaveOut.cpp +++ b/src/contrib/waveout/WaveOut.cpp @@ -115,7 +115,7 @@ bool WaveOut::Play(IBuffer *buffer, IPlayer *player) { /* if we have a different format, return false and wait for the pending buffers to be written to the output device. */ if (bufferCount > 0) { - bool formatChanged = + bool formatChanged = this->currentChannels != buffer->Channels() || this->currentSampleRate != buffer->SampleRate(); diff --git a/src/contrib/waveout/WaveOutBuffer.cpp b/src/contrib/waveout/WaveOutBuffer.cpp index 1d1fb0d90..b2fed4252 100644 --- a/src/contrib/waveout/WaveOutBuffer.cpp +++ b/src/contrib/waveout/WaveOutBuffer.cpp @@ -68,7 +68,6 @@ void WaveOutBuffer::Destroy() { this->header.dwFlags = WHDR_DONE; } - this->player->Notify(); /* WaveOut should do this... whatever it is. */ this->destroyed = true; } } diff --git a/src/core/audio/Player.cpp b/src/core/audio/Player.cpp index 9faca29fb..7adc7b091 100644 --- a/src/core/audio/Player.cpp +++ b/src/core/audio/Player.cpp @@ -81,7 +81,7 @@ Player::~Player() { void Player::Play() { boost::mutex::scoped_lock lock(this->mutex); this->state = Player::Playing; - this->waitCondition.notify_all(); + this->writeToOutputCondition.notify_all(); } void Player::Stop() { @@ -89,6 +89,7 @@ void Player::Stop() { boost::mutex::scoped_lock lock(this->mutex); this->state = Player::Quit; this->bufferQueue.clear(); + this->writeToOutputCondition.notify_all(); } if (this->output) { @@ -160,7 +161,7 @@ void Player::ThreadLoop() { { boost::mutex::scoped_lock lock(this->mutex); while (this->state == Precache) { - this->waitCondition.wait(lock); + this->writeToOutputCondition.wait(lock); } } @@ -168,6 +169,8 @@ void Player::ThreadLoop() { /* we're ready to go.... */ bool finished = false; + BufferPtr buffer; + while(!finished && !this->Exited()) { /* see if we've been asked to seek since the last sample was played. if we have, clear our output buffer and seek the @@ -184,21 +187,22 @@ void Player::ThreadLoop() { } } - BufferPtr buffer; - - /* the buffer queue may already have some buffers available if it was - prefetched. */ - if (!this->BufferQueueEmpty()) { + /* let's see if we can find some samples to play */ + if (!buffer) { boost::mutex::scoped_lock lock(this->mutex); - buffer = this->bufferQueue.front(); - } - /* otherwise, we need to grab a buffer from the stream and add it to the queue */ - else { - buffer = this->stream->GetNextProcessedOutputBuffer(); - if (buffer) { - boost::mutex::scoped_lock lock(this->mutex); - this->bufferQueue.push_back(buffer); - this->totalBufferSize += buffer->Bytes(); + + /* the buffer queue may already have some available if it was prefetched. */ + if (!this->bufferQueue.empty()) { + buffer = this->bufferQueue.front(); + this->bufferQueue.pop_front(); + } + /* otherwise, we need to grab a buffer from the stream and add it to the queue */ + else { + buffer = this->stream->GetNextProcessedOutputBuffer(); + + if (buffer) { + this->totalBufferSize += buffer->Bytes(); + } } } @@ -213,17 +217,17 @@ void Player::ThreadLoop() { know it's done with it. */ this->lockedBuffers.push_back(buffer); - /* TODO: don't understand this yet... seems like every time we enqueue a - new buffer to the output we remove the old one that was at the front of - */ - if (!this->bufferQueue.empty()) { - this->bufferQueue.pop_front(); - - // Set currentPosition - if (this->lockedBuffers.size() == 1) { - this->currentPosition = buffer->Position(); - } + if (this->lockedBuffers.size() == 1) { + this->currentPosition = buffer->Position(); } + + buffer.reset(); /* important! we're done with this one locally. */ + } + else { + /* the output device queue is full. we should block and wait until + the output lets us know that it needs more data */ + boost::mutex::scoped_lock lock(this->mutex); + writeToOutputCondition.wait(this->mutex); } } /* if we're unable to obtain a buffer, it means we're out of data and the @@ -268,11 +272,6 @@ void Player::ReleaseAllBuffers() { this->lockedBuffers.empty(); } -bool Player::BufferQueueEmpty() { - boost::mutex::scoped_lock lock(this->mutex); - return this->bufferQueue.empty(); -} - bool Player::PreBuffer() { /* don't prebuffer if the buffer is already full */ if (this->totalBufferSize > this->maxBufferSize) { @@ -285,7 +284,6 @@ bool Player::PreBuffer() { boost::mutex::scoped_lock lock(this->mutex); this->bufferQueue.push_back(newBuffer); this->totalBufferSize += newBuffer->Bytes(); - this->waitCondition.notify_all(); /* TODO: what's waiting on this? */ } return true; @@ -317,16 +315,13 @@ void Player::OnBufferProcessedByOutput(IBuffer *buffer) { this->currentPosition = this->lockedBuffers.front()->Position(); } - this->waitCondition.notify_all(); /* TODO: what's waiting on this? */ + /* if the output device's internal buffers are full, it will stop + accepting new samples. now that a buffer has been processed, we can + try to enqueue another sample. the thread loop blocks on this condition */ + this->writeToOutputCondition.notify_all(); return; } } } -void Player::Notify() { - this->waitCondition.notify_all(); -} - - - diff --git a/src/core/audio/Player.h b/src/core/audio/Player.h index 1ae869bd2..eb34be93c 100644 --- a/src/core/audio/Player.h +++ b/src/core/audio/Player.h @@ -62,7 +62,6 @@ namespace musik { namespace core { namespace audio { ~Player(void); virtual void OnBufferProcessedByOutput(IBuffer *buffer); - virtual void Notify(); void Play(); void Stop(); @@ -90,7 +89,6 @@ namespace musik { namespace core { namespace audio { void ThreadLoop(); bool PreBuffer(); int State(); - bool BufferQueueEmpty(); void ReleaseAllBuffers(); protected: @@ -114,7 +112,7 @@ namespace musik { namespace core { namespace audio { BufferList lockedBuffers; boost::mutex mutex; - boost::condition waitCondition; + boost::condition writeToOutputCondition; double volume; double currentPosition; diff --git a/src/core/audio/Stream.cpp b/src/core/audio/Stream.cpp index 0e9ba0cc4..a63af897f 100644 --- a/src/core/audio/Stream.cpp +++ b/src/core/audio/Stream.cpp @@ -134,7 +134,7 @@ BufferPtr Stream::GetNextBufferFromDecoder() { this->decoderSamplePosition += buffer->Samples(); /* calculate the position (seconds) in the buffer */ - buffer->position = ((double)this->decoderSamplePosition) / ((double)this->decoderSampleRate); + buffer->position = ((double) this->decoderSamplePosition) / ((double) this->decoderSampleRate); return buffer; } diff --git a/src/core/sdk/IDecoder.h b/src/core/sdk/IDecoder.h index 41f48eb2e..2cc72dcaa 100644 --- a/src/core/sdk/IDecoder.h +++ b/src/core/sdk/IDecoder.h @@ -54,12 +54,6 @@ class IDecoder{ ////////////////////////////////////////// virtual void Destroy() = 0; - ////////////////////////////////////////// - ///\brief - ///Get the length of the track in seconds - ////////////////////////////////////////// - //virtual double Length() = 0; - ////////////////////////////////////////// ///\brief ///Set the position in the source (in seconds) diff --git a/src/core/sdk/IPlayer.h b/src/core/sdk/IPlayer.h index dbc77f2fa..a18030dfe 100644 --- a/src/core/sdk/IPlayer.h +++ b/src/core/sdk/IPlayer.h @@ -43,7 +43,7 @@ namespace musik { namespace core { namespace audio { ///\brief ///Interface for the audio::Player to make IOuput plugins be able to make callbacks ////////////////////////////////////////// -class IPlayer{ +class IPlayer { public: ////////////////////////////////////////// ///\brief @@ -51,14 +51,6 @@ class IPlayer{ ///processing. ////////////////////////////////////////// virtual void OnBufferProcessedByOutput(IBuffer *buffer) = 0; - - ////////////////////////////////////////// - ///\brief - ///Notifies the Player that there may be buffer. - ///ready to be released in the output plugin. - ///TOOD: ugh... what? - ////////////////////////////////////////// - virtual void Notify() = 0; }; //////////////////////////////////////////////////////////////////////////////