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 sound pileup issue #14047

Merged
merged 12 commits into from Jul 13, 2023
Merged

Conversation

docEdub
Copy link
Contributor

@docEdub docEdub commented Jul 12, 2023

Sounds created with the loop or autoplay options set while the audio engine is locked are started automatically when the audio engine is unlocked. If the newly created sound is stopped before the audio engine is unlocked, then the sound still plays when the audio engine is unlocked. This is unexpected. Stopped sounds should not play when the audio engine is unlocked.

This change fixes the issue by clearing the 500 ms timeout used for double-checking the audio context state and removing the observer added to AudioEngine.onAudioUnlockedObservable when the sound stops, pauses, gets played again without being stopped, or when it gets disposed.

Reported on forum https://forum.babylonjs.com/t/sound-pileup-in-chrome/42266.
Tested on Edge browser with playground https://playground.babylonjs.com/#0CNZFE#21.

Copy link
Member

@sebavan sebavan left a comment

Choose a reason for hiding this comment

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

Should the cleanup code not be called in Play as well ?

packages/dev/core/src/Audio/sound.ts Outdated Show resolved Hide resolved
packages/dev/core/src/Audio/sound.ts Outdated Show resolved Hide resolved
@bjsplat
Copy link
Collaborator

bjsplat commented Jul 13, 2023

@docEdub docEdub added the bug label Jul 13, 2023
@docEdub docEdub marked this pull request as ready for review July 13, 2023 04:58
@docEdub docEdub enabled auto-merge July 13, 2023 04:58
@docEdub
Copy link
Contributor Author

docEdub commented Jul 13, 2023

Should the cleanup code not be called in Play as well ?

Yep, thanks. Unit test added for this case, too.

@BabylonJS BabylonJS deleted a comment from bjsplat Jul 13, 2023
@BabylonJS BabylonJS deleted a comment from bjsplat Jul 13, 2023
@BabylonJS BabylonJS deleted a comment from bjsplat Jul 13, 2023
@docEdub docEdub requested a review from sebavan July 13, 2023 05:13
@sebavan sebavan requested review from Popov72 and bghgary July 13, 2023 17:56
@docEdub docEdub merged commit a49048d into BabylonJS:master Jul 13, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants