[PATCH] dsound: Use event based threads
maarten.lankhorst at canonical.com
Mon Dec 3 16:40:51 CST 2012
Op 03-12-12 14:59, Joerg-Cyril.Hoehle at t-systems.com schreef:
> 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
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.
More information about the wine-devel