Regression: Sound component not playing audio asset across multiple entities in 0.8.*

[dsinni]

Description:

When multiple entities utilize the sound component, and use the same audio asset source, only one instance of audio is created in 0.8.*.

This does work as expected in 0.7.* as long as an <audio> element is used rather than an <a-asset-item>.

In 0.8.*, only the first entity of its kind in the code emits audio and TypeError: Argument of AudioContext.decodeAudioData can't be a detached buffer - three.js:37939:18 is thrown.

As I expected, poolSize has no effect, I'm assuming due to the fact that this is across multiple instances of the sound component and not contained within one component.

• A-Frame Version: 0.8.0 and 0.8.1
• Platform / Device: Chrome, Firefox, Edge on Windows 10 (all I've tested thus far)
• Reproducible Code Snippet or URL: https://codepen.io/dansinni/pen/yKOjem

[arpu]

looks like the three positional audio works normal
https://threejs.org/examples/#webaudio_sandbox

could it be this only happend if there are more than one sounds?

[dsinni]

@arpu Thanks for the reply. Yeah, it's not so much the positional audio that's not working; it's more specific to when you have multiple entities using the same audio asset, which is pretty common in my work, so this has been a blocking issue for me. It works well in 0.7.*, just not in 0.8.0. Hoping this is addressed soon so I can use 0.8.* in production.

[meatwallace]

we've resolved the issue on a local branch and will be getting a PR up in the next few hours.

[dsinni]

Rad. Thanks, @meatwallace.

[meatwallace]

@dsinni - sorry, jumped the gun a little - we'll be opening a PR, but the issue is in Three.js, so not certain what that means for A-Frame exactly as it's using a fork. regardless, i'll post back as soon as I open the PR.

[dsinni]

Ah, bummer. Okay. Thanks, @meatwallace. Appreciate it. Hoping it can be resolved without too much hassle. It may not be a breaking issue, but it really is an important bug, IMO; I'm surprised more people haven't noticed/reported it (or maybe they have on three.js).

[meatwallace]

@dsinni - the PR can be found here. Not certain it'll be accepted, as it might be more of a workaround rather than a fix.

We fork A-Frame to hack in some fixes specific to our project's implementation, and have temporarily modified the build/aframe-master.js with those changes just to get up and running. Given that A-Frame uses a fork of three, this seemed like the best (temporary) approach for us as we don't want to fork of that too.

@ngokevin - is there any information/advice/direction you can offer here?

[dmarcos]

FYI @meatwallace The fork we use in A-Frame is r90 + 1 cherry picked commit to fix a bug in the raycaster. The PR you opened if it lands will be available on THREE r92 that A-Frame will pick up in the next release. In the meantime you can build your own version of A-Frame as you did.

[dmarcos]

I don’t think there’s any action we have to take on the A-Frame side. Thanks all for investigating this

[meatwallace]

thanks @dmarcos. there's actually an existing issue already tracking it. I don't think my fix will land, so hopefully someone with more knowledge of three can solve it.

[dsinni]

This issue is still blocking me from using 0.8.* in production. 😢

[meatwallace]

@dsinni - fork A-Frame & edit the dist code directly to reflect the changes made in this PR

if you're importing it as a node module, be sure to update dist/aframe-master.js.

if you want 0.8.2, i'd branch from commit #46b1362 to play it safe, as the commit log around the release looks a little messy.

for what it's worth, we're using commit #d5bb456 (A-Frame 0.8.2 + Three.js R92) in production without any issues, but we're only targeting the Oculus Browser with the GearVR so I can't verify other platforms.

additionally, it seems like my PR will be part of Three.js R93, but I don't know when that will be wrapped up into A-Frame.

[dsinni]

Thanks, @meatwallace. I appreciate the info and will look into what you suggested. I'd just really love to see this in an official release since I often use the CDN version for prototypes.

This is pretty much crucial to doing anything with reusable audio assets, which I think is a pretty big deal, even more so because it's on the Web. I cannot stress this enough.

Does the PR allow for more than two instances of a sound?

[dmarcos]

I bumped three to r93 in master. Two options to try:

• Point your script tag to https://rawgit.com/aframevr/aframe/master/dist/aframe-master.js
• Point your package.json to "aframe": "aframevr/aframe#8a203f475b3"

Please let me know if the issue is solved.

[dsinni]

Yessssssss! Thank you, @dmarcos! Going to check it out right now.

[dsinni]

Nope, still not working, @dmarcos :(

I switched to the master in the demo and the issue persists: https://codepen.io/dansinni/pen/yKOjem

Only the rear, left entity is emitting the audio. If you don't hear any audio, you may need to refresh cache (Ctrl + F5, etc.).

Man, was really excited, too.

[dmarcos]

Is the issue still open in THREE then?

[dsinni]

I'm not entirely sure which issue it is in THREE, but the ones I have seen still appear to be open. @meatwallace 's patch appears to be awaiting merge. :/

mrdoob/three.js#10706
mrdoob/three.js#13710

[dsinni]

I just wanted to say that I bit the bullet and employed the changes outlined by @meatwallace and I can confirm that they are working with no noticeable side effects as of yet. A credit to you, sir. Thank you. I'm just trying to be patient as I wait for this to be merged in THREE and A-Frame so we can all move on with our lives. :)

[meatwallace]

looks like it's been pushed out to R94.

i'm pretty naive in this space so I may be incorrect, but I believe my fix introduces potentially non-trivial memory overhead if your app is loading a large audio asset in multiple places at the same time. it's a naive work around to the the loader 'race condition' described in this issue, which really needs to be fixed by rearchitecting the entire loader, coming with it's own set of considerations and is well beyond my capacity right now.

@dsinni - it might be worth profiling your app's memory consumption when your reused assets are loaded to see if it's significant in your use case.

[dsinni]

Looks like this is working in master now after bumping three to r94, so that's promising! Looking forward to seeing in an official release.

@meatwallace, I hadn't noticed any crippling side effects. Honestly, I'd rather have the the ability to reuse the audio at the cost of performance than not have the option; it's just too important of a feature to not have it.

Thanks to everyone for getting this closer to resolved.

[dmarcos]

Nice! Glad to hear that it got fixed in THREE r94.