Repository navigation
fix(cli): preserve ducking gain when music starts later - #5318
user-github-me wants to merge 8 commits into
Conversation
|
Thanks for this, @user-github-me. This PR now has merge conflicts with |
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
miguel-heygen
left a comment
There was a problem hiding this comment.
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.
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:
main, retaining the seven published skills and regenerating the manifest.The runtime tests and full CI workflow passed on commit
72b319cb. All 14 CLI lifecycle tests also passed locally after a CLI rebuild.