Skip to content

feat(studio): audio level meters at the timeline's right edge - #4193

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

miguel-heygen merged 7 commits into
mainfrom
hfoss21/audio-meters

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What

Studio gets audio level meters at the right edge of the timeline: one stereo meter with peak hold and a dB scale (0, -3, -6, -12, -24) for each audio group, plus Master, moving with the real playback level in the preview. A speaker button in the timeline toolbar hides and shows the strip and the choice is remembered. A project with no audio gets no strip and no button.

Read-only in this PR. Faders (each strip's volume through the existing group volume write path) follow in the next PR.

How

  • Core: window.__hf.audioMeter = { start, stop, read } (levelTap.ts, webAudioTransport.ts). start() adds a stereo analyser branch after each group bus and after master; read() returns the linear peak per channel; stop() disconnects everything. Taps are side branches, so the audible path is unchanged, and nothing exists until start().
  • Studio: AudioMeterStrip runs one rAF loop that re-reads the hook off the live preview window each frame, so a reloaded preview is followed and its taps started. Bars are written straight to the DOM; no React state per frame.
  • Groups that appear or disappear mid-session appear in or leave read().
  • Ungrouped clips play straight into master (no per-track bus), so they show in Master only.

Before

Timeline with audio groups and no meter strip

After

Meter strip at the timeline right edge reading Music, VO and Master while playing

Captured in Studio on this head, from a fixture built of generated tones in two audio groups plus Master. After is playing; the bars are filled, not an idle rail.

Checks

  • Rebased onto current origin/main. The previous regression red was the aggregate job of a cancelled shard (One or more regression shards failed); shard-6 was cancelled during Docker image build and never ran the fixtures.
  • Core webAudioTransport tests 77 passed, exit 0 (includes taps created only on start, side-branch wiring, idempotent start, group added or removed mid-session, metering started before audio context init).
  • Studio strip, meter math, preference and TimelinePane suites 25 passed, exit 0; tsc --noEmit clean for studio.
  • Independent adversarial review: recorded for this head.

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

Reviewed at a07357246a2d32d0d8d5f069ea1838cf62a7fbf1. Read every changed file in full plus the surrounding graph (webAudioTransport chain construction, scheduleWebAudioForActiveClips, resolveGroupElement, the Timeline container's own class contract), and ran the core suite in a worktree at this head.

The shape is good: the tap is a genuine side branch, nothing is created until start(), the rAF loop re-reads the hook off the live preview window instead of caching it, and the bars are written straight to the DOM. One blocking defect, one limitation I think the PR body overstates, and a small perf note.

Blocking — Master and the group meters sit on opposite sides of the monitor fader

createLevelTap(ctx, this._masterGain) hangs the master analyser off _masterGain's output (webAudioTransport.ts:526-530), and _masterGain.gain.value is the preview's monitor control: applyMasterGain() writes this._masterMuted ? 0 : this._masterVolume (:809-810), fed by setVolume / setMuted (:775, :804), which are wired to the Studio player's audioVolume / audioMuted — a per-user UI preference in localStorage (playerStore.ts:263-264, 409-415), reachable with the M key (usePlaybackKeyboard.ts:131).

The group taps hang off output (:500), which is upstream of _masterGain (output.connect(this._masterGain), :435).

Measured at this head, with your own mock context (probe appended to webAudioTransport.test.ts, one group vo playing):

masterGain.connect targets:  ["tap-mix"]
groupOutput.connect targets: ["masterGain","tap-mix"]
masterGain.gain.value at unity: 1
after setVolume(0.5):           0.5
after setMuted(true):           0
group output gain untouched:    1

So the monitor fader sits between the two sets of taps:

  • Preview volume at 50% → the Master bar reads ~6 dB under the groups that feed it. The dB scale printed next to it is then wrong relative to the strip beside it.
  • Press MMaster flatlines while every group meter keeps bouncing.

That second one is the reason I'm calling this blocking rather than a taste question. Under "the meter shows the program" the Master strip is wrong; under "the meter shows what you're hearing" (which is how I read "moving with the real playback level in the preview") the group strips are wrong. There's no reading of the widget where both halves are right, and the divergence is one keystroke away in the default configuration.

Smallest fix I can see that keeps your "audible path is unchanged" property intact: split the two roles that _masterGain is currently playing. Add a monitor gain downstream — everything keeps connecting to _masterGain exactly as it does now, _masterGain.connect(_monitorGain), _monitorGain.connect(destination), and applyMasterGain() writes to _monitorGain. The master tap then stays exactly where it is and becomes a program tap, the group taps already are one, and nothing else in the file changes its connection target. One extra unity gain node in the chain.

(The other direction — moving the master tap upstream of the fader — needs a summing node anyway, because ungrouped clips connect straight to _masterGain, so it lands in the same place.)

Important — the meters measure the WebAudio graph, not the audible output

scheduleWebAudioForActiveClips selects document.querySelectorAll("audio[data-start]") (init.ts:4054, and the paused-side sibling at :3915). A <video>'s soundtrack never enters the graph — it plays natively, with onSetVolume setting el.volume directly (:4212-4229). So it never reaches _masterGain and never reaches a tap.

Consequences:

  • A composition mixing audio clips with a video that has sound: the strip renders (there's an <audio>, so hasProjectAudio is true) and Master under-reports by exactly the video's contribution, silently.
  • Same for any <audio> clip that falls back to native playback — the decode-only route that fails to decode stays audible but is invisible to the tap.

Worth noting the flip side, because I think it's right: hiding the strip entirely for a video-only project (hasProjectAudio using isAudioTimelineElement, which excludes video) is the correct call here, not an oversight — showing a strip that could only ever read silence would be worse. It's the mixed case that misleads.

I'd either scope the PR-body claim ("the real playback level" → the audio-clip and group busses) or say the limitation out loud in the levelTap doc comment. This matters more once the faders land: someone will set a level against a Master reading that's missing a track.

Nit — readLevels() re-scans the preview document once per group, per frame

isPresent() calls resolveGroupElement(doc, groupId) (:468), which runs a full-document querySelectorAll("hf-audio-group[data-hf-render-id]") before falling back to getElementById (audioGroups.ts:100-104). readLevels() calls it for every tapped group, and the rAF loop calls readLevels() every frame with no idle path — it runs at 60 fps while the strip is visible even when the transport is paused and every bar is at rest.

Measured with 3 groups:

one readLevels():                {"querySelectorAll":3,"getElementById":3}
60 readLevels() (=1s at 60fps):  {"querySelectorAll":180,"getElementById":180}

That's 180 full-document scans per second inside the preview iframe — the same document that's rendering playback. And it's redundant for the only consumer: useStrips already derives the strip list from the player store, so Studio never asks for a group it doesn't have an element for. Resolving presence when the group set changes (or memoising it for the current read) would drop it to zero per frame.

Smaller things

  • useMeterLoop's state map never drops entries for strips that disappear (AudioMeterStrip.tsx:78). Bounded by the group count, so it's housekeeping, not a leak that matters.
  • startMetering / stopMetering aren't ref-counted. Fine with exactly one consumer; worth remembering when the fader PR adds a second reader of the same hook.

Verified negatives

Things I went looking for and did not find, so nobody re-derives them:

  • The render path is untouched. init.ts:2746-2750 assigns three closures and nothing else; attachGroupTap is guarded by _metering (:533), which only startMetering() sets, and only Studio calls it. A headless render creates no analyser, no splitter and no extra connect. The player-perf green at this head is consistent, but the guard is the actual reason.
  • The window.__hf global needs no readiness protocol here. The loop re-reads the hook every frame and start()s on identity change (AudioMeterStrip.tsx:81-94), and initSandboxRuntimeModular assigns a fresh object per run while teardown+re-init builds a new transport (init.ts:284-286, :297) — so a preview that mounts late, reloads, or is swapped is picked up on the next frame. This is the one case where the window-flag pattern is fine, because nothing is cached across the boundary.
  • The group id spaces line up. Studio writes audioGroup: groupId where groupId is the hf-audio-group element's id (timelineAudioGroupCreate.ts:251); the transport keys on the same id via audioGroupOf / resolveGroupElement. Strip keys and read().groups keys match.
  • The layout wrapper preserves the Timeline's height contract. The Timeline root is h-full (timelineProviderStateBuilders.ts:82); the new min-w-0 flex-1 wrapper is a stretched item in a flex min-h-0 flex-1 row, so its height is definite and height:100% still resolves. Before, the root was a column flex item shrinking to fit — same final height, and the strip returning null leaves the row with a single child.
  • rAF lifecycle is correct. raf is reassigned at the top of tick before the body, so the cleanup's cancelAnimationFrame(raf) always holds the pending handle; unmount also stops the live hook. No listeners or timers added anywhere in the diff.
  • PR-body test claim reproduces. I ran the core suite in a worktree at this head: webAudioTransport.test.ts 77 passed, and audioMeterMath.test.ts 15 passed. (I couldn't run the component suite in my borrowed rig — cn and @base-ui/react aren't in the snapshot I was linking against. That's my install, not your PR.)

Per the standing rule I didn't treat CI as a gate; the blocker above is a code finding, not a red check — everything is green at this head.

Happy to re-review as soon as the master tap moves. The rest of this is genuinely nice work — the side-branch design and the identity-keyed hook follow are both the right shape.

— Rames

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

fc14441 addresses the monitor-fader finding: packages/core/src/runtime/webAudioTransport.ts now puts _monitorGain downstream of _masterGain, and both level taps stay on the program bus. 38e254924 is only the oxfmt wrap of that same graph.

@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 at 38e2549249f93db769e8af3430591f787a8eabef. The delta over the head I blocked (a07357246) is exactly two commits and two files: fc1444118 plus the oxfmt wrap.

The blocker is resolved. You split the two roles instead of moving the tap, which is the version that leaves every existing connect alone.

I measured it two ways rather than reading it, because the injected-mock tests bypass init() entirely and so can't see the chain they're asserting about.

1 — the real init(), against a stubbed AudioContext (metering started and setVolume(0.5) called before init(), so the ordering at :192 is exercised too):

gain#0   = _masterGain    gain 1      connects -> gain#1, gain#2
gain#1   = _monitorGain   gain 0.5    connects -> destination
gain#2   = tap mix        gain 1      connects -> splitter#3 -> analyser#4, analyser#5
after init (volume 0.5 set pre-init)   _masterGain 1    _monitorGain 0.5
setMuted(true)                         _masterGain 1    _monitorGain 0
setMuted(false)                        _masterGain 1    _monitorGain 0.5
setVolume(99)                          _masterGain 1    _monitorGain 1     <- spec clamp

2 — the group case, same probe I ran in round 1 (one group vo playing), so the two rounds are directly comparable:

round 1 a07357246 round 2 38e254924
masterGain.connect targets ["tap-mix"] ["tap-mix"]
groupOutput.connect targets ["masterGain","tap-mix"] ["masterGain","tap-mix"]
masterGain.gain at unity 1 1
…after setVolume(0.5) 0.5 1
…after setMuted(true) 0 1
monitorGain.gain (unity / 0.5 / muted) 1 / 0.5 / 0
groupOutput.gain 1 1

Both taps now hang off nodes that sit at unity no matter where the preview fader is. M no longer flatlines Master while the group bars keep bouncing, and the audible product is unchanged — the fader changed node, not value.

Two places this shape usually leaks, both clear:

  • Nothing bypasses the new node. Every connect in the file is :190, :191, :361, :439, :450, :459, :651, and :191 is the only one that touches _ctx.destination. So _monitorGain still gates 100% of what the graph carries — mute mutes exactly what it muted before.
  • Nothing else writes the tapped node. _masterGain is assigned once (:188) and never written again; applyMasterGain is the only writer of either fader value, and no file outside webAudioTransport.ts references masterGain at all. There's no second writer that could put a level back onto the program tap.

keeps program taps off the monitor fader is a real pin, not a vacuous one: revert the split and applyMasterGain writes _masterGain.gain.value = 0, so expect(mock.masterGain.gain.value).toBe(1) fails.

Still open from round 1 — all non-blocking, none of it gates this

  1. The PR body claim. This now needs a slightly different edit than I asked for last round, because the fix deliberately decoupled the meters from the monitor fader: "moving with the real playback level in the preview" describes something the widget now intentionally does not do — pull the preview volume down and the bars don't move. Something like "moving with the program level, ahead of the preview volume control" is both accurate and a selling point. The original half stands too: scheduleWebAudioForActiveClips only takes audio[data-start], so a <video>'s soundtrack never enters the graph and a mixed composition under-reports on Master silently.
  2. readLevels() re-scans the preview document once per group per frame — 180 querySelectorAll per second at 3 groups, inside the document that's rendering playback, with no idle path.
  3. useMeterLoop's state map never drops vanished strips, and startMetering/stopMetering aren't ref-counted. Both are fine with one consumer; the second one is worth remembering when the fader PR adds a second reader of the same hook.

Nit

keeps the user's master volume spec-clamped seeds the monitor mock at 1 and expects 1 after setVolume(99), so it passes whether or not setVolume wrote anything. Seed it at a non-unity value — { gain: { value: 0.25 } } — and it distinguishes "clamped to 1" from "never written". The clamp itself is correct; measured 0.5 → setVolume(99) → 1 above. (Separately, applyMasterGain writes the monitor node now — applyMonitorGain would say what it does. Entirely optional.)

What I ran

webAudioTransport.test.ts at this head in a worktree against a borrowed install: 78 passed, which is your 77 plus the new program-tap test. The PR body's "77 passed" is stale by exactly that one.

Per the standing rule I didn't treat CI as a gate — everything above is the code plus the two probes.

Approving. Nice fix: one extra unity node, three lines of production change, and the widget now has a single consistent reading.

— Rames

@miguel-heygen
miguel-heygen merged commit 9742073 into main Sep 21, 2026
62 checks passed
@miguel-heygen
miguel-heygen deleted the hfoss21/audio-meters branch September 21, 2026 20:19
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