Skip to content

feat(studio): audio meter faders for groups and master - #4203

Merged
miguel-heygen merged 11 commits into
mainfrom
hfoss21/audio-meters-faders
Sep 21, 2026
Merged

miguel-heygen merged 11 commits into
mainfrom
hfoss21/audio-meters-faders

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

What

Audio meters are opt-in from the timeline toolbar. Fresh preferences keep the strip hidden; the speaker button shows it and persists the choice.

Each audio meter strip (from #4193) gets a volume fader. Dragging a group's fader writes its volume through the existing group volume attribute; dragging the Monitor fader goes through the existing preview volume control. Peak-hold meter bars also switch from a flat green fill to a green/amber/red backdrop matching the dB scale (green below -6, amber -6 to -3, red above -3), so a loud passage visibly lights the top of the scale instead of just a taller green bar.

How

  • Group faders reuse the clip-gain scale: unity at the midpoint, with gain up to +12 dB. Live and committed group writes use the shared gain formatter.
  • Monitor changes preview listening volume within [0, 1]. Its meter continues to show program level.
  • Gain readouts use the shared dB formatter. Pointer cancellation and lost capture finish the gesture once; arrows, PageUp/PageDown, Home, and End adjust the fader.

Before

Baseline 97420736d, meters-demo, 1600 × 1000 viewport. Live audio meters without faders.

Before: meters-demo with live meters and no faders

After

Head e847e8c6a, the same meters-demo fixture and viewport. Live audio meters with group faders and the Monitor fader, adjusted using its keyboard control. Monitor changes preview listening volume; the master meter continues to show the program level.

After: shared gain scale and Monitor fader

Checks

  • 30 focused meter/fader tests pass, including boosted gain, unity, ceiling, round-trips, Monitor limits, cancellation, and keyboard controls.
  • Reintroducing the unity clamp makes three regression tests fail.
  • Studio typecheck, lint, formatting, and Fallow pass.

@miguel-heygen
miguel-heygen force-pushed the hfoss21/audio-meters-faders branch 4 times, most recently from bd8acd4 to 44d0fce Compare September 21, 2026 19:54
Base automatically changed from hfoss21/audio-meters to main September 21, 2026 20:19
@miguel-heygen
miguel-heygen force-pushed the hfoss21/audio-meters-faders branch from 44d0fce to 2d14f0e Compare September 21, 2026 20:25

@jrusso1020 jrusso1020 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.

Requesting changes at b1a30c92 — one blocking issue in the fader's value range. The meter half of this is good work and the dB math is correct; the problem is that the fader borrows the meter's scale, and a meter and a fader do not have the same range.

Blocking — the group fader silently cuts any group authored above unity

AudioMeterStrip.tsx:248 sets the thumb from levelToFraction(Math.min(volume, 1)), and fractionToLevel (audioMeterMath.ts:31-44) returns at most 1. So the control's ceiling is 0 dB in both directions — display and write.

Group volume is not capped at 1 anywhere on the path this PR writes to:

  • core/src/audioGain.ts:8-9MAX_AUDIO_GAIN_DB = 12, MAX_AUDIO_GAIN = 10 ** (12/20)3.981
  • core/src/audioGain.ts:17-20clampAudioGain clamps to [0, MAX_AUDIO_GAIN], not [0, 1]
  • core/src/audioGroups.ts:45-49readAudioGroupVolume does not clamp at all
  • studio/src/hooks/timelineAudioGroupVolume.ts:46-49mirroredGroupVolume has no upper bound either
  • core/src/audioAutomation.ts:168-170VOLUME_RANGE.max is MAX_AUDIO_GAIN, so the automation lane for this same parameter already reaches +12 dB

Failure scenario. A composition authored with <hf-audio-group data-volume="2"> (+6 dB). Open Studio: Math.min(2, 1) → fraction 1, so the thumb is pinned at the top, visually identical to unity — nothing indicates the group is hot. Click once anywhere on the track. onPointerDown alone calls onLive, and onPointerUp calls onCommit (AudioMeterStrip.tsx:275-288), so a single click writes data-volume="1" through onSetAudioGroupAttributeQuiet. The group is permanently cut by 6 dB — 12 dB for a group at the 3.981 ceiling — with no warning and no visible thumb movement, because the thumb was already at the top before and after.

This is the same divergence core/src/runtime/webAudioTransport.ts:25-34 documents as an already-fixed bug, in nearly the same words:

an authored <hf-audio-group data-volume="2"> previewed at 1.0 and exported at 2.0, up to 6 dB quieter in the audition than in the file, and 12 dB at the ceiling

and against the invariant stated at audioGain.ts:1-7: "Keep the shared ceiling here so Studio, preview, and render cannot drift."

The repo already ships the fader this needs, and using it is less code than what is here. propertyPanelMediaSection.tsx:262-268 and propertyPanelFlatMediaSection.tsx:246-256 both drive clip gain with:

min={AUDIO_GAIN_FADER_MIN} max={AUDIO_GAIN_FADER_MAX}   // -100..100, unity at the midpoint
audioGainToFaderPosition(volume)                         // gain  -> position
audioFaderPositionToGain(next)                           // position -> gain
onSetAttribute("volume", formatAudioGain(gain))          // canonical serialisation
audioGainToText(volume)                                  // "+12.0 dB" readout

Switching Fader to those helpers fixes the range, the serialisation and the readout together. Note formatAudioGain also matters on its own: AudioMeterStrip.tsx:82,89 writes String(volume), bypassing the clamp and the 6-decimal rounding that audioGain.ts:27-40 exists to guarantee.

The Monitor strip is fine as written — playerStore.ts:412-416 clamps setAudioVolume to [0,1] deliberately, since that is preview monitoring rather than authored gain. Only the group faders need the wider range, which is another reason to take the range from the target rather than from the meter.

Non-blocking

No onPointerCancel / onLostPointerCapture. draggingRef (AudioMeterStrip.tsx:247) is cleared only in onPointerUp. On a cancelled gesture pointerup never fires, so the ref stays true while pointer capture is released — after that, plain hovering over the track satisfies the onPointerMove guard at line 281 and writes volume live, with no commit to close the undo entry. Every other drag surface in this repo handles this; propertyPanelPrimitives.tsx:70-78 is the closest precedent and wires one handler to all three of onPointerUp / onPointerCancel / onLostPointerCapture.

Keyboard is ArrowUp/ArrowDown only (:289-294). The slider role also implies Home/End and PageUp/PageDown. Minor, but aria-valuenow reporting percent-of-travel is less useful than the dB value audioGainToText already formats — worth an aria-valuetext.

Verified, so it does not get re-litigated

  • The restack is clean. #4193's merge commit 97420736 is an ancestor of b1a30c92, and is exactly merge-base(HEAD, origin/main). Nothing from the parent was dropped in the rebase, so rule-12-style "did the restack revert the parent" concerns do not apply here.
  • Main's drift is benign. 16fab47a8 and 4e9059ad5 landed since the merge-base and touch no file this PR touches.
  • The dB math is right. I checked levelToFraction / fractionToLevel by hand at every stop and between them; the marks land on 1.0/0.8/0.6/0.4/0.2 as the test asserts, and the round-trip holds. The piecewise scale and the peak hold/fall in stepChannel are sound.
  • The two red Windows checks are not from this diff. Both are timeouts in propertyPanelInputCoverage.test.tsx, a file this PR does not touch, on a runner that spent 276s just importing. This PR's own tests passed there. I am not asking for anything on CI.

One limit worth stating: the body says "All five review findings are closed." There are no reviews and no comments on this PR, so there was no artifact for me to check that against — I reviewed the head on its own merits rather than confirming closure of findings I could not see.

Happy to re-review as soon as the fader range moves onto the core gain helpers.

— Rames

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

@jrusso1020 Fixed in e847e8c.

Group faders now use the shared clip-gain coordinates and conversions, with unity at the midpoint and a +12 dB ceiling. Both live and committed writes use formatAudioGain; the tooltip and aria-valuetext use audioGainToText. The unused inverse meter mapping is removed. Monitor uses the same conversion with its maximum position at unity, matching setAudioVolume's [0, 1] range.

The tests cover authored gain 2 above unity, an exact unity write at the midpoint, the top writing MAX_AUDIO_GAIN, round-trips for 0.5/1/2/3.98, and low-gain precision. Restoring the unity clamp deliberately failed three tests. Restored code passed all 30 focused tests; Studio typecheck, lint, formatting, and Fallow also passed.

The same commit closes cancelled/lost pointer capture exactly once and prevents later hovering from writing gain. Home/End and PageUp/PageDown are covered as well. A fresh Studio capture shows the corrected group thumb positions and Monitor control. CI at the new head is pending.

@jrusso1020 jrusso1020 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.

APPROVE — re-review at e847e8c6a. My 5272421290 CHANGES_REQUESTED at b1a30c926 is cleared: both findings are fixed, and both are now covered by tests.

The blocker is fixed

The fader no longer borrows the meter's scale. AudioMeterStrip.tsx:260-262 drives the thumb from audioGainToFaderPosition over AUDIO_GAIN_FADER_MIN..maxPosition, and fractionToLevel — the function that capped the write at 0 dB — is deleted outright rather than worked around.

AudioMeterStrip.tsx:371 is the part worth calling out as the right call: the Monitor keeps its unity ceiling (maxPosition = audioGainToFaderPosition(1)) while groups get AUDIO_GAIN_FADER_MAX. That preserves the distinction — a meter tops out at 0 dBFS because that is what a signal level means; an authored gain goes to +12 dB. Widening both would have been the easy wrong fix.

Verified by executing packages/core/src/audioGain.ts directly (zero imports, so it runs standalone):

check result
the reported scenario, group at data-volume="2" position 50.17, thumb at 75.1%, readout +6.0 dB
one click at that same spot returns gain 2 — the silent cut is gone
End / group ceiling 3.981072 = +12.0 dB = MAX_AUDIO_GAIN
Home 0 (hard mute)
PageUp / PageDown 1.318257 / 0.251189 — matches the test expectations exactly
thumb fraction over 0, 0.001, 0.5, 1, 2, MAX, 10, 1e9, NaN, -5, Infinity stays in [0,1], cannot overflow
Monitor, every fraction never exceeds unity

The non-blocking finding is fixed too

onPointerCancel and onLostPointerCapture both route to finishDrag (:317-318), and draggingRef became number | null, so onPointerMove can no longer write live after a cancelled gesture — that was the hover bug. finishDrag self-guards on null, so the pointerup + lostpointercapture pair is idempotent.

The test at AudioMeterStrip.test.tsx:244 is the right shape: it fires pointerdown(50), then cancel, move, and up all at clientY=0, and asserts live and quiet were each called exactly once with "1" — proving it commits the pointerdown value rather than the hover position, and that the trailing move writes nothing.

Findings — all non-blocking, none needs to hold this

1. The fader accepts non-primary buttons. AudioMeterStrip.tsx:308-311 captures the pointer and calls moveTo with no e.button check, so a right-click or middle-click on the track starts a drag and writes volume. Fader is new in this PR, so this is in-diff rather than inherited. The sibling gesture that landed in #4250 guards it — TimelineClipFades.tsx:84, if (e.button !== 0 || !canEdit) return;. One line, and it makes the two new audio drag surfaces agree.

2. Cancel commits where the sibling reverts. finishDrag on pointercancel commits the last live value; TimelineClipFades reverts to the pre-gesture state. Committing is defensible here — onLive has already mutated what the user sees, and there is no stored pre-drag value to go back to — and the test pins it deliberately, so I read it as chosen rather than accidental. Flagging only so the divergence between the two new audio gestures is on record.

3. Out of diff, but this PR is what makes it load-bearing. audioGain.ts:34 claims "Six decimals round-trip every integer fader stop back to itself." Measured across all 201 integer stops, worst drift is 6.49e-3 position units at -99 — roughly 0.003% of travel, so the knob-jump-on-release problem the comment exists to prevent is genuinely solved and nothing misbehaves. "Every" is just slightly stronger than the measurement. audioGain.ts is untouched here; noting it because the fader is now the main consumer of that guarantee.

Negative results, published so they are not re-litigated

  • No interaction with #4250, which merged into main while this was in flight. Both PRs touched audioMeterMath.ts — exactly the overlap that becomes a merge-order bomb. It does not here: this push deletes the fractionToLevel that an earlier commit in the same PR had added, so audioMeterMath.ts is now net-unchanged versus base and the overlap is gone. Merge-base 97420736d, main 839ae426d, mergeable: MERGEABLE.
  • Deleting fractionToLevel is safe. Zero consumers at this head and zero on merged main. STOPS is still used by levelToFraction, so no orphaned constant.
  • ./runtime/levelTap subpath plumbing is complete across all three surfaces — package-subpaths.json, both the dev and dist export blocks in package.json, and tsconfig.json. That is the one that usually gets half-done.
  • PR body claim "30 focused meter/fader tests" is exact — 25 in AudioMeterStrip.test.tsx (15 plain plus 10 from three it.each blocks) and 5 in audioMeterMath.test.ts.
  • The three red checks are cancelled runs, not failures. Smoke: global install, Tests on windows-latest: ${{ matrix.lane }}, and regression-shards all report conclusion: cancelled, zero steps executed, started_at == completed_at. gh pr checks renders that as fail. I do not gate on CI either way; the reason it is worth saying is that no test lane has actually executed at this head, so the red is not evidence of breakage and the green is not evidence of health.
  • I could not run the suite locally either, so I am not claiming a pass I do not have. Vite resolves @hyperframes/core/audio-gain through the node condition into dist/, which needs a built core. That is environmental rather than yours: a pre-existing test on main with the identical import (TimelineAutomationLane.test.tsx) fails the same way in my checkout. I verified every numeric expectation in the new tests independently by executing the math instead.

— Rames

@jrusso1020 jrusso1020 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.

Re-reviewed the delta e847e8c6a..01c9d3d25 — three files, +45/-3. Approved at 01c9d3d2591fd3d943af526c36f484ef1041436c.

Nothing in this commit touches AudioMeterStrip.tsx's fader logic or audioGain.ts, so everything I verified for the approval at e847e8c6a stands unchanged. This is purely the opt-in flip.

The flip is complete, which is the part worth checking

audioMeterVisibility.ts:9 goes ?? true?? false. The thing that usually goes wrong with a default flip is a second place that still applies the old one, so I looked for every site:

  • audioMeterVisibility.ts:9 is the only place the default is applied. Both consumers read the store, not the preference — TimelineToolbar.tsx:147 and AudioMeterStrip.tsx:399.
  • studioUiPreferences.ts:145-146 copies audioMetersVisible only if (typeof parsed.audioMetersVisible === "boolean"), so an absent key stays undefined and actually reaches the ??. A looser check here (parsed.audioMetersVisible !== undefined, say) would have let a stored null shadow the default; it doesn't.
  • Both docstrings were updated with the behaviour — audioMeterVisibility.ts:4 ("off unless the user showed it") and studioUiPreferences.ts:28 ("hidden unless enabled here"). No stale contract left pointing the other way.

The gate placement does more than hide pixels

AudioMeterStrip.tsx:401 returns null before MeterStripBody mounts, and useStrips, useVolumeHandlers and useMeterLoop all live inside that child. So off-by-default doesn't just hide the strip — the meter loop never starts for a user who doesn't open it. That's the structure you want for this; had the gate been a className or a wrapper style, the flip would have shipped a permanent background loop for everyone.

TimelineToolbar.tsx:258 gates the toggle button itself on projectHasAudio, matching the strip's own !projectHasAudio half of the gate at :401. Consistent — no button that toggles nothing, no strip reachable without a button.

Test

Selectors are all real, which is worth stating because a wrong one here fails open on the first assertion: aria-label="Toggle audio meters" at TimelineToolbar.tsx:263, aria-pressed at :264, data-testid="audio-meter-strip" at AudioMeterStrip.tsx:416. Seeding usePlayerStore with a tag: "audio" element is what makes projectHasAudio true so both halves can render — correct setup, not incidental.

Two small notes, neither blocking:

  • The test asserts the default through useAudioMetersVisible.getInitialState(), which is a snapshot taken when the module was first evaluated, not a re-read of localStorage after the clear(). It's correct as written because the import happens with an empty store, and the failure mode if that ever stopped holding is a loud failure on aria-pressed, not a silent pass. Worth knowing it's load-order-coupled rather than reading the preference directly.
  • The explicit stored-false path isn't covered — only undefined and the opt-in transition. Harmless today since ?? false makes the two indistinguishable, but it means persistence of a deliberate opt-out is untested.

Still open from my last review

AudioMeterStrip.tsx:308-311 still has no e.button !== 0 guard on onPointerDown, so a right-click or middle-click on a fader captures the pointer and jumps the value. The sibling gesture on merged main does guard it — TimelineClipFades.tsx:84, if (e.button !== 0 || !canEdit) return;. Non-blocking, as before, and not a reason to hold this head; flagging so it isn't assumed fixed.

Could not run: the suite still doesn't execute in my worktree — vite resolves @hyperframes/core/* through the node condition into an unbuilt dist/, and the failure is identical on a test that predates this PR, so it's environmental rather than anything in the diff. The new test is verified by reading it against the source, not by execution.

— Rames

@miguel-heygen
miguel-heygen merged commit 6f6d242 into main Sep 21, 2026
64 checks passed
@miguel-heygen
miguel-heygen deleted the hfoss21/audio-meters-faders branch September 21, 2026 23:25
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.

2 participants