Skip to content

Conversation

@luisfonsivevo
Copy link
Contributor

Currently, if the call to context.decodeAudioData fails for whatever reason, AudioLoader.load (and loadAsync by extension) will hang forever because there is no error handling for it. This PR adds the extra error handling for that scenario.

Related (but closed and forgotten?): #20301

Currently, if the call to `context.decodeAudioData` fails for whatever reason, `AudioLoader.load` (and `loadAsync` by extension) will hang forever because there is no error handling for it. This PR adds the extra error handling for that scenario.
@github-actions
Copy link

github-actions bot commented Apr 7, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize Gzipped Diff from dev
627.7 kB 156.1 kB +21 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize Gzipped Diff from dev
419.2 kB 102.1 kB +0 B

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 7, 2023

Related (but closed and forgotten?):

The PR was closed at that time since the change does not play well with LoadingManager. Even if now an error in the decode happens, it does not prevent the loading manager from "completing" the loading process since the itemError() call comes too late.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 7, 2023

However, I changed my mind regarding #20301. No proper loading manager support does not mean we have to ignore the error callback of decodeAudioData().

@Mugen87 Mugen87 added this to the r152 milestone Apr 7, 2023
@mrdoob mrdoob merged commit f342395 into mrdoob:dev Apr 8, 2023
@luisfonsivevo luisfonsivevo deleted the patch-2 branch April 10, 2023 23:09
@luisfonsivevo luisfonsivevo restored the patch-2 branch April 10, 2023 23:09
@luisfonsivevo luisfonsivevo deleted the patch-2 branch April 10, 2023 23:10
@luisfonsivevo luisfonsivevo restored the patch-2 branch April 10, 2023 23:11
@luisfonsivevo luisfonsivevo deleted the patch-2 branch April 10, 2023 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants