resonance-audio / resonance-audio-web-sdk

Resonance Audio SDK for Web
https://resonance-audio.github.io/resonance-audio/
Apache License 2.0
200 stars 56 forks source link

setRoomProperties throws an error #16

Open brianchirls opened 6 years ago

brianchirls commented 6 years ago

The error happens in LateReflections.prototype.setDurations.

Per the ConvolverNode spec, you can only set the buffer once. If you try to set it a second time, it throws an InvalidStateError. Firefox ignores this, and it is only recently implemented in Chrome, which is probably why the problem didn't show up earlier.

I believe you'll need to create a new ConvolverNode when you want to change the buffer.

brianchirls commented 6 years ago

This doesn't seem to be an issue until Chrome 69, which is currently in Beta, so you might not see it in the current stable version. The next upgrade is scheduled for early next week, so it will go mainstream in a few days.

arpu commented 6 years ago

looks like i hit this problem with latest stable chrome Version 69.0.3497.81 (Offizieller Build) (64-Bit)

core:a-node:error Failure loading node: DOMException: Failed to set the 'buffer' property on 'ConvolverNode': Cannot set buffer to non-null after it has been already been set to a non-null buffer at LateReflections.setDurations (webpack:///./node_modules/resonance-audio/build/resonance-audio.js?:3382:26) at Room.setProperties (webpack:///./node_modules/resonance-audio/build/resonance-audio.js?:3110:13) at ResonanceAudio.setRoomProperties (webpack:///./node_modules/resonance-audio/build/resonance-audio.js?:3898:14) at NewComponent.setUpRoom (webpack:///./src/components/aframe-resonance.js?:131:33) at NewComponent.updateProps (webpack:///./src/components/aframe-resonance.js?:107:10) at NewComponent.init (webpack:///./src/components/aframe-resonance.js?:37:10) at NewComponent.updateProperties (webpack:///./node_modules/aframe/dist/aframe-master.js?:75069:12) at HTMLElement.value (webpack:///./node_modules/aframe/dist/aframe-master.js?:73830:19) at HTMLElement.value (webpack:///./node_modules/aframe/dist/aframe-master.js?:73804:14) at entityLoadCallback (webpack:///./node_modules/aframe/dist/aframe-master.js?:73600:14) aframe-master.js:3068 core:schema:warn Unknown property `map` for component/system `material`.

this breaks my app with the new chrome update

arpu commented 6 years ago

here is a online sample https://etiennepinchon.github.io/aframe-resonance/

brianchirls commented 6 years ago

@arpu I can confirm that I get the same error on my machine (OSX, same Chrome build version) with your link. This version came out of beta and went stable a few days ago, so a plurality if not a majority of Chrome users should be running this version pretty soon.

As a workaround until this gets fixed, I think you may need to recreate your resonance instance whenever the room dimensions change. You can pass the parameters to the constructor.

ianpetrarca commented 5 years ago

@drewbitllama @mgorzel It would be great to get some clarity on this issue. The official Resonance Examples page is completely broken along with about every other website that uses Resonance when viewed on the latest Chrome. Resonance now works better on Firefox which is a bit ironic.

Link to broken examples page:

https://cdn.rawgit.com/resonance-audio/resonance-audio-web-sdk/master/examples/hello-world.html

hoch commented 5 years ago

@brianchirls is correct.

This is because of the recent change in Chrome's Convolver node. We (AudioWG) reverted the spec decision due to the reasonable amount of breakage. Here's the revert CL for Chrome: https://chromium-review.googlesource.com/c/chromium/src/+/1249275

The revert patch was landed at 71.0.3569.0, so perhaps @drewbitllama needs to make a temporary fix that recreates a convolver node whenever a buffer changes. When 71 hits the stable, we can remove the temporary fix.

arpu commented 5 years ago

@hoch do you know a workaround for chrome < 71.0.3569.0 ?

hoch commented 5 years ago

This line: https://github.com/resonance-audio/resonance-audio-web-sdk/blob/c69e41dae836aea5b41cf4e9e51efcd96e5d0bb6/src/late-reflections.js#L212

Can be replaced with the following:

this._predelay.disconnect();
this._convolver.disconnect();
this._convolver = this._context.createConvolver();
this._convolver.buffer = buffer;
this._predelay.connect(this._convolver);
this._convolver.connect(this.output);

FYI I have not tested the code locally. (traveling atm)

arpu commented 5 years ago

@hoch thx a lot works!

ianpetrarca commented 5 years ago

@hoch I tested that code on Mac 10.12.16 Chrome Version 70.0.3538.67 and it fixed the Convolver error but it is now no longer outputting late-reflections for me. I am testing on this repo . I simply swapped your fix into the late reflections src.

hoch commented 5 years ago

https://github.com/ianpetrarca/resonance-audio-web-sdk/commit/56252b5a238e1de09ce282341cfb1d18fa8abd2e#diff-c87382122ba43153a840014fa7957897R220

Seems like you have a typo there. You can't connect a node to a buffer.