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.

Sign up for GitHub

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

Jump to bottom

Audio samples leaks #96068

Closed
blam23 opened this issue Aug 25, 2024 · 13 comments · Fixed by #96572
Closed

Audio samples leaks #96068

blam23 opened this issue Aug 25, 2024 · 13 comments · Fixed by #96572
Assignees
@adamscott
Labels
bug confirmed platform:web topic:audio
Milestone
4.4

Comments

@blam23
Copy link

blam23 commented Aug 25, 2024 •
edited
Loading

Tested versions

  • Reproducible in 4.3-stable, 4.3beta2, and 4.3beta3
  • Does not occur in 4.3beta1, or 4.2.2-stable

System information

Godot v4.3.stable - Windows 10.0.22631 - GLES3 (Compatibility) - NVIDIA GeForce RTX 3080 Ti (NVIDIA; 31.0.15.5161) - AMD Ryzen 9 5900X 12-Core Processor (24 Threads)

Issue description

Playing an AudioStream in a web build in 4.3 causes objects to be created that are not freed. A windows build of the same code doesn't cause the same leaks. I could not reproduce this with 4.3beta1, only the release 4.3 build (v4.3.stable.official [77dcf97]).

In games with lots of audio, this will lead to an eventual crash when the object count gets too high for the wasm/webgl buffer code to handle.

Steps to reproduce

Create any kind of AudioStream node, and call play on it.
On web, the object count goes up and never goes back down. Check using Performance.OBJECT_COUNT.

Minimal reproduction project (MRP)

MRP: web-leak-repro.zip

I've uploaded this to itch here with debug symbols and gdscript as text: https://blambear.itch.io/godot-web-audio-leak-bug the password is godot.

You can see the object count keep going up and up.

(Note the pop.mp3 file in here is an audio file I recorded, it's MIT licensed)
@blam23
Copy link
Author

blam23 commented Aug 25, 2024

Examples below of crash error output when the object count gets too high, for me this happens when the object count hits around 50k.

Firefox

image

Uncaught TypeError: WebGL2RenderingContext.bufferData: Argument 2 can't be an ArrayBuffer or an ArrayBufferView larger than 2 GB

Chrome

image

index.js:474 USER ERROR: Parameter "mem" is null.
index.js:474 USER ERROR: Out of memory

@blam23 blam23 changed the title Audio leaks objects on web exports in 4.3 Audio can leak objects on web exports in 4.3 Aug 25, 2024
@blam23 blam23 changed the title Audio can leak objects on web exports in 4.3 Audio leaks objects on web exports in 4.3 Aug 25, 2024
@blam23
Copy link
Author

blam23 commented Aug 25, 2024

Tried with 4.3beta2 and 4.3beta3, both seem to reproduce the issue with the object count going up and up. So it seems the issue was introduced between beta1 and beta2.

@akien-mga akien-mga added this to the 4.4 milestone Aug 25, 2024
@akien-mga
Copy link
Member

4.3-beta2 is the release where audio sample playback was added for the web platform, so that's probably where the bug originates.

@wheatear-dev

This comment was marked as outdated.

Sign in to view
@akien-mga

This comment was marked as outdated.

Sign in to view
@RainbowCemetery
Copy link

changing the audiostreamplayer playback type to sample has the same leaked object behavior when run in the editor

@blam23
Copy link
Author

blam23 commented Aug 30, 2024

Just for reference, I've uploaded versions exported from 4.3-beta1 and 4.3beta-2 to itch.io:

https://blambear.itch.io/web-leak-test-4-3-beta1
https://blambear.itch.io/web-leak-test-4-3-beta2

Password for both is again godot.

On my machine, in Firefox (131.0a1) and Chrome (128.0.6613.86), I get a stable object count in beta1 and a consistently increasing object count in beta2.

@wheatear-dev
Copy link
Contributor

I can reproduce this, using the first of the two itch.io links (4.3-beta1), on Safari for both macOS and iOS.

Chrome does not reproduce for this version, for me, on macOS.

trim.53DCD032-625F-492E-9843-CD85F94C56B8.MOV

@wheatear-dev
Copy link
Contributor

Okay git-bisect took a while! 😅

Good luck everyone, it's a big diff! 😌

52fa4f05f3945fdf511c249adede9b6d07c51beb is the first bad commit
commit 52fa4f05f3945fdf511c249adede9b6d07c51beb
Author: Adam Scott <ascott.ca@gmail.com>
Date:   Thu Apr 18 10:50:34 2024 -0400

    Add samples playback support
Methodology

Steps

  • Enable autoplay always alias ogc='open -a Google\ Chrome --args --autoplay-policy=no-user-gesture-required'
  • Build both MacOS editor and Web template_debug each time
  • Manually export
  • Open chrome (ogc alias), and confirm presence/absence of bug

Observations:

  • Only “bad” revisions seem to play sound at all?!
  • Before force enabling autoplay:
    • all locally built revisions would apparently demonstrate this memory leak
    • even those that were "good" when I tried again
    • but stop on a mouseclick (and maybe other interactions)
  • Something else is going on with Safari 🤔
    • it broke a lot earlier, at or before 4.3-beta1

@adamscott
Copy link
Member

@wheatear-dev Yes, it's when I added samples support. I'll investigate this beginning of next week.

@adamscott adamscott changed the title Audio leaks objects on web exports in 4.3 Audio samples leaks objects Aug 31, 2024
@adamscott adamscott changed the title Audio samples leaks objects Audio samples leaks Aug 31, 2024
@adamscott adamscott mentioned this issue Sep 4, 2024
Merged
@adamscott
Copy link
Member

#96572 should fix this issue. Now, the MRP's object numbers stay stable, whether the game runs on a desktop (with samples forced) or on the Web.

@PureAsbestos
Copy link
Contributor

PureAsbestos commented Sep 10, 2024 •
edited
Loading

This is a pretty serious bug with 4.3, will it be cherry-picked for a patch?
Edit: nevermind I see that it is already marked for that in the PR. Cheers!

@blam23
Copy link
Author

blam23 commented Sep 10, 2024

This is in 4.4.dev2, tested and it's fixed for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adamscott adamscott

Labels
bug confirmed platform:web topic:audio
Projects
None yet
Milestone
4.4
Development

Successfully merging a pull request may close this issue.

Fix leak when using audio samples instead of streams adamscott/godot
6 participants
@adamscott @blam23 @akien-mga @PureAsbestos @wheatear-dev @RainbowCemetery