[PATCH v9 0/1] MR9840: winepulse.drv: A potential solution to audio crackling

[Dzmitry Keremsha (＠Vyrolian)]

I have noticed that winepulse leaves too little headroom when draining the buffer, which very often leads to crackling that can't be fully resolved by manipulating latency.

Let's check some math:

Let's assume that the buffer (tlength) is at 33ms, and minreq is at 13.3ms. If 13.2 ms were drained, the refill would be denied. If the next timer trigger takes 14ms due to jitter, that leaves us with a 6.1ms margin. As such, if we get any jitter longer than that during refill, we get an underflow. I think this is what leads to crackles.

My solution increases timer polling to 5x the frequency, so if 13.2ms were drained, the next timer trigger would happen at around 16ms. That would leave us with a 17ms headroom.

The math might not be 100% representation of what happens, but I think it's quite close to reality. On practice, this does indeed help to fully get rid of crackling for an indefinite amount of time, when before they were unevitable.

I think this problem is quite urgent, because while this does not happen on every system, it's frequent enough that we see "Audio: Crackling" reports on most games on protondb and there are many forum threads about audio crackling. From my experience, the only solution was switching to winealsa, which is very consistent and does not crackle at all. My experience has been very consistent with forum threads when it came to the nature of crackling. I first noticed it in Balatro, which is a very light game and should not be crackling at all on any setup. Some games have very frequent crackling every 20-30 seconds, or buzzsaw crackling every 10-15 minutes.

I hope my idea brings us closer to the ultimate fix, I really look forward to your feedback!

--
  v9: winepulse.drv: Increase timer frequency
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9840

[PATCH v9 1/1] winepulse.drv: Increase timer frequency

[Dzmitry Keremsha]

From: Dzmitry Keremsha <[e-mail]>

---
 dlls/winepulse.drv/pulse.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c
index 7c8d872c604..7b509f192bc 100644
--- a/dlls/winepulse.drv/pulse.c
+++ b/dlls/winepulse.drv/pulse.c
@@ -85,6 +85,7 @@ struct pulse_stream
 
     struct list packet_free_head;
     struct list packet_filled_head;
+    pa_usec_t last_timer_tick;
 };
 
 typedef struct _ACPacket
@@ -1180,7 +1181,7 @@ static NTSTATUS pulse_create_stream(void *args)
 
     stream->bufsize_frames = ceil((params->duration / 10000000.) * params->fmt->nSamplesPerSec);
     bufsize_bytes = stream->bufsize_frames * pa_frame_size(&stream->ss);
-    stream->mmdev_period_usec = params->period / 10;
+    stream->mmdev_period_usec = params->period / 50;
 
     stream->share = params->share;
     stream->flags = params->flags;
@@ -1572,7 +1573,8 @@ static NTSTATUS pulse_timer_loop(void *args)
     struct pulse_stream *stream = handle_get_stream(params->stream);
     LARGE_INTEGER delay;
     pa_usec_t last_time;
-    UINT32 adv_bytes;
+    UINT32 adv_bytes=0;
+    UINT32 bytes_passed;
     int success;
 
     pulse_lock();
@@ -1582,7 +1584,7 @@ static NTSTATUS pulse_timer_loop(void *args)
 
     while (!stream->please_quit)
     {
-        pa_usec_t now, adv_usec = 0;
+        pa_usec_t now, diff, adv_usec = 0;
         int err;
 
         NtDelayExecution(FALSE, &delay);
@@ -1613,6 +1615,7 @@ static NTSTATUS pulse_timer_loop(void *args)
                     {
                         stream->just_started = FALSE;
                         last_time = now;
+                        stream->last_timer_tick = now;
                     }
                 }
                 else
@@ -1634,12 +1637,15 @@ static NTSTATUS pulse_timer_loop(void *args)
                 if (stream->dataflow == eRender)
                 {
                     pulse_write(stream);
-
-                    /* regardless of what PA does, advance one period */
-                    adv_bytes = min(stream->period_bytes, stream->held_bytes);
+               
+                    diff = now - stream->last_timer_tick;
+                    if (now < stream->last_timer_tick) diff = 0;
+                    bytes_passed = muldiv(diff, stream->ss.rate, 1000000) * pa_frame_size(&stream->ss);
+                    adv_bytes = min(bytes_passed, stream->held_bytes);
                     stream->lcl_offs_bytes += adv_bytes;
                     stream->lcl_offs_bytes %= stream->real_bufsize_bytes;
                     stream->held_bytes -= adv_bytes;
+                    stream->last_timer_tick += muldiv(adv_bytes / pa_frame_size(&stream->ss), 1000000, stream->ss.rate);
                 }
                 else if(stream->dataflow == eCapture)
                 {
@@ -1653,8 +1659,15 @@ static NTSTATUS pulse_timer_loop(void *args)
             }
         }
 
-        if (stream->event)
-            NtSetEvent(stream->event, NULL);
+       if (stream->event)
+        {
+            if (stream->dataflow == eCapture || 
+                stream->held_bytes == 0 || 
+               ((stream->held_bytes + adv_bytes) / stream->period_bytes != stream->held_bytes / stream->period_bytes))
+            {
+                NtSetEvent(stream->event, NULL);
+            }
+        }
 
         TRACE("%p after update, adv usec: %d, held: %u, delay usec: %u\n",
                 stream, (int)adv_usec,
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/9840

[Conor McCarthy (＠cmccarthy)]

Just a heads-up about !7696 and !8812 which improve audio crackling in specific cases, the former only if the MF session engine is used, and the latter if 2D video buffers are supplied, which can delay audio sample delivery. These won't necessarily avoid problems with pulse audio though.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9840#note_126537