Repository navigation
fix(core): keep long WebKit sources and unloaded clips on the audio clock - #7225
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · head d548e356
- blocker: the body has no
fails without the fix:line. "The 10 minute and the unloaded-source tests fail on main" (under Tests) doesn't name the tests. Add a line such asfails without the fix: "bare audio in WebKit > hands a 10 minute source to a decoded buffer" and "... > gives a clip that has not loaded its source's verdict", with what each one reports on main.
The code looks right to me. The clip with no duration yet uses the same decodeKey as _decoding and _bufferCache. A failed decode reports false, so its clips stay on their elements. The 600 s cap comes with its memory cost in the body.
Scope
I read all 3 files, and decodeAudioElement, fetchAndDecode and the cache pruning in webAudioTransport.ts. When I reviewed, Comments had passed and CI, the Safari player jobs and regression were still running.
— Genesis lane B
|
Blocker: the body now has the CI: Test (runtime) and Test (studio) on the last run ended cancelled after about 8 minutes, not failed; I reran them. Earlier reds were |
There was a problem hiding this comment.
Verdict: APPROVE — 1 resolved, 0 open, 0 new · d548e356 (same head, body edited)
✅ The body now has a fails without the fix: line. It names both WebKit tests and the assertion each one fails on main. Both tests match the change: 600 s is over main's 300 s limit, and main reads a NaN duration as "too long". The code is unchanged since my last review, and CI is green.
— Genesis lane B
* chore: release v0.9.16 * chore: format v0.9.16 release notes * chore: add heygen-com#7228 and heygen-com#7231 to v0.9.16 notes * chore: add heygen-com#7217 heygen-com#7224 heygen-com#7225 heygen-com#7226 heygen-com#7229 heygen-com#7233 to v0.9.16 notes
…eview Adopts or partially adopts 11 groups: multi-worker tween replay behind the timeline stand-in (heygen-com#7231), template entries in the shared compile stage (heygen-com#7217), HYPERFRAMES_CACHE_DIR and HYPERFRAMES_CONFIG_DIR for every per-user cache and settings file (heygen-com#7224, heygen-com#7229; HeyGen credentials stay under HEYGEN_CONFIG_DIR), WebKit audio clock (heygen-com#7225), packet-based video length (heygen-com#7233), HLG tone mapping (heygen-com#7234), fetch network cause (heygen-com#7236), .hyperframesignore in lint (heygen-com#7239), info --media (heygen-com#7235), lint priority (heygen-com#7244), check --budget (heygen-com#7245) and the unknown-command answer (heygen-com#7247). bun.lock gains only the lint `ignore` edge from heygen-com#7239, already resolved. Watermark advances to 0a6746e. Co-authored-by: Jerrai <[email protected]> Co-authored-by: Miguel Ángel <[email protected]> Co-authored-by: WaterrrForever <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
What
On WebKit, narrated cut edits of a long single source played the sound 250 to 350 ms behind the picture from the second cut on. Sound now stays on the audio clock for sources up to 10 minutes, and for every clip of a source whether or not its element has loaded.
Why
decodesFirsthands a clip's sound from its element to a decoded buffer on the context clock, because WebKit starts element audio late. It had two holes:el.durationisNaNfor a clip that has not loaded, andNaN <= 300is false. In a 10-take edit the last three clips never decoded, even under the cap.Now a clip with no duration yet takes the verdict of its source (
isDecodedOrDecoding), and the cap is 600 s (about 230 MB of stereo float PCM). The decode is already one fetch per source and is shared by all clips of it.Measured
System WebKit on macOS, the 10-take page with a 321 s source whose audio is a 1 kHz click on every source second. Clicks are read off the Web Audio graph (AudioWorklet tap), picture time off
requestVideoFrameCallback, the timeline off the player. Runtime built from main and from this branch; positive = sound later. Median per take, takes 2-10; 5 plays of this branch, 2 of main.Take 1 was already near 0 on main.
Not covered
Tests
init.test.ts, "bare audio in WebKit": a 10 minute source decodes, an hour stays on its element, an unloaded clip follows its source's verdict, and an unloaded clip of an undecoded source stays.fails without the fix: "bare audio in WebKit > hands a 10 minute source to a decoded buffer" and "bare audio in WebKit > gives a clip that has not loaded its source's verdict". On main both report
expected "releaseRouted" to be called with arguments: [ <audio> ]withNumber of calls: 0: the sound never leaves its element. The other two tests pass on main and on the branch (they pin the limit and the unchanged case).Size
48 changed lines, one lone fix: the two causes sit in one function (
decodesFirst) and its transport helper, and no open PR touches that path.