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

Fix AudioStreamPlayer3D still processing when out of range #96677

Conversation

Wierdox
Copy link
Contributor

@Wierdox Wierdox commented Sep 7, 2024

Fixes #96670

Although said in documentation, AudioStreamPlayer3D doesn't do the claimed optimization: avoiding audio mixing when further than max_distance from the AudioListener3D. So I did a seemingly simple change to the if statement for an early exit of its processing loop:

if (total_max > max_distance) {
	continue; //can't hear this sound in this listener
}

Into this,

if (dist > total_max || total_max > max_distance) {
	HashMap<StringName, Vector<AudioFrame>> bus_volumes;
	for (Ref<AudioStreamPlayback> &playback : internal->stream_playbacks) {
		// So the player gets muted and stops mixing when out of range.
		AudioServer::get_singleton()->set_playback_bus_volumes_linear(playback, bus_volumes);
	}
	continue; //can't hear this sound in this listener
}

Without set_playback_bus_volumes_linear(), the sound keeps playing if you exit the max_distance radius, and the audio server doesn't get the speed boost.
Note: the playhead position will continue to be updated even if this is called.

This oversight looks like it is also the case in 3.x, so it should probably be updated too.

@Wierdox Wierdox requested a review from a team as a code owner September 7, 2024 09:42
@Chaosus Chaosus added this to the 4.4 milestone Sep 7, 2024
@Wierdox

This comment was marked as outdated.

@Wierdox Wierdox force-pushed the fix_audio_stream_player_3d_still_processing_when_out_of_range branch from b834bf9 to 0b6d7be Compare September 8, 2024 03:13
@Mickeon
Copy link
Contributor

Mickeon commented Sep 17, 2024

So, when you retreat from an AudioStreamPlayer3D that is continuously playing audio, the audio stream will not stop playing audio in this PR, right?

Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the PR. I confirm the sound not shutting off when moving outside the distance.

I tested also that this PR doesn't shut off other AudioStream3D playing when moving. Here's the MRP:
audiostreamplayer3d-range-bug-local.zip

@adamscott
Copy link
Member

So, when you retreat from an AudioStreamPlayer3D that is continuously playing audio, the audio stream will not stop playing audio in this PR, right?

The audio playhead will still "continue", but no sound will be heard nor processed.

@adamscott adamscott requested a review from a team September 17, 2024 16:12
@akien-mga
Copy link
Member

I'm not too familiar with this class, but I see this is in a for loop for all Cameras. How does it handle cases where the player is out of range for one camera, but in range for another? Is this disabling processing globally or only for the current camera?

Also, should similar changes be done for AudioStreamPlayer2D, or does it already behave like this PR implements?

@masterthdev
Copy link

How does it handle cases where the player is out of range for one camera

Multiple Camera2D/3D or AudioListener2D/3Ds do not increase processing time. Looks like it only processes current camera per viewport. A subviewport with Camera3D increased processing time from 15 ms to 500 ms on my machine. Subviewport + Camera2D doubled processing time as expected.

Also, should similar changes be done for AudioStreamPlayer2D, or does it already behave like this PR implements?

Looks like it has same issue.

MRP for 2D: audiostreamplayer2d-range-mrp.zip

@Wierdox
Copy link
Contributor Author

Wierdox commented Sep 18, 2024

I can confirm what masterthedev said, for my unscientific tests I saw that:

  1. Even with 1 vs 1000 cameras there wasn't a noticeable change in audio/physics performance in this PR.
  2. Using a subviewport with a Camera3D(alongside a normal camera) in the MRP went from ~35ms on average in 4.3, to ~20ms in the PR. Not sure why it's so prohibitively expensive though.
  3. I tested the AudioStreamPlayer2D MRP and it seems to be a similar case of not changing performance when out/in range of max_distance, but it's different structure wise and insofar I haven't found a way to mitigate the audio server cost, only the physics time by a smallish amount. I'll note that the audio server costs were way higher(~6-20ms) compared to my testing with AudioStreamPlayer3D, at ~2-4ms with the same MRP settings on 4.3

I forgot to bring this up earlier:

For the early exit of _update_panning() processing, I'm not sure what the deal is with comparing max_distance with the area override reverb distance(assigned to total_max if > max_distance). Maybe it's intended for that case to only continue and not stop the mixing? Though that would disregard the comment //can't hear this sound in this listener.

@Calinou
Copy link
Member

Calinou commented Sep 19, 2024

For the early exit of _update_panning() processing, I'm not sure what the deal is with comparing max_distance with the area override reverb distance(assigned to total_max if > max_distance).

If you rotate the camera quickly, panning may need to update even though you're outside the range as you're still hearing the sound via the reverb effect.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased on top of master 694d3c2), it works as expected.

However, there's now an object leak that occurs on exit with the MRP:

master

$ bin/godot.linuxbsd.editor.x86_64.master --path /tmp/audiostreamplayer3d-range-bug --quit
Godot Engine v4.4.dev.custom_build.694d3c293 (2024-09-18 15:41:12 UTC) - https://godotengine.org
Vulkan 1.3.280 - Forward+ - Using Device #0: NVIDIA - NVIDIA GeForce RTX 4090
Requested V-Sync mode: Disabled

This PR

$ bin/godot.linuxbsd.editor.x86_64 --path /tmp/audiostreamplayer3d-range-bug --quit                                                 
Godot Engine v4.4.dev.custom_build.6819c26b1 (2024-09-19 12:31:25 UTC) - https://godotengine.org
Vulkan 1.3.280 - Forward+ - Using Device #0: NVIDIA - NVIDIA GeForce RTX 4090
Requested V-Sync mode: Disabled

WARNING: ObjectDB instances leaked at exit (run with --verbose for details).
     at: cleanup (./core/object/object.cpp:2329)
ERROR: 1 resources still in use at exit (run with --verbose for details).
   at: clear (./core/io/resource.cpp:592)

Edit: Nevermind, it happens with master too and is sporadic.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Profiling when outside the range (with the project running for ~7 seconds each time):

Before

image

image

After

image

image

I'm not sure why audio resampling is still being called though.

@Wierdox Wierdox force-pushed the fix_audio_stream_player_3d_still_processing_when_out_of_range branch from 0b6d7be to a3158d8 Compare September 20, 2024 05:37
@Wierdox
Copy link
Contributor Author

Wierdox commented Sep 20, 2024

If you rotate the camera quickly, panning may need to update even though you're outside the range as you're still hearing the sound via the reverb effect.

This would make sense, but the continue skips over the panning step in the function, so it doesn't seem to update that? Unless it is handled elsewhere, or I misunderstand you.

I'm not sure why audio resampling is still being called though.

No clue myself, perhaps an internal process. It's not very straightforward to stop the mixing, from my limited experience. By the way, the performance gain when using the ogg or mp3 version of the sound is lesser, probably from the overhead of still resampling. Will look into this more, maybe I can set something so the resampling doesn't occur.


I updated the PR to cache if it was out of range last frame, so it doesn't set the volume to zilch each time. Performance of audio server with all players out of range:

Using .wav Before After
800 Players ~26-30ms ~1.5-2.75ms
400 Players ~.6-1.5ms ~.6-1.5ms

Seems to only make a measurable difference when overwhelmed with players, but maybe it's more noticeable on a slower CPU.

@akien-mga akien-mga merged commit d2a5153 into godotengine:master Sep 20, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

AudioStreamPlayer3D doesn't stop processing when out of range
7 participants