mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-11 23:00:03 +00:00
fix(studio): make the volume fader tell the truth about the gain it writes (#3305)
* fix(studio): make the volume fader tell the truth about the gain it writes The fader travels in dB, so its stops are irrational values; serializing them through the generic two-decimal numeric formatter collapsed the bottom quarter of its travel onto "0" — a hard mute — and made the knob jump on release everywhere below unity. Both panels now use the exact serializer, which round-trips every integer stop back to itself. Raise the volume automation lane to the same ceiling the fader reaches. Clamping the lane at unity meant automating a boosted clip silently discarded the boost, and the panel disables the fader while a lane owns the level, so there was no way back. This rescales the lane's vertical axis: unity now sits a quarter of the way up rather than at the top. Add audio_volume_tween_overrides_gain. Tween values on `volume` are absolute — they replace the authored gain rather than scaling it — so a clip carrying both plays at whatever the tween names, and the fader gives no sign of it. The rule reuses the tween detector the sibling lane/tween rule already has. * fix(lint): treat a missing data-volume as unity, not as silence readAttr returns null when the attribute is absent, and Number(null) is 0 — finite, and not 1 — so a clip carrying NO data-volume cleared both filters and was reported as authored at silence. Both halves of that were false: absent means unity everywhere else in the runtime. It fired on exactly the case the rule exists to bless. The docs this PR edits say data-volume is the baseline for elements no tween touches, so a tweened clip is expected not to carry one — the common audio fade. A warning does not fail check, but an agent reading the fixHint would have written a gain to correct a level that was never wrong.
This commit is contained in:
@@ -429,6 +429,61 @@ describe("media_variable_src_no_fallback", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("audio_volume_tween_overrides_gain", () => {
|
||||
const withScript = (audioAttrs: string, script: string) => `<!DOCTYPE html><html><body>
|
||||
<div id="root" data-composition-id="main" data-start="0" data-width="1920" data-height="1080" data-duration="10">
|
||||
<audio id="bgm" src="a.wav" data-start="0" data-duration="10" ${audioAttrs}></audio>
|
||||
</div>
|
||||
<script>${script}</script>
|
||||
</body></html>`;
|
||||
|
||||
it("warns that the tween's values win over an authored gain", async () => {
|
||||
const res = await lintHyperframeHtml(
|
||||
withScript(`data-volume="1.949845"`, `tl.fromTo("#bgm", { volume: 0 }, { volume: 1 });`),
|
||||
);
|
||||
const finding = res.findings.find((f) => f.code === "audio_volume_tween_overrides_gain");
|
||||
expect(finding?.severity).toBe("warning");
|
||||
expect(finding?.elementId).toBe("bgm");
|
||||
expect(finding?.message).toMatch(/5\.8 dB/);
|
||||
});
|
||||
|
||||
it("warns about an attenuation the tween overrides, not just a boost", async () => {
|
||||
const res = await lintHyperframeHtml(
|
||||
withScript(`data-volume="0.3"`, `tl.to("#bgm", { volume: 1 });`),
|
||||
);
|
||||
expect(res.findings.some((f) => f.code === "audio_volume_tween_overrides_gain")).toBe(true);
|
||||
});
|
||||
|
||||
it("stays quiet on the fade the docs recommend, which carries no data-volume", async () => {
|
||||
// `Number(null)` is 0 — finite and not 1 — so a clip with NO `data-volume`
|
||||
// was reported as authored at silence. Both halves were false, and this is
|
||||
// the shape the docs recommend for a tweened clip: the baseline attribute is
|
||||
// for elements no tween touches. The rule fired on exactly the common fade.
|
||||
const res = await lintHyperframeHtml(
|
||||
withScript("", `tl.fromTo("#bgm", { volume: 0 }, { volume: 1 });`),
|
||||
);
|
||||
expect(res.findings.some((f) => f.code === "audio_volume_tween_overrides_gain")).toBe(false);
|
||||
});
|
||||
|
||||
it("stays quiet at unity, without a tween, or when a lane already owns the level", async () => {
|
||||
const unity = await lintHyperframeHtml(
|
||||
withScript(`data-volume="1"`, `tl.to("#bgm", { volume: 0 });`),
|
||||
);
|
||||
const noTween = await lintHyperframeHtml(
|
||||
withScript(`data-volume="2"`, `tl.to("#bgm", { x: 1 });`),
|
||||
);
|
||||
const lane = await lintHyperframeHtml(
|
||||
withScript(
|
||||
`data-volume="2" data-automation='{"version":1,"lanes":[{"target":"volume","points":[{"t":0,"v":1}]}]}'`,
|
||||
`tl.to("#bgm", { volume: 0 });`,
|
||||
),
|
||||
);
|
||||
for (const res of [unity, noTween, lane]) {
|
||||
expect(res.findings.some((f) => f.code === "audio_volume_tween_overrides_gain")).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("audio_volume_double_automation", () => {
|
||||
const withScript = (audioAttrs: string, script: string) => `<!DOCTYPE html><html><body>
|
||||
<div id="root" data-composition-id="main" data-start="0" data-width="1920" data-height="1080" data-duration="10">
|
||||
|
||||
@@ -629,8 +629,55 @@ export const mediaRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> =
|
||||
|
||||
// audio_volume_double_automation
|
||||
findVolumeDoubleAutomationFindings,
|
||||
|
||||
// audio_volume_tween_overrides_gain
|
||||
findVolumeTweenOverridesGainFindings,
|
||||
];
|
||||
|
||||
/**
|
||||
* Tween values on `volume` are ABSOLUTE gains, not multipliers of the authored
|
||||
* `data-volume`: the probed keyframes replace that baseline outright, in
|
||||
* preview and in the render alike. So a clip carrying both plays at whatever
|
||||
* the tween names — `{ volume: 1 }` is 0 dB even on a clip the fader says is
|
||||
* at +5.8 dB, and Studio's fader gives no sign of it.
|
||||
*
|
||||
* Silent before this rule, and easier to hit since the fader gained +12 dB of
|
||||
* boost and `normalize-audio` writes into the very same attribute.
|
||||
*/
|
||||
function findVolumeTweenOverridesGainFindings(ctx: LintContext): HyperframeLintFinding[] {
|
||||
const boosted = ctx.tags
|
||||
.filter((tag) => isMediaTag(tag.name))
|
||||
// Absent means unity, as it does everywhere else. Reading it raw gave
|
||||
// `Number(null)` — 0, finite and not 1, so a clip with NO `data-volume`
|
||||
// cleared both filters and was reported as authored at silence. That is the
|
||||
// shape the docs recommend for a tweened clip, so the rule fired on exactly
|
||||
// the case it exists to bless.
|
||||
.map((tag) => ({ tag, volume: Number(readAttr(tag.raw, "data-volume") ?? "1") }))
|
||||
.filter((entry) => Number.isFinite(entry.volume) && entry.volume !== 1)
|
||||
// A lane already has its own rule, and it wins over both of these.
|
||||
.filter((entry) => !readDecodedAttr(entry.tag.raw, "data-automation"))
|
||||
.map((entry) => ({ ...entry, id: readAttr(entry.tag.raw, "id") }))
|
||||
.filter((entry): entry is typeof entry & { id: string } => Boolean(entry.id));
|
||||
if (boosted.length === 0) return [];
|
||||
|
||||
const script = ctx.scripts.map((block) => stripJsComments(block.content)).join("\n");
|
||||
const findings: HyperframeLintFinding[] = [];
|
||||
for (const { tag, id, volume } of boosted) {
|
||||
if (!tweensVolumeInSameCall(script, id)) continue;
|
||||
const db = volume > 0 ? `${(20 * Math.log10(volume)).toFixed(1)} dB` : "silence";
|
||||
findings.push({
|
||||
code: "audio_volume_tween_overrides_gain",
|
||||
severity: "warning",
|
||||
message: `#${id} has data-volume="${volume}" (${db}) and a GSAP tween on \`volume\`. Tween values are absolute — they REPLACE this gain rather than scale it — so wherever the tween names a value the clip plays at that value, not at ${db}.`,
|
||||
elementId: id,
|
||||
fixHint:
|
||||
"Write the tween's targets in the same absolute gain (e.g. `volume: 1.95`, not `volume: 1`), or reset data-volume to 1 and let the tween carry the level on its own.",
|
||||
snippet: truncateSnippet(tag.raw),
|
||||
});
|
||||
}
|
||||
return findings;
|
||||
}
|
||||
|
||||
/**
|
||||
* A track can have its volume shaped by an automation lane or by a GSAP tween,
|
||||
* and only the lane is heard: the runtime reads `data-automation` first and
|
||||
|
||||
Reference in New Issue
Block a user