Op 19-10-12 15:40, Andrew Eikum schreef: > I like it. Before I can give a sign-off, I need to run it through my > usual battery of tests on all the platforms. I'll do that after patch > 3 gets figured out. > > Some thoughts below... > > On Tue, Oct 16, 2012 at 02:06:28PM +0200, Maarten Lankhorst wrote: >> + /* ALSA is retarded, add a timeout.. */ >> + ret = WaitForSingleObject(dev->sleepev, dev->sleeptime); > No arguments here :) but I'm curious what you mean. Presumably > winealsa isn't sending events when it ought to? I know Jorg has > discovered that failure, I wonder if it's worth testing and fixing > across all the drivers. Well I was abusing the fact that native drivers continue firing after being stopped, I have a followup patch which leaves IAudioClient always running unless you enter primary mode instead.
>> diff --git a/dlls/dsound/primary.c b/dlls/dsound/primary.c >> index 94bdf9c..9d9fa27 100644 >> --- a/dlls/dsound/primary.c >> +++ b/dlls/dsound/primary.c >> @@ -170,6 +172,7 @@ HRESULT DSOUND_ReopenDevice(DirectSoundDevice *device, >> BOOL forcewave) >> device->pwfx->wFormatTag = WAVE_FORMAT_PCM; >> device->pwfx->cbSize = 0; >> } >> + IAudioClient_SetEventHandle(device->client, device->sleepev); >> >> hres = IAudioClient_GetService(device->client, &IID_IAudioRenderClient, >> (void**)&device->render); >> @@ -203,6 +206,19 @@ HRESULT DSOUND_ReopenDevice(DirectSoundDevice *device, >> BOOL forcewave) >> WARN("GetService failed: %08x\n", hres); >> return hres; >> } >> + /* Now kick off the timer so the event fires periodically */ >> + IAudioClient_Start(device->client); >> + >> + IAudioClient_GetStreamLatency(device->client, &period); > I'm a big fan of error checking (or at least reporting), so these > unchecked calls make me nervous. They'll probably never fail, but if > they do, I think they should fail loudly. > Erm, dsound at this point will be worse off if you perform error reporting here than fail silently. It leaves a bigger mess since ReopenDevice is not atomic right now.. I do have a followup patch for this too though but I was going to see if this would go through first before checking if it is worth spending more time fixing things up. ~Maarten