OSS audio race fix
eric pouech
eric.pouech at wanadoo.fr
Fri Nov 9 14:25:36 CST 2001
as reported by Simon Brunnel, there was a race in OSS audio playback
(mainly, then, while processing a notification to a calling process,
the process called, within the notification, a waveOut function which
had to wait on the completion of the operation (like reset, close...))
this patch also applies to wave input functions the same in process
message ring as for wave output
A+
--
---------------
Eric Pouech (http://perso.wanadoo.fr/eric.pouech/)
"The future will be better tomorrow", Vice President Dan Quayle
-------------- next part --------------
Name: oss_race
ChangeLog: fixed some race conditions in notification vs. operation synchronization
using the same in process message ring in waveIn as in waveOut
GenDate: 2001/11/09 20:22:45 UTC
ModifiedFiles: dlls/winmm/wineoss/audio.c
AddedFiles:
===================================================================
RCS file: /usr/share/cvs/cvsroot/wine/wine/dlls/winmm/wineoss/audio.c,v
retrieving revision 1.44
diff -u -u -r1.44 audio.c
--- dlls/winmm/wineoss/audio.c 2001/08/18 16:09:41 1.44
+++ dlls/winmm/wineoss/audio.c 2001/11/09 20:21:37
@@ -81,13 +81,24 @@
#define WINE_WM_CLOSING (WM_USER + 4)
#define WINE_WM_HEADER (WM_USER + 5)
-#define WINE_WM_FIRST WINE_WM_PAUSING
-#define WINE_WM_LAST WINE_WM_HEADER
-
+typedef struct {
+ int msg; /* message identifier */
+ DWORD param; /* parameter for this message */
+ HANDLE hEvent; /* if message is synchronous, handle of event for synchro */
+} OSS_MSG;
+
+/* implement a in process message ring for better performance
+ * (compared to passing thru the server)
+ * this ring will be used by both the input & output plauback
+ */
typedef struct {
- int msg;
- DWORD param;
-} WWO_MSG;
+#define OSS_RING_BUFFER_SIZE 30
+ OSS_MSG messages[OSS_RING_BUFFER_SIZE];
+ int msg_tosave;
+ int msg_toget;
+ HANDLE msg_event;
+ CRITICAL_SECTION msg_crst;
+} OSS_MSG_RING;
typedef struct {
int unixdev;
@@ -109,15 +120,10 @@
DWORD dwRemain; /* number of bytes to write to end the current fragment */
/* synchronization stuff */
+ HANDLE hStartUpEvent;
HANDLE hThread;
DWORD dwThreadID;
- HANDLE hEvent;
-#define WWO_RING_BUFFER_SIZE 30
- WWO_MSG messages[WWO_RING_BUFFER_SIZE];
- int msg_tosave;
- int msg_toget;
- HANDLE msg_event;
- CRITICAL_SECTION msg_crst;
+ OSS_MSG_RING msgRing;
WAVEOUTCAPSA caps;
/* DirectSound stuff */
@@ -140,7 +146,8 @@
/* synchronization stuff */
HANDLE hThread;
DWORD dwThreadID;
- HANDLE hEvent;
+ HANDLE hStartUpEvent;
+ OSS_MSG_RING msgRing;
} WINE_WAVEIN;
static WINE_WAVEOUT WOutDev [MAX_WAVEOUTDRV];
@@ -161,16 +168,16 @@
int bytespersmpl;
int caps;
int mask;
- int i;
+ int i;
- /* start with output device */
+ /* start with output device */
- /* initialize all device handles to -1 */
- for (i = 0; i < MAX_WAVEOUTDRV; ++i)
- {
- WOutDev[i].unixdev = -1;
- }
+ /* initialize all device handles to -1 */
+ for (i = 0; i < MAX_WAVEOUTDRV; ++i)
+ {
+ WOutDev[i].unixdev = -1;
+ }
/* FIXME: only one device is supported */
memset(&WOutDev[0].caps, 0, sizeof(WOutDev[0].caps));
@@ -400,6 +407,70 @@
return 0;
}
+static int OSS_InitRingMessage(OSS_MSG_RING* omr)
+{
+ omr->msg_toget = 0;
+ omr->msg_tosave = 0;
+ omr->msg_event = CreateEventA(NULL, FALSE, FALSE, NULL);
+ memset(omr->messages, 0, sizeof(OSS_MSG) * OSS_RING_BUFFER_SIZE);
+ InitializeCriticalSection(&omr->msg_crst);
+ return 0;
+}
+
+static int OSS_AddRingMessage(OSS_MSG_RING* omr, int msg, DWORD param, BOOL wait)
+{
+ HANDLE hEvent;
+
+ EnterCriticalSection(&omr->msg_crst);
+ if ((omr->msg_tosave == omr->msg_toget) /* buffer overflow ? */
+ && (omr->messages[omr->msg_toget].msg))
+ {
+ ERR("buffer overflow !?\n");
+ LeaveCriticalSection(&omr->msg_crst);
+ return 0;
+ }
+
+ hEvent = wait ? CreateEventA(NULL, FALSE, FALSE, NULL) : INVALID_HANDLE_VALUE;
+
+ omr->messages[omr->msg_tosave].msg = msg;
+ omr->messages[omr->msg_tosave].param = param;
+ omr->messages[omr->msg_tosave].hEvent = hEvent;
+
+ omr->msg_tosave++;
+ if (omr->msg_tosave > OSS_RING_BUFFER_SIZE-1)
+ omr->msg_tosave = 0;
+ LeaveCriticalSection(&omr->msg_crst);
+ /* signal a new message */
+ SetEvent(omr->msg_event);
+ if (wait)
+ {
+ WaitForSingleObject(hEvent, INFINITE);
+ CloseHandle(hEvent);
+ }
+ return 1;
+}
+
+static int OSS_RetrieveRingMessage(OSS_MSG_RING* omr, int *msg, DWORD *param, HANDLE *hEvent)
+{
+ EnterCriticalSection(&omr->msg_crst);
+
+ if (omr->msg_toget == omr->msg_tosave) /* buffer empty ? */
+ {
+ LeaveCriticalSection(&omr->msg_crst);
+ return 0;
+ }
+
+ *msg = omr->messages[omr->msg_toget].msg;
+ omr->messages[omr->msg_toget].msg = 0;
+ *param = omr->messages[omr->msg_toget].param;
+ *hEvent = omr->messages[omr->msg_toget].hEvent;
+ omr->msg_toget++;
+ if (omr->msg_toget > OSS_RING_BUFFER_SIZE-1)
+ omr->msg_toget = 0;
+ LeaveCriticalSection(&omr->msg_crst);
+ return 1;
+}
+
/*======================================================================*
* Low level WAVE OUT implementation *
*======================================================================*/
@@ -522,48 +593,6 @@
}
-int wodPlayer_Message(WINE_WAVEOUT *wwo, int msg, DWORD param)
-{
- EnterCriticalSection(&wwo->msg_crst);
- if ((wwo->msg_tosave == wwo->msg_toget) /* buffer overflow ? */
- && (wwo->messages[wwo->msg_toget].msg))
- {
- ERR("buffer overflow !?\n");
- LeaveCriticalSection(&wwo->msg_crst);
- return 0;
- }
-
- wwo->messages[wwo->msg_tosave].msg = msg;
- wwo->messages[wwo->msg_tosave].param = param;
- wwo->msg_tosave++;
- if (wwo->msg_tosave > WWO_RING_BUFFER_SIZE-1)
- wwo->msg_tosave = 0;
- LeaveCriticalSection(&wwo->msg_crst);
- /* signal a new message */
- SetEvent(wwo->msg_event);
- return 1;
-}
-
-int wodPlayer_RetrieveMessage(WINE_WAVEOUT *wwo, int *msg, DWORD *param)
-{
- EnterCriticalSection(&wwo->msg_crst);
-
- if (wwo->msg_toget == wwo->msg_tosave) /* buffer empty ? */
- {
- LeaveCriticalSection(&wwo->msg_crst);
- return 0;
- }
-
- *msg = wwo->messages[wwo->msg_toget].msg;
- wwo->messages[wwo->msg_toget].msg = 0;
- *param = wwo->messages[wwo->msg_toget].param;
- wwo->msg_toget++;
- if (wwo->msg_toget > WWO_RING_BUFFER_SIZE-1)
- wwo->msg_toget = 0;
- LeaveCriticalSection(&wwo->msg_crst);
- return 1;
-}
-
/**************************************************************************
* wodPlayer_Notify [internal]
*
@@ -642,6 +671,7 @@
DWORD param;
DWORD tc;
BOOL had_msg;
+ HANDLE ev;
wwo->state = WINE_WS_STOPPED;
@@ -652,7 +682,7 @@
wwo->dwPlayedTotal = 0;
TRACE("imhere[0]\n");
- SetEvent(wwo->hEvent);
+ SetEvent(wwo->hStartUpEvent);
for (;;) {
/* wait for dwSleepTime or an event in thread's queue
@@ -680,21 +710,21 @@
TRACE("imhere[1] tc = %08lx\n", GetTickCount());
if (dwSleepTime)
- WaitForSingleObject(wwo->msg_event, dwSleepTime);
+ WaitForSingleObject(wwo->msgRing.msg_event, dwSleepTime);
TRACE("imhere[2] (q=%p p=%p) tc = %08lx\n", wwo->lpQueuePtr,
wwo->lpPlayPtr, GetTickCount());
had_msg = FALSE;
- while (wodPlayer_RetrieveMessage(wwo, &msg, ¶m)) {
+ while (OSS_RetrieveRingMessage(&wwo->msgRing, &msg, ¶m, &ev)) {
had_msg = TRUE;
switch (msg) {
case WINE_WM_PAUSING:
wodPlayer_Reset(wwo, uDevID, FALSE);
wwo->state = WINE_WS_PAUSED;
- SetEvent(wwo->hEvent);
+ SetEvent(ev);
break;
case WINE_WM_RESTARTING:
wwo->state = WINE_WS_PLAYING;
- SetEvent(wwo->hEvent);
+ SetEvent(ev);
break;
case WINE_WM_HEADER:
lpWaveHdr = (LPWAVEHDR)param;
@@ -711,14 +741,14 @@
break;
case WINE_WM_RESETTING:
wodPlayer_Reset(wwo, uDevID, TRUE);
- SetEvent(wwo->hEvent);
+ SetEvent(ev);
break;
case WINE_WM_CLOSING:
/* sanity check: this should not happen since the device must have been reset before */
if (wwo->lpQueuePtr || wwo->lpPlayPtr) ERR("out of sync\n");
wwo->hThread = 0;
wwo->state = WINE_WS_CLOSED;
- SetEvent(wwo->hEvent);
+ SetEvent(ev);
ExitThread(0);
/* shouldn't go here */
default:
@@ -888,21 +918,18 @@
}
wwo->dwFragmentSize = fragment_size;
- wwo->msg_toget = 0;
- wwo->msg_tosave = 0;
- wwo->msg_event = CreateEventA(NULL, FALSE, FALSE, NULL);
- memset(wwo->messages, 0, sizeof(WWO_MSG)*WWO_RING_BUFFER_SIZE);
- InitializeCriticalSection(&wwo->msg_crst);
+ OSS_InitRingMessage(&wwo->msgRing);
if (!(dwFlags & WAVE_DIRECTSOUND)) {
- wwo->hEvent = CreateEventA(NULL, FALSE, FALSE, NULL);
+ wwo->hStartUpEvent = CreateEventA(NULL, FALSE, FALSE, NULL);
wwo->hThread = CreateThread(NULL, 0, wodPlayer, (LPVOID)(DWORD)wDevID, 0, &(wwo->dwThreadID));
- WaitForSingleObject(wwo->hEvent, INFINITE);
+ WaitForSingleObject(wwo->hStartUpEvent, INFINITE);
+ CloseHandle(wwo->hStartUpEvent);
} else {
- wwo->hEvent = INVALID_HANDLE_VALUE;
wwo->hThread = INVALID_HANDLE_VALUE;
wwo->dwThreadID = 0;
}
+ wwo->hStartUpEvent = INVALID_HANDLE_VALUE;
TRACE("fd=%d fragmentSize=%ld\n",
wwo->unixdev, wwo->dwFragmentSize);
@@ -942,10 +969,8 @@
ret = WAVERR_STILLPLAYING;
} else {
TRACE("imhere[3-close]\n");
- if (wwo->hEvent != INVALID_HANDLE_VALUE) {
- wodPlayer_Message(wwo, WINE_WM_CLOSING, 0);
- WaitForSingleObject(wwo->hEvent, INFINITE);
- CloseHandle(wwo->hEvent);
+ if (wwo->hThread != INVALID_HANDLE_VALUE) {
+ OSS_AddRingMessage(&wwo->msgRing, WINE_WM_CLOSING, 0, TRUE);
}
if (wwo->mapping) {
munmap(wwo->mapping, wwo->maplen);
@@ -988,7 +1013,7 @@
lpWaveHdr->lpNext = 0;
TRACE("imhere[3-HEADER]\n");
- wodPlayer_Message(&WOutDev[wDevID], WINE_WM_HEADER, (DWORD)lpWaveHdr);
+ OSS_AddRingMessage(&WOutDev[wDevID].msgRing, WINE_WM_HEADER, (DWORD)lpWaveHdr, FALSE);
return MMSYSERR_NOERROR;
}
@@ -1047,8 +1072,7 @@
}
TRACE("imhere[3-PAUSING]\n");
- wodPlayer_Message(&WOutDev[wDevID], WINE_WM_PAUSING, 0);
- WaitForSingleObject(WOutDev[wDevID].hEvent, INFINITE);
+ OSS_AddRingMessage(&WOutDev[wDevID].msgRing, WINE_WM_PAUSING, 0, TRUE);
return MMSYSERR_NOERROR;
}
@@ -1067,8 +1091,7 @@
if (WOutDev[wDevID].state == WINE_WS_PAUSED) {
TRACE("imhere[3-RESTARTING]\n");
- wodPlayer_Message(&WOutDev[wDevID], WINE_WM_RESTARTING, 0);
- WaitForSingleObject(WOutDev[wDevID].hEvent, INFINITE);
+ OSS_AddRingMessage(&WOutDev[wDevID].msgRing, WINE_WM_RESTARTING, 0, TRUE);
}
/* FIXME: is NotifyClient with WOM_DONE right ? (Comet Busters 1.3.3 needs this notification) */
@@ -1095,8 +1118,7 @@
}
TRACE("imhere[3-RESET]\n");
- wodPlayer_Message(&WOutDev[wDevID], WINE_WM_RESETTING, 0);
- WaitForSingleObject(WOutDev[wDevID].hEvent, INFINITE);
+ OSS_AddRingMessage(&WOutDev[wDevID].msgRing, WINE_WM_RESETTING, 0, TRUE);
return MMSYSERR_NOERROR;
}
@@ -1756,23 +1778,21 @@
WINE_WAVEIN* wwi = (WINE_WAVEIN*)&WInDev[uDevID];
WAVEHDR* lpWaveHdr;
DWORD dwSleepTime;
- MSG msg;
DWORD bytesRead;
-
- audio_buf_info info;
- int xs;
-
- LPVOID buffer = HeapAlloc(GetProcessHeap(),
+ LPVOID buffer = HeapAlloc(GetProcessHeap(),
HEAP_ZERO_MEMORY,
- wwi->dwFragmentSize);
-
+ wwi->dwFragmentSize);
LPVOID pOffset = buffer;
+ audio_buf_info info;
+ int xs;
+ int msg;
+ DWORD param;
+ HANDLE ev;
- PeekMessageA(&msg, 0, 0, 0, 0);
wwi->state = WINE_WS_STOPPED;
wwi->dwTotalRecorded = 0;
- SetEvent(wwi->hEvent);
+ SetEvent(wwi->hStartUpEvent);
/* the soundblaster live needs a micro wake to get its recording started
* (or GETISPACE will have 0 frags all the time)
@@ -1783,7 +1803,7 @@
dwSleepTime = (wwi->dwFragmentSize * 1000) / wwi->format.wf.nAvgBytesPerSec;
TRACE("sleeptime=%ld ms\n", dwSleepTime);
- for (; ; ) {
+ for (;;) {
/* wait for dwSleepTime or an event in thread's queue */
/* FIXME: could improve wait time depending on queue state,
* ie, number of queued fragments
@@ -1891,16 +1911,17 @@
}
}
- MsgWaitForMultipleObjects(0, NULL, FALSE, dwSleepTime, QS_POSTMESSAGE);
+ WaitForSingleObject(wwi->msgRing.msg_event, dwSleepTime);
- while (PeekMessageA(&msg, 0, WINE_WM_FIRST, WINE_WM_LAST, PM_REMOVE)) {
+ while (OSS_RetrieveRingMessage(&wwi->msgRing, &msg, ¶m, &ev))
+ {
- TRACE("msg=0x%x wParam=0x%x lParam=0x%lx\n", msg.message, msg.wParam, msg.lParam);
- switch (msg.message) {
+ TRACE("msg=0x%x param=0x%lx\n", msg, param);
+ switch (msg) {
case WINE_WM_PAUSING:
wwi->state = WINE_WS_PAUSED;
/*FIXME("Device should stop recording\n");*/
- SetEvent(wwi->hEvent);
+ SetEvent(ev);
break;
case WINE_WM_RESTARTING:
{
@@ -1922,11 +1943,11 @@
read(wwi->unixdev, data, 4);
}
- SetEvent(wwi->hEvent);
+ SetEvent(ev);
break;
}
case WINE_WM_HEADER:
- lpWaveHdr = (LPWAVEHDR)msg.lParam;
+ lpWaveHdr = (LPWAVEHDR)param;
lpWaveHdr->lpNext = 0;
/* insert buffer at the end of queue */
@@ -1950,17 +1971,17 @@
}
}
wwi->lpQueuePtr = NULL;
- SetEvent(wwi->hEvent);
+ SetEvent(ev);
break;
case WINE_WM_CLOSING:
wwi->hThread = 0;
wwi->state = WINE_WS_CLOSED;
- SetEvent(wwi->hEvent);
+ SetEvent(ev);
HeapFree(GetProcessHeap(), 0, buffer);
ExitThread(0);
/* shouldn't go here */
default:
- FIXME("unknown message %d\n", msg.message);
+ FIXME("unknown message %d\n", msg);
break;
}
}
@@ -2080,11 +2101,13 @@
wwi->format.wf.nSamplesPerSec, wwi->format.wf.nChannels,
wwi->format.wf.nBlockAlign);
- wwi->hEvent = CreateEventA(NULL, FALSE, FALSE, NULL);
+ wwi->hStartUpEvent = CreateEventA(NULL, FALSE, FALSE, NULL);
wwi->hThread = CreateThread(NULL, 0, widRecorder, (LPVOID)(DWORD)wDevID, 0, &(wwi->dwThreadID));
- WaitForSingleObject(wwi->hEvent, INFINITE);
+ WaitForSingleObject(wwi->hStartUpEvent, INFINITE);
+ CloseHandle(wwi->hStartUpEvent);
+ wwi->hStartUpEvent = INVALID_HANDLE_VALUE;
- if (OSS_NotifyClient(wDevID, WIM_OPEN, 0L, 0L) != MMSYSERR_NOERROR) {
+ if (OSS_NotifyClient(wDevID, WIM_OPEN, 0L, 0L) != MMSYSERR_NOERROR) {
WARN("can't notify client !\n");
return MMSYSERR_INVALPARAM;
}
@@ -2111,9 +2134,7 @@
return WAVERR_STILLPLAYING;
}
- PostThreadMessageA(wwi->dwThreadID, WINE_WM_CLOSING, 0, 0);
- WaitForSingleObject(wwi->hEvent, INFINITE);
- CloseHandle(wwi->hEvent);
+ OSS_AddRingMessage(&wwi->msgRing, WINE_WM_CLOSING, 0, TRUE);
close(wwi->unixdev);
wwi->unixdev = -1;
wwi->dwFragmentSize = 0;
@@ -2147,9 +2168,9 @@
lpWaveHdr->dwFlags |= WHDR_INQUEUE;
lpWaveHdr->dwFlags &= ~WHDR_DONE;
lpWaveHdr->dwBytesRecorded = 0;
- lpWaveHdr->lpNext = NULL;
+ lpWaveHdr->lpNext = NULL;
- PostThreadMessageA(WInDev[wDevID].dwThreadID, WINE_WM_HEADER, 0, (DWORD)lpWaveHdr);
+ OSS_AddRingMessage(&WInDev[wDevID].msgRing, WINE_WM_HEADER, (DWORD)lpWaveHdr, FALSE);
return MMSYSERR_NOERROR;
}
@@ -2200,8 +2221,7 @@
return MMSYSERR_INVALHANDLE;
}
- PostThreadMessageA(WInDev[wDevID].dwThreadID, WINE_WM_RESTARTING, 0, 0);
- WaitForSingleObject(WInDev[wDevID].hEvent, INFINITE);
+ OSS_AddRingMessage(&WInDev[wDevID].msgRing, WINE_WM_RESTARTING, 0, TRUE);
return MMSYSERR_NOERROR;
}
@@ -2216,8 +2236,7 @@
return MMSYSERR_INVALHANDLE;
}
/* FIXME: reset aint stop */
- PostThreadMessageA(WInDev[wDevID].dwThreadID, WINE_WM_RESETTING, 0, 0);
- WaitForSingleObject(WInDev[wDevID].hEvent, INFINITE);
+ OSS_AddRingMessage(&WInDev[wDevID].msgRing, WINE_WM_RESETTING, 0, TRUE);
return MMSYSERR_NOERROR;
}
@@ -2232,8 +2251,7 @@
WARN("can't reset !\n");
return MMSYSERR_INVALHANDLE;
}
- PostThreadMessageA(WInDev[wDevID].dwThreadID, WINE_WM_RESETTING, 0, 0);
- WaitForSingleObject(WInDev[wDevID].hEvent, INFINITE);
+ OSS_AddRingMessage(&WInDev[wDevID].msgRing, WINE_WM_RESETTING, 0, TRUE);
return MMSYSERR_NOERROR;
}
More information about the wine-patches
mailing list