fix(windows): low-pass before decimating so 96/192 kHz sources stop aliasing - #644
fix(windows): low-pass before decimating so 96/192 kHz sources stop aliasing#644My-Denia wants to merge 2 commits into
Conversation
…liasing When the source rate is an integer multiple of the AAC target -- 96, 192 or 384 kHz loopback into a 48 kHz encoder -- convertAudioWithGain averaged each group of source frames. A box average is a low-pass FIR, but its stop-band attenuation is far short of what decimation needs: at 96 kHz -> 48 kHz a 36 kHz tone still passes at about 38% (-8.3 dB) and folds onto 12 kHz in the recording. The existing Nyquist test passed only because a two-tap mean has a zero at exactly 48 kHz, the one frequency it checked. Every frame now runs through a Kaiser-windowed sinc before frames are dropped: 80 * factor + 1 taps, beta 8.6, cutoff at 11/24 of the output rate (22 kHz for a 48 kHz output), unity gain at DC. The filter's history has to outlive a single packet, so it moves out of the byte-remainder buffer into AudioDecimatorState. AudioMixer owns one per stream, so system audio and the microphone keep independent histories, and it resets both where it used to reset the remainder (start, beginTimeline, setPaused). pendingFrames() replaces the leftover-bytes accounting and reports the same thing: how far into the current group the stream has got. Output is still exactly floor(frames / factor) whatever the packet boundaries, so nothing downstream sees a different frame count. Measured through the shipped code in float, for factors 2, 3, 4, 6, 12 and 24: -0.00 dB at 19.2 kHz, -6.02 dB at 22 kHz, and a worst stop-band response of -87.2 dB (factor 2) to -87.7 dB (factor 24), at the first sidelobe just above 24 kHz. Trade-offs: - Content between 20 and 24 kHz is attenuated: -6.02 dB at 22 kHz and below -87 dB from 24 kHz. That transition is the price of a stop band deep enough to drop frames safely. - Delay. The FIR is linear-phase, with a group delay of 40 * factor source frames. Each output frame is computed on the last source frame of its group, which takes back factor - 1 of those, so against the output timeline (output frame k at the time of source frame k * factor) a decimated stream lags by 39 + 1/factor output frames: 0.81 to 0.82 ms at a 48 kHz output. A stream that does not decimate has no such lag, so a mix of the two now carries that skew, where the box average's offset was under half an output frame. The suite asserts the value. - Level. The sharp transition needs negative taps, so a decimated peak can exceed its input, which a box average (all taps positive) never allows. Measured on square waves before the clamp: +1.38 to +1.45 dB for a 1 kHz square at the factors tested (2 to 24), and up to +3.82 dB for fundamentals up to 20 kHz. The ceiling for arbitrary bounded input is the taps' absolute sum, 2.22 to 2.23, about +7 dB -- a bound, not a typical value. Output is clamped, and the suite asserts it saturates rather than wraps, but full-scale material with sharp edges can clip where it did not before. - Non-finite input. Input is not sanitised, before or after this change. One NaN or Inf sample, which the box average confined to one output frame, now reaches 80 or 81 output frames (1.67 to 1.69 ms at 48 kHz) before the filter history clears. - Cost follows the source rate, not the factor: per source frame the filter does 80 + 1/factor multiply-adds per channel. In a bench on one development machine, a 10 ms packet from 192 kHz measured a steady-state p99 of 290 to 353 us, and a first-packet p99 (tap design included) of at most 373 us, across three runs; 192 -> 48 kHz and 192 -> 8 kHz steady-state means were within 4% of each other in those three. One of them also covered higher rates: 550 us steady p99 for a 384 kHz source, and 1.08 ms for 768 kHz, above the 1 ms per-packet budget this change set itself; no 768 kHz device was available. A fourth run, taken under background load, put 384 kHz at 1.07 ms, also above that budget, and the two 192 kHz means 5.5% apart. Tests: 95 assertions (18 before). A float layer measures the filter itself, with float32 in and out as WASAPI delivers, so no quantization floor hides the stop band; a PCM16 layer measures what the encoder receives. Beyond the response: bit-identical output under ragged packetization, frame accounting across all-zero packets, reset() equal to a cold start, the delay above, channel mapping and gain on this branch, the overshoot size, a sample-by-sample check of the clamped PCM16 write at gains 1, 2 and 8, and a case that drives the real AudioMixer, the only one that catches a mixer re-initialising the filter per packet. Real hardware, factors 2, 4 and 8: with a USB DAC as the loopback source, its shared-mode format set to 96, 192 and 384 kHz in turn against a 48 kHz target, the helper built from the base and from this branch recorded the same tones. In the fold bins, for 96 / 192 / 384 kHz, a 36 kHz tone (onto 12 kHz) reads -22.25 / -23.90 / -24.28 dBFS before and at most -115.32 / -138.02 / -111.54 dBFS after; a 42 kHz tone (onto 6 kHz) reads -28.20 / -30.40 / -30.96 dBFS before and at most -137.14 / -135.09 / -117.25 dBFS after. The after readings bound what the recordings hold and sit near each recording's floor; they are not the filter's leakage, which the design puts at -123 to -155 dBFS at these points. The 1 kHz reference reads -13.98 or -13.99 dBFS in every recording, and every before reading matches the box average's response to within 0.08 dB. Closes getopenscreen#582
…s it The Windows section already asks that a recording succeeds when the rate is snapped, and that a long take stays in sync. Neither would notice content folding down from above the new Nyquist, which is what getopenscreen#582 is about, so it gets its own item. The results row records the A/B rather than a claim. With a USB DAC as the loopback source at 96, 192 and 384 kHz in turn, the decimation branch runs at factors 2, 4 and 8, and the helper built from each side gives, for a 36 kHz tone folding onto 12 kHz, -22.25 / -23.90 / -24.28 dBFS before and at most -115.32 / -138.02 / -111.54 dBFS after; for 42 kHz folding onto 6 kHz, -28.20 / -30.40 / -30.96 before and at most -137.14 / -135.09 / -117.25 after -- bounds on what the recordings hold, set by the recording chain rather than by the filter's stop band. The 1 kHz reference is -13.98 or -13.99 dBFS in every recording, so what went away is the alias and not the signal. The item also writes down the traps that cost time here: a device's current format is not its capability, so check the format list and every endpoint; loopback reports the shared-mode format, so exclusive-mode support is not enough; and a probe tone has to sit well inside what AAC keeps.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe integer-factor AAC downsample path now uses a stateful Kaiser-windowed sinc decimator. Mixer sources retain independent decimator state across packets. Tests cover filtering, state behavior, signal integrity, mixer wiring, and Windows validation. ChangesAudio decimation
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant AudioMixer
participant convertAudioWithGain
participant AudioDecimatorState
AudioMixer->>convertAudioWithGain: append system or microphone packet
convertAudioWithGain->>AudioDecimatorState: consume source frames
AudioDecimatorState-->>convertAudioWithGain: filtered output frames
convertAudioWithGain-->>AudioMixer: converted PCM frames
Merge Risk: ⚪ Minimal · up to This change replaces box averaging with a proper anti-alias filter before downsampling high-rate Windows audio to 48 kHz, removing audible alias artifacts while keeping frame accounting unchanged. No unresolved defects were identified, and the behavior is covered by new automated tests and hardware measurements, so it is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…models and upstream fixes (#13) Capturia 2.1: new editor and capture features, plus the upstream open PRs that were worth taking. ## New features - **Record an area of the screen**: an Area tab in the source picker opens an overlay on the chosen display. You drag, move and resize a rectangle, and it shows the live size in physical pixels. The rectangle is validated and clamped in the main process, and the recording opens already cropped to it. Auto-zoom stays inside the area. Not offered on Wayland. - **Saved looks**: save the current appearance (background and frame, camera layout, cursor, caption style, and optionally the format) as a named preset, apply it in one undo step, and star one as the default for new projects. Regions, trims, zooms, crop and the transcript are never touched. - **Zooms at flagged moments**: Auto-enhance adds a zoom at every moment flagged while recording, using the same placement rules as auto-zoom. A flag that falls in a trim or on an existing zoom is reported, not duplicated. - **Right-click menu** on region pills and clips: Copy, Paste at playhead, Split, Delete. These call the same functions as the keyboard shortcuts. Also fixes Ctrl+C on audio pills, which did nothing before. - **Poster frames**: the project list and media cards show real thumbnails. They are generated by ffmpeg in the main process, cached, and made one at a time. - **Speech model choice**: Fast / Balanced / Accurate in AI settings. Each model is pinned to a SHA-256 digest and verified before it becomes active, and a failed switch keeps the previous model. - **Recordings folder**: choose where new takes are saved. The folder is set only through the OS picker. In that folder, only files Capturia itself names are reachable, after resolving symlinks, and it is never auto-cleaned. If the folder is unavailable, the app offers to use the default before the take starts. - **Pre-release update channel**: opt-in, and it never downgrades (`allowDowngrade` stays false). ## Taken from upstream open PRs Each one was rebuilt on our code where it no longer applied, and each carries its `Upstream-PR:` trailer: getopenscreen/openscreen#302, #386, #519, #520, #571, #617, #632, #640, #641, #642, #644. - #617 drops `node_modules` from `app.asar`. Verified: every npm dependency is bundled by Vite, since externals are Node builtins plus `electron`. `electron-updater` is a bundled chunk, and native addons load from `resourcesPath`. ## Fixes - **Windows Store verify step**: it looked the package up by the pre-rename name, `EtienneLescot.OpenScreen`, which is what failed the RC.3 Store job. It now reads the name from the generated `AppxManifest.xml`. - **Linux export on Intel Arc**: iHD accepts the dmabuf and then returns EIO on every encode, so every hardware export died at the first frame. Each export now probes one real frame and falls back to software if it fails. The mapped frame is also freed when `send_frame` fails. - **Windows microphone drift**: the 44.1→48 kHz path rounded every packet on its own, which added up to 3.75 s/h of growing mic lag. It now carries the position across packets with exact integer totals. 88.2/176.4/352.8 kHz devices now snap to 44.1 kHz, so they go through the anti-alias decimator. - **PipeWire test**: the vendored SPA 1.0.5 compares 64-bit values through an `int`, so the old probe modifier matched Intel X_TILED. The test now uses a modifier that cannot collide. CI now runs this crate's tests. ## Review and audit The integrated branch got an independent security audit and a separate bug hunt. Both were read-only, and every finding was verified by tracing the code. Fixed here: - **Self-update**: it could install a version other than the one the dialog named, or error out instead of falling back to "View Release". It now self-updates only when electron-updater's version matches. - **Recordings folder**: - The writable check always passed on Windows, because libuv ignores directory ACLs. It now creates and deletes a real probe file. - Renderer-named writes are contained after resolving symlinks. - A take keeps the path it opened with, so changing the folder's availability mid-take no longer reports "missing on disk". - The folder cannot be changed while a take is running. - **Poster cache**: one entry per source file, with no flicker when the duration arrives. - **Speech models**: switching is single-flight, and a settings dialog reopened mid-download joins the running download. - **Timeline**: a shift-click that deselects a pill no longer leaves it focused, which had made the menu delete the wrong pill. - **Area recording**: a flag zoom with no telemetry now centres on the recorded area. - **Saved looks**: applying a look is optimistic, so an edit made during its save is no longer lost. - **Saved-looks probe document**: it was invalid at import time. Caught in review before it could crash the editor. ## Verification - Both tsc projects exit 0. Biome is clean; the 26 warnings are the same as on main. The i18n check passes, with real translations in all 13 locales. - Vitest: 255 files, 3099 passed, 1 skipped, on the integrated branch. - Rust: compositor 216 lib tests plus integration tests, and pipewire-capture 84 tests. Both pass locally. - C++ `audio_sample_utils_test`: 97/97 under g++ on Linux, using stub headers. MSVC coverage comes from the `build.yml` dispatch on this branch, which never publishes without `release_tag`. - Every agent-reported claim was re-checked independently. For example, the model digests were checked against Hugging Face's LFS oids, and the electron-updater downgrade path was read in 6.8.9. ## Release note The speech-model change adds a `--dtw-preset` flag to the whisper helper. An older helper ignores unknown flags, so Balanced keeps working. The 2.1 release must still be cut **after** `build-whisper-stt.yml` has finished on main, so the installers stage a helper that understands the flag. Upstream-PR: getopenscreen/openscreen#302 Upstream-PR: getopenscreen/openscreen#386 Upstream-PR: getopenscreen/openscreen#519 Upstream-PR: getopenscreen/openscreen#520 Upstream-PR: getopenscreen/openscreen#571 Upstream-PR: getopenscreen/openscreen#617 Upstream-PR: getopenscreen/openscreen#632 Upstream-PR: getopenscreen/openscreen#640 Upstream-PR: getopenscreen/openscreen#641 Upstream-PR: getopenscreen/openscreen#642 Upstream-PR: getopenscreen/openscreen#644
Summary
Bug. When the capture rate is an integer multiple of the AAC target — 96,
192 or 384 kHz loopback into a 48 kHz encoder —
convertAudioWithGaindecimated by averaging each group of
factorsource frames. A box average is alow-pass FIR, but its stop-band attenuation is far short of what decimation
needs: at 96 kHz → 48 kHz a 36 kHz tone still passes at about 38% (−8.3 dB) and
folds onto 12 kHz in the recording. The existing Nyquist test passed only
because a two-tap mean has a zero at exactly 48 kHz, the one frequency it
checked.
Fix. Every frame now runs through a Kaiser-windowed sinc before frames are
dropped:
80·factor + 1taps, β = 8.6, cutoff at 11/24 of the output rate(22 kHz for a 48 kHz output), unity gain at DC. The filter's history outlives a
packet, so it moves out of the byte-remainder buffer into
AudioDecimatorState.AudioMixerowns one per stream — system audio and microphone keep independenthistories — and resets both where it used to reset the remainder (
start,beginTimeline,setPaused).pendingFrames()replaces the leftover-bytesaccounting. Output stays exactly
floor(frames / factor)however the input ispacketized, so nothing downstream sees a different frame count.
Filter response, measured through the shipped code in float:
Trade-offs
22 kHz and below −87 dB from 24 kHz. That transition is the cost of a stop
band deep enough to drop frames safely.
40·factorsourceframes. Each output frame is computed on the last source frame of its group,
which takes back
factor − 1of those, so against the output timeline(output frame k at the time of source frame
k·factor) a decimated streamlags by
39 + 1/factoroutput frames: 0.81–0.82 ms at a 48 kHz output. A48 kHz → 48 kHz stream does not decimate and has no such lag, so mixing the
two now carries that skew; the box average's offset was under half an output
frame. The suite asserts the value.
can exceed its input; the box average's all-positive taps never allowed that.
Measured before the clamp: +1.38 to +1.45 dB for a 1 kHz square at the factors
tested (2–24), and up to +3.82 dB for square fundamentals up to 20 kHz. The
ceiling for arbitrary bounded input is the taps' absolute sum, 2.22–2.23
(about +7 dB) — a bound, not a typical value. Output is clamped, and the
suite asserts saturation rather than wrap-around, but full-scale material
with sharp edges can clip where it did not before.
this change. One NaN/Inf sample, which the box average confined to one output
frame, now reaches 80 or 81 output frames (1.67–1.69 ms at 48 kHz) before the
filter history clears.
multiply-adds per source frame per channel. Bench on one development machine,
per 10 ms packet:
(tap design included) at most 373 µs. 192 → 48 kHz and 192 → 8 kHz
steady-state means were within 4% of each other in those three.
768 kHz 1.08 ms, above the 1 ms per-packet budget this change set itself
(bench only — no 768 kHz device was available).
1.07 ms, also above that budget, and the two 192 kHz means 5.5% apart.
Related issue
Fixes #582
Type of change
Release impact
Desktop impact
Screenshots / video
Not a visual change — it alters recorded audio content. Measurements are under
Testing.
Testing
npm run build:native:winfrom a deleted build directory: exit 0, 95assertions pass (18 before). No new compiler warnings; the six present are
pre-existing and identical in the base commit's build.
quantization floor hides the stop band): passband, the −6 dB point and the
worst fold-band response for factors 2, 3, 4, 6, 12 and 24.
192 kHz sources, each below −60 dB, with passband controls at 1 and 15 kHz.
accounting across all-zero packets,
reset()equal to a cold start, the39 + 1/factordelay, channel mapping and gain on the decimation branch, anda case that drives the real
AudioMixer.half an LSB) of the clamped PCM16 write on both channels at gains 1, 2 and 8.
Eleven deliberate mutations, each run on an isolated copy with the tests it
should break declared in advance, failed exactly those tests; a comment-only
control stayed green. They include restoring the box average, resetting the
filter every packet, giving
AudioMixer::appenda fresh decimator per call(caught only by the real-mixer case), and letting the PCM16 write wrap instead
of clamp.
Real hardware A/B (factors 2, 4 and 8)
A USB DAC was the default render endpoint, its shared-mode format set to
96000, 192000 and 384000 Hz in turn, so WASAPI loopback delivers that rate into
a 48 kHz target (factors 2, 4 and 8); the helper's own
audio-formateventconfirmed the rate in every run. The helper was built from the base commit and
from this branch, driven the way the app drives it (config JSON argument,
stopon stdin), and each recorded the same tone plus a 1 kHz reference. Fold-bin
readings:
the filter's leakage. Those readings sit near each recording's floor, while
the filter alone predicts −123 to −155 dBFS at these points — below five of
the six readings, and above the sixth (192 kHz, 36 kHz tone: −126.3 dBFS
predicted, −138.02 read), a residual the PCM16 + AAC chain evidently does
not carry.
alias went away and the signal did not.
pre-fix reading matches the box average's response (relative to the
reference) to within 0.08 dB.
(tracks are whole 1024-sample AAC frames); that cumulative output frame
counts do not drift is asserted by the suite.
The app's own record flow (source picker, system audio, record, pause/resume,
stop, editor playback) was also run end to end at 48 kHz, where nothing
decimates. A checklist item and a results row record the recipe.
Not covered
own flow ran only at 48 kHz.
and 8. A write that wrapped only above about 9 FS, or a clamp applied before a
gain below 1, would pass it. The microphone gain is a fixed 1.4 today.
Summary by CodeRabbit
Bug Fixes
Testing
Documentation