Fix audio pops when playing an AudioStreamPlaybackResampled after calling AudioStreamPlayer[2D/3D]::set_stream()

[Waridley]

AudioStreamPlayer2D and AudioStreamPlayer3D use setplay to communicate with the physics thread instead of setting setseek directly, but they were setting active in the play() function, which, depending on thread scheduling, could cause the audio thread to see the player as active, and try to read data without calling stream_playback->start(). This only surfaced for subclasses of AudioStreamPlaybackResampled (such as AudioStreamPlaybackOGGVorbis) because they use an internal buffer which starts uninitialized, while other stream types read the data directly.

This fixes the other serious popping issue mentioned by Jitspoe in #22016, but does not address either the "calling stop() on a looping sound" issue (which he says he worked around anyway), or calling set_stream() while a sound is still playing (which is obviously kind of a bad idea anyway, but was fixed for AudioStreamPlayer as mentioned in #25087 and #30827).

[jitspoe]

Can confirm this fixes the issue.

[ghost]

My issue was merged into this, but might be different. With this commit, I still get some subtle clicking sound when an audio player is restarted. Sounds sort of like a rewinding effect for a fraction of second.

Here is my minimal project. If you click around rapidly, some of the 2D audio will replay, and periodically have a clicking sound. Not sure if it is what you'd call a pop.

Audio Playback Issue.zip

[owstetra]

can this be merged in 3.2 ?

[akien-mga]

I need @reduz to review the changes.

[hoontee]

I need @reduz to review the changes.

This noticeably fixes pops I'm experiencing when playing OGG streams in 3.2. Any word on that review?

[CritCorsac]

I've been having similar issues when using OGG files but for me it's been occurring when using a regular AudioStreamPlayer (not specifically the 2D or 3D one). I'm not overly familiar with the source code, but I noticed the changes here were only for AudioStreamPlayer2D and AudioStreamPlayer3D respectively. I'm curious if the same fix would need to be applied to the regular AudioStreamPlayer too.

godot/scene/audio/audio_stream_player.cpp

Line 252 in 18fc857

active = true;

[Waridley]

@CristCorsac No, the regular AudioStreamPlayer doesn't interact with physics, so it would just end up never playing if this fix were applied... Have you checked if it's specifically related to calling set_stream? Or do you have a minimal example project that reproduces it?

[Waridley]

See https://github.com/Waridley/godot/blob/8fe0d31b925238289f2036114edf727a76132795/scene/2d/audio_stream_player_2d.cpp#L255 where active is set to true near the end of NOTIFICATION_INTERNAL_PHYSICS_PROCESS for the 2D player for example. That doesn't happen in the normal player.

[CritCorsac]

Sorry, I'm struggling to recreate an accurate minimal reproduction project as the bug seemingly occurs at totally random intervals. Sometimes not occurring for 5 minutes then suddenly cropping up.

In my setup I have a separate AudioStreamPlayer for each sound effect and I simply call play() on them when needed. The AudioStreamPlayer nodes are all instanced onto the main scene. In my experience the bug seems to be much more likely to occur after a scene change has been made through code.

As others mentioned in the issues posted in the initial comment here, this does not occur for WAV files and converting all my sound files to WAV fixed the issue for me.

[Waridley]

Yeah, that seems to be the pattern with audio issues in Godot 😆 They're always intermittent. Hence why it took me like 2-3 weeks of staring at the code to find this issue where I expected it to be all along.

[Waridley]

When are you calling set_stream()? E.g., in _process()? _physics_process()? And are you sure of whether the sound is still playing or not when you do? (It still shouldn't pop either way, just curious)

EDIT: Now I see you said you aren't changing the streams, only calling play()... So the stream is just set via the editor? In which case, the scene loader might be calling set_stream() when it loads the scene

[CritCorsac]

When are you calling set_stream()? E.g., in _process()? _physics_process()? And are you sure of whether the sound is still playing or not when you do? (It still shouldn't pop either way, just curious)

EDIT: Now I see you said you aren't changing the streams, only calling play()... So the stream is just set via the editor? In which case, the scene loader might be calling set_stream() when it loads the scene

I was using _physics_process() to call the .play() commands with the streams being set via the editor.

What I find strange is that the audio popping glitch seems to occur significantly more often when the scene that contains the AudioStreamPlayer nodes is loaded via a get_tree().change_scene() call. If the scene is the initial scene to load (set by application/run/main_scene in project settings) the audio glitch seems to occur a lot less often. In fact, as of right now, I can't even get the audio glitch to occur until the scene has been changed by a get_tree().change_scene() call, even if it's just reloading the current scene.

This is just speculation as I'm not familiar with the source code but could it be that changing the scene causes the problem to come to light?

[Waridley]

This is probably superseded by #46151

[akien-mga]

Indeed. Closing as superseded by #46151. Thanks for the contribution!