diff --git a/stepmania/src/arch/Sound/RageSoundDriver_DSound.cpp b/stepmania/src/arch/Sound/RageSoundDriver_DSound.cpp index c540d1ba14..f49e9075ca 100644 --- a/stepmania/src/arch/Sound/RageSoundDriver_DSound.cpp +++ b/stepmania/src/arch/Sound/RageSoundDriver_DSound.cpp @@ -42,7 +42,8 @@ void RageSound_DSound::MixerThread() if(!SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_ABOVE_NORMAL)) LOG->Warn(werr_ssprintf(GetLastError(), "Failed to set sound thread priority")); - while(!shutdown) { + while( !shutdown ) + { CHECKPOINT; /* Sleep for the size of one chunk. */ @@ -51,7 +52,7 @@ void RageSound_DSound::MixerThread() Sleep(int(1000 * sleep_secs)); CHECKPOINT; - LockMutex L(SOUNDMAN->lock); + LockMut( m_Mutex ); /* GetData() will return false if the buffer is sufficiently full. Interleave * reads: call GetData for each file before calling it on the same file twice. @@ -65,18 +66,48 @@ void RageSound_DSound::MixerThread() bool bMoreData = false; for(unsigned i = 0; i < stream_pool.size(); ++i) { - if(stream_pool[i]->state == stream_pool[i]->INACTIVE) + /* We're only interested in PLAYING and FLUSHING sounds. */ + if( stream_pool[i]->state != stream::PLAYING && + stream_pool[i]->state != stream::FLUSHING ) continue; /* inactive */ - if( stream_pool[i]->GetData(false) ) + bool bEOF; + if( stream_pool[i]->GetData( false, bEOF ) ) bMoreData = true; + + if( bEOF ) + { + /* FLUSHING tells the mixer thread to release the stream once str->flush_bufs + * buffers have been flushed. */ + stream_pool[i]->state = stream_pool[i]->FLUSHING; + + /* Keep playing until the data we currently have locked has played. */ + stream_pool[i]->flush_pos = stream_pool[i]->pcm->GetOutputPosition(); + } } if( !bMoreData ) break; } + + /* When sounds are in FLUSHING, and we've finished flushing, stop the sound + * and move the sound to FINISHED. Once we do this, it's owned by the main + * thread and we can't touch it anymore. */ + for( unsigned i = 0; i < stream_pool.size(); ++i ) + { + if( stream_pool[i]->state != stream_pool[i]->FLUSHING ) + continue; + + const int64_t ps = stream_pool[i]->pcm->GetPosition(); + if( ps < stream_pool[i]->flush_pos ) + continue; /* still flushing */ + + stream_pool[i]->pcm->Stop(); + + stream_pool[i]->state = stream::FINISHED; + } } - /* I'm not sure why, but if we don't stop the stream now, then the thread will take + /* I'm not sure why, but if we don't stop streams now, then the thread will take * 90ms (our buffer size) longer to close. */ for(unsigned i = 0; i < stream_pool.size(); ++i) if(stream_pool[i]->state != stream_pool[i]->INACTIVE) @@ -88,34 +119,30 @@ void RageSound_DSound::Update(float delta) /* SoundStopped might erase sounds out from under us, so make a copy * of the sound list. */ vector str = stream_pool; - ASSERT(SOUNDMAN); - LockMutex L(SOUNDMAN->lock); for(unsigned i = 0; i < str.size(); ++i) { - if(str[i]->state != str[i]->STOPPING) continue; - - const int64_t ps = str[i]->pcm->GetPosition(); - if(ps < str[i]->flush_pos) - continue; /* stopping but still flushing */ + if( str[i]->state != stream::FINISHED ) + continue; /* The sound has stopped and flushed all of its buffers. */ - if(str[i]->snd != NULL) - str[i]->snd->SoundIsFinishedPlaying(); + str[i]->snd->SoundIsFinishedPlaying(); str[i]->snd = NULL; - str[i]->pcm->Stop(); + /* Once we do this, the sound is once available for use; we must lock + * m_InactiveSoundMutex to take it out of INACTIVE again. */ str[i]->state = str[i]->INACTIVE; } } -/* If init is true, we're filling the buffer while it's stopped, so put - * data in the current buffer (where the play cursor is); otherwise put - * it in the opposite buffer. */ -bool RageSound_DSound::stream::GetData(bool init) +/* If init is true, we're filling the buffer while it's stopped, so fill the + * entire buffer. If false, fill only one chunk. If EOF is reached, set bEOF. */ +bool RageSound_DSound::stream::GetData( bool init, bool &bEOF ) { CHECKPOINT; + bEOF = false; + char *locked_buf; unsigned len; const int64_t play_pos = pcm->GetOutputPosition(); @@ -133,8 +160,8 @@ bool RageSound_DSound::stream::GetData(bool init) } /* It might be INACTIVE, when we're prebuffering. We just don't want to - * fill anything in STOPPING; in that case, we just clear the audio buffer. */ - if(state != STOPPING) + * fill anything in FLUSHING; in that case, we just clear the audio buffer. */ + if(state != FLUSHING) { pcm->SetVolume( snd->GetVolume() ); @@ -168,12 +195,7 @@ bool RageSound_DSound::stream::GetData(bool init) /* Fill the remainder of the buffer with silence. */ memset( locked_buf+got, 0, len-bytes_read ); - /* STOPPING tells the mixer thread to release the stream once str->flush_bufs - * buffers have been flushed. */ - state = STOPPING; - - /* Keep playing until the data we currently have locked has played. */ - flush_pos = pcm->GetOutputPosition(); + bEOF = true; } } else { /* Silence the buffer. */ @@ -190,7 +212,9 @@ RageSound_DSound::stream::~stream() delete pcm; } -RageSound_DSound::RageSound_DSound() +RageSound_DSound::RageSound_DSound(): + m_Mutex("DSoundMutex"), + m_InactiveSoundMutex("InactiveSoundMutex") { shutdown = false; @@ -253,39 +277,46 @@ RageSound_DSound::~RageSound_DSound() void RageSound_DSound::StartMixing( RageSoundBase *snd ) { - LockMutex L(SOUNDMAN->lock); + /* Lock INACTIVE sounds[], and reserve a slot. */ + m_InactiveSoundMutex.Lock(); /* Find an unused buffer. */ unsigned i; - for(i = 0; i < stream_pool.size(); ++i) { - if(stream_pool[i]->state == stream_pool[i]->INACTIVE) + for( i = 0; i < stream_pool.size(); ++i ) + if( stream_pool[i]->state == stream::INACTIVE ) break; - } - if(i == stream_pool.size()) { - /* We don't have a free sound buffer. Fake it. */ - /* XXX: too big of a hack for too rare of a case */ - // SOUNDMAN->AddFakeSound(snd); + if( i == stream_pool.size() ) + { + /* We don't have a free sound buffer. */ + m_InactiveSoundMutex.Unlock(); return; } + /* Place the sound in SETUP, where nobody else will touch it, until we put it + * in FLUSHING or PLAYING below. */ + stream_pool[i]->state = stream::SETUP; + m_InactiveSoundMutex.Unlock(); + /* Give the stream to the playing sound and remove it from the pool. */ stream_pool[i]->snd = snd; stream_pool[i]->pcm->SetSampleRate(snd->GetSampleRate()); stream_pool[i]->start_time = snd->GetStartTime(); - /* Pre-buffer the stream. */ - /* There are two buffers of data; fill them both ahead of time so the - * sound can start almost immediately. */ - stream_pool[i]->GetData(true); + /* Pre-buffer the stream, and start it immediately. */ + bool bEOF; + stream_pool[i]->GetData( true, bEOF ); stream_pool[i]->pcm->Play(); - /* Normally, at this point we should still be INACTIVE, in which case, - * tell the mixer thread to start mixing this channel. However, if it's - * been changed to STOPPING, then we actually finished the whole file - * in the prebuffering GetData calls above, so leave it alone and let it - * finish on its own. */ - if(stream_pool[i]->state == stream_pool[i]->INACTIVE) + /* If bEOF is true, we actually finished the whole file in the prebuffering + * GetData call above, and the sound should go straight to FLUSHING. Otherwise, + * set PLAYING. */ + if( bEOF ) + { + stream_pool[i]->state = stream_pool[i]->FLUSHING; + stream_pool[i]->flush_pos = stream_pool[i]->pcm->GetOutputPosition(); + } + else stream_pool[i]->state = stream_pool[i]->PLAYING; // LOG->Trace("new sound assigned to channel %i", i); @@ -297,8 +328,10 @@ void RageSound_DSound::StartMixing( RageSoundBase *snd ) * again. */ void RageSound_DSound::StopMixing( RageSoundBase *snd ) { + /* Lock, to make sure the decoder thread isn't running on this sound while we do this. */ + LockMut( m_Mutex ); + ASSERT(snd != NULL); - LockMutex L(SOUNDMAN->lock); unsigned i; for(i = 0; i < stream_pool.size(); ++i) @@ -319,8 +352,6 @@ void RageSound_DSound::StopMixing( RageSoundBase *snd ) int64_t RageSound_DSound::GetPosition( const RageSoundBase *snd ) const { - LockMutex L(SOUNDMAN->lock); - unsigned i; for(i = 0; i < stream_pool.size(); ++i) if(stream_pool[i]->snd == snd) break; @@ -330,6 +361,8 @@ int64_t RageSound_DSound::GetPosition( const RageSoundBase *snd ) const ASSERT(i != stream_pool.size()); + /* XXX: This isn't quite threadsafe: GetPosition uses two variables, and we might + * be caught in the middle. We don't want to lock, though ... */ return stream_pool[i]->pcm->GetPosition(); } diff --git a/stepmania/src/arch/Sound/RageSoundDriver_DSound.h b/stepmania/src/arch/Sound/RageSoundDriver_DSound.h index 06258924b3..13d12bf665 100644 --- a/stepmania/src/arch/Sound/RageSoundDriver_DSound.h +++ b/stepmania/src/arch/Sound/RageSoundDriver_DSound.h @@ -11,6 +11,10 @@ struct IDirectSoundBuffer; class RageSound_DSound: public RageSoundDriver { + /* The only place that takes sounds out of INACTIVE is StartMixing; this mutex + * serializes inactive sounds. */ + RageMutex m_InactiveSoundMutex; + struct stream { /* Actual audio stream: */ DSoundBuf *pcm; @@ -21,14 +25,16 @@ class RageSound_DSound: public RageSoundDriver enum { INACTIVE, + SETUP, PLAYING, - STOPPING + FLUSHING, + FINISHED } state; - int64_t flush_pos; /* state == STOPPING only */ + int64_t flush_pos; /* state == FLUSHING only */ RageTimer start_time; - bool GetData(bool init); + bool GetData( bool init, bool &bEOF ); stream() { pcm = NULL; snd = NULL; state=INACTIVE; } ~stream(); @@ -39,6 +45,8 @@ class RageSound_DSound: public RageSoundDriver DSound ds; + RageMutex m_Mutex; + bool shutdown; /* tells the MixerThread to shut down */ static int MixerThread_start(void *p); void MixerThread(); diff --git a/stepmania/src/arch/Sound/RageSoundDriver_DSound_Software.cpp b/stepmania/src/arch/Sound/RageSoundDriver_DSound_Software.cpp index 3261004664..99ed75a7ad 100644 --- a/stepmania/src/arch/Sound/RageSoundDriver_DSound_Software.cpp +++ b/stepmania/src/arch/Sound/RageSoundDriver_DSound_Software.cpp @@ -57,7 +57,6 @@ void RageSound_DSound_Software::MixerThread() int64_t RageSound_DSound_Software::GetPosition( const RageSoundBase *snd ) const { - LockMut(SOUNDMAN->lock); return pcm->GetPosition(); } diff --git a/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.cpp b/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.cpp index a79c0070d4..9fa24886d8 100644 --- a/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.cpp +++ b/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.cpp @@ -17,6 +17,7 @@ RageSound_Generic_Software::sound::sound() { snd = NULL; state = STOPPED; + available = true; } void RageSound_Generic_Software::sound::Init() @@ -40,6 +41,7 @@ void RageSound_Generic_Software::Mix( int16_t *buf, int frames, int64_t frameno, RAGE_ASSERT_M( m_DecodeThread.IsCreated(), "RageSound_Generic_Software::StartDecodeThread() was never called" ); static SoundMixBuffer mix; +//m_Mutex.Lock(); //XXX CHECKPOINT; for( unsigned i = 0; i < ARRAYSIZE(sounds); ++i ) @@ -48,9 +50,11 @@ void RageSound_Generic_Software::Mix( int16_t *buf, int frames, int64_t frameno, sound &s = sounds[i]; if( s.state == sound::HALTING ) { - /* The main thread is waiting for us. */ - s.buffer.clear(); + /* This indicates that this stream can be reused. */ s.state = sound::STOPPED; + s.available = true; + +// LOG->Trace("set %p from HALTING to STOPPED", sounds[i].snd); continue; } @@ -111,7 +115,14 @@ void RageSound_Generic_Software::Mix( int16_t *buf, int frames, int64_t frameno, p[0]->p += frames_to_read*channels; p[0]->frames_in_buffer -= frames_to_read; p[0]->position += frames_to_read; + CString foo = ssprintf("incr fr rd %i += %i", + (int) s.frames_read, (int) frames_to_read ); s.frames_read += frames_to_read; + +// LOG->Trace( "%s = %i (%i left) (state %i) (%p)", +// foo.c_str(), (int) s.frames_read, (int) s.frames_buffered(), s.state, s.snd ); + ASSERT( s.frames_read <= s.frames_written ); + got_frames += frames_to_read; frames_left -= frames_to_read; } @@ -123,6 +134,7 @@ void RageSound_Generic_Software::Mix( int16_t *buf, int frames, int64_t frameno, memset( buf, 0, frames*bytes_per_frame ); mix.read( (Sint16*)buf ); +//m_Mutex.Unlock(); //XXX } @@ -140,12 +152,16 @@ void RageSound_Generic_Software::DecodeThread() /* Fill each playing sound, round-robin. */ SDL_Delay( 1000*chunksize() / GetSampleRate(0) ); - LockMut(SOUNDMAN->lock); + LockMut( m_Mutex ); +// LOG->Trace("begin mix"); + unsigned i; - /* The volume can change while the sound is playing; update it. */ for( i = 0; i < ARRAYSIZE(sounds); ++i ) + { + /* The volume can change while the sound is playing; update it. */ if( sounds[i].state == sound::PLAYING || sounds[i].state == sound::STOPPING ) sounds[i].volume = sounds[i].snd->GetVolume(); + } /* Fill PLAYING sounds, prioritizing sounds that have less sound buffered. */ while( 1 ) @@ -174,8 +190,10 @@ void RageSound_Generic_Software::DecodeThread() { /* This sound is finishing. */ pSound->state = sound::STOPPING; +// LOG->Trace("mixer: (#%i) eof (%i buffered) (%p)", i, (int) pSound->frames_buffered(), pSound->snd ); } } +// LOG->Trace("end mix"); } } @@ -183,6 +201,7 @@ void RageSound_Generic_Software::DecodeThread() * return false. */ bool RageSound_Generic_Software::GetDataForSound( sound &s ) { +//m_Mutex.Lock(); //XXX sound_block *p[2]; unsigned psize[2]; s.buffer.get_write_pointers( p, psize ); @@ -196,18 +215,26 @@ bool RageSound_Generic_Software::GetDataForSound( sound &s ) s.buffer.advance_write_pointer( 1 ); + CString foo = ssprintf("incr fr wr %i += %i", + (int) s.frames_written, (int) b->frames_in_buffer ); + s.frames_written += b->frames_in_buffer; +// LOG->Trace( "%s = %i (%i left) (state %i) (%p)", +// foo.c_str(), (int) s.frames_written, (int) s.frames_buffered(), s.state, s.snd ); +//m_Mutex.Unlock(); //XXX + return !eof; } void RageSound_Generic_Software::Update(float delta) { - ASSERT(SOUNDMAN); - LockMut(SOUNDMAN->lock); - - for( unsigned i = 0; i < ARRAYSIZE(sounds); ++i ) + /* We must not lock here, since the decoder thread might hold the lock for a + * while at a time. This is threadsafe, because once a sound is in STOPPING, + * this is the only place it'll be changed (to STOPPED). */ + unsigned i; + for( i = 0; i < ARRAYSIZE(sounds); ++i ) { if( sounds[i].state != sound::STOPPING ) continue; @@ -215,54 +242,76 @@ void RageSound_Generic_Software::Update(float delta) if( sounds[i].buffer.num_readable() != 0 ) continue; - LOG->Trace("finishing sound %i", i); +// LOG->Trace("finishing sound %i", i); - /* This sound is done. */ - sounds[i].state = sound::STOPPED; sounds[i].snd->SoundIsFinishedPlaying(); + + /* This sound is done. Set it to HALTING, since the mixer thread might + * be accessing it; it'll change it back to STOPPED once it's ready to + * be used again. */ + sounds[i].state = sound::HALTING; +// LOG->Trace("set (#%i) %p from STOPPING to HALTING", i, sounds[i].snd); } } void RageSound_Generic_Software::StartMixing( RageSoundBase *snd ) { + /* Lock available sounds[], and reserve a slot. */ + m_SoundListMutex.Lock(); + unsigned i; for( i = 0; i < ARRAYSIZE(sounds); ++i ) - if( sounds[i].state == sound::STOPPED ) + if( sounds[i].available ) break; if( i == ARRAYSIZE(sounds) ) + { + m_SoundListMutex.Unlock(); return; + } sound &s = sounds[i]; + s.available = false; + + /* We've reserved our slot; we can safely unlock now. Don't hold onto it longer + * than needed, since prebuffering might take some time. */ + m_SoundListMutex.Unlock(); s.snd = snd; s.start_time = snd->GetStartTime(); s.frames_read = s.frames_written = 0; s.sound_id = snd->GetID(); s.volume = snd->GetVolume(); + s.buffer.clear(); + +// LOG->Trace("StartMixing(%s) (%p)", s.snd->GetLoadedFilePath().c_str(), s.snd ); /* Prebuffer some frames before changing the sound to PLAYING. */ bool ReachedEOF = false; while( !ReachedEOF && s.frames_buffered() < frames_to_buffer ) { +// LOG->Trace("StartMixing: (#%i) buffered %i of %i (%i writable) (%p)", i, (int) s.frames_buffered(), (int) frames_to_buffer, s.buffer.num_writable(), s.snd ); if( !GetDataForSound( s ) ) + { +// LOG->Trace("StartMixing: XXX hit EOF (%p)", s.snd ); ReachedEOF = true; + } } /* If we hit EOF already, while prebuffering, then go right to STOPPING. */ s.state = ReachedEOF? sound::STOPPING: sound::PLAYING; - LOG->Trace("finished prebuffering"); +// LOG->Trace("StartMixing: (#%i) finished prebuffering(%s) (%p)", i, s.snd->GetLoadedFilePath().c_str(), s.snd ); } void RageSound_Generic_Software::StopMixing( RageSoundBase *snd ) { /* Lock, to make sure the decoder thread isn't running on this sound while we do this. */ - LockMut(SOUNDMAN->lock); + LockMut( m_Mutex ); /* Find the sound. */ unsigned i; for( i = 0; i < ARRAYSIZE(sounds); ++i ) - if( sounds[i].snd == snd ) + if( !sounds[i].available && sounds[i].snd == snd ) break; if( i == ARRAYSIZE(sounds) ) { @@ -277,12 +326,16 @@ void RageSound_Generic_Software::StopMixing( RageSoundBase *snd ) return; } - /* Tell the mixing thread to flush the buffer. */ +// LOG->Trace("StopMixing: set %p (%s) to HALTING", sounds[i].snd, sounds[i].snd->GetLoadedFilePath().c_str()); + + /* Tell the mixing thread to flush the buffer. We don't have to worry about + * the decoding thread, since we've locked m_Mutex. */ sounds[i].state = sound::HALTING; /* Invalidate the snd pointer to guarantee we don't make any further references to * it. Once this call returns, the sound may no longer exist. */ sounds[i].snd = NULL; +// LOG->Trace("end StopMixing"); } @@ -304,7 +357,9 @@ void RageSound_Generic_Software::SetDecodeBufferSize( int frames ) frames_to_buffer = frames; } -RageSound_Generic_Software::RageSound_Generic_Software() +RageSound_Generic_Software::RageSound_Generic_Software(): + m_Mutex("RageSound_Generic_Software"), + m_SoundListMutex("SoundListMutex") { shutdown_decode_thread = false; SetDecodeBufferSize( 4096 ); diff --git a/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.h b/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.h index a9a71c9685..e02c76277d 100644 --- a/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.h +++ b/stepmania/src/arch/Sound/RageSoundDriver_Generic_Software.h @@ -33,7 +33,7 @@ class RageSound_Generic_Software: public RageSoundDriver * happen on the next iteration. * * The only state change made by the decoding thread is on EOF: the state is changed - * from PLAYING to STOPPING. This is done while SOUNDMAN->lock is held, to prevent + * from PLAYING to STOPPING. This is done while m_Mutex is held, to prevent * races with other threads. * * The only state change made by the mixing thread is from HALTING to STOPPED. @@ -58,6 +58,9 @@ class RageSound_Generic_Software: public RageSoundDriver int64_t frames_read, frames_written; int64_t frames_buffered() const { return frames_written - frames_read; } + /* If true, this sound is in STOPPED and available for use. */ + bool available; + enum { STOPPED, /* idle */ @@ -116,6 +119,14 @@ protected: */ void Mix( int16_t *frames, int nframes, int64_t frameno, int64_t current_frameno ); + /* This mutex is used for serializing with the decoder thread. Locking this mutex + * can take a while. */ + RageMutex m_Mutex; + + /* This mutex locks all sounds[] which are "available". (Other sound may safely + * be accessed, and sounds may be set to available, without locking this.) */ + RageMutex m_SoundListMutex; + public: virtual void Update(float delta);