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

Fixes THREE.Audio.load is deprecated, closes #1472 #1629

Merged
merged 6 commits into from Jul 14, 2016

Conversation

mkungla
Copy link
Contributor

@mkungla mkungla commented Jul 12, 2016

srcChanged
I had to update audio src which caused these warnings.
THREE.Audio: .load has been deprecated. Please use THREE.AudioLoader.

Although this is not big issue I came across another problem that I could not play previous audio again.
so changing src on same entity back to audio which was already played causes

The buffer passed to decodeAudioData contains an unknown content type.x
EncodingError: The given encoding is not supported.

This happens since Three. THREE.XHRLoader.load returns for already played audio cache which is instance of ArrayBuffer, but THREE.context.decodeAudioData expects instance of AudioBuffer

Changes proposed:

  • use THREE.AudioLoader instead of THREE.Audio.load
  • Do not let Tree to cache audio files

@@ -46,7 +47,14 @@ module.exports.Component = registerComponent('sound', {
}

// All sound values set. Load in `src`.
if (srcChanged) { sound.load(data.src); }
if (srcChanged) {
var audioLoader = this.audioLoader || new THREE.AudioLoader();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe initialize this in the init method?

@mkungla
Copy link
Contributor Author

mkungla commented Jul 13, 2016

@dmarcos

Can we maybe initialize this in the init method?

Yes, I added commit which initializes AudioLoader in the init method now.

Why do we need to remove from cache the audio to play it again?

Consider following scene using current aframe.js and that you have some tracks like 1.mp3 and 2.mp3

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1,maximum-scale=1,shrink-to-fit=no,user-scalable=no,minimal-ui">
<script src="https://raw.githubusercontent.com/aframevr/aframe/master/dist/aframe.js"></script>
</head>
<body>
<a-scene>
    <a-entity sound="src: 1.mp3; autoplay: true" position="0 1 0"></a-entity>
</a-scene>
</body>
</html>
  • As you load your scene track 1 will start playing
  • Now modify DOM audio src to src: 2.mp3 and you'll hear second track
  • When you modify DOM back to src: 1.mp3 you get silence and following errors
The buffer passed to decodeAudioData contains an unknown content type.{url}
EncodingError: The given encoding is not supported.

If you now use following aframe.js which has this PR applied

<script src="https://raw.githubusercontent.com/mkungla/aframe/sound-AudioLoader-test/dist/aframe.js"></script>

So removing file from TREE.Cache removes these errors and you can shuffle the tracks without any problems.

It seems to be problem with TREE.js Cache which passes ArrayBuffer to AudioLoader.decodeAudioData while last expects instance of AudioBuffer. I have opend issue at TREE as well about that and keep eye on where it goes. For now I find that using THREE.Cache.remove(data.src); is securest way to ensure that aframe's audio src can modified, setting new sources and setting it also back to already played source.

@@ -9,7 +9,7 @@ var warn = debug('components:sound:warn');
*/
module.exports.Component = registerComponent('sound', {
schema: {
src: { default: '' },
src: { type: 'src' },
on: { default: 'click' },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this default: '' while you're here?

@ngokevin
Copy link
Member

Yeah, can you update the examples and docs as well? Thanks!

@mkungla
Copy link
Contributor Author

mkungla commented Jul 14, 2016

@ngokevin I'll go over all filed issues and collect some additional information before I file new issue about sound component what we talked about. Since this PR improves a lot compared to masters if (srcChanged) { sound.load(data.src); } and it self does not introduce new issues then I find that these other questions should handled separately.

| Property | Description | Default Value |
|----------|-----------------------------------------------------------------------|---------------|
| autoplay | Whether to automatically play sound once set. | false |
| on | An event for the entity to listen to before playing sound. | click |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default value for this is no longer click

@ngokevin ngokevin merged commit 733ab95 into aframevr:master Jul 14, 2016
@mkungla mkungla deleted the sound-AudioLoader branch August 12, 2016 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants