webtools: record why the AudioContext resume is needed, with the measurement - #83
Merged
Merged
Conversation
…urement The resume itself landed in EnvelopSound#64. This adds the reasoning above it, so the next reader does not have to rediscover it: video.js builds the AudioContext at module-import time, before any user gesture, so an autoplay-policy browser hands it back suspended and the whole graph is stopped. Unmuting the element and raising a gain then change nothing audible, which is exactly what a user dragging the slider experiences. Measured on a live 16-channel stream, each step added alone (RMS into ctx.destination): gain up 0.0, plus element unmuted 0.0, plus resume() 5.6e-4. Only the resume makes it audible. Without this note a bare resume() call reads as defensive boilerplate and invites removal.
There was a problem hiding this comment.
Pull request overview
Adds an explanatory comment in GainSliderBox.setGain() documenting why audioContext.resume() is required when the AudioContext is created before a user gesture (autoplay policy), including a brief measurement showing the audible effect.
Changes:
- Document the autoplay-policy “suspended” AudioContext root cause in
setGain - Record measured RMS impact demonstrating that
resume()is the step that restores audibility - Clarify why
setGain(user gesture path) is the appropriate place to resume the context
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Video.js constructs the AudioContext at module-import time, which is | ||
| // before any user gesture, so browsers with an autoplay policy hand it back | ||
| // "suspended" and the whole graph is stopped. Unmuting the element and | ||
| // raising a gain then change nothing audible, which is exactly what a user |
'then change' put the adverb between a compound subject and its verb, which reads as a disagreement even though it is not one.
Collaborator
Author
|
The subject there is compound ("unmuting the element and raising a gain"), so the plural verb agrees and "changes" would be the error. It does read badly though, because |
mormegil6
added a commit
to mormegil6/Earshot
that referenced
this pull request
Aug 22, 2026
Both PRs were cut from this fork, so their changes arrive as conflicts against the local copies they were taken from. Upstream's side wins in all three: it is the same change plus the content-type line that review added. The deployment-only patches in these files are untouched.
This file contains hidden or 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
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.
The
audioContext.resume()insetGainlanded in #64. This adds the reasoning directly above it.video.js constructs the AudioContext at module-import time, before any user gesture, so a browser enforcing an autoplay policy hands it back
suspendedand the whole graph is stopped. Unmuting the element and raising a gain then change nothing audible, which is exactly what a user dragging the slider experiences: no sound and no clue why.Measured on a live 16-channel stream, each step added alone (RMS into
ctx.destination):resume()Only the resume makes it audible.
Comment only, no behaviour change. The reason for sending it: a bare
resume()with no explanation reads as defensive boilerplate and invites removal, and the next person to hit this would have to measure it again.