[PATCH] dsound: Use event based threads

[Maarten Lankhorst]

Op 03-12-12 14:59, Joerg-Cyril.Hoehle at 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%29.aspx

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.