fix: resume the audio context when a buffer source starts - #1272
Open
giaBaoJS wants to merge 1 commit into
Open
Conversation
AudioBufferSourceNode.start() and AudioBufferQueueSourceNode.start() override AudioScheduledSourceNode.start() without calling markRunningOnSourceStart(), so an AudioContext driven only by a buffer source keeps reporting 'suspended' and its native driver is never resumed. Add the call to both overrides, matching the base class and AudioFileSourceNode.play().
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.
No linked issue: found by reading
AudioScheduledSourceNode.start()againstthe two subclasses that override it.
None.
Introduced changes
#1239moved the implicit "a source started, so the driver is running now"transition out of C++ and into JS. It deleted this block from
AudioScheduledSourceNode::start():and replaced it with
this.context.markRunningOnSourceStart()inAudioScheduledSourceNode.start(). The C++ hook covered every scheduledsource, because they all inherit that
start(). The JS hook does not:AudioBufferSourceNode.start()andAudioBufferQueueSourceNode.start()override it (they take extra
offset/durationarguments) and never callthe base or the hook, so nothing publishes the transition for them.
AudioContext::start()on the C++ side now has no callers at all.Result, on a context that has not been resumed explicitly:
context.statestays'suspended'for the lifetime of the playback, andmarkRunningOnSourceStart()'sresume()never reaches the native driver.context.createOscillator().start()on the same context reports'running',because
OscillatorNodedoes not overridestart().The fix adds the one call to both overrides. It is the same call
Audio/AudioFileSourceNode.play()already makes for exactly this reason, withthe comment
copied from audioscheduledsourcenode.markRunningOnSourceStart()is a no-op on
BaseAudioContext, soOfflineAudioContextis unaffected, andAudioContext's override only acts while the state is'suspended', so analready-running context and a second
start()call both stay no-ops.Tests
tests/context-state-on-source-start.test.tsdrives the realcore/AudioContextover a stubbed JSI context and asserts, for each ofcreateOscillator,createBufferSourceandcreateBufferQueueSource, thatthe state is
'suspended'with noresume()beforestart()and'running'with exactly one
resume()after it.Without the fix, the oscillator case passes and both buffer cases fail with
Expected: "running" / Received: "suspended". RemovingmarkRunningOnSourceStart()fromAudioScheduledSourceNode.start()insteadturns all three red, so the assertions are not vacuous. Full suite: 82 passing.
Checklist
On the blank boxes: there is no issue to link, as noted at the top. No public
API, interface support or old-arch spec surface changes, so the docs, the
coverage table and the Android spec file are all unaffected. The web backend
delegates state entirely to the browser's own
AudioContext, sosrc/web-corehas no equivalent hook to fix.