dsound locking cleanup
Robert Reif
reif at earthlink.net
Mon Sep 6 15:38:44 CDT 2004
I'm trying to chase down an occasional deadlock in the dsound mixing code.
This patch consolidates secondary buffer list processing and locking.
Two new functions DSOUND_AddBuffer and DSOUND_RemoveBuffer
have been added and the locking has been moved into these functions.
Renamed IDirectSound lock to buffer_list_lock.
Removed redundant test for IID_IDirectSoundNotify and
IID_IDirectSoundNotify8 because they are the same.
Removed check for invalid buffers on list because that shouldn't
happen anymore because of fixed 3d buffer reference counting.
Minor code cleanups.
-------------- next part --------------
Index: dlls/dsound/buffer.c
===================================================================
RCS file: /home/wine/wine/dlls/dsound/buffer.c,v
retrieving revision 1.32
diff -u -r1.32 buffer.c
--- dlls/dsound/buffer.c 23 Aug 2004 19:39:51 -0000 1.32
+++ dlls/dsound/buffer.c 6 Sep 2004 20:31:43 -0000
@@ -382,9 +382,9 @@
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 @@
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 @@
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 @@
}
dsb->buffer->ref = 1;
}
+ err = DS_OK;
}
}
@@ -1231,37 +1216,23 @@
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(
Index: dlls/dsound/dsound.c
===================================================================
RCS file: /home/wine/wine/dlls/dsound/dsound.c,v
retrieving revision 1.18
diff -u -r1.18 dsound.c
--- dlls/dsound/dsound.c 31 Aug 2004 18:50:59 -0000 1.18
+++ dlls/dsound/dsound.c 6 Sep 2004 20:31:44 -0000
@@ -273,8 +273,8 @@
/* 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 @@
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 @@
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 @@
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 @@
InitializeCriticalSection(&(pDS->mixlock));
pDS->mixlock.DebugInfo->Spare[1] = (DWORD)"DSOUND_mixlock";
- RtlInitializeResource(&(pDS->lock));
+
+ RtlInitializeResource(&(pDS->buffer_list_lock));
*ppDS = (LPDIRECTSOUND8)pDS;
@@ -1834,6 +1820,78 @@
hr = DSOUND_Create8(lpcGUID, ppDS, pUnkOuter);
if (hr == DS_OK)
IDirectSoundImpl_Initialize((LPDIRECTSOUND8)dsound, lpcGUID);
+
+ 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;
}
Index: dlls/dsound/dsound_main.c
===================================================================
RCS file: /home/wine/wine/dlls/dsound/dsound_main.c,v
retrieving revision 1.106
diff -u -r1.106 dsound_main.c
--- dlls/dsound/dsound_main.c 23 Aug 2004 19:39:51 -0000 1.106
+++ dlls/dsound/dsound_main.c 6 Sep 2004 20:31:45 -0000
@@ -681,6 +681,12 @@
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;
Index: dlls/dsound/dsound_private.h
===================================================================
RCS file: /home/wine/wine/dlls/dsound/dsound_private.h,v
retrieving revision 1.23
diff -u -r1.23 dsound_private.h
--- dlls/dsound/dsound_private.h 20 Aug 2004 20:01:32 -0000 1.23
+++ dlls/dsound/dsound_private.h 6 Sep 2004 20:31:45 -0000
@@ -89,7 +89,7 @@
BOOL need_remix;
int nrofbuffers;
IDirectSoundBufferImpl** buffers;
- RTL_RWLOCK lock;
+ RTL_RWLOCK buffer_list_lock;
CRITICAL_SECTION mixlock;
PrimaryBufferImpl* primary;
DSBUFFERDESC dsbd;
@@ -451,6 +451,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 */
Index: dlls/dsound/mixer.c
===================================================================
RCS file: /home/wine/wine/dlls/dsound/mixer.c,v
retrieving revision 1.24
diff -u -r1.24 mixer.c
--- dlls/dsound/mixer.c 18 Aug 2004 00:30:37 -0000 1.24
+++ dlls/dsound/mixer.c 6 Sep 2004 20:31:46 -0000
@@ -838,11 +838,9 @@
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 @@
} 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 @@
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 @@
/* 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 @@
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 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 @@
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)
Index: dlls/dsound/primary.c
===================================================================
RCS file: /home/wine/wine/dlls/dsound/primary.c,v
retrieving revision 1.31
diff -u -r1.31 primary.c
--- dlls/dsound/primary.c 23 Aug 2004 19:39:51 -0000 1.31
+++ dlls/dsound/primary.c 6 Sep 2004 20:31:47 -0000
@@ -361,7 +361,7 @@
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 @@
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 @@
(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 @@
}
}
- RtlReleaseResource(&(dsound->lock));
+ RtlReleaseResource(&(dsound->buffer_list_lock));
/* **** */
return err;
@@ -450,6 +450,7 @@
IDirectSoundImpl* dsound = This->dsound;
DWORD ampfactors;
DSVOLUMEPAN volpan;
+ HRESULT hres = DS_OK;
TRACE("(%p,%ld)\n",This,vol);
@@ -474,13 +475,9 @@
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 @@
LeaveCriticalSection(&(dsound->mixlock));
/* **** */
- return DS_OK;
+ return hres;
}
static HRESULT WINAPI PrimaryBufferImpl_GetVolume(
@@ -778,6 +775,7 @@
IDirectSoundImpl* dsound = This->dsound;
DWORD ampfactors;
DSVOLUMEPAN volpan;
+ HRESULT hres;
TRACE("(%p,%ld)\n",This,pan);
@@ -802,13 +800,9 @@
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);
More information about the wine-patches
mailing list