[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 #34 from Alexey Loukianov <mooroon2 at mail.ru> 2011-11-06 19:48:58 CST ---
My tests on native system with shared mode mmdevapi shows that your estimation
looks like pretty correct, but OTOH it's far from the real situation XA2 deals
with on native system. Main point is that XA2 ignores any extra buffer space it
had been offered by mmdev device and behaves as it had been provided with the
buffer size it had requested. So if the default period size is something like
10ms (and that's approx the case for the hardware I have in my laptop) XA2
requests 20ms buffer and would use no more than 20ms from this buffer even if
GetBufferSize() shows that it had been provided with something bigger.

> 0. Before start, the app prefills 30ms according to your comment #19.

Em, I can't find where in the comment #19 I had been talking about 100% prefill
(you're considering 30ms duration / 10ms period case, right?). For period sizes
less than 10.5ms XA2 prefills 1/2 of duration before calling Start(), you can
check it using the values from the first PDF I had attached to this report.

Rest of your simulation seems to be pretty close to what one might see with
native XA2 playing data through mmdevapi in shared mode.

IMO, what we have here comes down to a XA2 relaying on the native mmdevapi
shared mode mixer behavior (which drains data in 10ms chunks every 10ms) and as
Wine's mmdevapi behaves differently - a problem arises. It had been masked by
the fact XA2 using 4xPeriodSize for Period sizes larger than 10.5ms, but popped
out of shadow as soon as DefaultPeriod size for Wine's mmdevapi had been cut
down to 10ms.

Considering the information you had provided about GetCurrentPadding() vs.
GetPosition(), I think that the "True Way (TM)" to fix it would be to:

a) Fix GetPosition() to use snd_pcm_delay() instead of basing it on
GetCurrentPadding() and thus on snd_pcm_avail_update().

b) Use snd_pcm_avail_update() purely for private purposes, don't expose it's
output to the client application - it's simply bogus to do so. Fix
GetCurrentPadding() to report actual space that's available in local buffer, do
not take into account ALSA buffer state when reporting padding.

c) Drain GetCurrentPadding() exactly in mmdev_period steps (in case we've got
in buffer more data than mmdev_period), and be sure that minimum period is non
less than 10ms for shared mode. Also, accordingly to MSDN, requested periods
size should be ignored for shared mode and default period should be always
used. As for duration - it is unclear from MSDN how should it be set WRT
shared/exclusive mode. At one place of the docs it is noted that an app should
specify here the minimal size of the buffer it requires to provide glitch-free
operation. Later on it is strictly stated that for shared mode with
event-driven stream duration and period should be set to 0. General idea looks
like that and an app shouldn't bother with buffer sizes in event-driven mode.
Instead it should call GetCurrentPadding() each time the event fires and
fill-in the buffer to its maximum. It's obvious that MS own XA2 violates this
requirement at least two times: it requests non-zero duration and it populates
buffer with non less than 10ms chunks when event fires.

d) Make sure we don't pump more frames to ALSA than the minimum required amount
to get xrun-free behavior. Failing to do so would introduce extra lag which is
undesirable. With current scheme when we pump-out data into ALSA once in
mmdev_period interval we have to make sure alsa buffer holds
2*mmdev_period_frames. Actually 2x it is a bit overkill but we have to have
something left in ALSA buffer so it wouldn't xrun at the very moment we are
pumping new data into it. Actual minimum for this might be something like
mmdev_period + alsa_period * (N + 1), where N is the time needed for
alsa_write_data() to complete expressed in alsa_period duration units and
rounded up. I.e. we have to be sure that ALSA has something to play until next
timer event pop-up (mmdev_period duration) and we pump-out more data to ALSA
(alsa_write_data() execution duration). IMO it's easier just to stick with
2*mmdev_period_frames and not to hunt for a bit reduced latency while having
risk to hit the xrun in unfortunate case.

e) Data that had been received through GetBuffer()/ReleaseBuffer() calls might
be immediately pumped out to ALSA in case the amount of data that had been
pumped out during previous timer callback was less than mmdev_pediod size. It
would save us from xruns in case frames amount left to be played in ALSA buffer
would be exhausted before next timer callback would arrive. Jörg, you had
mentioned that there are some constraints against acting in a such way, would
you please provide more info on the topic?

z) Unrelated to this bug, but would need fixing anyways: looks like that
current implementation of EXCLUSIVE mode in Wine alsa is total bogus. Docs
state that for event-driven exclusive mode stream period should equal to
duration and that audio engine should allocate two buffers period size each and
do a process called "ping-ponging" alternating the bugger that is being exposed
to the app each time the event fires. I can't see any sign of this
functionality implemented in winealsa mmdevdrv. OTOH, it might "just work" the
way it is now, but it is surely a thing that should be throughly tested.

I would attach a proposed patch that seems to fix issues in RAGE + XA2 reported
in this bug. With it applied I have xrun-free gameplay. There are still some
XRuns in log but they seem to be related to game itself being too busy or
re-initializing sound subsystem rather then winealsa.drv doing something wrong.
Grepping logs collected after about 10 minutes of gameplay reveals 5 xruns: two
of them were at game startup during sound subsystem init, one at map load
moment, one at sound subsystem re-init when quiting the campaign and going back
to the main menu, and one at the game exit. As a matter of test I had tested
with another patch modification that forces 3x mmdev_period smaples being sent
to ALSA and the amount of XRuns remained the same. "Make test" for mmdevapi
seems not to be affected in any way by proposed patch.

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