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

Audio subsystem fails if the first type it tries doesn't work #153

Closed
sporksmith opened this issue Jan 24, 2013 · 9 comments
Closed

Audio subsystem fails if the first type it tries doesn't work #153

sporksmith opened this issue Jan 24, 2013 · 9 comments
Milestone

Comments

@sporksmith
Copy link

The short version: audio.canPlayType in Chromium on Ubuntu returns 'maybe' for 'mp3', and 'probably' for other extensions. If 'mp3' was a format given to me.audio.init, we get an error when trying to load the mp3, and then audio completely falls over. If we fell back to the other supported audio types it would work.

More specifically, I ran into this when going through the tutorial (using tutorial_final from the git repo, modified to use an un-minified build of the current melonJS)

Uncaught melonJS: failed loading stomp.mp3 melonJS-0.9.5.js:6833

I can get through the tutorial by changing the call to me.audio.init to pass only 'ogg', and not 'mp3'. But, to further investigate, I added some logging to detectCapabilities:

        obj.detectCapabilities = function () {
            // init some audio variables
            var a = document.createElement('audio');
            if (a.canPlayType) {
                for (var c in obj.capabilities) {
                    var canPlay = a.canPlayType(obj.capabilities[c]);
                    console.log('capability', c, canPlay);
                    // convert the string to a boolean
                    obj.capabilities[c] = (canPlay !== "" && canPlay !== "no");
                    // enable sound if any of the audio format is supported
                    me.sys.sound |= obj.capabilities[c]; 
                }
            }

Which gives this output on the console:

capability mp3 maybe melonJS-0.9.5.js:6951
capability ogg probably melonJS-0.9.5.js:6951
capability m4a probably melonJS-0.9.5.js:6951
capability wav probably

A couple reasonable solutions I see are to 1) modify getSupportedAudioFormat and/or obj.detectCapabilities to prioritize formats reported as 'probably' over those reported as 'maybe', and/or 2) modify soundLoadError to try other (possibly) supported formats if the preferred one fails to load.

@melonjs
Copy link
Collaborator

melonjs commented Jan 25, 2013

Thank you for the feedback,

the way I see how to address this is through the audio init string
me.audio.init("mp3,ogg");

as actually melonJS will try to initialize the audio using the specify format, using the specified priority (if you rather specify "ogg,mp3" melonJS will first try with ogg and then mp3), which might not fully address your point, but might help with it.

also modifying soundLoadError to revert to another audio format, is yes probably a good idea ;)

@sporksmith
Copy link
Author

the way I see how to address this is through the audio init string
me.audio.init("mp3,ogg");

I agree that'd be another improvement if the order of that string affected the priority, rather than the priority being hard-coded inside melonjs. It doesn't fix this issue in the general case though. Since e.g. a different browser might respond maybe/probably to both mp3 and ogg and then fail on the ogg file, so there'd be no one audio init string that worked for all browsers.

also modifying soundLoadError to revert to another audio format, is yes probably a good idea ;)

Yeah, that seems like the most robust way to address this issue (though it might also be the trickiest to implement cleanly). Affecting the format priority through the order of the init string as you suggest, or by trying the 'probably's before the 'maybe's as I suggested initially both might be sensible things to do, but even then you could still have this problem that for some browser the first format that does actually ends up getting tried doesn't work.

@melonjs
Copy link
Collaborator

melonjs commented Jan 25, 2013

@sporksmith
actually the audio init string priority handling was supposed to be there already, but by just review the audio code, it seems that we kind of lost it through some past updates ! I will see to add it back :)

melonjs pushed a commit that referenced this issue Jan 25, 2013
… order of requested audio format was used as a priority (see #153)
@melonjs
Copy link
Collaborator

melonjs commented Jan 25, 2013

@sporksmith

I just added back the lost feature, where the order specified in the initialization string is used to define the format detection priority.

Could you pull the last version and give it a try ?

@sporksmith
Copy link
Author

Pulled the latest. Yes, changing the init string from "mp3, ogg" to "ogg, mp3" now causes it to try ogg first and not fall over for this browser. Again though, I don't think this really addresses the underlying issue since for any given game there'll still be a fixed priority that will be tried for all browsers, and things will go badly if the first format tried doesn't work.

@melonjs
Copy link
Collaborator

melonjs commented Jan 26, 2013

@sporkmisth
Fully agree, but my intention here was just to restore the missing/lost feature.

For the next step an easy way is probably to as well store the CanPlayType and do one first loop for the "probably" type, and if nothing found, a second one for the rest.

melonjs pushed a commit that referenced this issue Jan 27, 2013
…c support type, and prioritize codec being `probaby` supported
@melonjs
Copy link
Collaborator

melonjs commented Jan 27, 2013

@sporksmith

this last commit should do the trick, let me know what you think :)
thank you again for the feedback, that was a good idea !

melonjs pushed a commit that referenced this issue Jan 28, 2013
the test on 'probably' in the second loop can safely be removed, since
anyway we will stop at the first one if any of the codec can 'probably'
be played
@melonjs
Copy link
Collaborator

melonjs commented Jan 29, 2013

closing this one, as I don't see any further issue following the last change

@melonjs melonjs closed this as completed Jan 29, 2013
@sporksmith
Copy link
Author

Haven't had a chance to test it, but yeah this looks reasonable. Thanks!

Just to play devil's advocate though, if some browser returns 'probably' for, say, 'wav' and 'maybe' for everything else, it'd probably be preferred to actually try loading the compressed files before falling back to 'wav'. I realize that's a messier change to make though, so possibly not worth worrying about until when/if it actually comes up.

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

No branches or pull requests

1 participant