Skip to content

Replaced custom hack for preloading audios and fixed #299.#471

Closed
MarkusBordihn wants to merge 3 commits into
RaspberryPiFoundation:masterfrom
MarkusBordihn:patch-4
Closed

Replaced custom hack for preloading audios and fixed #299.#471
MarkusBordihn wants to merge 3 commits into
RaspberryPiFoundation:masterfrom
MarkusBordihn:patch-4

Conversation

@MarkusBordihn

@MarkusBordihn MarkusBordihn commented Jul 3, 2016

Copy link
Copy Markdown
Contributor

For preloading audio files the attribute "preload=auto" should be used.
See: https://developer.mozilla.org/en/docs/Web/HTML/Element/audio

Regenerated all files to include the changes.
Unit tests and workspace tests looks fine without any error in the console.

@MarkusBordihn MarkusBordihn changed the title Replaced custom hack for preloading audios and fixed #299. Replaced custom hack for preloading audios and fixed https://github.com/google/blockly/issues/299. Jul 3, 2016
@MarkusBordihn MarkusBordihn changed the title Replaced custom hack for preloading audios and fixed https://github.com/google/blockly/issues/299. Replaced custom hack for preloading audios and fixed #299. Jul 3, 2016
Comment thread core/workspace_svg.js
if (ext && audioTest.canPlayType('audio/' + ext[1])) {
// Found an audio format we can play.
sound = new window['Audio'](filename);
sound.setAttribute('preload', 'auto');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this work on iOS?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The preload attribute is an official attribute, so it will not break anything.
See https://www.w3.org/TR/html5/embedded-content-0.html#attr-media-preload

The former solution has not really worked on iOS:

// iOS can only process one sound at a time.  Trying to load more than one      
// corrupts the earlier ones.  Just load one and leave the others uncached.     
...

According MDN in most cases the mobile browsers will load the Metadata but not buffer the audio to 100%, which is by design and expected.
That's also the reason why the former solution not really worked on iOS.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change breaks iOS. The existing solution will ensure that the click sound plays, something we want to preserve. I've tried several variations of this PR on iOS but have not yet been able to come up with something that plays any sound.

@MarkusBordihn MarkusBordihn Jul 15, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Out of curiosity, why you are not using the "required" method (Web Audio API) from Apple to load the sound files on iOS to make sure that they are working as expected ?

See:

The current solution is "unsupported" for desktop browsers and will throw the given error message.
I'm not a friend of browser specific solutions, but in this case it seems there is no other way.

@MarkusBordihn

Copy link
Copy Markdown
Contributor Author

I will close this PR for now. Let me know if there is any interest for this solution in the future.

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