Audio Debugging View #4249
Audio Debugging View #4249
Conversation
keianhzo
force-pushed
the
panner-params
branch
4 times, most recently
from
ee096c9 to
241e3fe
Compare
|
Still haven't gotten around to reviewing this but I will try to soon.... The one thing I noticed off hand is that I think we should not change any falloff defaults in this PR. For me one of the main goals of the debug view will be to help us figure out what better defaults are. So we should first ship just the debug view (and hopefully some way to easily play with values) and then come up with defaults by getting in a room with a bunch of people and messing around with values till we find something we are happy with. |
|
I think the only one that I've changed is the I wanted to jump right away to the OB blitting issue so I haven't pushed the tweaking panel but I'll add that too. |
There was a problem hiding this comment.
This is looking great! I love the debug view and the debug panel will be really helpful for coming up with better defaults. We still have a few more stages of refactoring to clean up audio handling but this is a great first step that at least gives us the target for "source of truth" then we can start reading from. Then we can migrate it to actually being the source of truth and remove all the other places where we define these settings.
src/components/audio-params.js
Outdated
| const MUTE_DELAY_SECS = 1; | ||
|
|
||
| AFRAME.registerComponent("audio-params", { | ||
| dependencies: ["media-views", "avatar-audio-source"], |
There was a problem hiding this comment.
Hmm, would be very careful with this. If I remember correctly having this will cause aframe to create these components if they aren't already on the entity, that would cause avatar-audio-source components to get added to everything this is on which does not seem correct. Also media-views is not a component as far as I am aware.
There was a problem hiding this comment.
Humm, I thought it just affected the components initialization order. Removed.
|
@netpro2k Thanks a lot for the thorough review, it definitely looks better. I've a few questions but otherwise I think I've addressed all your concerns. |
|
I've also added debug support for |
| @@ -1,3 +1,38 @@ | |||
| export const AvatarAudioDefaults = Object.freeze({ | |||
There was a problem hiding this comment.
Yay for having the defaults all in one place!
src/systems/audio-debug.frag
Outdated
| @@ -0,0 +1,161 @@ | |||
| // Based on: https://www.shadertoy.com/view/Mll3D4# | |||
|
|
|||
| #define MAX_DEBUG_SOURCES 64 | |||
There was a problem hiding this comment.
Just FYI you can have threejs pass this in by putting it in defines on the material if you want it to be configurable from the js side. Not really needed for this since there is only ever a single instance of this shader anyway.
There was a problem hiding this comment.
oh that's cool, updated!
src/components/audio-params.js
Outdated
| const worldQuat = new THREE.Quaternion(); | ||
| avatarRigObj.getWorldQuaternion(worldQuat); | ||
| this.avatarRigOrientation.applyQuaternion(worldQuat); | ||
| return { |
There was a problem hiding this comment.
This is allocating several new objects per avatar every frame. I think that is going to lead to noticeable GC pressure.
There was a problem hiding this comment.
Moved this to a temp var to avoid allocation
|
@netpro2k Updated with the latest change requests. |
Support for a debugging view and a debugging panel to visualize and tweak the positional audio parameters.
audio_debugging.mp4
This PR adds:
audio-debugging-systemand a view that that can be enabled from preferences and renders the audio sources elements and it's parameters.audio-gain-systemthat handles all the audio gains and clips audio when attenuation is below certain thresholdCloses #4165
Closes #4166