sharing a global variable between threads without proper sync protection or atomic operation is the wrong thing to do moreover you're doing several unrelated changes in the same patch, please split up
A+
2011/3/28 Joerg-Cyril.Hoehle@t-systems.com
Hi,
- PlaySound_Alloc does not call _Free anymore, so it does not use WINMM_cs
anymore, which makes it easier to reason about use of critical sections.
- psStopEvent was never used like an event rather than a boolean.
- No need to muck with WHDR_DONE.
- No need to keep the unused thread handle object around.
- Abort the playloop in case of error, like I did for mciwave. The error
may not be transient.
- Free waveHdr and hEvent after closing hWave. Either you believe that
WAVERR_STILLPLAYING can happen, then obviously waveHdr is in use and must not yet be freed, or you believe checking for STILLPLAYING is a waste of code because waveOutReset must have done its job.
I should have added an ERR() message.
One should think again about what to do in such a case. Why/when would sleep help? User-friendlier might be not to wait, not to free WaveHdr and accept to leak resources in such a case.
Actually, waveOutClose can fail with STILLPLAYING because waveOutReset can fail. If you look at winealsa, you'll see it uses CreateEvent to synchronously wait for acknowledge of the message. Now guess what happens if CreateEvent fails because memory is tight? What happens in the app when waveOutClose's CreateEvent fails? It's always tough to reason about the many error paths.
Regards, Jörg Höhle