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

Safari Never Auto-Play setting causes long delay ending in fileerror with no information. #326

Open
curiousdustin opened this issue Feb 16, 2021 · 20 comments
Labels

Comments

@curiousdustin
Copy link

Issue Details

  • Version used: 1.0.0

  • Describe whats happening:
    If desktop Safari is set to Never Auto-Play, after registering a sound, there is a several second delay, and then a fileerror event. The event doesn't contain any info to determine that this setting was the cause.

Example

<!DOCTYPE html>

<html lang="en">
  <head>
    <meta charset="utf-8" />
    <title>Sound JS Test</title>
    <script src="https://code.createjs.com/1.0.0/soundjs.min.js"></script>
  </head>

  <body onload="setup()">
    <script>
      function setup() {
        console.log("setup");
        createjs.Sound.registerPlugins([createjs.HTMLAudioPlugin]);
      }

      function playSound() {
        console.log("playSound");
        
        createjs.Sound.on("fileload", function (result) {
          console.log("fileload", result);
          createjs.Sound.play("tone");
        });
        
        createjs.Sound.on("fileerror", function (error) {
          console.log("fileerror", error);
        });
        
        createjs.Sound.registerSound("audio/tone.mp3", "tone");
      }
    </script>

    <button onclick="playSound()">Play Sound</button>
  </body>
</html>

Resulting logs:

[Log] setup (sound-js, line 13)
[Log] playSound (sound-js, line 18)
[Log] fileerror (sound-js, line 24)

Event
  bubbles: false
  cancelable: false
  currentTarget: function()
  data: 100
  defaultPrevented: false
  eventPhase: 2
  id: "tone"
  immediatePropagationStopped: false
  propagationStopped: false
  removed: false
  sprite: undefined
  src: "audio/tone.mp3" 
  target: function()
  timeStamp: 1613432337722 
  type: "fileerror"
  Event Prototype
  • OS & Browser version
    Safari Version 14.0.3 (16610.4.3.1.4)

  • Do you know of any workarounds?
    Interestingly, if createjs.Sound.play("tone"); is called within the fileerror callback, the sounds still plays?! However, this is a ridiculous hack which breaks any other attempts at actually handling errors.

How can I react and or work around the Never Auto-Play setting in desktop Safari?

@danzen
Copy link
Contributor

danzen commented Feb 16, 2021

Will take a look.. I have not used registerSound directly... as I use PreloadJS for sounds - maybe PreloadJS uses registerSound in the background. Anyway - as a temporary fix, perhaps try PreloadJS for the sounds? It would be interesting to know if that works for you - as it is working for us. Or, I would recommend that you contact Apple and have them fix their sound.... okay... hahaha - teasing. Will do a little digging and if anyone else reading this has a thought, please pitch in. @curiousdustin - fyi, this is early in a community based help movement called loosely the CreateJS Consortium ;-). We have just been given access to make changes and are carefully approaching issues.

@curiousdustin
Copy link
Author

Thank you for the suggestion, however, I do not wish to preload sounds as my use case is streaming longer audio files. Which is also why I am using HTMLAudioPlugin.

@danzen
Copy link
Contributor

danzen commented Feb 16, 2021

@curiousdustin - just tested your code and it works on my Macbook AIR OS Sierra (10.12.6) with Safari 10.1.2. Is there a way that you might test on another computer and see how it does there? Also, try another sound just to make sure. I will wait to hear back and then could try upgrading my Mac. Cheers.

@curiousdustin
Copy link
Author

Per original description I see this issue in: Safari Version 14.0.3 (16610.4.3.1.4)

@danzen
Copy link
Contributor

danzen commented Feb 16, 2021

Yes - I know - I am on an older version. Do you have another computer to check on - or with a colleague. And, have you tried a different sound with your set-up. Or perhaps test my file here: https://zimjs.com/test/soundsafari.html Does it work in Safari for you? If not, I guess, I will update my Mac and test further.

@curiousdustin
Copy link
Author

Trying your test in Safari 14 results in the same error I get on my test.

No sound is played, and fileerror is reported after several second delay.

image

@danzen
Copy link
Contributor

danzen commented Feb 17, 2021

Okay - well that is seeming like there might have been a change in how Safari is handling things. I just hesitated to update my Safari because knowing Apple... I will probably have to update my OS... and that will mean I have to replace my computer. But... I will go update it and let you know what happens and then start digging - probably something like they stopped supporting HTML Audio or whatever the plugin works with. Wish me luck.

@danzen
Copy link
Contributor

danzen commented Feb 17, 2021

Torture! 25 minutes of constantly being thwarted as I try and update Safari - I think I have to update the OS. So I look for Catalina and can't figure out how to get it... it is not in the app store, it is not in the system updates, etc. I think it is because they are toting Big Sur how... so gave up and am downloading Big Sur...

@danzen
Copy link
Contributor

danzen commented Feb 17, 2021

@curiousdustin - okay - went to Big Sur with Safari 14.03 - it plays but I realize now, I forgot to check the Never Auto-Play option. Sigh... would have been nice to know if the older Safari had the same issue. Anyway - yes... with Never Auto-Play I get no sound playing after pressing the button. Looking into it.

@danzen
Copy link
Contributor

danzen commented Feb 23, 2021

@curiousdustin Hi Dustin - could you try using this version of CreateJS for your test and see if it works for you: zimjs.com/cdn/1.3.2/createjs_doc_sound.js

The CreateJS SoundJS is complex - like 7 levels of abstraction with register(), preload(), load(), etc. happening in many of them... it seems to all go back through Sound, HMTLAudioPlugin, AbstractPlugin, AbstractMediaLoader, AbstractLoader, SoundLoader to Loader which is really complex.

So... instead of going to the root of it, we are testing for Safari and HTMLAudioPlugin and polling if the sound is ready based on if the playState != "playFailed". When it is ready, it plays... but we want to dispatch an event. For some reason it once it plays, we can't play it again right away consistently... a few times my mac says it is playing but I can't hear it and it only resets when I restart - ARRRGGG Apple. So anyway... at the moment it has a timeout of 150 ms before playing (after it was ready to play) - it would be nice to remove that and we should be able to - but just for now, is it working for you with the changes in the zimjs.com/cdn/1.3.2/createjs_doc_sound.js file?

Here is our test file https://zimjs.com/test/soundsafari.html

@curiousdustin
Copy link
Author

@danzen, that does work in my simple test. I will try to test it in my main app soon, and let you know.

@curiousdustin
Copy link
Author

Actually, how can I compare the version you provided with the official version I was using? They have different formatting, so I can't just do a diff compare.

@curiousdustin
Copy link
Author

I guess what I am getting at... is this new version likely to break things in other browsers? Are the changes fairly minimal? I want to view the changes so I can understand the possible consequences.

@danzen
Copy link
Contributor

danzen commented Feb 23, 2021

Understandable - I am just getting used to CreateJS on GitHub here... so there is probably a way that I can set up a fork to try with existing code and merge it in once tested... I am just new at all that stuff. So for now, here is what I added to the Sound class under registerSound()

// this replaces:
// loader.on("complete", this._handleLoadComplete, this);
// loader.on("error", this._handleLoadError, this);

var pollSound = false;
if (s.activePlugin.toString()=="[HTMLAudioPlugin]") {
    var ua = navigator.userAgent.toLowerCase(); 
    if (ua.indexOf('safari') != -1) { 
        if (ua.indexOf('chrome') == -1) {
            pollSound = true;
        }
    }
}
if (pollSound) {    
    // poll here for Safari sound and call complete or error
    var tries = 0;
    var interval = 50;
    var num = (loadItem.loadTimeout||8000)/interval;
    // events expect a target.getItem() that has a src property of the sound
    var event = {target:{getItem:function(){return loadItem;}}};
    
    function testSound() {
        tries++;
        var cjss=createjs.Sound.play(loadItem.id, {volume:0});        
        if (cjss && cjss.playState != "playFailed") {
            clearInterval(inter);
            cjss.stop();
            // the sound actually plays here if we let it... but want to dispatch event
            // can't seem to replay the sound right away on dispatch - what the heck apple?
            setTimeout(function () {                
                s._handleLoadComplete(event);  
            }, 150); // 50 won't work - sigh...                                                                   
        } else {
            if (tries > num) this._handleLoadError(event);
        }                    
    }
    var inter = setInterval(testSound,interval);
    testSound(); // also test right away     
         
} else { // original event code
    
    loader.on("complete", this._handleLoadComplete, this);
    loader.on("error", this._handleLoadError, this);
    
}	

@curiousdustin
Copy link
Author

ok, so... that DOES work in the simple test playing a simple sound.

However, my use case is to stream larger audio files such as podcasts and such.

When I try within my actual project with actual audio files, this fails.

If I increase the 150 timeout to around 300 it works again. This makes me very think that this delay would be different from machine to machine and audio file to audio file...

I am reluctant to call this a solution.

@curiousdustin
Copy link
Author

curiousdustin commented Feb 23, 2021

Another thing about this fix is it appears to cause issues with the removeSound(id) method.

        p.removeSound = function (src) {
		if (!this._soundInstances[src]) { return; }
		for (var i = this._soundInstances[src].length; i--; ) {
			var item = this._soundInstances[src][i];
                         // Error! item is sometimes null
                        item.destroy();
		}
		delete(this._soundInstances[src]);
		delete(this._audioSources[src]);
		if(this._loaders[src]) { this._loaders[src].destroy(); }
		delete(this._loaders[src]);
	};

@danzen
Copy link
Contributor

danzen commented Mar 1, 2021

Yes, @curiousdustin, that is a suspicious timeout... it is strange because the sound actually plays once (cjss && cjss.playState != "playFailed") is true (you can hear it if the volume is set to 1) and needs to be stopped so the sound can be played when desired in the app code. But then it can't seem to be played again right away.

As a temporary fix we have set a delay for Safari with HTMLAudioPlugin to 300ms as default and provide a parameter for it in the registerSound(). If anyone comes up with a better solution, we are all ears. We considered moving the test to the play() as when the poll says it is ready, it does play... but think the delay in the initial complete event is better than a delay when the play is called for most situations.

In terms of error on stopping sound, we have added a test for null entries. Each of the polled sounds is being added to the _soundInstances for that source. So now the few null sound instances from polling are ignored.

Our test file is here https://zimjs.com/test/soundsafari2.html

Here are the changes - have a look and if all seems... okay... then we can look into updating the files here.

s.registerSound = function (src, id, data, basePath, defaultPlayProps, delay) {
    var loadItem = {src: src, id: id, data:data, defaultPlayProps:defaultPlayProps};

    // Polling for Safari due to HTMLAudioPlugin sound not playing with "Never Auto-Play"
    // see https://github.com/CreateJS/SoundJS/issues/326
    // Dan Zen - Feb 23, 2021
    if (delay==null && s.activePlugin.toString()=="[HTMLAudioPlugin]") {
        var ua = navigator.userAgent.toLowerCase(); 
        if (ua.indexOf('safari') != -1) { 
            if (ua.indexOf('chrome') == -1) {
                delay = 300; // safari
            }
        }
    }

    if (src instanceof Object && src.src) {
        basePath = id;
        loadItem = src;
    }
    loadItem = createjs.LoadItem.create(loadItem);
    loadItem.path = basePath;

    if (basePath != null && !(loadItem.src instanceof Object)) {loadItem.src = basePath + loadItem.src;}

    var loader = s._registerSound(loadItem);
    if(!loader) {return false;}

    if (!s._preloadHash[loadItem.src]) {s._preloadHash[loadItem.src] = [];}
    s._preloadHash[loadItem.src].push(loadItem);
    if (s._preloadHash[loadItem.src].length == 1) {
        // OJR note this will disallow reloading a sound if loading fails or the source changes

        // Dan Zen - HTMLAudioPlugin Safari - or delay set
        if (delay > 0) {    
            // poll here for Safari sound and call complete or error
            var tries = 0;
            var interval = 50;
            var num = (loadItem.loadTimeout||8000)/interval;
            // events expect a target.getItem() that has a src property of the sound
            var event = {target:{getItem:function(){return loadItem;}}};

            function testSound() {
                tries++;
                var cjss=createjs.Sound.play(loadItem.id, {volume:0});        
                if (cjss && cjss.playState != "playFailed") {
                    clearInterval(inter);
                    cjss.stop();
                    // the sound actually plays here if we let it... but want to dispatch event
                    // can't seem to replay the sound right away on dispatch
                    setTimeout(function () {                
                        s._handleLoadComplete(event);  
                    }, delay); // default 300 for Safari with HTMLAudioPlugin                                                                 
                } else {
                    if (tries > num) this._handleLoadError(event);
                }                    
            }
            var inter = setInterval(testSound,interval);
            testSound(); // also test right away     

        } else { // original event code

            loader.on("complete", this._handleLoadComplete, this);
            loader.on("error", this._handleLoadError, this);

        }			
        s.activePlugin.preload(loader);              
    } else {
        if (s._preloadHash[loadItem.src][0] == true) {return true;}
    }

    return loadItem;
};

and then the removeSound:

p.removeSound = function (src) {
	if (!this._soundInstances[src]) { return; }
	for (var i = this._soundInstances[src].length; i--; ) {
		var item = this._soundInstances[src][i];
		if (item) item.destroy(); // Dan Zen added conditional for null polled items stored - Mar 1 2021
	}
	delete(this._soundInstances[src]);
	delete(this._audioSources[src]);
	if(this._loaders[src]) { this._loaders[src].destroy(); }
	delete(this._loaders[src]);
};

@danzen
Copy link
Contributor

danzen commented Mar 6, 2021

@curiousdustin, did you get a chance to try these out... any thoughts?

@curiousdustin
Copy link
Author

After some testing, it works, but... it doesn't.

The delay needed to make it work seems to be different for every file I try.

And then, how do I know the delay will work on various machines? Will a poor performance machine require a longer delay?

Just seems like an unusable solution to me. I could use some insane delay like 2000ms, and maybe it would be... fine. But at the same time punishing all my users who have not selected this Never Auto Play setting?

A convoluted alternative I came up with, but still not happy with is based on your solution:

  • Instead of a delay, add an argument such as autoPlayOnSafari
  • In testSound, if this argument is true, let the sound play instead of stopping it.

Issues:

  • Since, registerSound isn't really meant to play a sound, the play in testSound doesn't automatically get the various props that would normally come from the play call.
  • I also need the instance of the sound, so that would need to be passed in the fileload event.

Like I said, convoluted, and I don't like it either...

Maybe there is some sort of approach related to this idea, allowing the sound to "auto play" on load?

@danzen
Copy link
Contributor

danzen commented Mar 27, 2021

Hey @curiousdustin sorry for the delay - missed the response. I certainly see what you mean and am never happy with a polling solution especially when there are users that will not need it and we can't seem to tell. Somehow I only seem to get in these binds with Apple.

What is frustrating is that the sound plays in the load test. We can capture that it plays. Yet when we stop it and try and play it again it will not play. Arrrg.

Your idea would at least get a sound playing. What would happen if we just paused the sound? Perhaps we can pass in a config object https://createjs.com/docs/soundjs/classes/PlayPropsConfig.html for the autoPlayOnSafari.

Anyway - will await your thoughts then see if we can code something. If you have a start, you are welcome to send it along. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants