http://bugs.winehq.org/show_bug.cgi?id=14717
--- Comment #276 from Andrew Eikum aeikum@codeweavers.com 2012-03-07 13:34:03 CST --- Created attachment 39233 --> http://bugs.winehq.org/attachment.cgi?id=39233 Alexander's resampler with simultaneous access fixed
(In reply to comment #274)
Created attachment 38883 [details] Updated patch
Updated to build against 1.4-rc2
Okay, great. Now that 1.4 is out, let's finally get this in. I took your latest patchset (plus modification below) and ran every patch through a series of tests. Specifically, every patch passes the dsound tests and plays the audio in the following games correctly on my Pulse/ALSA machine: Dracula (demo), Darwinia (demo), Alice (demo), Marble Arena 1, Highway Pursuit, Age of Kings (demo), and foobar2000 (both with and without resampling). I'm going to move on to testing with OSS and CoreAudio, but I don't expect problems.
I noticed you moved get_current_sample() from the last patch to the second-to-last patch. That way, we never return less than a full chunk of data. I agree with that change, it makes the code a bit simpler.
While testing the second-to-last patch, I got this FIXME a number of times: IDirectSoundBufferImpl_GetCurrentPosition Bad play position. playpos: 33440, buflen: 32768 It's caused by simultaneous access to the buffer structure in _GetCurrentPosition and in the mixer thread. We increment the buffer's sec_mixpos directly, and "fix" its overflow later. _GetCurrentPosition manages to sneak in before we fix it and dumps that FIXME.
Unfortunately, this isn't harmless. Consider a non-looping buffer that has overflowed. The _GetCurrentPosition implementation will report it as partway into the second loop, when really it should report 0.
To fix this, I kept the local copy of sec_mixpos in cp_fields, and do a single write to sec_mixpos at the end of the function. This only affects the last two patches.
Really, dsound needs a thorough thread safety review, but now isn't the time for that.
You own the first four patches so, if you're still happy with the patchset, you can get those sent off at your leisure (no hurry, the first week or two after release tends to be a zoo anyway).
I'll then get the next two in.
Then we can work on getting the resampler patch itself in. firgen.c (both the idea and the code itself) is a little funky and might require some tweaking to be acceptable in Wine.