Skip to content

Commit 228eabd

Browse files
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.
1 parent b3c43e2 commit 228eabd

15 files changed

Lines changed: 243 additions & 108 deletions

packages/core/src/audioAutomation.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
type HfAutomationLane,
1919
} from "./audioAutomation.js";
2020
import { mintAudioFxNodeId, parseAudioFxChain, type HfAudioFxChain } from "./audioFx.js";
21+
import { MAX_AUDIO_GAIN } from "./audioGain.js";
2122

2223
const chain: HfAudioFxChain = {
2324
version: 1,
@@ -116,7 +117,7 @@ describe("normalisation", () => {
116117
]);
117118
});
118119

119-
it("clamps volume into 0..1 at parse time", () => {
120+
it("clamps volume into the authoring gain range at parse time", () => {
120121
const parsed = parseAutomation(
121122
JSON.stringify({
122123
version: 1,
@@ -131,7 +132,9 @@ describe("normalisation", () => {
131132
],
132133
}),
133134
);
134-
expect(parsed.lanes[0]!.points.map((p) => p.v)).toEqual([1, 0]);
135+
// The lane shares the fader's ceiling. Clamping it at unity discarded the
136+
// boost of any clip authored above 0 dB the moment it was automated.
137+
expect(parsed.lanes[0]!.points.map((p) => p.v)).toEqual([MAX_AUDIO_GAIN, 0]);
135138
});
136139

137140
it("refuses malformed input instead of silently losing an envelope", () => {

packages/core/src/audioAutomation.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import { getAudioFxDef, type HfAudioFxChain } from "./audioFx.js";
15+
import { MAX_AUDIO_GAIN } from "./audioGain.js";
1516

1617
export const HF_AUDIO_AUTOMATION_ATTR = "data-automation";
1718

@@ -135,8 +136,9 @@ export const PRESET_RANGE: AutomationRange = {
135136
/**
136137
* The value range a lane is drawn and clamped against.
137138
*
138-
* Volume is linear 0..1, matching `data-volume` and the existing volume
139-
* envelope machinery — no dB conversion enters the volume path. Everything
139+
* Volume is linear over the full authoring gain range, matching `data-volume`
140+
* and the existing volume envelope machinery — no dB conversion enters the
141+
* volume path. Everything
140142
* else is read from the effect registry, so a lane can never offer a value the
141143
* renderer would reject, and the log-scaled knobs sweep the way a DAW's do.
142144
*/
@@ -151,9 +153,15 @@ export interface AutomationRange {
151153
default: number;
152154
}
153155

156+
/**
157+
* One ceiling for the fader, the lane, the preview transport and the render
158+
* mixer. Capping the lane at unity while the fader reached +12 dB made
159+
* automating a boosted clip silently discard the boost — and the panel
160+
* disables the fader while a lane owns it, so there was no way back.
161+
*/
154162
export const VOLUME_RANGE: AutomationRange = {
155163
min: 0,
156-
max: 1,
164+
max: MAX_AUDIO_GAIN,
157165
step: 0.01,
158166
unit: "",
159167
label: "Volume",

packages/lint/src/rules/media.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,61 @@ describe("media_variable_src_no_fallback", () => {
429429
});
430430
});
431431

432+
describe("audio_volume_tween_overrides_gain", () => {
433+
const withScript = (audioAttrs: string, script: string) => `<!DOCTYPE html><html><body>
434+
<div id="root" data-composition-id="main" data-start="0" data-width="1920" data-height="1080" data-duration="10">
435+
<audio id="bgm" src="a.wav" data-start="0" data-duration="10" ${audioAttrs}></audio>
436+
</div>
437+
<script>${script}</script>
438+
</body></html>`;
439+
440+
it("warns that the tween's values win over an authored gain", async () => {
441+
const res = await lintHyperframeHtml(
442+
withScript(`data-volume="1.949845"`, `tl.fromTo("#bgm", { volume: 0 }, { volume: 1 });`),
443+
);
444+
const finding = res.findings.find((f) => f.code === "audio_volume_tween_overrides_gain");
445+
expect(finding?.severity).toBe("warning");
446+
expect(finding?.elementId).toBe("bgm");
447+
expect(finding?.message).toMatch(/5\.8 dB/);
448+
});
449+
450+
it("warns about an attenuation the tween overrides, not just a boost", async () => {
451+
const res = await lintHyperframeHtml(
452+
withScript(`data-volume="0.3"`, `tl.to("#bgm", { volume: 1 });`),
453+
);
454+
expect(res.findings.some((f) => f.code === "audio_volume_tween_overrides_gain")).toBe(true);
455+
});
456+
457+
it("stays quiet on the fade the docs recommend, which carries no data-volume", async () => {
458+
// `Number(null)` is 0 — finite and not 1 — so a clip with NO `data-volume`
459+
// was reported as authored at silence. Both halves were false, and this is
460+
// the shape the docs recommend for a tweened clip: the baseline attribute is
461+
// for elements no tween touches. The rule fired on exactly the common fade.
462+
const res = await lintHyperframeHtml(
463+
withScript("", `tl.fromTo("#bgm", { volume: 0 }, { volume: 1 });`),
464+
);
465+
expect(res.findings.some((f) => f.code === "audio_volume_tween_overrides_gain")).toBe(false);
466+
});
467+
468+
it("stays quiet at unity, without a tween, or when a lane already owns the level", async () => {
469+
const unity = await lintHyperframeHtml(
470+
withScript(`data-volume="1"`, `tl.to("#bgm", { volume: 0 });`),
471+
);
472+
const noTween = await lintHyperframeHtml(
473+
withScript(`data-volume="2"`, `tl.to("#bgm", { x: 1 });`),
474+
);
475+
const lane = await lintHyperframeHtml(
476+
withScript(
477+
`data-volume="2" data-automation='{"version":1,"lanes":[{"target":"volume","points":[{"t":0,"v":1}]}]}'`,
478+
`tl.to("#bgm", { volume: 0 });`,
479+
),
480+
);
481+
for (const res of [unity, noTween, lane]) {
482+
expect(res.findings.some((f) => f.code === "audio_volume_tween_overrides_gain")).toBe(false);
483+
}
484+
});
485+
});
486+
432487
describe("audio_volume_double_automation", () => {
433488
const withScript = (audioAttrs: string, script: string) => `<!DOCTYPE html><html><body>
434489
<div id="root" data-composition-id="main" data-start="0" data-width="1920" data-height="1080" data-duration="10">

packages/lint/src/rules/media.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,8 +629,55 @@ export const mediaRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> =
629629

630630
// audio_volume_double_automation
631631
findVolumeDoubleAutomationFindings,
632+
633+
// audio_volume_tween_overrides_gain
634+
findVolumeTweenOverridesGainFindings,
632635
];
633636

637+
/**
638+
* Tween values on `volume` are ABSOLUTE gains, not multipliers of the authored
639+
* `data-volume`: the probed keyframes replace that baseline outright, in
640+
* preview and in the render alike. So a clip carrying both plays at whatever
641+
* the tween names — `{ volume: 1 }` is 0 dB even on a clip the fader says is
642+
* at +5.8 dB, and Studio's fader gives no sign of it.
643+
*
644+
* Silent before this rule, and easier to hit since the fader gained +12 dB of
645+
* boost and `normalize-audio` writes into the very same attribute.
646+
*/
647+
function findVolumeTweenOverridesGainFindings(ctx: LintContext): HyperframeLintFinding[] {
648+
const boosted = ctx.tags
649+
.filter((tag) => isMediaTag(tag.name))
650+
// Absent means unity, as it does everywhere else. Reading it raw gave
651+
// `Number(null)` — 0, finite and not 1, so a clip with NO `data-volume`
652+
// cleared both filters and was reported as authored at silence. That is the
653+
// shape the docs recommend for a tweened clip, so the rule fired on exactly
654+
// the case it exists to bless.
655+
.map((tag) => ({ tag, volume: Number(readAttr(tag.raw, "data-volume") ?? "1") }))
656+
.filter((entry) => Number.isFinite(entry.volume) && entry.volume !== 1)
657+
// A lane already has its own rule, and it wins over both of these.
658+
.filter((entry) => !readDecodedAttr(entry.tag.raw, "data-automation"))
659+
.map((entry) => ({ ...entry, id: readAttr(entry.tag.raw, "id") }))
660+
.filter((entry): entry is typeof entry & { id: string } => Boolean(entry.id));
661+
if (boosted.length === 0) return [];
662+
663+
const script = ctx.scripts.map((block) => stripJsComments(block.content)).join("\n");
664+
const findings: HyperframeLintFinding[] = [];
665+
for (const { tag, id, volume } of boosted) {
666+
if (!tweensVolumeInSameCall(script, id)) continue;
667+
const db = volume > 0 ? `${(20 * Math.log10(volume)).toFixed(1)} dB` : "silence";
668+
findings.push({
669+
code: "audio_volume_tween_overrides_gain",
670+
severity: "warning",
671+
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}.`,
672+
elementId: id,
673+
fixHint:
674+
"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.",
675+
snippet: truncateSnippet(tag.raw),
676+
});
677+
}
678+
return findings;
679+
}
680+
634681
/**
635682
* A track can have its volume shaped by an automation lane or by a GSAP tween,
636683
* and only the lane is heard: the runtime reads `data-automation` first and

packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx

Lines changed: 14 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,9 @@ describe("FlatMediaSection — cutout", () => {
131131
});
132132

133133
describe("FlatMediaSection — volume/rate/media-start", () => {
134-
it("renders volume at its stored percentage and commits a new value on drag", () => {
134+
it("renders unity volume as neutral 0 dB at the slider midpoint", () => {
135135
const onSetAttribute = vi.fn();
136-
const element = makeVideoElement({ dataAttributes: { volume: "0.5" } });
136+
const element = makeVideoElement({ dataAttributes: { volume: "1" } });
137137
const host = document.createElement("div");
138138
document.body.append(host);
139139
const root = createRoot(host);
@@ -149,48 +149,16 @@ describe("FlatMediaSection — volume/rate/media-start", () => {
149149
/>,
150150
);
151151
});
152-
expect(host.textContent).toContain("50%");
152+
expect(host.textContent).toContain("0.0 dB");
153+
expect(
154+
host.querySelector('[data-flat-slider-track="true"]')?.getAttribute("aria-valuenow"),
155+
).toBe("0");
153156
act(() => root.unmount());
154157
});
155158

156-
it("refuses to commit from the percent slider on a clip authored above unity", () => {
157-
// The control tops out at 100%, so any commit from it would cap a boosted
158-
// clip and silently drop up to 12 dB that now genuinely renders. Held until
159-
// the dB fader that can represent these levels replaces it.
159+
it("commits +12 dB of boost from the upper half of the volume fader", () => {
160160
const onSetAttribute = vi.fn();
161-
const element = makeVideoElement({ dataAttributes: { volume: "1.949845" } });
162-
const host = document.createElement("div");
163-
document.body.append(host);
164-
const root = createRoot(host);
165-
act(() => {
166-
root.render(
167-
<FlatMediaSection
168-
projectDir={null}
169-
element={element}
170-
styles={{}}
171-
onSetStyle={vi.fn()}
172-
onSetAttribute={onSetAttribute}
173-
onSetHtmlAttribute={vi.fn()}
174-
/>,
175-
);
176-
});
177-
178-
const volumeTrack = host.querySelectorAll('[data-flat-slider-track="true"]')[0];
179-
Object.defineProperty(volumeTrack, "getBoundingClientRect", {
180-
value: () => ({ left: 0, width: 100, top: 0, height: 2, right: 100, bottom: 2 }),
181-
});
182-
act(() => {
183-
volumeTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 50 }));
184-
volumeTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 50 }));
185-
});
186-
187-
expect(onSetAttribute).not.toHaveBeenCalled();
188-
act(() => root.unmount());
189-
});
190-
191-
it("commits a new volume value on slider track pointerdown", () => {
192-
const onSetAttribute = vi.fn();
193-
const element = makeVideoElement({ dataAttributes: { volume: "0.2" } });
161+
const element = makeVideoElement({ dataAttributes: { volume: "1" } });
194162
const host = document.createElement("div");
195163
document.body.append(host);
196164
const root = createRoot(host);
@@ -211,11 +179,13 @@ describe("FlatMediaSection — volume/rate/media-start", () => {
211179
value: () => ({ left: 0, width: 100, top: 0, height: 2, right: 100, bottom: 2 }),
212180
});
213181
act(() => {
214-
volumeTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 50 }));
215-
volumeTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 50 }));
182+
volumeTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 100 }));
183+
volumeTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 100 }));
216184
});
217-
// starting volume 0.2 (draft=20); min=0, max=100, ratio=0.5 -> raw=50 -> commit(50) -> 50/100=0.5 -> "0.5"
218-
expect(onSetAttribute).toHaveBeenCalledWith("volume", "0.5");
185+
// Six decimals, not two: at two the bottom of the dB fader collapses onto
186+
// "0" (a hard mute) and every stop below unity writes a value the knob then
187+
// jumps away from.
188+
expect(onSetAttribute).toHaveBeenCalledWith("volume", "3.981072");
219189
act(() => root.unmount());
220190
});
221191

packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ import {
1313
import { FlatSelectRow, FlatSlider } from "./propertyPanelFlatPrimitives";
1414
import { FlatToggle } from "./propertyPanelFlatToggle";
1515
import { AutomationToggle } from "./propertyPanelFxControls";
16+
import {
17+
AUDIO_GAIN_FADER_MAX,
18+
AUDIO_GAIN_FADER_MIN,
19+
audioFaderPositionToGain,
20+
formatAudioGain,
21+
audioGainToFaderPosition,
22+
audioGainToText,
23+
} from "@hyperframes/core/audio-gain";
1624

1725
// fallow-ignore-next-line complexity
1826
export function FlatMediaSection({
@@ -54,7 +62,7 @@ export function FlatMediaSection({
5462
const el = element.element;
5563

5664
const volume = parseNumericValue(element.dataAttributes.volume ?? "") ?? 1;
57-
const volumePercent = Math.round(volume * 100);
65+
const volumeFaderPosition = audioGainToFaderPosition(volume);
5866
const mediaStart =
5967
Number.parseFloat(
6068
element.dataAttributes["media-start"] ?? element.dataAttributes["playback-start"] ?? "0",
@@ -207,27 +215,24 @@ export function FlatMediaSection({
207215
<>
208216
{/* The slider is disabled while a lane owns the level: a value set
209217
here would be overwritten by the envelope on the next tick. The
210-
toggle beside it carries the tooltip.
211-
212-
It is also disabled above unity, for the same reason in a
213-
different guise — this control tops out at 100%, so committing
214-
from it would silently cap a boosted clip and drop up to 12 dB
215-
that now genuinely renders. A hold, not a fix: the dB fader that
216-
can represent these levels replaces this control outright. */}
218+
toggle beside it carries the tooltip. */}
217219
<div
218220
className="hf-volume-row flex items-center gap-1"
219221
data-volume-automated={volumeAutomated ? "" : undefined}
220222
>
221223
<div className="min-w-0 flex-1">
222224
<FlatSlider
223225
label="Volume"
224-
value={volumePercent}
225-
min={0}
226-
max={100}
227-
tier={volumePercent === 100 ? "default" : "explicitCustom"}
228-
displayValue={`${volumePercent}%`}
229-
disabled={volumeAutomated || volume > 1}
230-
onCommit={(next) => void onSetAttribute("volume", formatNumericValue(next / 100))}
226+
value={volumeFaderPosition}
227+
min={AUDIO_GAIN_FADER_MIN}
228+
max={AUDIO_GAIN_FADER_MAX}
229+
tier={volume === 1 ? "default" : "explicitCustom"}
230+
displayValue={audioGainToText(volume)}
231+
disabled={volumeAutomated}
232+
centerTick
233+
onCommit={(next) =>
234+
void onSetAttribute("volume", formatAudioGain(audioFaderPositionToGain(next)))
235+
}
231236
/>
232237
</div>
233238
<AutomationToggle

packages/studio/src/components/editor/propertyPanelMediaSection.tsx

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ import {
1313
} from "./propertyPanelHelpers";
1414
import { Section, SegmentedControl, SelectField, SliderControl } from "./propertyPanelPrimitives";
1515
import { useTrackDesignInput } from "../../contexts/DesignPanelInputContext";
16+
import {
17+
AUDIO_GAIN_FADER_MAX,
18+
AUDIO_GAIN_FADER_MIN,
19+
audioFaderPositionToGain,
20+
formatAudioGain,
21+
audioGainToFaderPosition,
22+
audioGainToText,
23+
} from "@hyperframes/core/audio-gain";
1624

1725
// fallow-ignore-next-line complexity
1826
export function MediaSection({
@@ -47,7 +55,7 @@ export function MediaSection({
4755
const el = element.element;
4856

4957
const volume = parseNumericValue(element.dataAttributes.volume ?? "") ?? 1;
50-
const volumePercent = Math.round(volume * 100);
58+
const volumeFaderPosition = audioGainToFaderPosition(volume);
5159

5260
const mediaStart =
5361
Number.parseFloat(
@@ -246,23 +254,18 @@ export function MediaSection({
246254

247255
{(isVideo || isAudio) && (
248256
<>
249-
{/* Held above unity: this control tops out at 100%, so committing
250-
from it would silently cap a boosted clip and drop up to 12 dB
251-
that now genuinely renders. The dB fader that can represent
252-
these levels replaces this control outright. */}
253257
<div className="grid min-w-0 gap-1.5">
254258
<span className={LABEL}>Volume</span>
255259
<SliderControl
256260
trackName="Volume"
257-
value={volumePercent}
258-
min={0}
259-
max={100}
261+
value={volumeFaderPosition}
262+
min={AUDIO_GAIN_FADER_MIN}
263+
max={AUDIO_GAIN_FADER_MAX}
260264
step={1}
261-
disabled={volume > 1}
262-
displayValue={`${volumePercent}%`}
263-
formatDisplayValue={(next) => `${Math.round(next)}%`}
265+
displayValue={audioGainToText(volume)}
266+
formatDisplayValue={(next) => audioGainToText(audioFaderPositionToGain(next))}
264267
onCommit={(next) => {
265-
void onSetAttribute("volume", formatNumericValue(next / 100));
268+
void onSetAttribute("volume", formatAudioGain(audioFaderPositionToGain(next)));
266269
}}
267270
/>
268271
</div>

0 commit comments

Comments
 (0)