[Bug 28723] Sound stutter in Rage when emulated windows version is set to "Windows 7" (XAudio2 -> mmdevapi sound output path)

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=28723

--- Comment #42 from Andrew Eikum <aeikum at codeweavers.com> 2011-11-11 10:51:30 CST ---
(In reply to comment #39)
> Created attachment 37437 [details]
> Proposed patchset, try #2
> 

The first patch gave me tons of underruns (all of the following is without
PulseAudio). I have the following statistics in IAC::Initialize():

trace:alsa:AudioClient_Initialize alsa_period_frames: 470
trace:alsa:AudioClient_Initialize ALSA buffer size: 2352 frames
trace:alsa:AudioClient_Initialize MMDevice period: 0x186a0 * 100ns, 220 frames
trace:alsa:AudioClient_Initialize MMDevice buffer size: 2560 frames

But then I see:
err:alsa:alsa_write_data avail: 1930, held_frames: 2496
trace:alsa:alsa_write_data XRun state, recovering

So it looks like ALSA is reporting an underrun when there isn't enough data in
the ALSA buffer to take a full alsa_period_frames chunk (2352 - 1930 = 422 <
470). So perhaps we need to tweak the write_limit calculation to ensure there's
always at least three mmdev periods _and_ three ALSA periods (three in case we
get the period pattern Wine,Alsa-->Alsa,Wine).

This fixes the problem for me (should clean up the frames-in-alsa-buffer
calculation redundancy, and you'll need to add the alsa_period_frames member):

-    write_limit = This->mmdev_period_frames;
-    /* Make sure ALSA has enough samples to play till next timer event, to get
-       xrun-free behavior we need ALSA to have 2x mmdev_period frames to play.
*/
-    if((This->alsa_bufsize_frames - avail) <= This->mmdev_period_frames)
write_limit += This->mmdev_period_frames;
+    write_limit = 0;
+    while(This->alsa_bufsize_frames - avail + write_limit <
This->mmdev_period_frames * 3 ||
+            This->alsa_bufsize_frames - avail + write_limit <
This->alsa_period_frames * 3)
+        write_limit += This->mmdev_period_frames;

I tested this with a few games with PulseAudio as well, and it worked. Does
this continue to work with RAGE and your other test games?


Some comments on the code in your patches:

-        TRACE("PCM state: %u, avail: %ld, pad: %u\n",
-                snd_pcm_state(This->pcm_handle), avail_frames, *out);
+        *out = This->held_frames;
+        TRACE("pad: %u\n", *out);

I think the snd_pcm_state() value is useful here, if only for debugging the
PulseAudio bug. Unless you have a reason to get rid of it, I'd rather keep it
in.



+#if 0
...
+endif

When you are getting ready to submit the patches, you can just remove this
code.
This patch also creates an unused variable which you can remove,
alsa_period_us.



+    n = max(duration / 10, This->mmdev_period_rt * 3 / 10) ; /* duration
converted to us or 3x our period size */
+    d = 0; 

I know they're taken from my hackish test patch, but please choose a better
variable name than "n" (alsa_buffer_us or something). And you can get rid of
"d" entirely and pass NULL instead.



+    TRACE("n: %u, d: %d\n", n, d);

We trace these values more formally below in that patch, so you can just get
rid of these debug TRACEs.



(In reply to comment #41)
> Changing dsound's primary.c so it don't try to set period size to pre-defined
> value of 50000 seems to fix the problem - no more unexpected xruns, I've got
> only four at expected places after ~5 minutes of RAGE gameplay.

This change is obviously correct, so there should be no problem submitting it.

> specs require to ignore requested period size and use default instead if
> AudioClient is being opened in shared more, which is not the case with the
> current implementation.

This is pretty easy to fix with tests. It's on my list, but feel free to beat
me to it :)

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.