-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Audio Zones #4399
Audio Zones #4399
Conversation
- Audio zone component that changes the settings of the audio sources inside it based on audio zones. - Global gain nodes for voice/media - Refactor audio-params
05d3fdc
to
26e34af
Compare
6ca8b65
to
221d945
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking really good! I like a lot of the consolidation into audio-params
which removes a lot of duplicate code. Also centralizing the mixer logic like we discussed simplifies things a lot too, and I think opens up room for a pretty nice UI to control things as we break down stuff into more granular mixer tracks like "environment", "music", "videos", "ui" etc.
The audio zones are also looking really good. I think they will give people a lot of flexibility to play with different things in their scenes.
I made a bunch of little suggestions (many are just GC related) but also a few points that I think are worth hopping on a call and brainstorming a bit. Namely I wonder if we can remove some of the incidental complexity of having multiple components (audio-params, audio-zone-souce, and audio-zone-entity) since it seems like we always want all 3 on everything that produces audio, so we may be able to combine them. And I wonder if there is anything we can do to simplify the work of resolving zones.
|
||
// Adds a zone to the current zones array. | ||
addZone(zone) { | ||
const index = this.zones.indexOf(zone); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should use a Set instead... not sure what the perf characteristics are..
dependencies: ["audio-zone-entity"], | ||
|
||
init() { | ||
this.position = new THREE.Vector3(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is it useful to have this extra copy of the position? If not we might just replace this component with a tag... but don't think its a big deal either way..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I get this one.
src/systems/audio-zones-system.js
Outdated
// We always apply the outmost active zone audio params, the zone that's closest to the listener | ||
const inOutParams = source.entity | ||
.getZones() | ||
.filter(zone => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very much like writing code like this with filter/map/reduce but am a bit concrned about the GC characteristics.. I wonder if there is a good way we can find out if this is something we should still be worried about..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same but this should only be called whenever there is a change in zones in a source or a listener so shouldn't happen very often so I thought it might be ok.
src/systems/audio-zones-system.js
Outdated
.getZones() | ||
.filter(zone => { | ||
const zoneBBAA = zone.getBoundingBox(); | ||
this.intersectTarget = new THREE.Vector3(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be creating a lot of Vector3s every frame.
src/systems/audio-zones-system.js
Outdated
const zoneBBAA = zone.getBoundingBox(); | ||
this.intersectTarget = new THREE.Vector3(); | ||
this.ray.intersectBox(zoneBBAA, this.intersectTarget); | ||
return this.intersectTarget !== null && zone.data.inOut && !this.listener.entity.getZones().includes(zone); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not too big a deal since we are only doing a Ray x Box raycast here and not a general scene raycast, but we can avoid it in the case zone.data.inOut == false
.
} | ||
|
||
// Updates zones states in case they have changed. | ||
_updateZones() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are doing the work to update the list of zones for each source every frame anyway, so I wonder if we can somehow combine this logic with the logic in tick into a single loop without keeping state... Might be worth trying to talk through and see if we can come up with something. Though this is probably fine since the number of zones and sources we are talking about is fairly low.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are two different updates, in the zones update we iterate over the zones and then we iterate over the sources to update their states based on the zones state. Not sure if it could be merged...
@netpro2k I've updated most of the items. Let's discuss the rest. |
@netpro2k I've updated all the audio zones, I think it should be good to go. I've updated with a fix to correctly show the audio debugger when switching scenes and I have also removed all the code for handling audio zones with meshes. Now we always assume an empty object with a scale. One thing that bugs me is that by default blender empty is two meters wide (although the display size is set to 1 which is the default) so blender exported empties are half size the Spoke exported ones in the Hubs scene. I think probably the best thing to do it document that in the Blender add-on and ask people to set the empty cube dimension to 0.5. |
This PR adds support for Audio Zones, a 3D box volume that modifies the audio properties of audio sources (avatars, videos and audios) based on the source's and listener's positions with respect to the audio zone. One obvious application would be to dim audio sources' volumes based on 3D areas like rooms to mimic the real world behavior.
You can try it in this scene: https://hubs.mozilla.com/scenes/PXxP3be
Components
audio-zone-entity
Represents an entity in the
audio-zones-system
. Records the audio zones where the entity is and returns current/past states. This entity is used together with theaudio-zone-listener
oraudio-zone-source
components to composite actual entities.audio-zone-source
Represents an
audio-zone-entity
that has an audio component in theaudio-zones-system
. Expects anaudio-params
component. It can be an audio, video, avatar or an audio target.audio-zone-listener
Represents an
audio-zone-entity
that is the scene's audio listener in theaudio-zones-system
.audio-zone
Represents an 3D box area in the
audio-zones-system
that can containaudio-zone-entities
. I has anaudio-params
components which values are used to override the audio sources audio properties based on the source's and listener's position.It can be of
inOut
or/andoutIn
types.audio-params
to anaudio-zone-source
when the source is inside and listener is outside. i.e. Mute or dim audio sources inside the audio zone when the listener is outside.audio-params
to the anaudio-zone-source
when the listener is inside and the source is outside. i.e. Mute or dim audio sources from outside the audio zone when the listener is inside.Systems
audio-zones-system
This system updates the
audio-zone-sources
audio-params
based on theaudio-zone-listener
position. On every tick it computes theaudio-zone-source
andaudio-zone-listener
positions to check if the listener is inside/outside anaudio-zone
and applies the zone's audio parameters. It only updates in case there has been any changes in the sources or listener's positions.If several audio-zones are in between the
audio-zone-listener
and theaudio-zone-source
, the applied audio parameters is a reduction of theaudio-zones
most restrictive audio parameters. i.e. If there are two audio-zones in between the listener and the source and the first one has gain == 0.1 and the other has gain == 1.0, gain == 0.1 is applied to the source.This PR also refactors the current global media/avatar volume handling to use a global gain node instead of updating individual audio's gain values
Related Hubs-Foundation/Spoke#1156
Related Hubs-Foundation/hubs-blender-exporter#36
Fixes #4167