Skip to content

fix(export): stop a voiceover piece where the next one starts - #1362

Merged
EtienneLescot merged 1 commit into
mainfrom
pilot/fix-1283
Oct 23, 2026
Merged

EtienneLescot merged 1 commit into
mainfrom
pilot/fix-1283

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Refs #1283

Cause. In buildSceneDescription, each voiceover piece left by a cut starts at its head projected through the speed regions, but keeps its full source window at 1x. A sped-up stretch inside a piece makes it outlast the gap to the next piece's projected start, so both play at once (0.9 s in the #1283 measurement).

Fix. Each piece's trimEndSec is capped at the next piece's projected start. Pieces without a speed region in them are unchanged. Music and looping tracks are untouched.

Tests. New case in sceneDescription.test.ts: a 2x region before a cut. Before: [[0, 0, 4], [3, 6, 10]] (1 s overlap). After: [[0, 0, 3], [3, 6, 10]]. npx vitest --run src/native src/lib/ai-edition/timeline (578 passing), both tsc configs and npm run lint clean.

Not measured. No sweep export through the real pipeline: the headless CLI export needs a fresh compositor addon, which this worktree does not have. The sweep re-run from #1283 remains to be done. The P3 missing fade-out from #1283 is not addressed here.

Overlap with #1355. It rewires the same voiceover block (pill lookup) but not the piece durations, so it does not fix this. The two hunks are separate.

🤖 Generated with Claude Code

A piece plays at 1x but its head is projected through the speed regions, so a
sped-up stretch before a cut left it playing over the next piece. Refs #1283.
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2010af79-5174-4f41-aa07-c6764996e7e5
📥 Commits

Reviewing files that changed from the base of the PR and between 1e91710 and 2c57198.

📒 Files selected for processing: 2
  • src/native/sceneDescription.test.ts
  • src/native/sceneDescription.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Voiceover piece mapping now caps each piece’s trimEndSec at the next piece’s projected start interval. A regression test checks output timing and source ranges across a cut after a 2× speed region.

Changes: Voiceover Piece Timing

Layer / File(s) Summary
Piece timing and regression coverage
src/native/sceneDescription.ts, src/native/sceneDescription.test.ts
Voiceover pieces now end at the next piece’s projected start interval. The regression test checks the output times and source ranges after a 2× speed region and cut.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: beetix

Merge Risk: ⚪ Minimal · up to 2c571

The change prevents the tested voiceover overlap without changing the inspected no-speed case. No issue identified here needs resolution before merge.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly identifies the primary fix: stopping a voiceover piece when the next piece starts. It is concise and specific.
Description check Passed The description explains the cause, fix, affected behavior, test coverage, validation results, and known limitations. It uses a valid related-issue reference and provides sufficient testing details. I…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the cause, fix, affected behavior, test coverage, validation results, and known limitations. It uses a valid related-issue reference and provides sufficient testing details. It does not include the template headings or explicit release and platform impact selections, but the content is mostly complete.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@EtienneLescot

Copy link
Copy Markdown
Member Author

Before/after measurement in the real export pipeline (headless CLI export, Windows)

Setup: 20 s synthetic screen video. The voiceover over the whole take is a frequency sweep 0.5*sin(2*PI*(200*t+100*t*t)), so track time = (f − 200) / 200. A 2x speed region over raw 2..6 s and a cut over raw 10..12 s, the case from #1283. Expected output: 16 s, cut at output 8 s, where track time should jump from 8 to 12. Same project, same native addon and DLL set for main (b664d49) and the PR head (2c57198). Audio scanned in 50 ms windows, two strongest peaks per window.

main PR head
Two pieces at once 2.00 s (output 8.0..10.0, two peaks at equal level) none (second peak at least 61 dB down in every window)
Cut piece 2 starts at output 8.0 with track time 12.0 while piece 1 keeps playing track 8..10 output 8.0: track time jumps 7.98 → 12.03, clean
Slope, track time over output time 1.000 on every segment, incl. under 2x 1.000 on every segment
Length / end 16.0 s, ends at track time 19.98 16.0 s, ends at track time 19.98

The overlap is gone, with no regression: same cut position, slope 1 under the speed region, same length and ending.

@EtienneLescot
EtienneLescot merged commit 0b8b406 into main Oct 23, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant