[PATCH v7 0/3] MR9840: winepulse.drv: A potential solution to audio crackling
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 understand that this might be not the most elegant approach, the 5x increase is arbitrary. I had to add another if condition to NtSetEvent to avoid issues with spatialaudio. We need as much headroom as possible and tests were failing above 1.8, so I set it to 1.75 to be sure. 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 a buzzsaw crackling every 10-15 minutes. I hope my idea brings us closer to the ultimate fix, I really look forward to your feedback! -- v7: winepulse.drv: Prevent unsigned underflow on clock jitter winepulse.drv: Revert to adv_bytes and fix spatial audio winepulse.drv: Adjust timer period https://gitlab.winehq.org/wine/wine/-/merge_requests/9840
From: Dzmitry Keremsha <vyro@lumencoil.com> --- dlls/winepulse.drv/pulse.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 7c8d872c604..93f10499c01 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -1180,7 +1180,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; @@ -1451,6 +1451,9 @@ static void pulse_write(struct pulse_stream *stream) to_write = stream->real_bufsize_bytes - stream->pa_offs_bytes; TRACE("writing small chunk of %u bytes\n", to_write); write_buffer(stream, buf, to_write); + stream->held_bytes -= to_write; + stream->lcl_offs_bytes += to_write; + stream->lcl_offs_bytes %= stream->real_bufsize_bytes; stream->pa_held_bytes -= to_write; to_write = bytes - to_write; stream->pa_offs_bytes = 0; @@ -1461,6 +1464,9 @@ static void pulse_write(struct pulse_stream *stream) TRACE("writing main chunk of %u bytes\n", to_write); write_buffer(stream, buf, to_write); + stream->held_bytes -= to_write; + stream->lcl_offs_bytes += to_write; + stream->lcl_offs_bytes %= stream->real_bufsize_bytes; stream->pa_offs_bytes += to_write; stream->pa_offs_bytes %= stream->real_bufsize_bytes; stream->pa_held_bytes -= to_write; @@ -1572,7 +1578,6 @@ 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; int success; pulse_lock(); @@ -1634,12 +1639,6 @@ 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); - stream->lcl_offs_bytes += adv_bytes; - stream->lcl_offs_bytes %= stream->real_bufsize_bytes; - stream->held_bytes -= adv_bytes; } else if(stream->dataflow == eCapture) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9840
From: Dzmitry Keremsha <vyro@lumencoil.com> Signed-off-by: Dzmitry Keremsha <vyro@lumencoil.com> --- dlls/winepulse.drv/pulse.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 93f10499c01..b25b8eb6802 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 @@ -1451,9 +1452,6 @@ static void pulse_write(struct pulse_stream *stream) to_write = stream->real_bufsize_bytes - stream->pa_offs_bytes; TRACE("writing small chunk of %u bytes\n", to_write); write_buffer(stream, buf, to_write); - stream->held_bytes -= to_write; - stream->lcl_offs_bytes += to_write; - stream->lcl_offs_bytes %= stream->real_bufsize_bytes; stream->pa_held_bytes -= to_write; to_write = bytes - to_write; stream->pa_offs_bytes = 0; @@ -1464,9 +1462,6 @@ static void pulse_write(struct pulse_stream *stream) TRACE("writing main chunk of %u bytes\n", to_write); write_buffer(stream, buf, to_write); - stream->held_bytes -= to_write; - stream->lcl_offs_bytes += to_write; - stream->lcl_offs_bytes %= stream->real_bufsize_bytes; stream->pa_offs_bytes += to_write; stream->pa_offs_bytes %= stream->real_bufsize_bytes; stream->pa_held_bytes -= to_write; @@ -1578,6 +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 bytes_passed; int success; pulse_lock(); @@ -1587,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); @@ -1618,6 +1615,7 @@ static NTSTATUS pulse_timer_loop(void *args) { stream->just_started = FALSE; last_time = now; + stream->last_timer_tick = now; } } else @@ -1639,6 +1637,14 @@ static NTSTATUS pulse_timer_loop(void *args) if (stream->dataflow == eRender) { pulse_write(stream); + + diff = now - stream->last_timer_tick; + bytes_passed = muldiv(diff, stream->ss.rate, 1000000) * pa_frame_size(&stream->ss); + adv_bytes = min(bytes_passed, (UINT32)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) { @@ -1652,8 +1658,11 @@ static NTSTATUS pulse_timer_loop(void *args) } } - if (stream->event) - NtSetEvent(stream->event, NULL); + if (stream->event) + { + if (stream->held_bytes < (stream->period_bytes * 7 / 4)) + 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
From: Dzmitry Keremsha <vyro@lumencoil.com> --- dlls/winepulse.drv/pulse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index b25b8eb6802..3b7faba7d22 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -1639,6 +1639,7 @@ static NTSTATUS pulse_timer_loop(void *args) pulse_write(stream); 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, (UINT32)stream->held_bytes); stream->lcl_offs_bytes += adv_bytes; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9840
participants (2)
-
Dzmitry Keremsha -
Dzmitry Keremsha (＠Vyrolian)