Fix muting behavior with the MediaElement backend #1966
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thijstriemstra
suggested changes
Jun 15, 2020
|
Done! |
thijstriemstra
approved these changes
Jun 15, 2020
|
hey folks, this PR broke a few tests (that were previously hidden behind some eslint issues). I'm not sure if the tests are incorrect or if the code is, but maybe someone wants to look? |
sandiz
pushed a commit
to sandiz/wavesurfer.js
that referenced
this pull request
Sep 1, 2021
* Fix muting behavior with the MediaElement backend * Document changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short description of changes:
wavesurfer.jscurrently handles muting the media by setting the backend's volume to 0 and remembering the previous value, but it also checks whether the backend maintains muting state and uses it to override its own. This is currently used by theMediaElementto make wavesurfer aware of muting/volume changes performed directly on theAudioelement, but muting throughwavesurfer.jsitself triggers a backend volume change without changing the backend'sisMutedstate, so the end result is the volume switching to 0 and the media being unmuted, leading to inconsistent and erratic behavior.This PR adds an optional
setMutemethod to backends so that theMediaElementbackend can handle muting by itself.Breaking in the external API:
None.
Breaking changes in the internal API:
None (the
setMutemethod is optional so that other backends do not need change—but if we are ok with breaking the internal API by requiring thesetMutemethod, we can move thewavesurfer.jscode to the WebAudio backend).Related Issues and other PRs:
I believe the “immediately unmuting” behavior was introduced by #1635. This PR fixes it while still handling external changes to the
Audioelement state.refs #1931
refs #1703