Skip to content

fix(cli): preserve ducking gain when music starts later - #5318

Queued
user-github-me wants to merge 8 commits into
heygen-com:mainfrom
user-github-me:fix/duck-lane-clip-origin
Queued

user-github-me wants to merge 8 commits into
heygen-com:mainfrom
user-github-me:fix/duck-lane-clip-origin

Conversation

@user-github-me

@user-github-me user-github-me commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

A music clip starting after narration begins now enters its ducking lane at the gain for that composition time. Starting partway through an attack or release preserves the remaining ramp; speech that already ended cannot create a spurious dip at the clip's beginning.

Build the lane in composition time, interpolate its value at the clip start, then retain the future points in clip-local time. Packaged and standalone copies remain identical, and the audio guide explains delayed music behavior.

Validation:

  • Updated against current main, retaining the seven published skills and regenerating the manifest.
  • CLI rebuild and typecheck passed; the workspace was built before tests.
  • All 25 ducking and copy-parity tests passed, covering starts before/during/after speech and empty voice placement.
  • Lint, formatting, manifest freshness, reachability, normal hooks and both comment checks passed.
  • Mint documentation validation and broken-link checks passed.

The runtime tests and full CI workflow passed on commit 72b319cb. All 14 CLI lifecycle tests also passed locally after a CLI rebuild.

@jrusso1020

Copy link
Copy Markdown
Collaborator

Thanks for this, @user-github-me. This PR now has merge conflicts with main. Could you rebase it onto the latest main? Once it's conflict-free we'll review it and get it into the merge queue.

@mintlify

mintlify Bot commented Oct 24, 2026

Copy link
Copy Markdown

Preview approval required

This pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit 72b319c.

Approve preview

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and sorry it waited on rebases. The fix is right: building the lane in composition time and reading the gain at the clip's start is what keeps a late-starting music clip at the level it would have had, and it gives the same output as before when the clip starts at 0. I checked the edge cases (no speech, a clip that starts after all speech, a negative start) and they all fall back sensibly, and the interpolation can't divide by zero because the point after the start is always strictly later. The two copies of duck.mjs are identical, and the table of start positions in the tests covers the attack and release cases well.

One optional nit: the opening value at duck.mjs:72-77 isn't passed through round3 like the other points, so a start such as 1.1004 writes 0.2987999999999997 into the lane. Rounding it would keep the attribute tidy; it doesn't change the sound.

Verdict: APPROVE
Reasoning: Correct fix in the one function the ducking script uses, identical packaged copy, tests cover each start position, CI green and no conflict with main.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 25, 2026

This branch has not been deployed

No deployments
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.

3 participants