godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.1k stars 20.2k forks source link

Audio clipping / static / interference on rapid intervals of sound (fixed in `master`) #22016

Open kezzbracey opened 6 years ago

kezzbracey commented 6 years ago

Godot version: 3.1 alpha

OS/device including version: Kubuntu 18.04 and Windows 10

Issue description: Creating a rapidly firing weapon, if sounds are played at intervals less than 0.5 or 0.6 seconds there is a clipping or static sound.

Demo: https://www.dropbox.com/s/q5tx4y6272lgdae/2018-09-12%2017-16-00.mp4?dl=0

Improvement can come from modifying sound files with fade in and fade out at either end, but the issue is still present.

Demo: https://www.dropbox.com/s/hnvlu161x1t62fl/secondtest.mp4?dl=0

In the case of testing with a sine wave sound file, instead of clipping / static there is a sort of interference or modulation sound instead.

Demo: https://www.dropbox.com/s/nd2rcsrdrqxo6sd/2018-09-12%2021-19-52.mp4?dl=0

The clipping / static sound can seemingly be eliminated by setting a LowPassFilter and LowShelfFilter on a sound bus, both with a cutoff of 1000Hz.

These filters don't eliminate the modulation type of sound that comes up when using a sine testing sound.

Steps to reproduce: Create a timer with an interval of about 0.3 seconds and have either a single sound repeat at that interval, or rotate between two different sounds.

Issue present in the editor on Linux and Windows, in builds for Linux and Windows, on 2 PCs in my house, through 3 headphone sets and two speaker sets. Also verified by another user who tested on 2 of their headphone sets.

Minimal reproduction project: SoundTesting.zip

Tests and attempted solutions that didn't help Before trying the filters, I also tried:

Extra information

I spent a few hours with the helpful folks at the Godot discord last night going through various test to try and isolate the source of the clipping / interference sounds. Our tests did lead us to believe it wasn't an issue localized to my setup alone.

Different people heard different effects from the videos and test project above. Some heard clipping / static on every sound, some heard none at all. One other person heard it as clearly as I did, and confirmed the same on two of their headphone sets.

@starry-abyss had the theory it was something to do with too many high frequency sounds at once, hence we tried the low pass / shelf filters which eliminated the clipping / static sounds.

Given the effect of the filters I'm not sure this is actually an issue as opposed to a sound management technique being required.

If we can confirm this is just about handling sound in a certain way, perhaps I can contribute to the docs with some information for people in the future who run into this problem.

Calinou commented 5 years ago

Try reimporting sounds with the Trim property disabled:

image

I've found Godot's automatic trimming to be too aggressive, resulting in part of the sound not playing.

Also, beware that Godot 3 currently does not provide polyphony (https://github.com/godotengine/godot/issues/23541), so instancing an AudioStreamPlayer for every sound played is a good idea if you need several of them to play at the same time. (Of course, don't forget to queue_free() them when they are done playing.)

Aaron-Fleisher commented 5 years ago

FWIW this does not happen on 3.0.6 for me. I sometimes get a random "static sound" (very rare, but it happens) at the beginning of certain sounds that are played (.ogg). Never heard that kind of static in 3.0.6. W10, 64-bit

By the way, that's a very extensive issue report, kudos.

lawnjelly commented 5 years ago

Godot version : 3.1 stable OS : Linux Mint 18.2 Cinnamon 64 bit Issue description : Glitches in audio when reusing AudioStreamPlayer3D node

It sounds like this may be the same issue I have been dealing with in the past couple of days. I have a workaround that works, and a possible reason for what I now believe is probably a bug rather than calling the AudioStreamPlayer incorrectly.

Minimal reproduction project: godot_audiostreamplayer_test.zip

Full description of the problem Rather than creating new audioplayer nodes (in my case AudioStreamPlayer3D) each time I need a sound effect, I would expect to either:

Both of these cases involve reuse of the node to play multiple times.

A When using this scheme, if the AudioStreamPlayer translation and / or volume is changed prior to playing, audio glitches occur. This is not to do with the import settings / normalization / trim, it occurs with these off. It is not clipping. The glitches on my system are approx 1/20th second, probably one of the audio tiles used in a circular audio buffer.

Picture of glitch Screenshot from 2019-04-04 14-04-00

The glitch can be seen bottom left waveform, at the start. Each sound should be playing for 1 second, and the glitch is around 0.05 second.

Audio of glitch https://soundcloud.com/lawnjelly/godot-audio-glitch-test

B When attached to a game object node, there are not problems from translate (as it is not set prior to calling play()), however changing volume still results in glitches.

Workaround: By chance I found a different method of playing sounds by Calinou, which I slightly modified and solves the issue:

func PlaySound_Node(var stype, var node, var volume = 1.0):
    assert (node != null)
    var player = AudioStreamPlayer3D.new()
    node.add_child(player)
    player.stream = StreamFromSType(stype)
    player.set_max_db(-60 + (volume * 60.0))
    player.connect("finished", player, "queue_free")
    player.play()

This seems to solve the issue, and works as a workaround. However, the question is, why? Creating new AudioStreamPlayer for each sound and attaching to the parent node and deletion on completion doesn't sound the most efficient way of playing sounds.

At a guess, I suspect the reason it works may be because creating a new AudioStreamPlayer is forcing creation and blanking of an audio buffer (or position within a buffer) whereas reuse of AudioStreamPlayer is not getting rid of previous data from playing (buffer, position within buffer, historical record of position or volume?).

I also discussed the issue here: https://godotdevelopers.browse-tutorials.com/discussion/comment/27395

starry-abyss commented 5 years ago

@lawnjelly Just a quick thought: do you initialize the state of objects (i.e. make sure all internal variables are not holding leftover values) before reusing them from the pool? As this is a common caveat with pools. When the new one is created (case B), it's clean, but is so the one in case A?

lawnjelly commented 5 years ago

starry-abyss, the example project I posted should show exactly. I don't do anything to the AudioStreamPlayer between playing one sound and the next, just move the translation, change the volume, and call play() again after the previous sound has finished. Is there anything else that should be done?

As a user I would assume that a sound would autostop at the end, and another play would reset everything necessary. I did actually try calling stop() before play just in case but that didn't change anything.

Calinou, the problem still occurs when trimming / normalizing is turned off during import (you can try this on the example project I posted, and reimport the wav). So I'm assuming it isn't to do with trimming / normalization.

Calinou commented 5 years ago

Calinou, the problem still occurs when trimming / normalizing is turned off during import (you can try this on the example project I posted, and reimport the wav). So I'm assuming it isn't to do with trimming / normalization.

I realized that just after posting the comment, which is why I deleted it :wink: Sorry for the confusion.

jitspoe commented 4 years ago

There's also a problem with pops in the AudioStreamPlayer3D. Just going to paste the contents of my duplicate bug here for easy reference:

Godot version: 3.1.2, 3.2 rc4

OS/device including version: Windows 8.1

Issue description: There are a few situations where playing a sound AudioStreamPlayer3D results in pops or crackles. The easiest to reproduce has something to do with the positional logic. The surround sound logic changed between 3.1 and 3.2, so I thought maybe it was fixed, but even with the new 3.2 positional code, it still has the pops.

I replaced my character's footstep sounds with sine waves, and it became VERY clear that there was something wrong. image

It looks like there are actually 2 problems: One, seen in the top channel, is an abrupt volume/amplitude change. The second, shown in the bottom channel, shows the sine wave getting out of phase. I think it's actually playing the sound again at a later time and blending it (also followed by an abrupt amplitude change).

The abrupt change seems to happen at 21ms, which, at a 48000 sample rate is pretty close to the 1024 buffer size. My guess is the first buffer somehow has bad data and is mixing/fading incorrectly.

I dug into the code for a while, but couldn't identify the exact problem. From the looks of it:

1) AudioStreamPlayer3D::play() indicates the player is active and enables the physics process. 2) AudioStreamPlayer3D::_mix_audio() is called from a separate thread. Data isn't set yet, so it fills the buffer with 0's (this is probably not great, as it likely delays the sound, but shouldn't cause a pop). 3) _calc_output_vol() called when there's a NOTIFICATION_INTERNAL_PHYSICS_PROCESS, which sets proper data.

Seems there is something wrong with the first buffer every time: image You can see here that sometimes the amplitude is scaled up, and sometimes it's scaled down. Rarely is it ever at the volume it should be (unless maybe the sound is played at a location very close to where it played previously?)

Other pops:

There are 2 other pops I'm aware of that may or may not be part of the same issue. Perhaps they should have their own bug entry once this is fixed.

Steps to reproduce: Play a sound on an AudioStreamPlayer3D, then move it to a new location relative to the camera and play it again. Really obvious when you move from the left to right side of the listener (see attached project).

Minimal reproduction project: sound_pop.zip

lawnjelly commented 4 years ago

3. _calc_output_vol() called when there's a NOTIFICATION_INTERNAL_PHYSICS_PROCESS, which sets proper data.

This may be the problem. I had assumed before it would be something to do with the audio buffer tiles (without looking at the source code), but it may simply be the audio is only changing things on physics ticks.

Besides using physics ticks (which is kind of dodgy, because it means changing physics affects the audio) It needs to be doing interpolation for everything between the ticks, because audio needs to be continuously interpolated. I'm guessing it isn't somewhere.

Also I suspect the problem I was having with changing position and playing a AudioStreamPlayer again was because it was waiting to the next physics tick to set the new output, instead of doing this via a forced path every time play is called.

I can't look at it now, busy with other stuff, but hopefully this might provide ideas for anyone else working on the audio.

jitspoe commented 4 years ago

I tried a quick hack where I forced a call to update the physics-related stuff to try to ensure the data was set before the audio thread tried to process it, but that didn't seem to fix it. :(

void AudioStreamPlayer3D::play(float p_from_pos) {

    if (stream_playback.is_valid()) {
        setplay = p_from_pos;
        output_ready = false;
        set_physics_process_internal(true);
        _notification(NOTIFICATION_INTERNAL_PHYSICS_PROCESS); // HACK - force position update before audio driver tries to process this.
        active = true;
    }
}

The audio thread keeps track of the previous data and lerps between the previous and current values. This appears to work properly everywhere except for the first buffer.

jitspoe commented 4 years ago

In case it's helpful, here's a clip from when I got a pop while hardcoding the relative position of the listener (WARNING: TURN VOLUME DOWN): https://clips.twitch.tv/ViscousCooperativeBasenjiMoreCowbell Another: https://clips.twitch.tv/JollyDignifiedCarabeefItsBoshyTime (you can also hear the pop when the elevator looping sound stops, which is another issue).

Unfortunately, the compression makes it almost impossible to tell what the original wave form actually did.

I think there are 3+ unique pop bugs.

Calinou commented 4 years ago

Unfortunately, the compression makes it almost impossible to tell what the original wave form actually did.

You could upload a short video with uncompressed audio, put it in a ZIP archive and upload it here. Or just upload a WAV file of the video's uncompressed audio in a ZIP archive :slightly_smiling_face:

jitspoe commented 4 years ago

I don't have an uncompressed recording. Whenever I try to catch it in the act, I can't seem to reproduce it. I just happened to have these because I'm live streaming the whole development.

jitspoe commented 4 years ago

1/3 of the pops fixed with Waridley's change! That was probably the biggest one. There's still this rare pop that happens even after the change. Only happens like once a day for me. It can be really harsh, so I hope we can track that down, too. Not sure if the stop pop is still there, as I worked around that one.

jitspoe commented 4 years ago

@akien-mga Sadly the other pop is still pretty prevalent. Seems to happen most frequently when sounds are first played. I'm wondering if something is uninitialized (prev_outputs, maybe?). Happens both on Windows and Linux (completely different machine). Actually seems more prevalent on Linux, but maybe that was just by chance.

jitspoe commented 4 years ago

Ok, I've finally managed to capture a recording of the sound pop with audacity, so it's a little cleaner. Unfortunately, there's some resampling going on so it's not perfect, but at least there's no compression:

image

Wav file: godot_random_pop.zip

What's crazy here is that audacity scales the volume based on my windows settings, so normally if there were a pop like that, it wouldn't peak unless I had everything at max volume. Playing the sound back and recording it with my volume set at 43%, it looks like this: image

Meaning that somehow Godot is bypassing the max volume in windows and just flat out spiking as loud as the sound card and headphones will let it go. That explains why these random pops are so much worse than the other ones (that were fixed).

This really needs to get fixed. It could blow a speaker or damage somebody's hearing.

jitspoe commented 4 years ago

Leaning toward this being an uninitialized value. I changed the AudioFrame() constructor to set l and r to like 99999, and it seems to happen more often now, but I can't find a consistent repro case for it. Sometimes it seems to happen when changing direction. Sometimes it happens like every footstep. Wav file where it was happening a lot: godot_random_pop3.zip

It seems to always pop upward and stay flat for a period of time, so maybe it's not just a weird volume scale.

Ok, I've recompiled with the AudioFrame() constructor defaulting l and r to 0, and I can't seem to get the pop to happen anymore, but it is somewhat random. I don't think this is the solution. It likely masks the problem and introduces another (more subtle) problem where the audio is too quiet and fades in.

jitspoe commented 4 years ago

Yep, definitely an uninitialized value somewhere. I had the constructor set l = 99999 and r = 0, and now the popping only happens in the left channel. What's weird is that it doesn't happen EVERY time a sound is played, and I can't seem to make a simple repro case scene. It's possible it's a threading issue that only happens when enough stuff is happening simultaneously.

Side note: The volume for 2 channels vs 6 (surround) channels is different. If I have my sound configured to use 6 channels then output for headphones, it's considerably louder than if I use 2 channels. Also, the panning seems really harsh. If the character is slightly to the left of the listener, there's almost nothing coming out of the right channel when using 2 channels. Sounds more natural with 6.

jitspoe commented 4 years ago

Slowly honing in on this. Seems the problem is in the buffer coming from the stream playback sample is bad, now, not the volume. It might be related to changing the pitch of an ogg file. I was randomizing pitches, which would explain why the problem happens randomly.

Here's the buffer (note left channel is large and right channel is 0 all the way up to just before sample 256):

image

jitspoe commented 4 years ago

Just posting all this info bit by bit because I haven't had time to sit down and focus on it, but the problem appears to be with the buffer in AudioStreamPlaybackResampled::mix()

internal_buffer has uninitialized data. INTERNAL_BUFFER_LEN is 256, which explains why the data clears up after 256. Now, initializing the data to 0 would fix the pop, but there's another issue where sounds sometimes don't play (every once in a while, I'll get a silent footstep, for example) which might be related. Need to track down why the data isn't getting set before mix() is called.

Edit: Seems it only happens with ogg vorbis. I have a consistent repro project now (might only happen consistently if you set the default AudioFrame constructor to init l and r to large values): sound_pop2.zip

Edit2: Side note, it would be nice if we could load ogg vorbis files into memory so we could keep game file sizes down without the perf hit of streaming them.

Edit3: I forgot to set the ogg to not loop, so there's ANOTHER popping issue when a looping sound is interrupted which should also probably be addressed. If you disable loop on the project and reimport, the pop only happens every 1 in 4 runs or so.

Waridley commented 4 years ago

Jitspoe and I determined his issue is due to calling set_stream(), and not doing so seems to eliminate the pops. https://github.com/godotengine/godot/issues/25087 and https://github.com/godotengine/godot/issues/30827 already mention this issue and have not yet been resolved. It was fixed for AudioStreamPlayer by https://github.com/godotengine/godot/commit/040b59c010f3cce63b4c45956c418b74079e24e6, but not the 2D or 3D versions.

I will tackle porting that fix to the other 2 classes in the next few days.

jitspoe commented 4 years ago

I'm hoping that fixes it, but I think there may be something more involved than simply avoiding cutting off a playing buffer and fading it out. The problem seems to happen ONLY with ogg vorbis streams, so I don't think the problem is necessarily with the stream PLAYER, but the stream itself.

Related question: Is it better to have a bunch of different stream players or have fewer players and swap the streams out for different sounds? For example, if I have 50 different footstep variations, is it better to have just a couple stream players and swap the streams out, or have 50 stream players, one for each unique stream? The overhead of all the nodes seems like it would be bad to have that many, but threads get locked and memory gets allocated and freed when swapping streams, which also seems bad.

fabriceci commented 4 years ago

I experience the same with AudioStreamPlayer3D. In my game it happens nearly each time the sound is played, it could damage the speaker. I made a reproducible project but in this one, it appears sometime (after few seconds on mac), I don't know on windows.

Here are what I found that could help:

bugClip.zip

jitspoe commented 4 years ago

The fix hasn't been accepted yet. If you compile the engine yourself, it's just a matter of commenting out or removing one line:

In audio_stream_player_3d.cpp in the AudioStreamPlayer3D::play() function, remove or comment out the active = true; line.

Torguen commented 3 years ago

The problem also happens with WAV.

Torguen commented 3 years ago

Jitspoe and I determined his issue is due to calling set_stream(), and not doing so seems to eliminate the pops. #25087 and #30827 already mention this issue and have not yet been resolved. It was fixed for AudioStreamPlayer by 040b59c, but not the 2D or 3D versions.

I will tackle porting that fix to the other 2 classes in the next few days.

Hello, I use an audiostreamplayer (not 2d or 3d) and version 3.2.4 beta 3 mono and the problem continues, you said you solved it but in my case the problem still exists. Any solution to the creaks?

Waridley commented 3 years ago

@Torguen The actual solution is in this PR: https://github.com/godotengine/godot/pull/38216

I actually had the port of the "fix" for calling set_stream() on a playing sound working on AudioStreamPlayer2D and I think maybe even 3D, but when it didn't fix the problem and ended up making very large changes to those classes, I ended up abandoning it and continued looking for the actual problem, which turned out to be a one-line fix related to the fact that those nodes interact with physics. I think what really needs to happen at some point is a refactor of all 3 AudioStreamPlayer nodes to at least be more consistent with each other, or preferably share code instead of duplicating son much.

Waridley commented 3 years ago

@Torguen WAIT I was not awake this morning. Just realized you said you were using the AudioStreamPlayer and NOT 2D or 3D... In which case I have NO idea what the problem is because the fix for that has been in stable releases for a while now. Unless you're just dealing with performance issues or something like that.

All the more reason for a refactor I suppose... I know there are also several TODOs in the audio code such as pooling audio memory and other performance improvements.

ellenhp commented 3 years ago

Here's a repro project for 4.0. Based on a different repro project, but I distilled it down and added some options so you can test a lot of different things. I'm getting some pretty consistent pops and crackling pops with mp3s and oggs. Investigating.

sound_pop.zip

Edit: Please be careful with your volume. On my macbook at around 1/3 volume some of these pops are loud enough that I worry about the speakers.

ellenhp commented 3 years ago

I think this might be fixed. There are definitely still pops if you:

But I think that's it. This issue seems to specifically track pops fixed by #46151 and #46202 (both of which will be in 3.2.4). The remaining pops are tracked by #25087 and #21312.

ellenhp commented 2 years ago

I think I'm going to close this because I'm 95% sure I fixed it, feel free to reopen though if you see similar issues.

hhyyrylainen commented 2 years ago

Which Godot version is the fix in? I think we have had sound issues even with Godot 3.3.0: https://github.com/Revolutionary-Games/Thrive/issues/1268#issuecomment-902165158 (which I think I incorrectly called 3.2.4 there) These sound issues have been plaguing us for a long time and I think we have tried to apply all the advice available in this issue, but still whenever someone makes a let's play of our game, they have a pretty good chance to get the audio glitch.

ellenhp commented 2 years ago

In 4.0 you should not get pops under any circumstances. In 3.x you still get pops during these 3 things:

But I don't think I'm smart enough to fix them. It would be an enormous and incredibly error prone change to make. It took a pretty substantial rewrite of the audio system to fix those pops for 4.0 and it's just too big of a change to backport. It's usually easy to avoid doing those 3 things though. Definitely let me know if you're doing anything other than those 3 things and seeing pops though because those are the only cases I'm aware of right now.

hhyyrylainen commented 2 years ago

If it's coming in 4.0, I think we can live with that. There's already a few other big fixes in 4.0 that we are waiting for already.

Calinou commented 2 years ago

Reopening, as the bug still exists in 3.x and we should try to get it fixed there, or at least document how to avoid it.

sketchyfun commented 2 years ago

Just attaching the project from my most recent ticket about this, so that it can be easily tested. Project shows the bug occuring when the sound is rapidly played. Doesn't occur when a regular (non 2D/3D) AudioStreamPlayer node is used

audio_pop.zip

wizbane commented 2 years ago

Here's a demo project for 3.4.3 that shows popping/clicking occurring when an AutoLoad AudioStreamPlayer is playing, and more AudioStreamPlayers get added to the scene. It occurs when they are added to the scene, but not playing, and -80db, stream paused. This is primarily an issue when streams with OGG files get added, occuring when even 1-2 are added in rapid succession. It does occur when 500 or so WAV or MP3 streams get added too, but not at more reasonable numbers.

audio_pop_demo.zip

ellenhp commented 2 years ago

I'm not entirely sure @wizbane's issue is the same as what was originally reported in this bug, but regarding @wizbane's issue, I think it's just caused by the AudioStreamPlayer nodes holding a global audio server lock during most of the execution of the set_stream method. Stream playback instancing and initialization like that prevent the audio thread from mixing, which is bad news. It gave me bad vibes during the 4.0 audio mixing rewrite so I removed the need for locking, but solving this in 3.x is probably still worth doing.

ellenhp commented 2 years ago

If anyone has a repro on this bug and would be willing to test it against #59413 that would be wonderful! I tested the project in https://github.com/godotengine/godot/issues/22016#issuecomment-1074729858 already but it seems like there might be other ways of causing this to happen?

akien-mga commented 2 years ago

Fixed by #59413.

sketchyfun commented 2 years ago

I've just tried applying the changes from #59413 but when I run the project I attached in my comment above I'm still getting popping noises, so I don't believe the issue has been completely fixed. Can you try the file I attached to my previous comment and see if it's fixed for you?

ellenhp commented 2 years ago

That repro project still displays the pop. Seems like the cross-fade is broken. The 3.x audio system is really tricky to work on but I bet it would be possible to move the non-spatial player crossfade logic into the spatial players. No promises though because my availability for Godot work has been really inconsistent lately.

ellenhp commented 2 years ago

As a workaround, you could try using multiple audio players and allowing playback for each to expire normally when the audio stream ends.

sketchyfun commented 2 years ago

As a workaround, you could try using multiple audio players and allowing playback for each to expire normally when the audio stream ends.

This happens in many places throughout my game, so I don't think multiple audio players would be a suitable solution for me. I'll have a dig through the regular audio player code and see if I can figure out what can be transplanted into the spatial audio player to fix my problem

ronyeh commented 2 years ago

Hi!

If any devs on the Godot Audio system need me to help with testing or bug repros, please @ or message me. I've been running into issues with my app regarding rapidly repeating sound effects (and pitch shifting), so I am very much interested in seeing the audio system improved. I'm happy to help out in any way that I can.

Thanks for all your hard work. Godot is awesome. :-)

elvisish commented 1 year ago

Is there any workaround for this in 3.x? Even using 4+ audio players and iterating through an array to assign which one to play doesn’t work for some reason.

elvisish commented 1 year ago

Is this fixing the problem with zero-crossings causing the pops? I opened 5697 but it was closed as it was suggested to be a duplicate of this, but I don't think so. I think Godot needs to automatically add a micro fade out at the end of sounds when it stops playing them to avoid pops/clicks on zero-crossings. This will happen any time a sound is stopped and it lands on a zero crossing, be it destroying the audio player, stopping the stream, changing the stream or seeking. It's not a bug really, it's just a natural mathematical inconvenience that should be accounted for in the audio engine.