Repository navigation
fix: caption timing in trimmed clips, in-flight resolve dedup, idempotency keys - #226
Merged
Merged
Conversation
…tency keys
Three fixes from the ep5 production incident:
1. Captions in trimmed clips (double lines + timing drift):
- flatten.ts: HoistedCaption carries the leaf clip's trim window
(cutFrom/cutTo/duration)
- merge-ass.ts: transformCue — single shift/drop/clamp primitive.
Effective shift = clipStartOffset - cutFrom (was + offset only, so
captions lagged by cutFrom). Cues outside [cutFrom, cutTo] are
dropped; partially-outside cues are clamped, so a whisper tail can
no longer bleed over the next clip's captions (the double-line bug).
Both the single-caption path and mergeAssFiles use it; emoji
overlays get the same treatment; withAudio caption tracks now carry
cutFrom/cutTo into the mix. Clips without trims behave exactly as
before (covered by tests).
2. In-flight resolve dedup (52 jobs for one image):
- memoizedElementResolve in resolve.ts, WeakMap-keyed by element
identity, wraps all standalone resolvers (image/video/speech/music/
talking-head/audio). Graph invariant: one element resolves at most
once no matter how many concurrent paths lead to it. Previously the
nested path (async component -> speechRange -> resolveVideoElement
-> resolveImageElement) bypassed both makeThenable memoization and
the renderers' pendingFiles dedup, so N components sharing one
dependency image fired N parallel generations. Meta write-back keeps
renderers/executePlan/compile seeing resolved elements; rejected
promises stay memoized (no silent re-spend on re-await).
3. Idempotency-Key on job submission (retry storm multiplier):
- varg.ts submitJob sends a stable Idempotency-Key (hash of
capability + canonicalized params), so 429/network retries reference
the same job instead of creating duplicates. Verified live: the API
stores the key on created jobs.
Also: named element type exports (VideoElement/SpeechElement/
ImageElement), key prop in JSX IntrinsicAttributes, and the native-audio
target-API example that now typechecks against them.
Verified against the real API with a cheap flux-schnell reproducer of
the ep5 dependency shape: 4 unique requests -> exactly 4 jobs, shared
nested image generated once, zero rate-limit warnings.
Refs #225
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Whisper word timestamps are not guaranteed monotonic — in ep5 the word "can't" started at 3.18s while the previous words ended at 3.56/3.72s. Cue generation trusted these timings, producing overlapping Dialogue events that ASS renders stacked: two caption lines on screen at once INSIDE a single clip (distinct from the cross-clip overlap fixed by transformCue — that one was at clip boundaries, this one is anywhere whisper emits a non-monotonic timestamp). monotonizeEntries() clamps each entry's start to the previous entry's end (and end >= start) before grouping, applied in both convertSrtToAss and convertSrtToAssGrouped. Already-monotonic input passes through unchanged. Verified on ep5 (cached re-render): 221 cues, zero overlapping pairs including intra-clip (previous check only counted cross-clip pairs); frame at 00:03 now shows a single caption line.
Whisper is a seq2seq transcriber, not a frame-level speech detector: its word timestamps come from attention alignment, and leading silence/noise frames have no tokens — their attention mass is absorbed by the FIRST word. In ep5 clip 1, "I" got start=0.000 under ~0.9s of footsteps and birdsong, so (a) captions appeared before anyone spoke and (b) speechRange-based auto-trim computed cutFrom=0 and kept the silent lead-in. Both consumers were consistent with each other and equally wrong about the actual audio. - primitives/silence.ts: refineWordTimings(words, silences, duration) — clamp the first word's start to the end of measured leading silence and the last word's end to the start of trailing silence (ffmpeg silencedetect measures signal energy directly and knows the truth whisper lost). Middle words are never touched. No silence -> no-op. - audio-element.ts: all AudioNode analysis is now memoized per node per option set (transcribe / silenceSegments / range / speechRange), so every consumer — DialogueClip's speechRange, Captions, user code — shares one transcription and one silencedetect run. transcribe() applies refinement by default for whisper words (native ElevenLabs alignment is trusted and never refined); refine: false opts out, noiseDb is tunable (default -35dB). Refinement runs on top of the disk cache — raw transcripts stay the cached source of truth, no retranscription. - captions.ts: an AudioNode src now uses the node's own memoized transcribe() instead of re-running whisper, so captions and auto-trim agree on when speech starts. Verified on ep5 (cached re-render, $0, zero rate-limit): first caption cue moved 0.00 -> 0.30 (clip now actually trimmed by speechRange), frame at 0.15s has no captions, frame at 1.0s shows "I really love" during actual speech; 217 cues, zero overlapping pairs; 99 tests green.
Date.now()-only temp paths have millisecond resolution. Once caption transcripts became memoized (previous commit), all 12 clips' ASS/SRT conversions landed in the same millisecond — 12 paths collapsed into 7 files, neighboring clips overwrote each other, and mergeAssFiles composed clip N's captions over clip M's video (three consecutive clips all showed 'If you can't' while the speech said 'between work, bad weather'). The memoization speedup exposed the latent race. uniqueTempPath(prefix, ext) in renderers/utils.ts appends a process-wide counter; all 6 temp-path sites in captions.ts and merge-ass.ts now use it. Regression test: 100 rapid calls produce 100 unique paths. Verified on ep5 (cached re-render): 12 distinct temp files for 12 clips, each clip's first cue now matches its own dialogue.
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
1 participant
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.
Summary
Three fixes from the formula-care-ep5 production incident (rate-limit storm + caption timing bugs). Targets
feature/ir-compile-audio(PR #223). Closes the root cause analyzed in #225.1. Captions in trimmed clips — double lines + timing drift
Caption cues are generated in RAW (untrimmed) clip time, but clips trimmed with
cutFrom/cutTo(e.g. byspeechRange()auto-trim) start elsewhere on the timeline. Previously only+ clipStartOffsetwas applied:cutFromseconds per clip. Correct shift isclipStartOffset − cutFrom.cutToand bled over the next clip's captions — the double-line frames at clip boundaries (~82s in ep5).Fix:
flatten.ts:HoistedCaptioncarries the leaf clip's trim window (cutFrom/cutTo/duration)merge-ass.ts: newtransformCue— single shift/drop/clamp primitive used by both the single-caption path andmergeAssFiles; cues fully outside the window are dropped, partial ones clamped (cue end of clip N can never exceed cue start of clip N+1)render.ts: emoji overlays go through the same transform;withAudiocaption tracks carrycutFrom/cutTointo the audio mix2. In-flight resolve dedup — 52 jobs for one image (#225 root cause)
Production DB showed 123 jobs in 7 min for ~8 unique requests. The standalone resolve path (
resolveImageElementetc.) had no in-flight dedup: N concurrent async components racing throughspeechRange() → resolveAudioElement → resolveVideoElement → resolveImageElementtoward one shared dependency image all sawmeta === undefinedand fired N parallel generations — bypassing bothmakeThenablememoization (nested elements are resolved via direct calls) and the renderers'pendingFiles.Fix:
memoizedElementResolveinresolve.ts— WeakMap keyed by element identity, wrapping all standalone resolvers (image/video/speech/music/talking-head/audio). Graph invariant: one element resolves at most once no matter how many concurrent paths lead to it. Meta write-back keeps renderers/executePlan/compile seeing resolved elements; rejected promises stay memoized (no silent re-spend on re-await).3. Idempotency-Key on job submission — retry storm multiplier
Every 429 retry in
submitJobcreated a brand-new job (all 123 incident jobs hadidempotency_key = NULL), refilling the rate window as it reset. Fix: stableIdempotency-Keyheader — hash of (capability, canonicalized params) — so retries reference the same job. Verified live: the API stores the key on created jobs.Also included
VideoElement/SpeechElement/ImageElement(no moreReturnType<typeof Video>in user code)keyprop in JSXIntrinsicAttributesexamples/async/native-audio-scene1.tsx— the target-API example, now typecheckingVerification
merge-ass.test.ts(15 — shift/drop/clamp/boundary/legacy),resolve-dedup.test.ts(5 — concurrent dedup, ep5 shape, failure memoization, distinct-elements),varg-idempotency.test.ts(6 — key stability/canonicalization)be_varg_jobs)bunx tsc --noEmitcleanNot done (deliberate)
Refs #225