http://bugs.winehq.org/show_bug.cgi?id=28723
--- Comment #34 from Alexey Loukianov mooroon2@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.
- 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.