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

Properly resume AudioContext on iOS platform. #3499

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

jpauloruschel
Copy link
Contributor

@jpauloruschel jpauloruschel commented Sep 20, 2021

Fixes #3212

This PR makes sure that the AudioContext is properly resumed on the iOS platform when context is switched by using the AudioContext#resume API. It returns a future, so the callback is only triggered once AudioContext is actually resumed. For other platforms, or if AudioContext is not being used, the logic is unchanged.

According to https://developer.mozilla.org/en-US/docs/Web/API/BaseAudioContext/state , iOS Safari sets the state to interrupted.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

Comment on lines 84 to 87
const resumeFunction = function () {
this.suspended = false;
this.fire('resume');
}.bind(this);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const resumeFunction = function () {
this.suspended = false;
this.fire('resume');
}.bind(this);
const resumeFunction = () => {
this.suspended = false;
this.fire('resume');
}

Comment on lines 84 to 87
const resumeFunction = function () {
this.suspended = false;
this.fire('resume');
}.bind(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const resumeFunction = function () {
this.suspended = false;
this.fire('resume');
}.bind(this);
const resumeFunction = () => {
this.suspended = false;
this.fire('resume');
};

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming the above code suggestion works OK. But using the arrow function should avoid the need to use bind.

Copy link
Member

Choose a reason for hiding this comment

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

Beat ya! :D

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, except you deleted the trailing semi-colon, @slimbuck! 😆

@willeastcott
Copy link
Contributor

I have a minor concern here. There another bug you might want to check out: #2383
So it seems that pc.platform.ios returns false on iPad at the moment? So your fix here might now work as expected on iPad.

@jpauloruschel
Copy link
Contributor Author

I have a minor concern here. There another bug you might want to check out: #2383
So it seems that pc.platform.ios returns false on iPad at the moment? So your fix here might now work as expected on iPad.

Calling AudioContext.resume() works on all platforms - it is only the case that on iOS it is required. To be safe, I can remove the platform.ios entirely and always call AudioContext.resume when it is being used. What do you think?

@willeastcott
Copy link
Contributor

Oh interesting. Yeah, that sounds like a good solution to me.

@jpauloruschel jpauloruschel merged commit 33eeee2 into master Sep 30, 2021
@jpauloruschel jpauloruschel deleted the jpauloruschel-ios-audio-context branch September 30, 2021 12:08
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.

iOS audio context suspended on visibility change
4 participants