Skip to content

feat(gpui): normalize exported audio to -14 LUFS - #2777

Open
YoavLevavi wants to merge 4 commits into
CapSoftware:mainfrom
YoavLevavi:feat/gpui-export-loudness
Open

YoavLevavi wants to merge 4 commits into
CapSoftware:mainfrom
YoavLevavi:feat/gpui-export-loudness

Conversation

@YoavLevavi

@YoavLevavi YoavLevavi commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

GPUI port of #2379 (the Tauri version).

  • cap_export::loudness: measures integrated loudness and true peak, then renders the export's audio to the -14 LUFS social target (gain capped, peak-limited), replacing the file in place.
  • AudioConfiguration::normalize_loudness (defaults off, old projects deserialize unchanged).
  • GPUI: a Normalize Loudness toggle in the editor's audio tab; run_export runs the pass after MP4 and MOV exports. GIF and cursor-only exports skip it. A failed pass keeps the unnormalized export and logs a warning.

Testing: cargo test -p cap-export --lib loudness, cargo test -p cap-project --lib, cargo check in apps/desktop-gpui.

A/V sync check (CAP_LOUDNESS_AV_FILE=<file> cargo test -p cap-export --lib normalizing_a_real_export_keeps_av_sync -- --ignored --nocapture). Each run normalizes a copy, compares every video packet's pts/dts (µs) and size, and cross-correlates the decoded audio before and after:

Cap MP4 export:   before -17.48 LUFS (TP 0.60), gain +3.48 dB, after -14.01 LUFS (TP 0.90), video packets 1959 identical, audio lag 0.00 ms
MOV (screen rec): before -30.10 LUFS (TP 0.15), gain +16.10 dB, after -14.06 LUFS (TP 0.89), video packets 259 identical, audio lag 0.00 ms
MOV (iPhone):     before -19.29 LUFS (TP 0.95), gain +7.07 dB, after -14.25 LUFS (TP 0.94), video packets 651 identical, audio lag 0.00 ms
1h47m MP4:        before -18.14 LUFS (TP 1.07), gain +4.14 dB, after -14.22 LUFS (TP 0.91), video packets 89556 identical, audio lag 0.00 ms

The MOVs end 64 samples (1.3 ms) longer because of AAC end padding; the start is sample-exact.

🤖 Generated with Claude Code

RetriggerView in GreptileConfidence Score: 5/5

The reviewed update appears safe to merge.

Summary

Adds optional export audio normalization to -14 LUFS.

  • GPUI exports can normalize MP4 and MOV audio after rendering.
  • The export audio tool measures loudness and replaces audio at a chosen target.

Reviews (2) · Last reviewed commit: "Report the gain the saved audio was rend..." · Reviewed by Greptile

Adds the loudness pass (cap_export::loudness) and an audio
setting to run it after MP4 and MOV exports from the GPUI editor.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@vercel

vercel Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

@YoavLevavi is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

@YoavLevavi

Copy link
Copy Markdown
Contributor Author

@greptileai

@superagent-security superagent-security Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superagent found 1 security concern(s).

Comment thread crates/export/src/loudness.rs Outdated
path = %path.display(),
"Normalizing export loudness"
);
let temp_path: PathBuf = path.with_extension(format!(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Predictable loudness temp file lets a local attacker redirect the export rewrite through a symlink

Loudness scratch file uses predictable <output>.loudness.<ext>; a pre-planted symlink is followed and the target truncated.

Use tempfile in the destination directory and persist after inode verification instead of a predictable with_extension path.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="crates/export/src/loudness.rs">
<violation number="1" location="crates/export/src/loudness.rs:307">
<priority>P2</priority>
<title>Predictable loudness temp file lets a local attacker redirect the export rewrite through a symlink</title>
<evidence>normalize_file_loudness derives its scratch path as `path.with_extension("loudness.&lt;ext&gt;")` (lines 307-310), so the name is fully predictable from the chosen export filename. rewrite_audio then calls `format::output(temp_path)` on that path, which opens/truncates it through any pre-existing symlink, and the result is later renamed over the export. The export directory is normally selected by the user and can be a shared or world-writable location (for example /tmp or a shared volume). Another local user who can create entries there can pre-create `&lt;name&gt;.loudness.mp4` as a symlink to any file the exporting user can write; the loudness pass opens it with victim privileges and truncates/overwrites that target with encoded media. The same predictable name also silently clobbers any unrelated existing `&lt;name&gt;.loudness.&lt;ext&gt;` file. The existing export code already avoids this class by staging through tempfile's randomized names and verifying the staged inode was not replaced (crates/export/src/mp4.rs:1189-1215 and :1161-1177, present at the merge-base), but the new loudness scratch file does not use that mechanism.</evidence>
<recommendation>Create the scratch file with tempfile::Builder::new().tempfile_in(the export's parent directory) (or reuse the existing TemporaryExportOutput/prepare_new_output helper), keep the open file handle, write through it, and persist/replace the destination only after verifying the same inode was not swapped. Do not derive a fixed path with with_extension, and add a Unix regression test showing a pre-existing symlink at the old predictable location is not followed.</recommendation>
</violation>
</file>

@richiemcilroy

Copy link
Copy Markdown
Member

Thanks for porting this to GPUI @YoavLevavi, the toggle and the default-off serde field look good and existing exports stay untouched. A few things before we can merge: 1) crates/export/src/loudness.rs:307 writes to a predictable <name>.loudness.<ext> next to the export, which can clobber a user's own file with that name and is what the open Superagent thread is about. Please create the scratch file with tempfile::Builder in the destination directory and persist it over the export. 2) render_at_target (loudness.rs:327) re-muxes the full video stream on every pass, so a long export can be rewritten three times after the progress bar already shows 100%. Could you iterate on an audio-only scratch file and remux the video only once at the end? 3) In apps/desktop-gpui/src/editor_export.rs:2892 the pass ignores cancel and reports no progress, so please check the cancel flag between passes and surface a short "Normalizing audio" phase in the export dialog. 4) Since this rewrites the audio track, please share a quick A/V sync check of an exported recording with the toggle on (for example a clap or beep test compared against the same export with it off). Thanks again!

- The gain search encodes only the audio, into a private temp directory;
  the video is remuxed once at the end, interleaved by decode time.
- The rewritten export is written to a tempfile::Builder file in the
  destination directory (exclusive, random name), verified to still be that
  same file, then persisted over the export. The old predictable
  <name>.loudness.<ext> path is gone.
- Every pass and streaming loop checks the cancel flag; a cancelled run
  leaves the export untouched and the GPUI export ends as cancelled.
- GPUI shows a "Normalizing audio" phase (cancellable) after rendering.
- Tests: no scratch left and a file with the old name is untouched; cancel
  keeps the export byte-identical; a click stays within 1ms after
  normalization. An ignored test (CAP_LOUDNESS_AV_FILE) checks a real
  export's video timeline and audio offset.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@richiemcilroy

Copy link
Copy Markdown
Member

Thanks for the quick and thorough update @YoavLevavi, this addresses almost everything: the scratch file is now a random tempfile::Builder file beside the export with the inode check before persist, the gain search only re-encodes audio in a private temp dir with a single video remux at the end, cancel is checked between passes and inside the streaming loops, and the export dialog shows a cancellable "Normalizing audio" phase. The new timing and cancel tests are great too. Two small things before we merge: 1) tempfile creates files with mode 0600 on macOS and Linux, and scratch.persist(path) in normalize_file_loudness (crates/export/src/loudness.rs) then puts that file in place of the export. So with Normalize Loudness on, the exported video ends up owner-only, where today it gets the usual umask permissions (0644). Could you copy the original file's permissions onto the scratch file before persisting, e.g. std::fs::set_permissions(scratch.path(), std::fs::metadata(path)?.permissions())?, and add an assert for that to never_touches_a_file_with_the_old_scratch_name_and_leaves_no_scratch? 2) For the A/V sync check, could you run the ignored normalizing_a_real_export_keeps_av_sync test against a real Cap MP4 export (and a MOV one if you can) with CAP_LOUDNESS_AV_FILE, and paste the printed line (gain, video packet count, audio lag) into the PR description? Optional nit: per the comments policy in AGENTS.md, the doc comments on ExportProgress, its variants and ExportPhase::Normalizing just restate the code and can go. Thanks again!

@YoavLevavi

Copy link
Copy Markdown
Contributor Author

Thanks @richiemcilroy, all four are addressed in f2194c1:

  1. Scratch file: the rewritten export now goes to a tempfile::Builder file in the destination directory (exclusive create, random name). Before it is persisted over the export, it is checked to still be that same file (same inode/dev). The predictable <name>.loudness.<ext> path is gone.
  2. One remux: the gain-search passes encode only the audio, into a private temp dir. The video is remuxed once at the end, interleaved by decode time.
  3. Cancel and progress: every pass and streaming loop checks cancel. A cancelled run leaves the export untouched and the export ends as cancelled. The dialog shows a cancellable "Normalizing audio" phase after rendering.
  4. A/V sync: there's a new ignored test, normalizing_a_real_export_keeps_av_sync (CAP_LOUDNESS_AV_FILE=... cargo test -p cap-export --lib -- --ignored). It normalizes a copy, then compares every video packet's pts/dts (µs) and size, and cross-correlates the decoded audio before and after.
    • A Cap export at -17.5 LUFS went to -14.0 LUFS. All 1,959 video packets were identical and the audio lag was 0.00 ms.
    • A 1h47m 1920x1118 screen recording went from -18.1 to -14.2 LUFS (true peak 1.07 → 0.91). All 89,556 video packets were identical and the audio lag was 0.00 ms.
    • A unit test also checks that a click stays within 1 ms of where it was through a normalization.
    • The other two new unit tests: a file using the old scratch name is never touched and no scratch is left behind, and a cancelled run leaves the export byte-identical.

- Merges origin/main, folding its latest-progress coalescing into the
  ExportProgress loop.
- tempfile makes the scratch owner-only, so the original export's
  permissions are copied onto it before it replaces the export (asserted
  in the scratch-name test).
- The A/V check keeps the source extension, so MOV exports are tested as
  MOV. Drops the doc comments on ExportProgress and ExportPhase::Normalizing.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@YoavLevavi

Copy link
Copy Markdown
Contributor Author

Thanks @richiemcilroy, both are done in 74f255a (also merged main):

  1. The scratch file now gets the original export's permissions (set_permissions from metadata(path)) before persist. never_touches_a_file_with_the_old_scratch_name_and_leaves_no_scratch asserts the export keeps its permissions and isn't 0600.
  2. The A/V check results are in the PR description, from runs against a Cap MP4 export, two MOVs and a 1h47m MP4. All have identical video packet timelines and a 0.00 ms audio lag. The test now keeps the source extension, so MOVs are muxed as MOV.

I also dropped the doc comments on ExportProgress and ExportPhase::Normalizing.

@greptileai review

if shortfall.abs() < MIN_ADJUSTMENT_DB || (next - *gain).abs() < f32::EPSILON {
return Ok(true);
}
*gain = next;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Returned gain was never applied

When all three passes finish without reaching the target, render_at_target updates gain after the last render. normalize_file_loudness then returns that new value, although the saved audio used the previous one. This makes callers and manual test output report the wrong adjustment. Keep the last applied gain separate from the next proposed gain.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/export/src/loudness.rs
Line: 517

Comment:
**Returned gain was never applied**

When all three passes finish without reaching the target, `render_at_target` updates `gain` after the last render. `normalize_file_loudness` then returns that new value, although the saved audio used the previous one. This makes callers and manual test output report the wrong adjustment. Keep the last applied gain separate from the next proposed gain.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@YoavLevavi

Copy link
Copy Markdown
Contributor Author

Fixed in c10f698: on the last pass render_at_target now returns without proposing a new gain, so normalize_file_loudness reports the gain the saved audio was rendered with.

@greptileai review

This branch has not been deployed

No deployments
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