Rewrite sound locking. Having a single global mutex for the sound system

is causing contention, leading to skips.  A detail on this is probably
coming to -devs soon ... (this will break the ALSA hw driver compile until
I update it; an hour or so)

stepmania/src/RageSound.cpp

@@ -57,10 +57,10 @@ RageSoundParams::RageSoundParams():
 	StopMode = M_AUTO;
 }
 
-RageSound::RageSound()
+RageSound::RageSound():
+	m_Mutex( "RageSound" )
 {
 	ASSERT(SOUNDMAN);
-	LockMut(SOUNDMAN->lock);
 
 	original = this;
 	Sample = NULL;
@@ -88,13 +88,24 @@ RageSound::~RageSound()
 }
 
 RageSound::RageSound(const RageSound &cpy):
-	RageSoundBase( cpy )
+	RageSoundBase( cpy ),
+	m_Mutex( "RageSound" )
 {
 	ASSERT(SOUNDMAN);
-	LockMut(SOUNDMAN->lock);
 
 	Sample = NULL;
 
+	*this = cpy;
+
+	/* Register ourself, so we receive Update()s.  We have a different ID than
+	 * our parent. */
+	ID = SOUNDMAN->RegisterSound( this );
+}
+
+RageSound &RageSound::operator=( const RageSound &cpy )
+{
+	LockMut(cpy.m_Mutex);
+
 	original = cpy.original;
 	m_Param = cpy.m_Param;
 	decode_position = cpy.decode_position;
@@ -103,19 +114,20 @@ RageSound::RageSound(const RageSound &cpy):
 	playing_thread = 0;
 
 	databuf.reserve(internal_buffer_size);
+	delete Sample;
 	Sample = cpy.Sample->Copy();
 
 	/* Load() won't work on a copy if m_sFilePath is already set, so
 	 * copy this down here. */
 	m_sFilePath = cpy.m_sFilePath;
 
-	/* Register ourself, so we receive Update()s.  We have a different ID than
-	 * our parent. */
-	ID = SOUNDMAN->RegisterSound( this );
+	return *this;
 }
 
 void RageSound::Unload()
 {
+	LockMut(m_Mutex);
+
 	if(IsPlaying())
 		StopPlaying();
 
@@ -181,17 +193,8 @@ bool RageSound::Load(CString sSoundFilePath, int precache)
 	return true;
 }
 
-/* Read data at the rate we're playing it.  We only do this to smooth out the rate
- * we read data; the sound thread will always read more if it's needed. 
- *
- * Actually, this isn't a good idea.  The sound driver will read in small chunks,
- * interleaving between files.  For example, if four files are playing, and each
- * is two chunks behind, it'll read a chunk from each file twice, instead of reading
- * two chunks for each file at a time, which reduces the chance of underrun. */
 void RageSound::Update(float delta)
 {
-	LockMut(SOUNDMAN->lock);
-
 	/* Erase old pos_map data. */
 //	CleanPosMap( pos_map );
 }
@@ -265,8 +268,6 @@ void RageSound::RateChange(char *buf, int &cnt,
  * actually read; 0 = EOF. */
 int RageSound::FillBuf( int frames )
 {
-	LockMut(SOUNDMAN->lock);
-
 	ASSERT(Sample);
 
 	bool got_something = false;
@@ -401,6 +402,13 @@ void FadeSound( Sint16 *buffer, int frames, float fStartVolume, float fEndVolume
 	}
 }
 
+/* RageSound::GetDataToPlay and RageSound::FillBuf are the main threaded API.  These
+ * need to execute without blocking other threads from calling eg. GetPositionSeconds,
+ * since they may take some time to run.
+ Sample (r), databuf (r)
+ decode_position (r), databuf (r)
+ * 
+ */
 /* Retrieve audio data, for mixing.  At the time of this call, the frameno at which the
  * sound will be played doesn't have to be known.  Once committed, and the frameno
  * is known, call CommitPCMData.  size is in bytes.
@@ -414,9 +422,10 @@ bool RageSound::GetDataToPlay( int16_t *buffer, int size, int &sound_frame, int
 {
 	int NumRewindsThisCall = 0;
 
-	LockMut(SOUNDMAN->lock);
+	/* We only update decode_position; only take a shared lock, so we don't block the main thread. */
+//	LockMut(m_Mutex);
 
-	ASSERT(playing);
+	ASSERT_M( playing, ssprintf("%p", this) );
 
 	frames_stored = 0;
 	sound_frame = decode_position;
@@ -455,7 +464,7 @@ bool RageSound::GetDataToPlay( int16_t *buffer, int size, int &sound_frame, int
 					return false;
 				}
 
-				/* Rewind and start over. */
+				/* Rewind and start over.  XXX: this will take an exclusive lock */
 				SetPositionSeconds( m_Param.m_StartSecond );
 
 				/* Make sure we can get some data.  If we can't, then we'll have
@@ -517,8 +526,6 @@ void RageSound::CommitPlayingPosition( int64_t frameno, int pos, int got_frames
  * Be careful; this is called in a separate thread. */
 int RageSound::GetPCM( char *buffer, int size, int64_t frameno )
 {
-	LockMut(SOUNDMAN->lock);
-
 	ASSERT(playing);
 
 	/*
@@ -553,8 +560,6 @@ int RageSound::GetPCM( char *buffer, int size, int64_t frameno )
  * playing, Stop is called. */
 void RageSound::StartPlaying()
 {
-	LockMut(SOUNDMAN->lock);
-
 	// If no volume is set, use the default.
 	if( m_Param.m_Volume == -1 )
 		m_Param.m_Volume = SOUNDMAN->GetMixVolume();
@@ -568,11 +573,16 @@ void RageSound::StartPlaying()
 			GetLoadedFilePath().c_str(), m_Param.StartTime.Ago() );
 
 	/* Tell the sound manager to start mixing us. */
+//	LOG->Trace("set playing true for %p (StartPlaying) (%s)", this, this->GetLoadedFilePath().c_str());
+
 	playing = true;
 	playing_thread = RageThread::GetCurrentThreadID();
 
 	SOUNDMAN->StartMixing(this);
-	SOUNDMAN->playing_sounds.insert( this );
+
+	SOUNDMAN->RegisterPlayingSound( this );
+
+//	LOG->Trace("StartPlaying %p finished (%s)", this, this->GetLoadedFilePath().c_str());
 }
 
 void RageSound::StopPlaying()
@@ -585,35 +595,47 @@ void RageSound::StopPlaying()
 	/* Tell the sound driver to stop mixing this sound. */
 	SOUNDMAN->StopMixing(this);
 
-	SOUNDMAN->lock.Lock();
-	SOUNDMAN->playing_sounds.erase( this );
-	SOUNDMAN->lock.Unlock();
+	SOUNDMAN->UnregisterPlayingSound( this );
 
+	/* Lock the mutex after calling UnregisterPlayingSound.  We must not make driver
+	 * calls with our mutex locked (driver mutex < sound mutex).  Nobody else will
+	 * see our sound as not playing until we set playing = false. */
+	m_Mutex.Lock();
+
+//	LOG->Trace("set playing false for %p (StopPlaying) (%s)", this, this->GetLoadedFilePath().c_str());
 	playing = false;
 	playing_thread = 0;
 
 	pos_map.Clear();
+//	LOG->Trace("StopPlaying %p finished (%s)", this, this->GetLoadedFilePath().c_str());
+
+	m_Mutex.Unlock();
 }
 
 /* This is similar to StopPlaying, except it's called by sound drivers when we're done
  * playing, rather than by users to as us to stop.  (The only difference is that this
  * doesn't call SOUNDMAN->StopMixing; there's no reason to tell the sound driver to
- * stop mixing, since they're the one telling us we're done.) */
+ * stop mixing, since they're the one telling us we're done.)
+ *
+ * This is only called from the main thread. */
 void RageSound::SoundIsFinishedPlaying()
 {
 	if(!playing)
 		return;
+	m_Mutex.Lock();
 
 	stopped_position = (int) GetPositionSecondsInternal();
 
-	SOUNDMAN->lock.Lock();
-	SOUNDMAN->playing_sounds.erase( this );
-	SOUNDMAN->lock.Unlock();
+	SOUNDMAN->UnregisterPlayingSound( this );
 
+//	LOG->Trace("set playing false for %p (SoundIsFinishedPlaying) (%s)", this, this->GetLoadedFilePath().c_str());
 	playing = false;
 	playing_thread = 0;
 
 	pos_map.Clear();
+//	LOG->Trace("SoundIsFinishedPlaying %p finished (%s)", this, this->GetLoadedFilePath().c_str());
+
+	m_Mutex.Unlock();
 }
 
 RageSound *RageSound::Play( const RageSoundParams *params )
@@ -645,7 +667,7 @@ float RageSound::GetLengthSeconds()
 /* Get the position in frames. */
 int64_t RageSound::GetPositionSecondsInternal( bool *approximate ) const
 {
-	LockMut(SOUNDMAN->lock);
+	LockMut(m_Mutex);
 
 	if( approximate )
 		*approximate = false;
@@ -667,9 +689,6 @@ int64_t RageSound::GetPositionSecondsInternal( bool *approximate ) const
 	/* Get our current hardware position. */
 	int64_t cur_frame = SOUNDMAN->GetPosition(this);
 
-	/* Before using pos_map, flush any incoming positions. */
-	SOUNDMAN->FlushPosMapQueue();
-
 	return pos_map.Search( cur_frame, approximate );
 }
 
@@ -684,7 +703,7 @@ int64_t RageSound::GetPositionSecondsInternal( bool *approximate ) const
 
 float RageSound::GetPositionSeconds( bool *approximate, RageTimer *Timestamp ) const
 {
-	LockMut(SOUNDMAN->lock);
+	LockMut(m_Mutex);
 
 	if( Timestamp )
 	{
@@ -715,10 +734,7 @@ int RageSound::GetSampleRate() const
 
 bool RageSound::SetPositionFrames( int frames )
 {
-	/* This can take a while.  Only lock the sound buffer if we're actually playing. */
-	LockMutex L(SOUNDMAN->lock);
-	if(!playing)
-		L.Unlock();
+	LockMut(m_Mutex);
 
 	{
 		/* "decode_position" records the number of frames we've output to the
@@ -788,6 +804,16 @@ float RageSound::GetVolume() const
 	return m_Param.m_Volume;
 }
 
+void RageSound::LockSound()
+{
+	m_Mutex.Lock();
+}
+
+void RageSound::UnlockSound()
+{
+	m_Mutex.Unlock();
+}
+
 float RageSound::GetPlaybackRate() const
 {
 	return float(m_Param.speed_input_samples) / m_Param.speed_output_samples;

stepmania/src/RageSound.h

@@ -70,6 +70,7 @@ public:
 	RageSound();
 	~RageSound();
 	RageSound(const RageSound &cpy);
+	RageSound &operator=( const RageSound &cpy );
 
 	/* If cache == true (1), we'll preload the entire file into memory if
 	 * it's small enough.  If this is done, a large number of copies of the
@@ -107,6 +108,10 @@ public:
 	bool IsPlaying() const { return playing; }
 	unsigned GetPlayingThread() const { return playing_thread; }
 
+	/* Lock and unlock this sound. */
+	void LockSound();
+	void UnlockSound();
+
 	float GetPlaybackRate() const;
 	RageTimer GetStartTime() const;
 	float GetVolume() const;
@@ -119,6 +124,8 @@ private:
 	 * this is ourself. */
 	RageSound *original;
 
+	mutable RageMutex m_Mutex;
+
 	SoundReader *Sample;
 	CircBuf<char> databuf;
 	int FillBuf(int bytes);

stepmania/src/RageSoundManager.cpp

@@ -14,10 +14,27 @@
 #include "arch/Sound/RageSoundDriver.h"
 #include "SDL_audio.h"
 
+/*
+ * This mutex is locked before Update() deletes old sounds from owned_sounds.  Lock
+ * this mutex if you want to ensure that sounds remain valid.  (Other threads may
+ * still delete them; this only guarantees that RageSoundManager won't.)
+ *
+ * The lock ordering requirements are:
+ * g_DeletionMutex before RageSound::Lock
+ * RageSound::Lock before g_SoundManMutex
+ * RageSound::Lock must not be locked when calling driver calls (since the driver
+ * may lock a mutex and then make RageSound calls back)
+ *
+ * (This is important: you must not make RageSound calls that might lock while holding
+ * g_SoundManMutex, but you can do so while holding g_DeletionMutex.)
+ */
+
+static RageMutex g_DeletionMutex("SoundDeletionMutex");
+static RageMutex g_SoundManMutex("SoundMan");
+
 RageSoundManager *SOUNDMAN = NULL;
 
-RageSoundManager::RageSoundManager(CString drivers):
-	lock("RageSoundManager")
+RageSoundManager::RageSoundManager(CString drivers)
 {
 	/* needs to be done first */
 	SOUNDMAN = this;
@@ -39,12 +56,14 @@ RageSoundManager::RageSoundManager(CString drivers):
 
 RageSoundManager::~RageSoundManager()
 {
-	lock.Lock();
+	g_SoundManMutex.Lock(); /* lock for access to owned_sounds */
+	set<RageSound *> sounds = owned_sounds;
+	g_SoundManMutex.Unlock(); /* finished with owned_sounds */
+
 	/* Clear any sounds that we own and havn't freed yet. */
-	set<RageSound *>::iterator j = owned_sounds.begin();
-	while(j != owned_sounds.end())
+	set<RageSound *>::iterator j = sounds.begin();
+	while(j != sounds.end())
 		delete *(j++);
-	lock.Unlock();
 
 	/* Don't lock while deleting the driver (the decoder thread might deadlock). */
 	delete driver;
@@ -67,11 +86,12 @@ int64_t RageSoundManager::GetPosition( const RageSoundBase *snd ) const
 
 void RageSoundManager::Update(float delta)
 {
-	LockMut(lock);
-
 	FlushPosMapQueue();
 
+	g_DeletionMutex.Lock();
+
 	/* Scan the owned_sounds list for sounds that are no longer playing, and delete them. */
+	g_SoundManMutex.Lock(); /* lock for access to owned_sounds */
 	set<RageSound *>::iterator it;
 	set<RageSound *> ToDelete;
 	for( it = owned_sounds.begin(); it != owned_sounds.end(); ++it )
@@ -79,14 +99,18 @@ void RageSoundManager::Update(float delta)
 			ToDelete.insert( *it );
 
 	for( it = ToDelete.begin(); it != ToDelete.end(); ++it )
-	{
-		delete *it;
 		owned_sounds.erase( *it );
-	}
+	g_SoundManMutex.Unlock(); /* finished with owned_sounds */
 
-	for(set<RageSound *>::iterator i = all_sounds.begin();
-		i != all_sounds.end(); ++i)
-		(*i)->Update(delta);
+	/* We can safely delete sounds while holding g_DeletionMutex, but not while
+	 * holding g_SoundManMutex (see the mutex ordering at the top of the file). */
+	for( it = ToDelete.begin(); it != ToDelete.end(); ++it )
+		delete *it;
+	g_DeletionMutex.Unlock();
+
+//	for(set<RageSound *>::iterator i = all_sounds.begin();
+//		i != all_sounds.end(); ++i)
+//		(*i)->Update(delta);
 
 	driver->Update(delta);
 }
@@ -94,7 +118,7 @@ void RageSoundManager::Update(float delta)
 /* Register the given sound, and return a unique ID. */
 int RageSoundManager::RegisterSound( RageSound *p )
 {
-	LockMut(lock);
+	LockMut(g_SoundManMutex); /* lock for access to all_sounds and iID */
 
 	all_sounds.insert( p );
 
@@ -104,8 +128,23 @@ int RageSoundManager::RegisterSound( RageSound *p )
 
 void RageSoundManager::UnregisterSound( RageSound *p )
 {
-	LockMut(lock);
+	g_SoundManMutex.Lock(); /* lock for access to all_sounds */
 	all_sounds.erase( p );
+	g_SoundManMutex.Unlock(); /* finished with all_sounds */
+}
+
+void RageSoundManager::RegisterPlayingSound( RageSound *p )
+{
+	g_SoundManMutex.Lock(); /* lock for access to playing_sounds */
+	SOUNDMAN->playing_sounds.insert( p );
+	g_SoundManMutex.Unlock(); /* finished with playing_sounds */
+}
+
+void RageSoundManager::UnregisterPlayingSound( RageSound *p )
+{
+	g_SoundManMutex.Lock(); /* lock for access to playing_sounds */
+	SOUNDMAN->playing_sounds.erase( p );
+	g_SoundManMutex.Unlock(); /* finished with playing_sounds */
 }
 
 void RageSoundManager::CommitPlayingPosition( int ID, int64_t frameno, int pos, int got_frames )
@@ -120,27 +159,36 @@ void RageSoundManager::CommitPlayingPosition( int ID, int64_t frameno, int pos,
 	pos_map_queue.write( &p, 1 );
 }
 
+RageSound *RageSoundManager::GetSoundByID( int ID )
+{
+	LockMut(g_SoundManMutex); /* lock for access to all_sounds */
+
+	/* Find the sound with p.ID. */
+	set<RageSound *>::iterator it;
+	for( it = all_sounds.begin(); it != all_sounds.end(); ++it )
+		if( (*it)->GetID() == ID )
+			return *it;
+	return NULL;
+}
+
+/* This is only called by RageSoundManager::Update. */
 void RageSoundManager::FlushPosMapQueue()
 {
-	LockMut(SOUNDMAN->lock);
 	queued_pos_map_t p;
 
+	/* We don't need to lock to access pos_map_queue. */
 	while( pos_map_queue.read( &p, 1 ) )
 	{
-		/* Find the sound with p.ID. */
-		set<RageSound *>::iterator it;
-		for( it = all_sounds.begin(); it != all_sounds.end(); ++it )
-			if( (*it)->GetID() == p.ID )
-				break;
+		RageSound *pSound = GetSoundByID( p.ID );
 
 		/* If we can't find the ID, the sound was probably deleted before we got here. */
-		if( it == all_sounds.end() )
+		if( pSound == NULL )
 		{
 			LOG->Trace("ignored unknown (stale?) commit ID %i", p.ID);
 			continue;
 		}
 
-		(*it)->CommitPlayingPosition( p.frameno, p.pos, p.got_frames );
+		pSound->CommitPlayingPosition( p.frameno, p.pos, p.got_frames );
 	}
 }
 
@@ -156,8 +204,6 @@ int RageSoundManager::GetDriverSampleRate( int rate ) const
 
 RageSound *RageSoundManager::PlaySound( RageSound &snd, const RageSoundParams *params )
 {
-	LockMut(lock);
-
 	RageSound *sound_to_play;
 	if(!snd.IsPlaying())
 		sound_to_play = &snd;
@@ -166,7 +212,9 @@ RageSound *RageSoundManager::PlaySound( RageSound &snd, const RageSoundParams *p
 		sound_to_play = new RageSound(snd);
 
 		/* We're responsible for freeing it. */
+		g_SoundManMutex.Lock(); /* lock for access to owned_sounds */
 		owned_sounds.insert(sound_to_play);
+		g_SoundManMutex.Unlock(); /* finished with owned_sounds */
 	}
 
 	if( params )
@@ -180,45 +228,83 @@ RageSound *RageSoundManager::PlaySound( RageSound &snd, const RageSoundParams *p
 	return sound_to_play;
 }
 
+/* Stop playing all playing sounds derived from the same parent as snd. */
 void RageSoundManager::StopPlayingAllCopiesOfSound(RageSound &snd)
 {
-	LockMut(lock);
+	g_DeletionMutex.Lock();
 
-	/* Stop playing all playing sounds derived from the same parent as snd. */
 	vector<RageSound *> snds;
-	GetCopies(snd, snds);
-	for(vector<RageSound *>::iterator i = snds.begin(); i != snds.end(); i++)
+	GetCopies( snd, snds );
+
+	vector<RageSound *>::iterator it;
+	for( it = snds.begin(); it != snds.end(); ++it )
 	{
-		if((*i)->IsPlaying())
-			(*i)->StopPlaying();
+		if( (*it)->IsPlaying() )
+			(*it)->StopPlaying();
 	}
+
+	g_DeletionMutex.Unlock();
+}
+
+/* XXX: If this is ever called from a thread, it should take a bLockSounds parameter,
+ * like GetCopies. */
+set<RageSound *> RageSoundManager::GetPlayingSounds() const
+{
+	LockMut(g_SoundManMutex); /* lock for access to playing_sounds */
+	return playing_sounds;
 }
 
 void RageSoundManager::StopPlayingSoundsForThisThread()
 {
 	/* Lock to make sure sounds don't become invalidated below before we get to them. */
-	LockMut(lock);
+	g_DeletionMutex.Lock();
 
 	set<RageSound *> Sounds = GetPlayingSounds();
-	for( set<RageSound *>::iterator it = Sounds.begin(); it != Sounds.end(); ++it )
+	set<RageSound *>::iterator it;
+	for( it = Sounds.begin(); it != Sounds.end(); ++it )
 	{
 		if( (*it)->GetPlayingThread() != RageThread::GetCurrentThreadID() )
 			continue;
 		(*it)->Stop();
 	}
+	g_DeletionMutex.Unlock();
 }
 
-void RageSoundManager::GetCopies(RageSound &snd, vector<RageSound *> &snds)
+/*
+ * If bLockSounds is true, all returned sounds will be locked; you must call Unlock()
+ * on all returned sounds when you're done.  This is used for thread safety: without
+ * it, if this is called in a separate thread, returned copies might stop playing
+ * and be deleted by RageSoundManager::Update before you're done with them.
+ *
+ * If bLockSounds is false, sounds are not locked.  This is only safe to use in the same
+ * thread as RageSoundManager::Update (the gameplay thread).
+ */
+void RageSoundManager::GetCopies( RageSound &snd, vector<RageSound *> &snds, bool bLockSounds )
 {
-	LockMut(lock);
+	snds.clear();
 
+	/* Locking this means that Update() will not delete sounds.  g_SoundManMutex does that,
+	 * too, but we can't hold g_SoundManMutex when we lock sounds. */
+	g_DeletionMutex.Lock();
+
+	g_SoundManMutex.Lock(); /* lock for access to all_sounds */
+	set<RageSound *> sounds = all_sounds;
+	g_SoundManMutex.Unlock(); /* finished with owned_sounds */
+	
 	RageSound *parent = snd.GetOriginal();
 
-	snds.clear();
-	for(set<RageSound *>::iterator i = playing_sounds.begin();
-		i != playing_sounds.end(); i++)
-		if((*i)->GetOriginal() == parent)
-			snds.push_back(*i);
+	set<RageSound *>::iterator it;
+	for( it = sounds.begin(); it != sounds.end(); ++it )
+	{
+		if( (*it)->GetOriginal() != parent )
+			continue;
+		if( bLockSounds )
+			(*it)->LockSound();
+
+		snds.push_back( *it );
+	}
+
+	g_DeletionMutex.Unlock();
 }
 
 /* Don't hold the lock when we don't have to.  We call this function from other
@@ -234,17 +320,17 @@ void RageSoundManager::PlayOnce( CString sPath )
 	/* We're responsible for freeing it.  Add it to owned_sounds *after* we start
 	 * playing, so RageSoundManager::Update doesn't free it before we actually start
 	 * it. */
-	LockMut(lock);
-	lock.Lock();
+	g_SoundManMutex.Lock(); /* lock for access to owned_sounds */
 	owned_sounds.insert(snd);
-	lock.Unlock();
+	g_SoundManMutex.Unlock(); /* finished with owned_sounds */
 }
 
 void RageSoundManager::SetPrefs(float MixVol)
 {
-	LockMut(lock);
-
+	g_SoundManMutex.Lock(); /* lock for access to MixVolume */
 	MixVolume = MixVol;
+	g_SoundManMutex.Unlock(); /* finished with MixVolume */
+
 	driver->VolumeChanged();
 }
 

stepmania/src/RageSoundManager.h

@@ -4,7 +4,6 @@
 #include <set>
 #include <map>
 #include "SDL_utils.h"
-#include "RageThreads.h"
 #include "RageUtil_CircularBuffer.h"
 
 class RageSound;
@@ -17,7 +16,11 @@ class RageSoundManager
 	/* Set of sounds that we've taken over (and are responsible for deleting
 	 * when they're finished playing): */
 	set<RageSound *> owned_sounds;
+	set<RageSound *> playing_sounds;
 
+	/* A list of all sounds that currently exist. */
+	set<RageSound *> all_sounds;
+	
 	RageSoundDriver *driver;
 
 	/* Prefs: */
@@ -31,8 +34,6 @@ class RageSoundManager
 	CircBuf<queued_pos_map_t> pos_map_queue;
 
 public:
-	RageMutex lock;
-
 	RageSoundManager(CString drivers);
 	~RageSoundManager();
 
@@ -45,11 +46,12 @@ public:
 	int64_t GetPosition( const RageSoundBase *snd ) const;	/* used by RageSound */
 	int RegisterSound( RageSound *p );		/* used by RageSound */
 	void UnregisterSound( RageSound *p );	/* used by RageSound */
+	void RegisterPlayingSound( RageSound *p );	/* used by RageSound */
+	void UnregisterPlayingSound( RageSound *p );	/* used by RageSound */
 	void CommitPlayingPosition( int ID, int64_t frameno, int pos, int got_bytes );	/* used by drivers */
-	void FlushPosMapQueue();				/* used by RageSound */
 	float GetPlayLatency() const;
 	int GetDriverSampleRate( int rate ) const;
-	const set<RageSound *> &GetPlayingSounds() const { return playing_sounds; }
+	set<RageSound *> GetPlayingSounds() const;
 
 	void PlayOnce( CString sPath );
 
@@ -60,15 +62,13 @@ public:
 	 * before exiting a thread. */
 	void StopPlayingSoundsForThisThread();
 
-	/* A list of all sounds that currently exist.  RageSound adds and removes
-	 * itself to this. */
-	set<RageSound *> all_sounds;
-	
-	/* RageSound adds and removes itself to this. */
-	set<RageSound *> playing_sounds;
-	void GetCopies(RageSound &snd, vector<RageSound *> &snds);
+	void GetCopies( RageSound &snd, vector<RageSound *> &snds, bool bLockSounds=false );
 
 	static void AttenuateBuf( Sint16 *buf, int samples, float vol );
+
+private:
+	void FlushPosMapQueue();
+	RageSound *GetSoundByID( int ID );
 };
 
 /* This inputs and outputs 16-bit 44khz stereo input. */