test: require real AAC packets from native capture - #83
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7db984191a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| WITH_SYSTEM_AUDIO && | ||
| (audioEvidence.rms < AUDIO_RMS_MIN || | ||
| audioEvidence.peak < AUDIO_PEAK_MIN || | ||
| Math.abs(audioEvidence.frequency - SYSTEM_AUDIO_TONE_HZ) > TONE_TOLERANCE_HZ) |
There was a problem hiding this comment.
When npm run test:wgc-mixed-audio:win enables both system audio and a real microphone, this condition still requires the zero-crossing frequency of the combined signal to be within 10 Hz of 997 Hz. Ambient microphone input—amplified by the configured microphoneGain—can add crossings and fail an otherwise valid mixed-audio recording. Analyze the tone spectrally, isolate the loopback channel, or apply this frequency assertion only to system-audio-only runs.
Useful? React with 👍 / 👎.
| if ( | ||
| WITH_SYSTEM_AUDIO && | ||
| (Number(audioStream.sample_rate) !== AUDIO_SAMPLE_RATE || | ||
| Number(audioStream.channels) !== AUDIO_CHANNELS) |
There was a problem hiding this comment.
On a Windows machine whose default render endpoint is configured for 44.1 kHz, both helpers legitimately encode 44.1 kHz AAC, but this assertion makes test:wgc-audio:win fail. The production format normalization explicitly preserves a nonzero source rate in electron/native/rust/wgc-capture/src/audio/format.rs:43-47 (and the legacy implementation does the same), so 48 kHz is only a property of the tested laptop, not the helper contract; the parity harness repeats the same hardcoded assumption. Compare against the reported endpoint/audio format or allow supported AAC sample rates instead.
Useful? React with 👍 / 👎.
| assert!( | ||
| decoded_bytes >= AUDIO_FRAMES * 4, | ||
| "decoded PCM tail was truncated: {decoded_bytes} bytes" | ||
| ); |
There was a problem hiding this comment.
This length assertion does not prove that the final short write_audio call was retained. The fixture writes 48,000 frames in full chunks and then a final 123-frame chunk; if that last call is dropped, AAC can pad the preceding 48,000 frames to 47 1,024-sample access units (48,128 decoded frames), which still satisfies decoded_bytes >= AUDIO_FRAMES * 4. Validate the decoded tone/content through the expected tail—or choose counts that cannot be masked by AAC padding—so the regression actually detects loss of the trailing chunk it is named for.
Useful? React with 👍 / 👎.
Description
Hardens the Windows native audio regression gates so an AAC metadata-only MP4 cannot pass as a successful audio capture.
This change adds:
stop()waits for an in-flight trailing output callback;eRender/eConsoleendpoint evidence and bounded stimulus cleanup;Production capture, exporter, Electron, version, packaging, and release code are unchanged.
Motivation
The fork.4 release smoke initially appeared to show an exporter dropping AAC. Packet-level inspection instead found that the source MP4 had AAC configuration metadata but zero audio packets. A deterministic active loopback stimulus then proved that the unmodified Rust and legacy helpers both produce real AAC packets and decoded frames; the earlier idle-loopback smoke had not generated system audio.
The compatibility boundary was therefore a validation gap: existing checks could accept an AAC track header without proving packet production, decode, endpoint stimulus, or trailing audio retention. This PR closes that gap without applying an unjustified runtime workaround or generating silent AAC.
Type of Change
Related Issue(s)
None. This is the release-blocker follow-up discovered during the v1.5.0-fork.4 candidate smoke.
Screenshots / Video
Not applicable; there is no UI change and retained local capture/export artifacts were deleted after verification.
Testing
cargo +1.96.1 fmt --checkcargo +1.96.1 clippy --locked --target x86_64-pc-windows-msvc --all-targets -- -D warningscargo +1.96.1 test --locked --target x86_64-pc-windows-msvc— 42 WGC tests passed, including the new WARP/MF and mixer-tail regressionsnpm run test— 489 tests passednpm run lintnpm run i18n:checknpx tsc --noEmitnpm run build-vitenpm run test:browser— 7 tests passednpm run build:native:winnpm run test:wgc-audio:win— Rust and legacy each produced AAC 48 kHz stereo with 235 packets/frames and a fully decoded approximately 997 Hz signalCLOSESCREEN_PARITY_AUDIO=1 npm run test:wgc-parity:win— both backends retained the expected tone in the first, middle, and trailing audible windows across pause/resumenpm run test:wgc-helper:win— explicit video-only/no-audio behavior passed for both backendsnpm run test:wgc-fault:win— bounded shutdown passed for both backendsIndependent execution audit and Codex diff review both passed with no blocking findings. Human listening was not used as evidence; decoded non-silent PCM and frequency analysis provide the deterministic media assertion.
Checklist