Fix looping sounds that specify a start time and duration

[willeastcott]

Description

A SoundInstance (or sound slot) that has both a start time and a duration set does not loop — it plays a single iteration, fires end and stops.

Root cause

SoundInstance passed this._duration as the third argument to AudioBufferSourceNode.start() even when the instance was looping. Per the Web Audio spec that argument is a hard limit on total playback — the source stops after that many seconds of buffer content, "including any whole or partial loop iterations" — so the source stopped at the end of the first iteration, onended fired, and _onEnded() turned that into an end event plus stop().

This only ever worked because the engine assigns source.loop after start() (the "moved to be after start() because of Chrome bug" comment), and Blink decided whether to honour the grain duration at start() time:

// third_party/blink/renderer/modules/webaudio/audio_buffer_source_handler.cc
// AudioBufferSourceHandler::ClampGrainParameters(), called from StartSource()
if (is_duration_given_ && Loop()) {
  end_time_ = start_time_ + grain_duration_;   // spec behaviour
} else {
  grain_duration_ = ClampTo(grain_duration_, 0.0, buffer_duration - grain_offset_);
}

With loop still false at start(), no end time was scheduled and the duration was effectively discarded. That accident is now gone everywhere:

Engine	Behaviour
Firefox ≥ 84 (Nov 2020)	honours the duration regardless of ordering (bug 1618225) — the original report
Chromium ≤ 149	legacy is_duration_given_ && Loop() gate — engine works by accident
Chromium ≥ 150 (branch 7871, stable 2026-06-30)	handler rewrite applies the limit unconditionally in the render loop — engine breaks

which matches the recent report on the issue that Chrome and Brave now fail too. I bisected the milestone branches to confirm 149 → 150 is the flip.

Fix

1. Only pass the duration to the source for non-looping instances (_startSource()). For a looping instance, loopStart/loopEnd already define the region to repeat, which every browser honours identically.
2. Since a looping source is now started without a duration, disabling loop mid-playback would let it run to the end of the buffer. _stopAtEndOfLoopRegion() schedules a stop at the end of the current iteration so the duration is still respected.
3. Clamp loopEnd instead of wrapping it with a modulo — see below.

Second, independent bug fixed here

this.source.loopEnd = Math.max(this.source.loopStart, capTime(this._startTime + this._duration, buffer.duration));

capTime() is a modulo, so whenever startTime + duration landed on or past a multiple of the buffer duration the wrapped value collapsed onto loopStart. Both the spec and Blink's ResolveLoopPoints() treat loopEnd <= loopStart as "loop the whole buffer". Measured on a 4 s clip, startTime 1 / duration 3 ("loop the last 3 s"), startTime 2 / duration 2 and startTime 3 / duration 2 all looped the entire clip. This one is browser-independent and was simply masked by looping being broken anyway.

Verification

Verified against a real Web Audio implementation using OfflineAudioContext (4 s buffer with a distinct DC level per second, so the rendered samples identify which part of the buffer is playing):

• Reproduced the ordering dependence directly — loop = true before start(0, 1, 2) stops after 2 s and fires onended; set after start() it loops forever.
• With the fix, the loop region renders correctly for startTime/duration of 1/1, 1/2, 1/3, 2/2, 3/2, 0/2 and 0/4, including the three cases that previously fell back to looping the whole buffer.
• Validated _stopAtEndOfLoopRegion() by suspending the render mid-playback: for startTime 1, duration 2, disabling loop at t=3.5 s schedules the stop at exactly 4.0 s and every sampled output value matches the prediction.

ESLint is clean on the changed file.

Notes for reviewers

• The source.loop assignment is still left after start() — unchanged, since that ordering also relates to the volume/gain setup next to it.
• The inverse transition (enabling loop mid-playback on an instance that was started non-looping with a duration) still stops at the duration, because the duration was baked into start(). Fixing that would need the source to be recreated; it is the same root cause but a separate, much rarer case, so it is left alone here.
• Two adjacent pre-existing bugs found while investigating are deliberately not touched: resume() passes the full _duration rather than the remaining duration, so pause/resume extends a non-looping grain; and _updateCurrentTime() derives _currentTime from the absolute buffer offset while the currentTime setter treats the value as relative to _startTime, so currentTime is wrong unless startTime is an integer multiple of duration. Happy to file these separately.
• No unit test added — there is no existing SoundInstance test and covering this needs a real (or offline) AudioContext rather than a mock. Say the word if you'd like a test harness for it.

Fixes #4712

Checklist

• I have read the contributing guidelines
• My code follows the project's coding standards
• This PR focuses on a single change

🤖 Generated with Claude Code

[github-actions]

Build size report

This PR changes the size of the minified bundles.

Bundle	Minified	Gzip	Brotli
playcanvas.min.js	2387.7 KB (+1.0 KB, +0.04%)	613.6 KB (+0.2 KB, +0.04%)	476.1 KB (+0.4 KB, +0.08%)
playcanvas.min.mjs	2385.1 KB (+1.0 KB, +0.04%)	612.4 KB (+0.3 KB, +0.04%)	475.4 KB (+0.0 KB, +0.01%)

[mvaligursky]

Automated PR review by Codex (GPT-5) at exact head eb28263204109ccb92350c76e4d760c7c7eed836.

The primary fix is well targeted: looping sources no longer receive a finite start() duration, and clamping loopEnd avoids the whole-buffer fallback when the previous modulo collapsed the loop points. I found one actionable issue in the new mid-playback loop-disable path.

Verification:

• Reviewed the complete PR diff and surrounding SoundInstance play, resume, pause, pitch, loop, and onended state transitions.
• Reproduced the changed behavior in a real Chromium OfflineAudioContext: the basic start-time/duration loop remained active, but an off/on loop toggle still stopped at the previously scheduled deadline, and a post-schedule pitch change also stopped at the stale deadline.
• git diff --check passed.
• All GitHub build, lint, type, unit-test, docs, example, API, size, and deployment checks are green.

The absence of a real-Web-Audio regression test currently leaves this state-transition case uncovered.