Skip to content

fix(macos-helper): set audio probe frameLength before PCM copy - #3821

Draft
jipeng6036-del wants to merge 1 commit into
callstack:mainfrom
jipeng6036-del:fix/3771-audio-probe-framelength
Draft

jipeng6036-del wants to merge 1 commit into
callstack:mainfrom
jipeng6036-del:fix/3771-audio-probe-framelength

Conversation

@jipeng6036-del

@jipeng6036-del jipeng6036-del commented Oct 19, 2026 •

Copy link
Copy Markdown

Summary

audio-probe always reported -90 dBFS because AudioProbeStatusWriter.add(sampleBuffer:) called CMSampleBufferCopyPCMDataIntoAudioBufferList while AVAudioPCMBuffer.frameLength was still 0, so every destination mDataByteSize was 0 and the copy returned -12731.

This sets frameLength before the copy. AudioProbeStatusWriter is internal so the regression can call add(sampleBuffer:) under @testable. Two files; no public API change. Fixes #3771.

Validation

  • Base: 256aa126 (upstream main). Head: b8e31769.
  • Diff: git show --shortstat b8e31769 → 2 files, +131/−2.
  • Local host is Linux, so pnpm test:macos-helper / live ScreenCaptureKit audio-probe were not run here. CI's macos-helper gate should run swift test --package-path apple/macos-helper, including AudioProbeTests.testAddSampleBufferRecordsANonSilentPeakFromRealPCM (synthetic float PCM → heard and peak > -90). That test fails if frameLength is set after the copy.
  • check:affected --run --head not run locally (no Swift toolchain). Relying on CI to serialize the macos-helper gate on this head.
  • swift test on macOS and a live audio-probe run haven't been done yet. I'll mark this ready once they have.
  • Risk: none beyond the helper path covered by the new unit test.

View guided diff

AVAudioPCMBuffer starts with frameLength 0, so mDataByteSize stays 0
and CMSampleBufferCopyPCMDataIntoAudioBufferList returns -12731. Set
frameLength first, then copy. Add a regression that feeds a real float
PCM sample buffer through add(sampleBuffer:) and asserts a non-silent
peak.

Fixes callstack#3771
@jipeng6036-del
jipeng6036-del marked this pull request as draft October 19, 2026 13:52
@thymikee

Copy link
Copy Markdown
Member

This change is not ready to merge yet. At b8e3176 the fix looks right, but nothing shows it works on a real audio capture. The change affects the ScreenCaptureKit audio path in AudioProbe.swift. The PR body says no Swift tests and no live audio probe ran. Without a live run, a silent drop back to -90 dBFS would pass every CI lane.

Please run swift test --package-path apple/macos-helper on a macOS host (13 or newer, Screen Recording granted) and post the AudioProbeTests pass. Then build the helper from this head and run agent-device-macos-helper audio-probe for about 3 s while system audio plays, for example afplay /System/Library/Sounds/Glass.aiff in a loop. The output must show heard:true and at least one peakDbfs/rmsDbfs bucket above -90. Run it once more in silence and show heard:false. Post the commands and the JSON output in the PR.

Not blocking, and you can take or leave it: AudioProbeTests.swift builds a mono buffer, but ScreenCaptureKit sends two-channel non-interleaved float32, and the checks are loose enough that a copy filling one channel would still pass. Would a stereo case with a different level per channel and an exact peak check (for example -6 dBFS) make the test stronger?

CI shows one check pending and none failing, but I could not confirm it is the macos-helper gate, so a green result may not mean the new test ran. There are no conflicts. Once the macOS test and live run are posted, mark the draft ready.

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.

macOS helper audio-probe always reports -90 dBFS: AVAudioPCMBuffer frameLength set after the copy

2 participants