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

Webaudio migration #6098

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Webaudio migration #6098

wants to merge 24 commits into from

Conversation

keianhzo
Copy link
Contributor

@keianhzo keianhzo commented May 25, 2023

This PR migrates the client code from ThreeJS Audio to use the WebAudio API directly. This has a number of benefits:

  • It simplifies the scene graph, reducing the number of Objects that were there just there to represent the audio entities. Now we update the panner node tranform on demand.
  • It allow us to add some performance improvements and update audios only when necessary.
  • Simplifies audio code interfacing directly with the WebAudio API.

The performance improvements added in this PR are:

  • The audio listener transform is not updated if the listener object world transform hasn't changed
  • The audio source transforms is only updated if the related object world transform has changed either because the object is moving or being rotated
  • The audio source transforms is not updated if the audio source is muted, clipped or paused.

In the quite common scenario where a few people are gathered not moving and listening to someone talking this translates into no audio updates.

@keianhzo keianhzo changed the title Webaudio-migration Webaudio migration May 25, 2023
@keianhzo keianhzo marked this pull request as ready for review May 30, 2023 15:58
@keianhzo
Copy link
Contributor Author

@netpro2k I've tried to cover as many audio update cases as I could think of but I might be missing some.

@johnshaughnessy
Copy link
Contributor

johnshaughnessy commented Jun 1, 2023

In updateAudioSettings, we check whether we are switching from panning to stereo modes, and fire an event accordingly:

    if (
      (audio.panner === undefined && settings.audioType === AudioType.PannerNode) ||
      (audio.panner !== undefined && settings.audioType === AudioType.Stereo)
    ) {
      el.emit("audio_type_changed");
    }

In the event handlers, we delete the three js Audio (or PositionalAudio) node and replace it with its counter part. And we do the same thing in swapAudioType of audio-emitter-system.ts.

When we control the web audio node graph directly, changing from Stereo to Panner does not require deleting resources -- just rewiring of the graph to go through (or not go through) a PannerNode.

This also allows us to delete swapObject3DComponent, which was a hack written specifically for this situation.

I thought this was one of the main motivations for making this change, so I am surprised to not see it in the diff. Are there plans to follow up with that change, or did I misunderstand the problem you're intending to solve?

@keianhzo
Copy link
Contributor Author

keianhzo commented Jun 2, 2023

Good catch! I totally overlooked that case.

I thought this was one of the main motivations for making this change, so I am surprised to not see it in the diff. Are there plans to follow up with that change, or did I misunderstand the problem you're intending to solve?

The main goal of this PR is:

  • Simplify the scene and audio node graphs remove unnecessary intermediate nodes
  • Improve the audio performance reducing the amount of panner nodes and audio listener transform updates

Swapping audios is not a common case case, it only happens when you disable L/R panning so it doesn't really happen most of the time but now we can simplify that so that's a good side win.

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

Successfully merging this pull request may close these issues.

None yet

2 participants