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

Fix audio player volume control on Safari #23187

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

minacle
Copy link
Contributor

@minacle minacle commented Jan 21, 2023

<Audio> is playing audio using AudioContext, not using HTMLAudioElement directly.
Therefore, changing volume and muted of HTMLAudioElement are not working at all.

This PR makes <Audio> to handle volume change correctly by using GainNode for it.

@ClearlyClaire
Copy link
Contributor

Hi! Quickly looking over the change, it seems fine, but I am unable to replicate the issue it aims to fix. Which browser did you encounter the issue with?

@minacle
Copy link
Contributor Author

minacle commented Jan 23, 2023

@ClearlyClaire

Which browser did you encounter the issue with?

I am using Safari 16.2 on macOS 13.1 (22C65). I think the issue will be replicable on iOS or iPadOS, but I did not tested on these OSes.

@minacle
Copy link
Contributor Author

minacle commented Jan 23, 2023

I've verified that existing code works fine in other major browsers.
And so, I have changed the code to make the experience consistent across all browsers.

Now fixes an issue where the effect of Gain may be applied twice (because of volume of HTMLAudioElement)

@ClearlyClaire ClearlyClaire mentioned this pull request Jan 23, 2023
2 tasks
@minacle minacle force-pushed the fix-audio-volume branch 2 times, most recently from 383ed0a to 0763d0b Compare January 23, 2023 17:59
app/javascript/mastodon/features/audio/index.js Outdated Show resolved Hide resolved
app/javascript/mastodon/features/audio/index.js Outdated Show resolved Hide resolved
app/javascript/mastodon/features/audio/index.js Outdated Show resolved Hide resolved
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for your contribution!

@ClearlyClaire ClearlyClaire merged commit 3cf60ba into mastodon:main Jan 24, 2023
minacle referenced this pull request in uri-life/mastodon Jan 24, 2023
@minacle minacle changed the title Fix the volume of audio media could not be adjusted Fix audio player volume control on Safari Jan 25, 2023
btrd pushed a commit to btrd/mastodon that referenced this pull request Feb 22, 2023
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