Consolidate secondary buffer list processing and locking.

dlls/dsound/buffer.c

@@ -382,9 +382,9 @@ static DWORD WINAPI IDirectSoundBufferImpl_AddRef(LPDIRECTSOUNDBUFFER8 iface) {
 	return ref;
 }
 
-static DWORD WINAPI IDirectSoundBufferImpl_Release(LPDIRECTSOUNDBUFFER8 iface) {
+static DWORD WINAPI IDirectSoundBufferImpl_Release(LPDIRECTSOUNDBUFFER8 iface)
+{
 	ICOM_THIS(IDirectSoundBufferImpl,iface);
-	int	i;
 	DWORD ref;
 
 	TRACE("(%p) ref was %ld, thread is %04lx\n",This, This->ref, GetCurrentThreadId());
@@ -392,22 +392,7 @@ static DWORD WINAPI IDirectSoundBufferImpl_Release(LPDIRECTSOUNDBUFFER8 iface) {
 	ref = InterlockedDecrement(&(This->ref));
 	if (ref) return ref;
 
-	RtlAcquireResourceExclusive(&(This->dsound->lock), TRUE);
-	for (i=0;i<This->dsound->nrofbuffers;i++)
-		if (This->dsound->buffers[i] == This)
-			break;
-	if (i < This->dsound->nrofbuffers) {
-		/* Put the last buffer of the list in the (now empty) position */
-		This->dsound->buffers[i] = This->dsound->buffers[This->dsound->nrofbuffers - 1];
-		This->dsound->nrofbuffers--;
-		This->dsound->buffers = HeapReAlloc(GetProcessHeap(),0,This->dsound->buffers,sizeof(LPDIRECTSOUNDBUFFER8)*This->dsound->nrofbuffers);
-		TRACE("(%p) buffer count is now %d\n", This, This->dsound->nrofbuffers);
-	}
-	if (This->dsound->nrofbuffers == 0) {
-		HeapFree(GetProcessHeap(),0,This->dsound->buffers);
-		This->dsound->buffers = NULL;
-        }
-	RtlReleaseResource(&(This->dsound->lock));
+	DSOUND_RemoveBuffer(This->dsound, This);
 
 	DeleteCriticalSection(&(This->lock));
 
@@ -979,8 +964,7 @@ static HRESULT WINAPI IDirectSoundBufferImpl_QueryInterface(
 		return E_NOINTERFACE;
 	}
 
-	if ( IsEqualGUID( &IID_IDirectSoundNotify, riid ) ||
-	     IsEqualGUID( &IID_IDirectSoundNotify8, riid ) ) {
+	if ( IsEqualGUID( &IID_IDirectSoundNotify, riid ) ) {
 		if (!This->notify)
 			IDirectSoundNotifyImpl_Create(This, &(This->notify));
 		if (This->notify) {
@@ -1188,6 +1172,7 @@ HRESULT WINAPI IDirectSoundBufferImpl_Create(
 				}
 				dsb->buffer->ref = 1;
 			}
+			err = DS_OK;
 		}
 	}
 
@@ -1231,37 +1216,23 @@ HRESULT WINAPI IDirectSoundBufferImpl_Create(
 	InitializeCriticalSection(&(dsb->lock));
         dsb->lock.DebugInfo->Spare[1] = (DWORD)"DSOUNDBUFFER_lock";
 
-	/* register buffer */
-	RtlAcquireResourceExclusive(&(ds->lock), TRUE);
+	/* register buffer if not primary */
 	if (!(dsbd->dwFlags & DSBCAPS_PRIMARYBUFFER)) {
-		IDirectSoundBufferImpl **newbuffers;
-		if (ds->buffers)
-			newbuffers = HeapReAlloc(GetProcessHeap(),0,ds->buffers,sizeof(IDirectSoundBufferImpl*)*(ds->nrofbuffers+1));
-		else
-			newbuffers = HeapAlloc(GetProcessHeap(),0,sizeof(IDirectSoundBufferImpl*)*(ds->nrofbuffers+1));
-
-		if (newbuffers) {
-			ds->buffers = newbuffers;
-			ds->buffers[ds->nrofbuffers] = dsb;
-			ds->nrofbuffers++;
-			TRACE("buffer count is now %d\n", ds->nrofbuffers);
-		} else {
-			ERR("out of memory for buffer list! Current buffer count is %d\n", ds->nrofbuffers);
+		err = DSOUND_AddBuffer(ds, dsb);
+		if (err != DS_OK) {
 			if (dsb->buffer->memory)
 				HeapFree(GetProcessHeap(),0,dsb->buffer->memory);
 			if (dsb->buffer)
 				HeapFree(GetProcessHeap(),0,dsb->buffer);
 			DeleteCriticalSection(&(dsb->lock));
-			RtlReleaseResource(&(ds->lock));
 			HeapFree(GetProcessHeap(),0,dsb->pwfx);
 			HeapFree(GetProcessHeap(),0,dsb);
-			*pdsb = NULL;
-			return DSERR_OUTOFMEMORY;
+			dsb = NULL;
 		}
 	}
-	RtlReleaseResource(&(ds->lock));
+
 	*pdsb = dsb;
-	return S_OK;
+	return err;
 }
 
 HRESULT WINAPI IDirectSoundBufferImpl_Destroy(

dlls/dsound/dsound.c

@@ -273,8 +273,8 @@ static ULONG WINAPI IDirectSoundImpl_Release(
         /* The sleep above should have allowed the timer process to expire
          * but try to grab the lock just in case. Can't hold lock because
          * IDirectSoundBufferImpl_Destroy also grabs the lock */
-        RtlAcquireResourceShared(&(This->lock), TRUE);
-        RtlReleaseResource(&(This->lock));
+        RtlAcquireResourceShared(&(This->buffer_list_lock), TRUE);
+        RtlReleaseResource(&(This->buffer_list_lock));
 
         /* It is allowed to release this object even when buffers are playing */
         if (This->buffers) {
@@ -301,7 +301,7 @@ static ULONG WINAPI IDirectSoundImpl_Release(
         if (This->driver)
             IDsDriver_Release(This->driver);
 
-        RtlDeleteResource(&This->lock);
+        RtlDeleteResource(&This->buffer_list_lock);
         DeleteCriticalSection(&This->mixlock);
         HeapFree(GetProcessHeap(),0,This);
         dsound = NULL;
@@ -409,7 +409,7 @@ static HRESULT WINAPI DSOUND_CreateSoundBuffer(
         if (from8 && (dsbd->dwFlags & DSBCAPS_CTRL3D) && (dsbd->lpwfxFormat->nChannels != 1)) {
             WARN("invalid parameter: 3D buffer format must be mono\n");
             return DSERR_INVALIDPARAM;
-        } 
+        }
 
         hres = IDirectSoundBufferImpl_Create(This, (IDirectSoundBufferImpl**)&dsb, dsbd);
         if (dsb) {
@@ -599,39 +599,24 @@ static HRESULT WINAPI IDirectSoundImpl_DuplicateSoundBuffer(
 
     InitializeCriticalSection(&(dsb->lock));
     dsb->lock.DebugInfo->Spare[1] = (DWORD)"DSOUNDBUFFER_lock";
+
     /* register buffer */
-    RtlAcquireResourceExclusive(&(This->lock), TRUE);
-    {
-        IDirectSoundBufferImpl **newbuffers;
-        if (This->buffers)
-            newbuffers = HeapReAlloc(GetProcessHeap(),0,This->buffers,sizeof(IDirectSoundBufferImpl**)*(This->nrofbuffers+1));
-        else
-            newbuffers = HeapAlloc(GetProcessHeap(),0,sizeof(IDirectSoundBufferImpl**)*(This->nrofbuffers+1));
-
-        if (newbuffers) {
-            This->buffers = newbuffers;
-            This->buffers[This->nrofbuffers] = dsb;
-            This->nrofbuffers++;
-            TRACE("buffer count is now %d\n", This->nrofbuffers);
-        } else {
-            ERR("out of memory for buffer list! Current buffer count is %d\n", This->nrofbuffers);
-            IDirectSoundBuffer8_Release(psb);
-            DeleteCriticalSection(&(dsb->lock));
-            RtlReleaseResource(&(This->lock));
-            HeapFree(GetProcessHeap(),0,dsb->buffer);
-            HeapFree(GetProcessHeap(),0,dsb->pwfx);
-            HeapFree(GetProcessHeap(),0,dsb);
-            *ppdsb = 0;
-            return DSERR_OUTOFMEMORY;
-        }
+    hres = DSOUND_AddBuffer(This, dsb);
+    if (hres != DS_OK) {
+        IDirectSoundBuffer8_Release(psb);
+        DeleteCriticalSection(&(dsb->lock));
+        HeapFree(GetProcessHeap(),0,dsb->buffer);
+        HeapFree(GetProcessHeap(),0,dsb->pwfx);
+        HeapFree(GetProcessHeap(),0,dsb);
+        *ppdsb = 0;
+    } else {
+        hres = SecondaryBufferImpl_Create(dsb, (SecondaryBufferImpl**)ppdsb);
+        if (*ppdsb) {
+            dsb->dsb = (SecondaryBufferImpl*)*ppdsb;
+            IDirectSoundBuffer_AddRef((LPDIRECTSOUNDBUFFER8)*ppdsb);
+        } else
+            WARN("SecondaryBufferImpl_Create failed\n");
     }
-    RtlReleaseResource(&(This->lock));
-    hres = SecondaryBufferImpl_Create(dsb, (SecondaryBufferImpl**)ppdsb);
-    if (*ppdsb) {
-        dsb->dsb = (SecondaryBufferImpl*)*ppdsb;
-        IDirectSoundBuffer_AddRef((LPDIRECTSOUNDBUFFER8)*ppdsb);
-    } else
-        WARN("SecondaryBufferImpl_Create failed\n");
 
     return hres;
 }
@@ -973,7 +958,8 @@ HRESULT WINAPI IDirectSoundImpl_Create(
 
     InitializeCriticalSection(&(pDS->mixlock));
     pDS->mixlock.DebugInfo->Spare[1] = (DWORD)"DSOUND_mixlock";
-    RtlInitializeResource(&(pDS->lock));
+
+    RtlInitializeResource(&(pDS->buffer_list_lock));
 
     *ppDS = (LPDIRECTSOUND8)pDS;
 
@@ -1837,3 +1823,75 @@ HRESULT WINAPI DirectSoundCreate8(
 
     return hr;
 }
+
+/*
+ * Add secondary buffer to buffer list.
+ * Gets exclusive access to buffer for writing.
+ */
+HRESULT DSOUND_AddBuffer(
+    IDirectSoundImpl * pDS,
+    IDirectSoundBufferImpl * pDSB)
+{
+    IDirectSoundBufferImpl **newbuffers;
+    HRESULT hr = DS_OK;
+
+    TRACE("(%p, %p)\n", pDS, pDSB);
+
+    RtlAcquireResourceExclusive(&(pDS->buffer_list_lock), TRUE);
+
+    if (pDS->buffers)
+        newbuffers = HeapReAlloc(GetProcessHeap(),0,pDS->buffers,sizeof(IDirectSoundBufferImpl*)*(pDS->nrofbuffers+1));
+    else
+        newbuffers = HeapAlloc(GetProcessHeap(),0,sizeof(IDirectSoundBufferImpl*)*(pDS->nrofbuffers+1));
+
+    if (newbuffers) {
+        pDS->buffers = newbuffers;
+        pDS->buffers[pDS->nrofbuffers] = pDSB;
+        pDS->nrofbuffers++;
+        TRACE("buffer count is now %d\n", pDS->nrofbuffers);
+    } else {
+        ERR("out of memory for buffer list! Current buffer count is %d\n", pDS->nrofbuffers);
+        hr = DSERR_OUTOFMEMORY;
+    }
+
+    RtlReleaseResource(&(pDS->buffer_list_lock));
+
+    return hr;
+}
+
+/*
+ * Remove secondary buffer from buffer list.
+ * Gets exclusive access to buffer for writing.
+ */
+HRESULT DSOUND_RemoveBuffer(
+    IDirectSoundImpl * pDS,
+    IDirectSoundBufferImpl * pDSB)
+{
+    int i;
+    HRESULT hr = DS_OK;
+
+    TRACE("(%p, %p)\n", pDS, pDSB);
+
+    RtlAcquireResourceExclusive(&(pDS->buffer_list_lock), TRUE);
+
+    for (i = 0; i < pDS->nrofbuffers; i++)
+        if (pDS->buffers[i] == pDSB)
+            break;
+
+    if (i < pDS->nrofbuffers) {
+        /* Put the last buffer of the list in the (now empty) position */
+        pDS->buffers[i] = pDS->buffers[pDS->nrofbuffers - 1];
+        pDS->nrofbuffers--;
+        pDS->buffers = HeapReAlloc(GetProcessHeap(),0,pDS->buffers,sizeof(LPDIRECTSOUNDBUFFER8)*pDS->nrofbuffers);
+        TRACE("buffer count is now %d\n", pDS->nrofbuffers);
+    }
+
+    if (pDS->nrofbuffers == 0) {
+        HeapFree(GetProcessHeap(),0,pDS->buffers);
+        pDS->buffers = NULL;
+    }
+
+    RtlReleaseResource(&(pDS->buffer_list_lock));
+
+    return hr;
+}

dlls/dsound/dsound_main.c

@@ -681,6 +681,12 @@ BOOL WINAPI DllMain(HINSTANCE hInstDLL, DWORD fdwReason, LPVOID lpvReserved)
     case DLL_PROCESS_DETACH:
         TRACE("DLL_PROCESS_DETACH\n");
         break;
+    case DLL_THREAD_ATTACH:
+        TRACE("DLL_THREAD_ATTACH\n");
+        break;
+    case DLL_THREAD_DETACH:
+        TRACE("DLL_THREAD_DETACH\n");
+        break;
     default:
         TRACE("UNKNOWN REASON\n");
         break;

dlls/dsound/dsound_private.h

@@ -89,7 +89,7 @@ struct IDirectSoundImpl
     BOOL                        need_remix;
     int                         nrofbuffers;
     IDirectSoundBufferImpl**    buffers;
-    RTL_RWLOCK                  lock;
+    RTL_RWLOCK                  buffer_list_lock;
     CRITICAL_SECTION            mixlock;
     PrimaryBufferImpl*          primary;
     DSBUFFERDESC                dsbd;
@@ -452,6 +452,11 @@ void DSOUND_RecalcVolPan(PDSVOLUMEPAN volpan);
 void DSOUND_AmpFactorToVolPan(PDSVOLUMEPAN volpan);
 void DSOUND_RecalcFormat(IDirectSoundBufferImpl *dsb);
 
+/* dsound.c */
+
+HRESULT DSOUND_AddBuffer(IDirectSoundImpl * pDS, IDirectSoundBufferImpl * pDSB);
+HRESULT DSOUND_RemoveBuffer(IDirectSoundImpl * pDS, IDirectSoundBufferImpl * pDSB);
+
 /* primary.c */
 
 HRESULT DSOUND_PrimaryCreate(IDirectSoundImpl *This);

dlls/dsound/mixer.c

@@ -838,11 +838,9 @@ static DWORD DSOUND_MixToPrimary(IDirectSoundImpl *dsound, DWORD playpos, DWORD
 	IDirectSoundBufferImpl	*dsb;
 
 	TRACE("(%ld,%ld,%ld,%d)\n", playpos, writepos, mixlen, recover);
-	for (i = dsound->nrofbuffers - 1; i >= 0; i--) {
+	for (i = 0; i < dsound->nrofbuffers; i++) {
 		dsb = dsound->buffers[i];
 
-		if (!dsb || !dsb->lpVtbl)
-			continue;
 		if (dsb->buflen && dsb->state && !dsb->hwbuf) {
 			TRACE("Checking %p, mixlen=%ld\n", dsb, mixlen);
 			EnterCriticalSection(&(dsb->lock));
@@ -853,12 +851,12 @@ static DWORD DSOUND_MixToPrimary(IDirectSoundImpl *dsound, DWORD playpos, DWORD
 			} else {
 				if ((dsb->state == STATE_STARTING) || recover) {
 					dsb->primary_mixpos = writepos;
-					memcpy(&dsb->cvolpan, &dsb->volpan, sizeof(dsb->cvolpan));
+					dsb->cvolpan = dsb->volpan;
 					dsb->need_remix = FALSE;
 				}
 				else if (dsb->need_remix) {
 					DSOUND_MixCancel(dsb, writepos, TRUE);
-					memcpy(&dsb->cvolpan, &dsb->volpan, sizeof(dsb->cvolpan));
+					dsb->cvolpan = dsb->volpan;
 					dsb->need_remix = FALSE;
 				}
 				len = DSOUND_MixOne(dsb, playpos, writepos, mixlen);
@@ -885,11 +883,9 @@ static void DSOUND_MixReset(IDirectSoundImpl *dsound, DWORD writepos)
 	nfiller = dsound->pwfx->wBitsPerSample == 8 ? 128 : 0;
 
 	/* reset all buffer mix positions */
-	for (i = dsound->nrofbuffers - 1; i >= 0; i--) {
+	for (i = 0; i < dsound->nrofbuffers; i++) {
 		dsb = dsound->buffers[i];
 
-		if (!dsb || !dsb->lpVtbl)
-			continue;
 		if (dsb->buflen && dsb->state && !dsb->hwbuf) {
 			TRACE("Resetting %p\n", dsb);
 			EnterCriticalSection(&(dsb->lock));
@@ -900,7 +896,7 @@ static void DSOUND_MixReset(IDirectSoundImpl *dsound, DWORD writepos)
 				/* nothing */
 			} else {
 				DSOUND_MixCancel(dsb, writepos, FALSE);
-				memcpy(&dsb->cvolpan, &dsb->volpan, sizeof(dsb->cvolpan));
+				dsb->cvolpan = dsb->volpan;
 				dsb->need_remix = FALSE;
 			}
 			LeaveCriticalSection(&(dsb->lock));
@@ -960,7 +956,7 @@ void DSOUND_PerformMix(IDirectSoundImpl *dsound)
 	BOOL forced;
 	HRESULT hres;
 
-	TRACE("()\n");
+	TRACE("(%p)\n", dsound);
 
 	/* the sound of silence */
 	nfiller = dsound->pwfx->wBitsPerSample == 8 ? 128 : 0;
@@ -1123,8 +1119,10 @@ void DSOUND_PerformMix(IDirectSoundImpl *dsound)
 void CALLBACK DSOUND_timer(UINT timerID, UINT msg, DWORD dwUser, DWORD dw1, DWORD dw2)
 {
         IDirectSoundImpl* This = (IDirectSoundImpl*)dwUser;
+	DWORD start_time =  GetTickCount();
+        DWORD end_time;
 	TRACE("(%d,%d,0x%lx,0x%lx,0x%lx)\n",timerID,msg,dwUser,dw1,dw2);
-        TRACE("entering at %ld\n", GetTickCount());
+        TRACE("entering at %ld\n", start_time);
 
 	if (dsound != This) {
 		ERR("dsound died without killing us?\n");
@@ -1133,15 +1131,15 @@ void CALLBACK DSOUND_timer(UINT timerID, UINT msg, DWORD dwUser, DWORD dw1, DWOR
 		return;
 	}
 
-	RtlAcquireResourceShared(&(This->lock), TRUE);
+	RtlAcquireResourceShared(&(This->buffer_list_lock), TRUE);
 
-	if (This->ref) {
+	if (This->ref)
 		DSOUND_PerformMix(This);
-	}
 
-	RtlReleaseResource(&(This->lock));
+	RtlReleaseResource(&(This->buffer_list_lock));
 
-	TRACE("completed processing at %ld\n", GetTickCount());
+	end_time = GetTickCount();
+	TRACE("completed processing at %ld, duration = %ld\n", end_time, end_time - start_time);
 }
 
 void CALLBACK DSOUND_callback(HWAVEOUT hwo, UINT msg, DWORD dwUser, DWORD dw1, DWORD dw2)

dlls/dsound/primary.c

@@ -361,7 +361,7 @@ static HRESULT WINAPI PrimaryBufferImpl_SetFormat(
 	      wfex->wBitsPerSample, wfex->cbSize);
 
 	/* **** */
-	RtlAcquireResourceExclusive(&(dsound->lock), TRUE);
+	RtlAcquireResourceExclusive(&(dsound->buffer_list_lock), TRUE);
 
 	if (wfex->wFormatTag == WAVE_FORMAT_PCM) {
             alloc_size = sizeof(WAVEFORMATEX);
@@ -390,12 +390,12 @@ static HRESULT WINAPI PrimaryBufferImpl_SetFormat(
                     err = DSOUND_PrimaryOpen(dsound);
 		    if (err != DS_OK) {
 			    WARN("DSOUND_PrimaryOpen failed\n");
-			    RtlReleaseResource(&(dsound->lock));
+			    RtlReleaseResource(&(dsound->buffer_list_lock));
 			    return err;
 		    }
 		} else {
 			WARN("waveOutOpen failed\n");
-			RtlReleaseResource(&(dsound->lock));
+			RtlReleaseResource(&(dsound->buffer_list_lock));
 			return err;
 		}
 	} else if (dsound->hwbuf) {
@@ -409,14 +409,14 @@ static HRESULT WINAPI PrimaryBufferImpl_SetFormat(
 							  (LPVOID)&(dsound->hwbuf));
 			if (err != DS_OK) {
 				WARN("IDsDriver_CreateSoundBuffer failed\n");
-				RtlReleaseResource(&(dsound->lock));
+				RtlReleaseResource(&(dsound->buffer_list_lock));
 				return err;
 			}
 			if (dsound->state == STATE_PLAYING) dsound->state = STATE_STARTING;
 			else if (dsound->state == STATE_STOPPING) dsound->state = STATE_STOPPED;
 		} else {
 			WARN("IDsDriverBuffer_SetFormat failed\n");
-			RtlReleaseResource(&(dsound->lock));
+			RtlReleaseResource(&(dsound->buffer_list_lock));
 			return err;
 		}
                 /* FIXME: should we set err back to DS_OK in all cases ? */
@@ -437,7 +437,7 @@ static HRESULT WINAPI PrimaryBufferImpl_SetFormat(
 		}
 	}
 
-	RtlReleaseResource(&(dsound->lock));
+	RtlReleaseResource(&(dsound->buffer_list_lock));
 	/* **** */
 
 	return err;
@@ -450,6 +450,7 @@ static HRESULT WINAPI PrimaryBufferImpl_SetVolume(
 	IDirectSoundImpl* dsound = This->dsound;
 	DWORD ampfactors;
 	DSVOLUMEPAN volpan;
+        HRESULT hres = DS_OK;
 
 	TRACE("(%p,%ld)\n",This,vol);
 
@@ -474,13 +475,9 @@ static HRESULT WINAPI PrimaryBufferImpl_SetVolume(
             volpan.lVolume=vol;
             DSOUND_RecalcVolPan(&volpan);
             if (dsound->hwbuf) {
-                HRESULT hres;
                 hres = IDsDriverBuffer_SetVolumePan(dsound->hwbuf, &volpan);
-                if (hres != DS_OK) {
-                    LeaveCriticalSection(&(dsound->mixlock));
+                if (hres != DS_OK)
                     WARN("IDsDriverBuffer_SetVolumePan failed\n");
-                    return hres;
-                }
             } else {
                 ampfactors = (volpan.dwTotalLeftAmpFactor & 0xffff) | (volpan.dwTotalRightAmpFactor << 16);
                 waveOutSetVolume(dsound->hwo, ampfactors);
@@ -490,7 +487,7 @@ static HRESULT WINAPI PrimaryBufferImpl_SetVolume(
 	LeaveCriticalSection(&(dsound->mixlock));
 	/* **** */
 
-	return DS_OK;
+	return hres;
 }
 
 static HRESULT WINAPI PrimaryBufferImpl_GetVolume(
@@ -778,6 +775,7 @@ static HRESULT WINAPI PrimaryBufferImpl_SetPan(
 	IDirectSoundImpl* dsound = This->dsound;
 	DWORD ampfactors;
 	DSVOLUMEPAN volpan;
+        HRESULT hres;
 
 	TRACE("(%p,%ld)\n",This,pan);
 
@@ -802,13 +800,9 @@ static HRESULT WINAPI PrimaryBufferImpl_SetPan(
             volpan.lPan=pan;
             DSOUND_RecalcVolPan(&volpan);
             if (dsound->hwbuf) {
-                HRESULT hres;
                 hres = IDsDriverBuffer_SetVolumePan(dsound->hwbuf, &volpan);
-                if (hres != DS_OK) {
-                    LeaveCriticalSection(&(dsound->mixlock));
+                if (hres != DS_OK)
                     WARN("IDsDriverBuffer_SetVolumePan failed\n");
-                    return hres;
-                }
             }
             else {
                 ampfactors = (volpan.dwTotalLeftAmpFactor & 0xffff) | (volpan.dwTotalRightAmpFactor << 16);