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

Three.Audio evolution #7485

Closed
vincent-courtalon opened this issue Oct 28, 2015 · 28 comments
Closed

Three.Audio evolution #7485

vincent-courtalon opened this issue Oct 28, 2015 · 28 comments

Comments

@vincent-courtalon
Copy link
Contributor

Hello, i recently upgraded from R71 to R73 and am pretty happy with the evolution of Three.Audio

I am currently developing some sort of game, and i need different type of sound capabilities
A) ambient sound (no position, just some background)
B) "event" sound, like footstep, punch... (once again, no position needed)
C) positional sound (like a torch on a wall, a monster, a fountain...)

Three.audio is not really useful for A and B, which is ok (but you loose some of the ease of use that provide Three.audio)
C is clearly the intended use of Three.audio, with the use of the panner to position the sound seamlessly with position update from Three.

but i could see some enhancement (some easy, some more complex) added.

  1. first and foremost, the ability to directly create a Three.Audio with a pre-loaded buffer, thus avoiding a request if the sound is already loaded. Particularly useful if a sound is used at a lot of different positions (like 'medieval' torch burning in my case for example).
  2. if you want to go further, and more powerful/generic, we could provide directly a way to connect an audio source/node to three.Audio (instead of a buffer or file), which would open all (or almost) the possibilities of the webAudio api while still benefiting from Three.Audio functionality
  3. maybe a way to add/remove filter in Three.Audio, instead of just setting one (which is already a big plus), but it would maybe become too complicated/cumbersome...

what do other peoples think of this ? any idea ?
if it is interesting and possible to have that in Three, i could try and do an implementation candidate for a pull request instead of just having my own code (case in point, if it is something needed and acceptable for Three).

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

Couldn't you do A and B by adding the Audio as child of the AudioListener?

@vincent-courtalon
Copy link
Contributor Author

i think you are right, it could work for A and B. But i remember (maybe i am wrong) trying to simply copy my camera position in the Audio (i add the listener to the camera) and had weird result when rotating, which is when i think now probably because i did not copy the rotation. I ended placing the audio where i look at (1 meter forward) and it worked for me. i will try this.
thank you for the suggestion (still learning...)

still, i think adding the possibility to reuse same buffer, or connect to a node would be nice.
Another way to achieve the reuse would be with a Audio clone method not cloning the buffer i think...

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

Maybe we need a AudioSource class?

@vincent-courtalon
Copy link
Contributor Author

It is a possibility

The AudioSource could "abstract/hide" from where the audio data come (file, buffer, or node).
But what would be the responsibility of Audio then?
Currently, Audio, as i understand it, manage the loading, update to position and basic filters (in particular a panner with correct positioning) and playback control (play/stop, loop, volume, etc...)

If we introduce an AudioSource class, it will surely encapsulate loading/source, and not the positioning (which is Three.Audio role), but what about playback control?

@vincent-courtalon
Copy link
Contributor Author

i am still thinking, but what i am trying to see which could be a good addition.
if it is just sharing of buffer, an AudioSource class is possible, but will not do anything besides loading...
if it is more advanced (connection to another audio node), then the playback and controls are not so simple...i think, and an AudioSource could help

@vincent-courtalon
Copy link
Contributor Author

just a little followup concerning case A and B.

There definitively is something strange happening when i add a THREE.Audio to the Listener, or to the camera (the listener itself is added to the camera).
the sound plays, but only on one side (right for me). This was the problem i had originaly for "ambient" sound. as soon as i add the Audio to an object in front of the camera, back to normal (well almost, there are sometime some weird sound artifact).
maybe i am doing something wrong, i tested on chromium on linux...

just for testing purpose, i added a little function to my custom Audio test class

THREE.Audio.prototype.createFromBuffer = function ( buffer ) {
    this.source.buffer = buffer;
    if ( this.autoplay ) this.play();
    return this;
};

and preloading/decoding myself the audio buffer. Seems to work normally.

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

Another option would be AudioPlayer and Audio. I guess that's more intuitive?

@vincent-courtalon
Copy link
Contributor Author

I honestly am not sure which one is the more intuitive.

if we try to (very broadly) categorize what is available in webaudio API, there is 3 type of "object".

  • the source objects, generating a sound either from buffer (like Audio class which load it from a file now), or with for example an oscillator. These objects are the ones with playback control (start, stop, loop...)
  • the 'transformation' object, like gain node(volume), panner (position), and plenty other effect (dynamic compression, echo, delay...). these ones have no playback control
  • and finally the Listener

right now, Audio does the two first roles.

I suppose the best is to not break (or not much) existing code relying on Audio while providing more flexibility? Plus, I don't think 'encapsulating' to much of webaudio api is really useful (for now).

what we could do:
have an AudioBase class with common fonctionnality to all Audio 'types'. two classes inherits from it:

  • Audio class is used for file or buffer audio, and has playback control
  • AudioStream(or source, or whatever) is used for AudioNode source audio (ie it is connected to a webaudio source), and thus has no playback control.

regarding AudioBase itself, i can see also a use case for disabling the panner filter if the user wants to have an 'ambient' sound with no position. Also, there are some setter/getter which can be added for finer control of panner (like distanceModel and maxDistance).

And one last thing, but i do not know yet where (or if) we could put this (maybe in the listener), generally in game, you can have multiple sound playing at the same time and using a DynamicCompressor Node just at the end can avoid clipping sound/volume problems. So having this possibility would be useful(but only just before the Listener...)

@vincent-courtalon
Copy link
Contributor Author

I would add, if you prefer better naming (but with side effect of "breaking" old code, though easy to fix):

  • Audio would be the baseClass name
  • AudioPlayer for the file or buffer Audio
  • AudioSource (or another name) for the 'connected' audio

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

I was suggesting AudioSource to abstract the buffer part.

So the API would be something like this...

var source = new THREE.AudioSource().load( file );
var audio1 = new THREE.Audio( listener ).setSource( source );
var audio2 = new THREE.Audio( listener ).setSource( source );

Or maybe this would be more intuitive...

var buffer = new THREE.AudioBuffer().load( file );
var audio1 = new THREE.Audio( listener ).setBuffer( buffer );
var audio2 = new THREE.Audio( listener ).setBuffer( buffer );

As per Ambient audio... Maybe we can make Audio panner-less and create PositionAudio which extends Audio and adds a panner.

@vincent-courtalon
Copy link
Contributor Author

Ok, i understand now what you are suggesting.

That would solve all the buffer sharing quite simply. I am leaning for THREE.AudioBuffer because, well, in fact it is just an webaudio audioBuffer inside with convenience method for loading.

PositionAudio with the panner seems a good idea to me too.

with that, positioning sounds, ambient sounds and buffer sharing would be handled easily.

now, we can also try to go a little further (the whole "connecting any audio source") with, i think, not a lot more code.

this is the second reason i prefer THREE.AudioBuffer, because we could add a second (or more) class for no-buffer source, which could be named THREE.AudioSource

like that for example:

var source = new THREE.AudioSource().setInputNode( audioNode );
var audio1 = new THREE.Audio( listener ).setSource( source );
var audio2 = new THREE.Audio( listener ).setSource( source );

there could be any audio source (or audio node graph) connected like that ( OscillatorNode, MediaElementAudioSourceNode, MediaStreamAudioSourceNode) without having to handle the details oursevles (you want to connect you're own AudioNode to an audio, you handle their details/setup).
only thing is, in that case, the THREE.Audio with an AudioSource (and not an AudioBuffer) would have play, pause and stop not have any effect...

last, currently, the THREE.Audio directly connect to the listener with this line:

this.gain.connect( this.context.destination );

i propose to change it like this

this.gain.connect( listener.getOutputNode() );

where getOutputNode() is a method of THREE.AudioListener returning a gain node which has been setup internally when THREE.AudioListener is created. thus, we would have some sort of master volume (for the output) and could add a setfilter if someone want to add for example a DynamicCompressor or whatever he wants just before global audio output.

if it is ok with you, i can try to do a quick proof of concept?

@mrdoob
Copy link
Owner

mrdoob commented Nov 1, 2015

if it is ok with you, i can try to do a quick proof of concept?

Sounds good!

@vincent-courtalon
Copy link
Contributor Author

just a little followup. I am still working on it (should have more time this week-end). One thing i noticed while refactoring the code and reading about the details of pannerNode, which maybe the source of the strange effect i had while adding the THREE.Audio to the camera. in the original code of THREE.Audio

return function updateMatrixWorld( force ) {
        THREE.Object3D.prototype.updateMatrixWorld.call( this, force );
        position.setFromMatrixPosition( this.matrixWorld );
        this.panner.setPosition( position.x, position.y, position.z );
    };

here the panner has its position updated... but a panner also has an orientation

https://developer.mozilla.org/en-US/docs/Web/API/PannerNode/setOrientation

this one is currently never updated which may be important if the panner use a soundcone. There is no definition of the soundcone in the current code, which i suppose mean it must use the "default" soundcone of the navigator.

i suppose for now the soundcone is 360° by default, so maybe it is not important here, but we should check it later...

@mrdoob
Copy link
Owner

mrdoob commented Nov 4, 2015

Yep! At the moment is omnidirectional.

@vincent-courtalon
Copy link
Contributor Author

I Have now a working, albeit not complete, evolution of the Audio system.
I would like to have you're input before more development.

here my three.js github clone with the work done on audio (branch dev_audio_#7485)

https://github.com/vincent-courtalon/three.js/tree/dev_audio_%237485

I modified THREE.Audio (remove panner/position and add buffer support) and THREE.AudioListener(added filter and master volume capacity).
I added THREE.PositionAudio (like the previous THREE.Audio), THREE.AudioBuffer (for loading sound once, use in multiple source)
and finally modified sound example and build file list.

Some controls are still missing (like finer control of panner in PositionAudio) and i have not yet added the possibility to 'connect' a node as a source for Three.Audio.

what do you think, should we just silently ignore when you call play/pause/start... on a THREE.Audio connected to a node source ( and not a buffer) or should we generate an error?
Or is there another solution?

When these are added and if you are ok with it, i will submit you a pull request.

ps: i did not commit the build, you need to build first to test the example

@mrdoob
Copy link
Owner

mrdoob commented Nov 9, 2015

Do you mind doing Pull Request? That way it'll be easy to see what changed.

@antont
Copy link
Contributor

antont commented Nov 9, 2015

@mrdoob - i used this to see the changes: dev...vincent-courtalon:dev_audio_#7485

but yes it was trouble to dig that up and pullreqs show everything nicely so certainly the right way at this point.

just figured to share that comparison link for convenience.

@mrdoob
Copy link
Owner

mrdoob commented Nov 9, 2015

Yeah, that's what Pull Requests are for, to show the changes. A Pull Request can still be modified until is good to merge.

@mrdoob
Copy link
Owner

mrdoob commented Nov 9, 2015

@mrdoob - i used this to see the changes: dev...vincent-courtalon:dev_audio_#7485

However, that does the trick too.

@vincent-courtalon I would rename PositionAudio to PositionalAudio, but the rest looks good!

@vincent-courtalon
Copy link
Contributor Author

Sorry for the lack of pull Request, i will make one now (i commit the renaming of PositionAudio just now).
Some project don't like to have pull request modified 'later'. sorry to have assumed that (and i agree it is easier to have a 'moving' pull request).

edit: made the pull request: #7562

@Zob1
Copy link

Zob1 commented Nov 9, 2015

Vincent, just wanted to add I'm very excited to see this move forward. Thanks for your work!

@vincent-courtalon
Copy link
Contributor Author

Thanks, glad to know it will be useful to other peoples!

I just added support for connecting an audioNode to a THREE.Audio.
basically now, when you create a THREE.Audio (or THREE.PositionalAudio), you either chose to call setBuffer with an AudioBuffer, or call setNodeSource to connect to an AudioNode

i updated the example with a very basic oscillator:

var sound3 = new THREE.PositionalAudio( listener );
var oscillator = listener.context.createOscillator();
oscillator.type = 'sine';
oscillator.frequency.value = 144;
oscillator.start(0);
sound3.setNodeSource(oscillator);

if anyone has any simple more interesting example...

I am not completely happy with the code yet, but it seem to work (although i only tested on chrome and firefox).

I will next add finer panner controls (like distance model) and i think i will then be ok for now.

@vincent-courtalon
Copy link
Contributor Author

I Will test it more, but i don't plan to add more feature right now.
@mrdoob I added check/guards to generate warning when trying to play/start/stop/etc. an Audio wich is connected to a sourceNode (and not an AudioBuffer), but maybe it is better to not have this check and let these functions as they were before, because it can work if the sourcenode is playable (but fail otherwise). two properties added also, 'hasPlaybackControl' (set to false if using a sourceNode), and sourceType ('empty', 'audioNode','buffer') which is updated at creation/loading, but not used otherwise for now. they are here more in case of future addition/update to Audio, but could be removed if it is preferred.

@vincent-courtalon
Copy link
Contributor Author

Hello, just a little followup after the merge.
I have done more testing and no problems so far, but one missing thing i think.
I was considering two strategy for the case (like in game) where you create/destroy as you go new sound source, either have really clean up THREE.Audio after use or reuse them.
for now, i will reuse them (more efficient) but stumbled on a little problem (not specific to the re-use), there was no simple/clean way to "remove" a filter once added to a THREE.Audio (cause of the test used in connect/disconnect). So i changed the code to always have a filter property, only null initialized at the creation. Now, it is easy to remove a filter, just call setFilter with null.
also, i simplfied and removed duplicate code in PositionalAudio (with the help a little function added getOuput wich return the gainNode or pannerNode depending on type of object), so no more specific version of connect/disconnect for positionalAudio.

I will make a pull request now

@mrdoob
Copy link
Owner

mrdoob commented Nov 13, 2015

Sounds good!

@spiricom
Copy link

I just want to comment that this is excellent work - I'm doing a project using the THREE.js library that is very focused on audio now and it's very clear. If I have some time, I may make an example that shows how to add a filter, and how to use envelopes for gain changes (to avoid clicking caused by instantaneous setters).
Great job!

@mrdoob
Copy link
Owner

mrdoob commented Jun 28, 2016

@spiricom That sounds good!

@mrdoob
Copy link
Owner

mrdoob commented Jun 28, 2016

Closing this as the suggested features have now been implemented.

@mrdoob mrdoob closed this as completed Jun 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants