Fixed audio output latency inaccuracies

[benjarmstrong]

Fixed issue #38215

A couple of notes on the changes:

• AudioDriver::get_latency() was made pure virtual (instead of non-pure virtual). I did this because the only audio driver implementing this method was for PulseAudio, and the other 9 drivers used the default implementation (reporting 0 latency). My reasoning is that this is something all audio drivers should be forced to implement for correct behaviour and consistency across platforms.
• Drivers may still select an actual latency that is different from the audio "output latency" setting, and on many platforms there is no way around this. For example the WASAPI driver currently has no control over it's buffer size due to limitations in WASAPI. Other drivers select the nearest allowed latency to the specified amount. Either way these drivers should report non-0 latency now.
• AudioDriver::get_mix_buffer_size() tells the AudioServer how larger the audio mixer buffers should be. This was implemented in every audio driver based on how many frames they pass to their audio_server_process(..) method so they may synchronize. It was named get_mix_buffer_size instead of get_buffer_size because the mixer can run at a different buffer size if the audio driver would like it to - this could be useful in the future when improving inflexible audio drivers (like WASAPI) to allow for greater latency beyond what the native toolkit can provide.
• I have compiled and tested on Windows and Linux, but all other platforms are currently untested. Any testing would be thoroughly appreciated.
• There's a small conflict between this and my pr #38210 for bad latency handling in WASAPI; if either is merged I'll update the other.
• I need use these changes in Godot 3.2 for a project of mine that requires low-latency audio, so I'll happily put in the work to cherry pick this.

[akien-mga]

Could you squash commits into one?

[fire]

I'm not sure how to test this to solve the problem of "scheduled" audio play this was referenced in.

[fire]

Are you able to rebase on master? I can also rebase for you.

[ellenhp]

I really like these changes overall. Just a bit of a note about whether the mix buffer needs to always be a power of two or not? I know that's convention. There's even a get_nearest_power_of_two function (or something with a similar name) somewhere in the audio system to guarantee it.

[ellenhp]

It's normal for Godot's mix buffers to be powers of two from what I've seen. Perhaps someone else with more deep knowledge about the audio system can comment on why or whether that's truly necessary?

[benjarmstrong]

@ellenhp

Are you able to rebase on master? I can also rebase for you.

I would do it but I'm extremely busy with work at the moment - if you want to go ahead, I would certainly appreciate it.

Just a bit of a note about whether the mix buffer needs to always be a power of two or not? I know that's convention. There's even a get_nearest_power_of_two function (or something with a similar name) somewhere in the audio system to guarantee it.

Good catch, I didn't notice this.

I can't personally find a hard requirement for this. I've been developing with Godot using this PR for about 15 months now on a Windows 10 system with a variety of audio drivers that give Godot different buffer lengths to use which are rarely a power of two. This development has mainly been using the Vive Pro and USB audio drivers, for which the drivers typically tell Godot to buffer 441 samples per callback and I have yet to experience any issues. I believe @EIREXE shipped Project Heartbeat with this patch backported to 3.x; maybe he has some insights into any issues this does/could cause.

Perhaps someone else with more deep knowledge about the audio system can comment on why or whether that's truly necessary?

One possible advantage of the buffer size being a power of two: the audio stream resampling is hard-coded to work in batches of 256 samples (IIRC), so using a power of two would reduce latency between audio streams that resample before handing it over to the audio mixer. If the resampling isn't performed in multiples of the buffer size or lower then this could introduce latency with a worst-case of adding 255 samples of delay. However:

• IMO 255 samples of delay is negligible and arguably a non-issue in all but the most specific use cases
• If this is an issue, it's beyond the scope of this PR/issue
• This patch combined with the default Godot audio latency is still an improvement on most systems, although it depends on the buffer sizes that the OS/audio subsystem/hardware driver allows. For example using Win 10 / WASAPI / Vive pro audio driver that only allows a buffer size of 441 (coincidentally Godot's default audio latency at 44100hz); the worst case latency for a resampled audio stream on this system is 441+1023 without this PR, and 441+255 with this PR (1023 being the worst case latency via hard-coded mixer buffer, 255 being the worst case latency via hard-coded stream resampling buffer).

I completely agree with you that someone with more experience on Godot's audio mixer should take a look.

[ellenhp]

Interesting. I'm not sure what to make of that to be honest (edit: that sentence sounded kind of rude; just to clarify I'm referring to the fact that buffers of size 441 work correctly). Audio driver buffer sizes and audio server buffer sizes look like they were decoupled for a reason, but I can't quite piece together why. It's possible that it's related to drivers potentially having weird buffer sizes, I suppose. I'm supportive of the driver and the audio server having the same or very similar buffer sizes though. Currently a user can set latency to be a very low value in the driver but the audio server still mixes 1024 samples per batch so the latency doesn't actually go down below that.

@reduz would be the only one I can think of who might know more about this.

[benjarmstrong]

I'm supportive of the driver and the audio server having the same or very similar buffer sizes though. Currently a user can set latency to be a very low value in the driver but the audio server still mixes 1024 samples per batch so the latency doesn't actually go down below that.

Currently it also increases the chances of audio artifacts due to buffer underruns.

For example if the Godot audio driver is outputting at 256 samples per batch then every 4th batch will have the audio server mix 1024 frames, and the next 3 audio driver batches will use the remainder of the last audio server mix. This means that some audio driver callbacks take much longer than others, and if the audio driver callback doesn't finish before the audio needs to go to the speaker then we get audio artifacts.

When the audio driver and audio server are perfectly in-sync then the CPU time spent in the audio server's mixing is relatively constant across each driver batch and these glitches are less likely to occur. This means that with this PR you can take the latency much lower than you could before with less risk of audio glitches.

For me making the callbacks less glitchy under high CPU load was just as important as reducing the latency, as my game has a hefty audio synthesiser node that makes audio server mixes unusually expensive for a game.

Audio driver buffer sizes and audio server buffer sizes look like they were decoupled for a reason, but I can't quite piece together why.
It's possible that it's related to drivers potentially having weird buffer sizes, I suppose.

I think this is it. Up until this PR Godot couldn't dynamically set the buffer size on Windows, and just had to roll with whatever the system audio driver gave it.

@reduz would be the only one I can think of who might know more about this.

I'm guessing @reduz wrote most of Godot's audio code on account of all the music software he's authored, so he's probably the one to ask.

[EIREXE]

I believe @EIREXE shipped Project Heartbeat with this patch backported to 3.x; maybe he has some insights into any issues this does/could cause.

I don't think I can offer much more insight, since I am not really an audio professional, I just know that after I shipped the change people said the game felt more responsive, which matched the feel I had when playing with it.

[ellenhp]

@benjarmstrong If you're too busy to rebase this, would you mind if I took these changes and made a new PR to get them merged? I'll list you as a co-author on the commit if you'd like. I'm not super familiar with git workflows but I think that's the normal way to salvage old work. I might modify some logic a bit but I really like these changes overall and I want to make sure this gets merged in some form or another.

[benjarmstrong]

@benjarmstrong If you're too busy to rebase this, would you mind if I took these changes and made a new PR to get them merged?

If you could I would really appreciate it, I'm really snowed under at the moment.

[ellenhp]

Salvaged in #52626. Thank you again Ben for putting this together! I'll try to backport it to 3.x too if it's accepted.

[akien-mga]

Superseded by #52626. Thanks for the contribution!