Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 52e6da77b135861b49039098e00873cf895b9c18
https://github.com/WebKit/WebKit/commit/52e6da77b135861b49039098e00873cf895b9c18
Author: Chris Dumez <[email protected]>
Date: 2026-08-23 (Sun, 23 Aug 2026)
Changed paths:
M
LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-playbackrate-negative-expected.txt
M
LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated-loop.html
M
LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated.html
M LayoutTests/webaudio/audiobuffersource-negative-playbackrate-loop.html
M LayoutTests/webaudio/audiobuffersource-negative-playbackrate.html
M Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp
Log Message:
-----------
AudioBufferSourceNode ignores the start() offset for a negative playbackRate
https://bugs.webkit.org/show_bug.cgi?id=320870
rdar://184484940
Reviewed by Darin Adler.
adjustGrainParameters() placed the initial playhead at the *end* of the grain
when the playback rate was negative, at offset + duration - 1, rather than at
offset. With no explicit duration, m_grainDuration defaults to the remainder of
the buffer, so the playhead always landed on the last frame of the buffer and
the caller's offset had no effect at all: start(0, 2 / sampleRate) on an
8-frame buffer at rate -1 played 8,7,6,5,4,3,2,1 instead of 3,2,1.
The spec places the playhead at offset regardless of the sign of the rate, then
clamps it only in the direction playback is heading:
if (loop && computedPlaybackRate >= 0 && offset >= actualLoopEnd)
offset = actualLoopEnd;
if (computedPlaybackRate < 0 && loop && offset < actualLoopStart)
offset = actualLoopStart;
https://webaudio.github.io/web-audio-api/#playback-AudioBufferSourceNode
Blink already implements exactly this, and this patch brings us in line with it.
Every change below matches the structure of Blink's
AudioBufferSourceHandler::RenderFromBuffer(), which splits the playhead setup
into an explicit "directional playhead setup" with a forward branch and a
reverse branch.
renderFromBuffer() applied only the forward half of the spec's clamping,
wrapping to loopStart whenever the playhead was at or past virtualMaxFrame. For
a negative rate that is the wrong direction: an offset past loopEnd should
descend into the loop from above, not jump to the loop start. The forward wrap
is now forward-only and the reverse path gets the clamp the spec asks for, the
same split Blink makes between its "Forward Loop Clamping" and "Reverse Loop
Clamping" cases. Because skipping the forward wrap also removes its implicit
end-of-buffer guard, a reverse playhead starting at or past the end of the
buffer now returns false so process() zeroes the output bus instead of leaving
it partially written.
Two smaller bugs in the reverse path fell out of the above. The sub-quantum
startFrameOffset adjustment advanced the playhead forward for both signs of the
rate, moving a reverse playhead away from where it was about to read; it now
subtracts for a negative rate, as Blink's "Reverse Start Time Adjustment" does.
The pitchRate == -1 fast path clamped the read index down to loopEnd, which
defeats the point of not clamping the offset, and now only bounds it by the
buffer length.
The offset is converted with a new timeToFractionalSampleFrame() helper rather
than AudioUtilities::timeToSampleFrame(). The latter rounds to whole frames,
which would quantize away a genuine sub-sample offset; the helper keeps the
sub-sample position but still snaps values that differ from a whole frame only
by floating-point round-off, preserving the existing property that playback of a
whole-frame offset at |rate| == 1 is a straight copy of the PCM data rather than
an interpolation. Blink splits these two cases apart instead, rounding only when
the rate is exactly 1 and the detune is 0 and multiplying raw otherwise; the
helper gets the same result for both without branching on the rate.
This fixes 3 of the 6 failing subtests in the WPT test: an offset inside the
loop, an offset past loopEnd, and sub-sample interpolation backwards. The
remaining 3 are unrelated to the offset handling and stay in the rebaselined
expectation. Two are out-of-bounds offsets, which Blink handles with a reverse
out-of-bounds catch-up loop that renders silence until the playhead descends
into the buffer. The third is a negative loopStart, where we implement the
algorithm's guard and so fall back to looping the entire buffer, while the test
asserts the attribute definition's clamp to [0, loopEnd). The spec contradicted
itself here; the Audio WG resolved in
https://github.com/WebAudio/web-audio-api/issues/2689 to make the algorithm
clamp the loop points before validating them, which is what Blink already does
in UpdateEffectiveLoopPoints(). Both are larger changes than the offset fix and
are left for follow-up.
The prose describing offset in the spec still states the negative-rate condition
backwards, saying an offset *greater* than loopStart is clamped. That is a
transcription error, not a second normative rule; the Audio WG resolved to
correct the prose to match the algorithm in
https://github.com/WebAudio/web-audio-api/issues/2690. This patch follows the
algorithm, as Blink does.
*
LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-playbackrate-negative-expected.txt:
Rebaselined: 3 subtests now pass.
* LayoutTests/webaudio/audiobuffersource-negative-playbackrate.html:
*
LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated.html:
* LayoutTests/webaudio/audiobuffersource-negative-playbackrate-loop.html:
*
LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated-loop.html:
These called start(0) or start(0, loopStart) with a negative rate and depended
on the playhead jumping to the end of the buffer or loop. Under the spec that
starts playback at frame 0 or at loopStart, which immediately runs off the front
and renders silence. Each now passes the explicit offset it relied on
implicitly, so they keep testing what they were written to test rather than
being rebaselined to the new output.
* Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:
(WebCore::timeToFractionalSampleFrame):
(WebCore::AudioBufferSourceNode::renderFromBuffer):
(WebCore::AudioBufferSourceNode::adjustGrainParameters):
Canonical link: https://commits.webkit.org/319674@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
