[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 #43 from Alexey Loukianov <mooroon2 at mail.ru> 2011-11-11 11:28:20 CST ---
(In reply to comment #42)
> 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

Main problem here is that your ALSA period is bigger than MMDev period. As I
had already mentioned you should expect underruns in such case. Probably we
should not try to get of ALSA period setup during init but try to set it to be
equal to mmdev period instead.

> 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).

Having three mmdev periods in ALSA buffer won't help in case
(3*mmdev_period_frames < alsa_period_frames) == TRUE. What can be done when
calculating write_limit is to also take into account the alsa_period timeframe.
Easiest way is to take period=max(alsa,mmdev), but it would add excess latency
which might be avoided by doing more precise calculations. I would see what can
be done on this topic and post a new patch with proposed changes when done.

> This fixes the problem for me ...
> 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?

The code you had written is exactly what I've been writing about in previous
paragraph. I'm not too content with using 3*period while it should be enough to
have 2*period - extra latency is not a good thing and it should be avoided if
possible. 3*period would be especially harmful if we don't set ALSA period and
it would be auto-set by ALSA to something really big.

> 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.
> 
OK. I just had removed it from there as with the modifications I made
GetCurrentPadding had been no longer relaying on any data from ALSA. Maybe it
would be reasonable to move snd_pcm_state() trace into alsa_write_buffer and/or
to timer callback handler - IMO there it would be more logical place to call
ALSA stuff.

> 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.

Sure, I just had used the patch you had submitted as-is, without any attempts
to make it more pretty. Having comments by preprocessor ifs in final patchset
is no-no :-). I would clean it up in case we would head on "don't set ALSA
period road" or uncomment it back in case "set ALSA period to match mmdev
period" road would be chosen instead.

> I know they're taken from my hackish test patch...
> 
Sure it would be cleaned up. Just had been using your quick hackish patch as-is
while testing.

> (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.
> 
This change is correct, but mmdev hitting xruns with my patch in + 5ms period -
is wrong. My tests show that most likely xruns I hit are yet again caused by
ALSA period ending up being bigger than mmdev period. Going to test more
thoroughly today.

Thank you for review, I would come back with a new patchset for more :-).

-- 
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.