Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All sound stops playing when you set the Quaternion of a node with Camera3D and AudioStreamPlayer3D to zero. #80648

Open
Tracked by #76797
m4rr5 opened this issue Aug 15, 2023 · 15 comments

Comments

@m4rr5
Copy link
Contributor

m4rr5 commented Aug 15, 2023

Godot version

4.1.1

System information

Godot v4.1.1.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2070 SUPER (NVIDIA; 31.0.15.3179) - Intel(R) Core(TM) i9-9900KS CPU @ 4.00GHz (16 Threads)

Issue description

If you accidentally set the quaternion for a node in 3D space to (0, 0, 0, 0) and attached to this node is a Camera3D and AudioStreamPlayer3D then this will instantly stop all sound from playing.

Steps to reproduce

Using the project below, you can simply start it. After a short countdown, the world.gd script will set the Quaternion of the MeshInstance3D (with cam and audio attached) and all sound will stop playing.

Minimal reproduction project

sound-bug.zip

@AThousandShips
Copy link
Member

AThousandShips commented Aug 15, 2023

How would playing audio from/to an undefined/invalid transform work?

Does this happen if only one player has invalid transform and there are other players as well? I.e. does one audio stream player with a broken transform mute all other sound, when the camera has a valid transform

@m4rr5
Copy link
Contributor Author

m4rr5 commented Aug 15, 2023

That's a good question. Let me ask you another one. Why would Godot allow you to set an undefined/invalid transform?

Note that the bug is that all sounds stop playing. If you take the minimum sample project I provided and attach to the same "World" an "AudioStreamPlayer" that plays the same mp3 then it too stops playing. And AudioStreamPlayer is, according to its documentation, "non-positional" so why would that ever stop playing?

@AThousandShips
Copy link
Member

AThousandShips commented Aug 15, 2023

Okay! Will take a deeper look, your issue didn't mention the non-positional case so that changes things, could be related to #46204 but will have to investigate the maths involved

An invalid transform can arise in many ways and checking for it isn't always cheap, nor is it always desirable to flood error messages, so it's a balance, generally it's expected that the user knows what they are doing

@m4rr5
Copy link
Contributor Author

m4rr5 commented Aug 15, 2023

Yeah, I probably should have been more explicit in my explanation. Thanks for taking a look.

@m4rr5
Copy link
Contributor Author

m4rr5 commented Aug 15, 2023

Regarding the comment about flooding error messages, I agree that is a balance and actually there is some error message that already shows up in the debugger every time you set this invalid Quaternion, but it's currently very hard to figure out that it is related to this issue:

E 0:00:06:0358   instance_set_transform: Condition "!v.is_finite()" is true.
  <C++ Source>   servers/rendering/renderer_scene_cull.cpp:907 @ instance_set_transform()

@AThousandShips
Copy link
Member

And that's my point there, invalid transforms can occur in unforseen places, for example a Node3D can be invalid and it shows up as the transform for a child mesh instance, hard to debug

@m4rr5
Copy link
Contributor Author

m4rr5 commented Aug 15, 2023

For sure, I spent over 10 hours in the last couple of days pinning this down. It happened in a multiplayer scenario when a new client spawned (and existing clients would loose all audio because they initially got a position with this invalid transform).

@AThousandShips
Copy link
Member

I haven't tested it yet, might be able to later today or tomorrow, but I suspect it has to do with the audio being poisoned with NaN values due to corrupted transforms, will look into it and some checks to be done

But some checks:

  • Does this occur if you only use a AudioStreamPlayer with the camera having an invalid transform, or only when either the camera or AudioStreamPlayer3D has an invalid transform
  • Does it occur when only the camera has invalid transform
  • Does it occur when only the AudioStreamPlayer3D has invalid transform

If you are able to test, I'll try the same tests when I have the time if you are unable

Thank you

@m4rr5
Copy link
Contributor Author

m4rr5 commented Aug 15, 2023

Responses to those checks:

  • This bug does NOT occur if there is an AudioStreamPlayer in the world but no AudioStreamPlayer3D that is playing sound. In this case, setting an invalid transform on the mesh that the camera is attached to has no influence on the sound playback.
  • This bug does also occur if the Camera3D has an invalid transform but the AudioStreamPlayer3D is static (attached directly to the World in the example project instead of to the mesh).
  • Thes bug does also occur if only the AudioStreamPlayer3D is attached to the mesh with the invalid transform. That said, it does NOT occur if I don't give the mesh but the AudioStreamPlayer3D the invalid transform! In that last case, the error message I mentioned above (instance_set_transform) is also not present. I think that makes some sense as it is irrelevant what the orientation of the actual sound source is (given that it's considered a point source).

@AThousandShips
Copy link
Member

AThousandShips commented Aug 15, 2023

It's not the orientation but the scale that is relevant here, using this invalid Quaternion breaks that, the issue here is that if an audio sample with invalid values is passed it "poisons" all the audio, because of how NaN works

Thank you for checking

@MJacred
Copy link
Contributor

MJacred commented Aug 24, 2023

@ellenhp
Copy link
Contributor

ellenhp commented Sep 3, 2023

We should probably check for NaN in the volume scalars in the AudioServer volume setters, since IIRC we do volume ramps at the AudioServer level so that's the last chance to catch the NaN before it enters the audio buffer.

I think I'd be against NaN checks in the audio streams, unless there's some kind of preprocessor flag we can include to disable them in release and release_debug builds. Checking all values from all stream playbacks sounds expensive. Very cache-friendly but it's still a lot of operations that we don't need to be performing. If more of these bugs crop up we could talk about adding it to development builds only, perhaps.

What's expected behavior here? WARN_PRINT_ONCE and setting the volume to zero would be my first idea, but we could also halt the AudioStreamPlayback.

@m4rr5
Copy link
Contributor Author

m4rr5 commented Sep 4, 2023

In my opinion, the expected behavior should be that an AudioStreamPlayer3D with an invalid transform:

  • should not stop all sound playback;
  • can have its volume set to zero (with a warning printed once as you suggested).

I would prefer setting the volume to zero as this would be the least disruptive. It's not unlikely for this node to move around, so there might be other code that moves it to a valid position in the future. If you stopped the sound before, it would not get a chance to play anymore.

@jitspoe
Copy link
Contributor

jitspoe commented Mar 6, 2024

I'm not sure if this is related, but I ran into an issue where creating an AudioStreamPlayer3D at 0, 0, 0 would cause ALL audio to mute/cut out until the sound finished playing. Even music playing on an AudioStreamPlayer.

Here's the code I was using to spawn in the player:

func play_world_sound(sound_position : Vector3, sound : AudioStream):
	var sound_player : AudioStreamPlayer3D
	if (sound_players_world.size() < MAX_WORLD_SOUNDS):
		sound_player = AudioStreamPlayer3D.new()
		sound_players_world_index = sound_players_world.size()
		sound_players_world.append(sound_player)
		Global.main_scene.add_child(sound_player)
		sound_player.bus = &"Sound"
	else:
		sound_players_world_index += 1
		if (sound_players_world_index >= MAX_WORLD_SOUNDS):
			sound_players_world_index = 0
		sound_player = sound_players_world[sound_players_world_index]
	sound_player.stop() # make sure we stop any existing sound if this gets reused
	sound_player.stream = sound
	sound_player.global_position = sound_position
	sound_player.play()

Using 0,0,0 for the sound_position caused the issue. Changing it to a non-zero value worked fine.

@jitspoe
Copy link
Contributor

jitspoe commented Mar 7, 2024

Update: It seems playing at 0,0,0 works. Perhaps the debug display was incorrect? I can reproduce it by trying to play at Vector3.INF, though.

Edit: Pretty sure the values were INF or -INF, and the debugger just wasn't showing them due to this bug: #88006

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants