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

#2176 No audio in Safari #2202

Merged
merged 6 commits into from
Aug 5, 2020
Merged

#2176 No audio in Safari #2202

merged 6 commits into from
Aug 5, 2020

Conversation

shokiami
Copy link
Collaborator

@shokiami shokiami commented Aug 2, 2020

Resolves #2176

This PR solves the lack of audio in Safari. Specifically, it converts several of the audio files from wav (which seems to not work in Safari) to mp3 files as well as ensures that each audio is loaded before being played.

Here is the documentation which mentioned the need to convert the files to mp3:
https://www.reddit.com/r/HTML/comments/9zqncy/play_audio_in_safari_using_the_audioplay_function/

However, there are a few concerns:
This will probably require editing lines 44 and line 53 of _readme_and_license.txt.
Also, due to Safari requiring audio to be loaded (line 78 of AudioEffect.js), there is a sliver of delay every time an audio is played in Safari. However, this is not an issue in Chrome and Firefox, where everything still works perfectly.

@misaugstad
Copy link
Member

Awesome, thanks for fixing this!

it converts several of the audio files from wav (which seems to not work in Safari) to mp

That works! We shouldn't notice any difference in sound between the two, but can you keep the .wav files in the repo since they are the original uncompressed versions? You can use the MP3 everywhere, I'd just like to keep the originals on hand.

This will probably require editing lines 44 and line 53 of _readme_and_license.txt.

Why is that? What edits?

Also, due to Safari requiring audio to be loaded (line 78 of AudioEffect.js), there is a sliver of delay every time an audio is played in Safari.

Does the audio need to be loaded every time it is played? Or would you be able to just load it the first time and then it remains available? And is it possible to load the audio earlier to decrease/remove the delay?

@shokiami
Copy link
Collaborator Author

shokiami commented Aug 2, 2020

@misaugstad I added the original .wav audio files back!

Sorry, I thought that the lines regarding the .wav audio files in _readme_and_license.txt would need to be removed as well, but if we are keeping the files then never mind then.

Unfortunately, I have had no success with loading the audio earlier to remove the slight delay in Safari.

I think this has to do with "preload" being disabled on Safari. However, all sources say that this is the case for iOS
https://stackoverflow.com/questions/12804028/safari-with-audio-tag-not-working
but do not mention macOS.

Also, the solution I arrived upon of loading the audio every time before playing comes from this source
https://stackoverflow.com/questions/9335577/html5-audio-sound-only-plays-once
as the issue detailed aligns exactly with what I was seeing in Safari. However, what is confusing is that this source concerns Chrome, not Safari.

As you can probably tell, this was a very annoying one to figure out. I understand the sources above may not be the best, but I have been really struggling to find documentation on the topic. However, please let me know what you think; I'd be happy to dig around again!

@misaugstad
Copy link
Member

I added the original .wav audio files back!

Thank you!

Unfortunately, I have had no success with loading the audio earlier to remove the slight delay in Safari.

How far back have you tried? Like what if you made a new load audio function that you called first thing after the click to place a label, then the play is called later on? When you say "no success", does this mean that if you put the load any earlier, the play wouldn't work, or that it didn't matter how much earlier you put it, there was always a delay?

@shokiami
Copy link
Collaborator Author

shokiami commented Aug 3, 2020

@misaugstad Just got it to work by coding it so that the audio is loaded when the user selects the label type (way before even placing the label)! To answer your question, before I tried loading the audio in the constructor method AudioEffect, which caused the play to not work, and I also tried loading the audio when the user clicks to place the label, but the difference was very negligible. I hadn't thought about the fact that the user has to select the label type before placing each label.

@misaugstad
Copy link
Member

Nice!! That's what I was hoping for! 😁 one last thing to check: can you test it by clicking on the label type, then waiting a long period of time (2 minutes?) and see if the sound still plays? If it doesn't, I'd like to know what that threshold is. If it still plays then awesome :)

@shokiami
Copy link
Collaborator Author

shokiami commented Aug 3, 2020

@misaugstad Uh oh... it doesn't! From my testing, if one takes longer than 15 seconds to place the label after selecting the label type, the sound does not play. Again, not an issue on other browsers such as Chrome and Firefox.

@misaugstad
Copy link
Member

Yeah that's good enough for me. 15 seconds is reasonable

@misaugstad
Copy link
Member

Well is there a way to check if the audio is loaded or not? Then right before you play it you could check if it has been loaded, and if not you can load it again.

@shokiami
Copy link
Collaborator Author

shokiami commented Aug 3, 2020

@misaugstad From what I'm seeing online, it looks like you can tell when an audio html element is loaded by using event listeners, but I don't think there are any event listeners to check when the element is no longer loaded...

@misaugstad
Copy link
Member

Does the ready state property work?

@shokiami
Copy link
Collaborator Author

shokiami commented Aug 3, 2020

@misaugstad Yeah so I think the readyState property listens for when the element loads and updates accordingly, however, it does not update when the element unloads. The readyState property returns 5 stages of being ready to play 0 (no data) to 4 (has enough data), and from my testing in Safari, I found that when you place a label and it plays the audio, it says that the element was at stage 4, and if you place a label after the 15 seconds threshold so it does not play the audio, it still says the element was at stage 4.

@misaugstad
Copy link
Member

Okay that's unfortunate :( but what you've laid out is definitely good enough for us. Is it ready for review now?

@shokiami
Copy link
Collaborator Author

shokiami commented Aug 3, 2020

@misaugstad Yeah sorry about that :(
I would say it's ready for review!

@misaugstad
Copy link
Member

Great! @ThatOneGoat could you test on Safari, and I can test on the other browsers?

@michaelduan8
Copy link
Collaborator

Everything that Sho described is working for me!

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Works for me and looks good! Just want you to add one comment.

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Great work @shokiami !

@misaugstad misaugstad merged commit c5ae70f into develop Aug 5, 2020
@misaugstad misaugstad deleted the 2176-safari-no-audio branch August 5, 2020 00:40
@misaugstad misaugstad mentioned this pull request Aug 5, 2020
@misaugstad misaugstad mentioned this pull request Sep 17, 2020
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