Op 03-12-12 14:59, Joerg-Cyril.Hoehle@t-systems.com schreef:
Hi,
Maarten Lankhorst queried:
Bump, anything wrong with this patch?
Here's my 0.0$ ... (standard DSound disclaimer here...)
Using mmdevapi's events in DSound is basically TRT. Then, the DSound mixer will be synchronized with mmdevapi writes.
/* ALSA is retarded, add a timeout.. */ret = WaitForSingleObject(dev->sleepev, dev->sleeptime);How does that comment help the reader of the code?
- if (period_ms <= 15)
device->sleeptime = period_ms * 5 / 2;- else
device->sleeptime = period_ms * 3 / 2;I expect such threshold functions to be continuous or at least monotonic. A sawtooth is certainly unexpected. What is its purpose?
What I would understand is a comment in the code saying that as of 2012, all 3 wine*.drv mmdevapi drivers have a bug that they stop sending events after IAudioClient_Stop. Therefore add a short timeout to the wait.
Yeah, but in a followup commit I'm going to stop sending IAudioClient_Stop entirely. :-)
If that's the reason for the timeout, then period_ms * 1.5 is perfectly fine. There's no reason for a particular threshold. Note that neither 1.5 nor 2.5 give you regular spacing around the time of transition from playing to stopped.
I just didn't want to make the timeout too short in case no processing is done yet. Alsa's native period is ~ 22ms (1024 samples / 44100 or 48000) with dmix despite claiming it to be otherwise.. I thought about doing something more complicated to make it smoother, but I'm really just increases it >2 buffers to compensate for jitter on lower latencies, so if it the timeout would be beyond 20 ms I don't care any more..
- IAudioClient_GetStreamLatency(device->client, &period);
- period_ms = (period + 9999) / 10000;
If IAC_Stop is the reason for the sleep time out, then it's obvious that GetStreamLatency has no business here, rather than the device period.
I could understand a use of GetStreamLatency when it comes to computing a reasonable size of the primary buffer.
Got it right.. and this is a perfect valid use of IAudioClient GetStreamLatency here, the device could also be dead (AUDCLNT_E_DEVICE_INVALIDATED), in which case events are probably not fired any more.
GetStreamLatency is also used for calculating the period size, see http://msdn.microsoft.com/en-us/library/windows/desktop/dd370874%28v=vs.85%2...
I don't think it's used as such in this commit yet, but in the mixer rework it's used it to calculate fragment length.
ret = WaitForSingleObject(dev->sleepev, dev->sleeptime);... if (ret)WaitFor* return values are defined in terms of WAIT_OBJECT_0, WAIT_TIMEOUT and WAIT_FAILED etc., not zero or non-zero.
Except WAIT_OBJECT_0 is defined as 0.
- device->sleepev = CreateEventW(0, 0, 0, 0);
- device->thread = CreateThread(0, 0, DSOUND_mixthread, device, 0, 0);
I haven't checked the overall style of DSound, but I prefer FALSE and TRUE be used for things advertised as BOOL. (I tend to write NULL for null pointer initialisation. Note that I easily read "if (foo) ..." for boolean and pointer types; I don't expect "if (foo == TRUE) or (foo != NULL) ...")
BTW, I still believe that mixing and resampling would find their best place in mmdevapi, not DSound.
Well some way for dsound and winmm to use them both, so winmm can do resampling internally.