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

Add audio engine v2 #15839

Draft
wants to merge 46 commits into
base: master
Choose a base branch
from
Draft

Add audio engine v2 #15839

wants to merge 46 commits into from

Conversation

docEdub
Copy link
Contributor

@docEdub docEdub commented Nov 18, 2024

This change adds the initial audio engine v2 implementation and introduces the API for sound and streaming sound creation and playback.

This change adds the initial audio engine v2 implementation and introduces the API for sound and streaming sound creation and playback.
@bjsplat
Copy link
Collaborator

bjsplat commented Nov 18, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

This change gets the CI's `Test UMD build` step to pass.
@bjsplat
Copy link
Collaborator

bjsplat commented Nov 18, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@ryantrem
Copy link
Member

Am I remembering correctly from previous conversations that the new audio engine will implement the existing interfaces, or no?

@bjsplat
Copy link
Collaborator

bjsplat commented Nov 18, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Nov 18, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Nov 18, 2024

@docEdub
Copy link
Contributor Author

docEdub commented Nov 18, 2024

Am I remembering correctly from previous conversations that the new audio engine will implement the existing interfaces, or no?

Yes, this is something we discussed as a possibility.

If not given, the `engine` defaults to the last created engine.
A document body is required for the underlying HTMLMediaElement used in `WebAudioStreaminSoundInstance`.
When a streaming instance is paused before it transitions from `Starting` to `Started`, the start offset is being set incorrectly when resumed. This change fixes the issue by setting the related timing variables correctly and initializing the start time to `Infinity` to indicate that the instance has not actually started, yet.
@bjsplat
Copy link
Collaborator

bjsplat commented Nov 20, 2024

Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/15839/merge/testResults/webgl1/index.html

If tests were successful afterwards, this report might not be available anymore.

@bjsplat
Copy link
Collaborator

bjsplat commented Nov 22, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Nov 22, 2024

It is not needed. The sound plays without adding it to the document body.
This is particularly important for Vision Pro because it suspends/interrupts the audio context when exiting immersive mode.
The WebAudio-based streaming sound class's underlying `HTMLMediaElement` play and stop functions don't have `waitTime` parameters like the static sound class's `AudioBufferSourceNode` does. I tried implementing them in the streaming sound class using `setTimeout`, but the timing is too flaky, so this change removes the parameter completely.

Note that the `waitTime` parameter for the static sound classes remains, but it is moved to the end of the parameter list so the streaming and static sound class play signatures are consistent with each other.
@bjsplat
Copy link
Collaborator

bjsplat commented Dec 11, 2024

Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/15839/merge/testResults/webgl1/index.html

If tests were successful afterwards, this report might not be available anymore.

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.

4 participants