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

Wed Nov 30 04:07:41 CST 2011

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

--- Comment #58 from Jörg Höhle <hoehle at users.sourceforge.net> 2011-11-30 04:07:41 CST ---
>when I request 20ms buffer for 44100Hz stream I got 896 samples buffer
I conclude that you have an Intel HDA.  That uses no 10ms period rather than a
10.1587ms one with 448 frames.  IOW, you need to repeat your tests (441->448)!

>((20ms * nSamplesPerSec)/16 + 1)*16
Some month ago, I tried to predict buffer sizes etc. in shared and excl. mode. 
I wasn't successful with the HDA's alignment requirements, yet the following
code predicts non-HDA engines.  Note the differences in rounding (round is like
MulDiv, unlike floor & ceiling etc. -- see the Common Lisp HyperSpec).  In
particular, round is not C's / operator.

(defun mmdevapi (duration period freq)
  (let* ((shared (round (* (float duration) freq) 10000000))
     (fragment (round (* (float period) freq) 10000000))
     (parts (round (float duration) period))
     (partu (ceiling (float duration) period)))
    (list shared fragment parts (* fragment parts)
          (* (round (* fragment partu) 128) 128))))
(mmdevapi 5000000 101587 44100)

It would be good if you could find the correct formula for exclusive mode with
Intel HDA.

>using 32bit IEEE_FLOAT
FP is convenient for mixing, but I'd expect the output of the mixer to be
typical 16 bits.  OTOH, some cards *do* support FP, so why would native perform
an extra conversion then?

---- GetPosition
AE>I removed the upper bound (5 seconds) on delay_frames in GetPosition().
AE>How strongly do you feel we need that type of logic?
Well, Alexey even complains about 5ms :-)
AL>With Wine I still got devposition lagging behind about 5ms

I introduced that logic to specifically acknowledge the idea that a true
speaker position may be up to 5s behind the audio data we feed in the presence
of *huge* latencies (audio over a network?).  Latencies cause speakers to
continue playing even after the front buffer hits an underrun.

GetPosition MUST ensure that it'll reach sum_written eventually.

Removing that logic also removed the underrun protection that worked in
free-running mode.  Something else is needed with the current XRUN=>stop mode.

Hopefully I can complete my GetPosition patch tonight.  I'll also add the
"IAC_Stop must not use snd_pcm_drop" one to pass even more of my render tests.

Alexey's comment is a bit troubling.  If there was a 28ms lag while feeding
data, I'd expect GetPosition to slowly reach sum_written within 28ms after
feeding ends.  He seems to say that GetPosition is bumped to sum_written as
soon as there's no more data to feed.  This would be important behaviour to
mimic.
Can you add please some logs to show this and report back?

---- Log File Analysis
Analysis of my WINETEST_DEBUG=2 render.ok output shows that GetPosition is
41-83ms(!) behind frames_written-padding in Wine with the 4 patches from
comment #54, and while GetPosition advances as regularly as the high
performance counter, it's the padding that varies wildly at each iteration.
The cause is probably the large ALSA period with dmix:
AudioClient_Initialize ALSA buffer size: 8192 frames
AudioClient_Initialize ALSA period size: 1024 frames
AudioClient_Initialize MMDevice period: 480 frames
AudioClient_Initialize MMDevice buffer size: 24000 frames
or (period still not being ignored in shared mode):
AudioClient_Initialize MMDevice period: 720 frames

---- Patches & Ownership
I claim 0002.
Date: Wed, 2 Nov 2011 09:27:46 +0100
Subject: [PATCH] winealsa: Don't set ALSA's period time.
http://www.winehq.org/pipermail/wine-patches/2011-November/108479.html

Something is missing from patch 0003.  IMHO, any patch that touches
+    if(period)
+        This->mmdev_period_rt = period;
should clamp period and duration first, something like:

if(!duration) /* 3 times is compatible with native and our requirements*/
    duration = 3 * DefaultPeriod;
if(SHAREDMODE){
    This->mmdev_period_rt = DefaultPeriod;
    if(duration > 2s) 2s
    if(duration < 3 * DefaultPeriod) duration = 3 * DefaultPeriod;
}else{
    if(period < MinPeriod) ...
    if(duration ...)

---- Alternatives
It's good news that your patch set works well (the key seems to be the ALSA
buffer size limitation?).  Here's what I had sketched during the last days.

alsa_set_period_max 10ms /* guarantees start when writing 10ms */
alsa_set_buffer_min 30ms /* stream with at least 3 periods */
TODO set_buffer_max approx. duration, perhaps 30ms more when tiny.

if (to_write){
    if (alsa_pad <= 0 /* just starting */
     && to_write <= 10ms){ /* we want 3 periods for streaming */
        write(10ms silence lead);
    }
    write(alsa_avail); /* as much as possible, not 10ms */
    /* alsa_buffer >= 20ms => will write at least 10ms initially */
}
if(alsa_avail>=10ms)
    if(gcp>10ms) # gcp = max(0,gcp-10ms);
        gcp -= 10ms;
    else gcp = 0;

It allows the callback to be late by up to 9ms in the case of Rage, but needs
more thought about when the audio and system clocks differ or when e.g. due to
system load or paging several events are missed.
It should allow large ALSA buffers and a heuristic to write less often than
every 10ms if ALSA already has a lot of data.

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