Skip to content

feat(desktop): record system audio + mic (ScreenCaptureKit) so calls capture both sides - #3

Open
alonmuroch wants to merge 1 commit into
feat/layout-v0.4from
feat/system-audio-recording
Open

feat(desktop): record system audio + mic (ScreenCaptureKit) so calls capture both sides#3
alonmuroch wants to merge 1 commit into
feat/layout-v0.4from
feat/system-audio-recording

Conversation

@alonmuroch

Copy link
Copy Markdown
Contributor

What & why

The recorder captured the microphone only, so on a call the other person's voice — which comes out of the speakers as system audio — was never recorded; conversations came out as near-silence (only the local half). This adds system-output audio capture via ScreenCaptureKit (macOS 13+) and mixes it with the mic into the single mono WAV, so a two-way call records both sides.

The recently-fixed mic selection / permission logic, the silence guard (recording-guard.ts), and the native pill are untouched — this is purely additive.

How

  • system_audio.rsSCStream audio tap via raw objc (same ClassDecl/msg_send! path as overlay_native.rs; no new deps). SCShareableContentSCContentFilter (first display, all apps) → SCStreamConfiguration.capturesAudio = YES (+ excludesCurrentProcessAudio). A BrainsSCAudioDelegate bridges the async stream:didOutputSampleBuffer:ofType: callback → a mono-f32 ring buffer; PCM is pulled via CMSampleBufferGetAudioBufferListWithRetainedBlockBuffer (planar + interleaved float32), downmixed + resampled to the WAV rate. The delegate points at a heap SinkCtx via an ivar, owned by SystemAudioCapture for the stream's whole life so the callback can't touch freed memory. ScreenCaptureKit/CoreMedia force-linked.
  • Permission gateensure_screen_permission() mirrors ensure_mic_permission() (CGPreflight/CGRequestScreenCaptureAccess). Non-fatal: on deny / <macOS 13 / SCStream start failure → degrade to mic-only, recording why.
  • audio_mix.rs — pure, unit-tested DSP: downmix_to_mono, resample_linear, mix_sum_limit (sum + soft-clip so simultaneous speech can't wrap), to_i16_with_peak.
  • recorder.rs — mic callback now pushes into a ring; a mixer/writer loop drains both rings, mixes, writes the mono WAV, and feeds CAPTURE_PEAK from the mixed signal (so the silence guard now means "neither side has sound"). Ordered teardown: stop SCStream → stop mic → finalize WAV.
  • Capture-mode signalrecorder_capture_mode()mic+system | mic-only-denied | mic-only-unsupported, surfaced as a subtle "Mic only ⓘ" note in the toolbar.

Verified here

  • cargo check clean; cargo clippy — no new findings on the changed files (only the shared cocoa-crate deprecation + msg_send! cargo-clippy cfg warnings that the pre-existing overlay_native.rs already emits); cargo fmt --check OK.
  • npm run check0 errors (140 pre-existing warnings, unchanged).
  • New DSP unit tests (9) + existing recorder tests (9) pass. (One flaky diagnostics latency test is pre-existing and unrelated.)

Needs a live run (can't be tested here)

  • The actual SCStream Screen-Recording permission prompt + grant flow.
  • Real two-way call capture and mix quality (levels, resample artifacts).
  • SCStream teardown under load (no leaked stream/threads across many start/stop cycles).
  • The mic-only fallback actually triggering on a denied grant.

Safety caveats

  • The delegate→mixer bridge is the riskiest part. stop() waits (bounded) on stopCaptureWithCompletionHandler before freeing the SinkCtx, and clears the ivar first — mitigating but not 100% eliminating a theoretical in-flight-callback race. Worth a careful live soak before merging past stage.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V2pJsw7KBLViKjXGGo4Nwg

…capture both sides

On a call the far end's voice comes out of the speakers as system output audio;
the mic only hears the local user. Add a ScreenCaptureKit (macOS 13+) SCStream
audio tap and mix it with the existing cpal mic capture into the single mono WAV,
so a two-way conversation records both sides.

- system_audio.rs: SCStream audio capture via raw objc (SCShareableContent →
  SCContentFilter → SCStreamConfiguration.capturesAudio). A BrainsSCAudioDelegate
  (ClassDecl, same pattern as overlay_native.rs) bridges the async
  stream:didOutputSampleBuffer:ofType: callback → a mono f32 ring buffer via a
  heap SinkCtx held as an ivar; owned for the stream's lifetime so the callback
  can't touch freed memory. ScreenCaptureKit/CoreMedia force-linked.
- Screen-Recording permission gate ensure_screen_permission() mirrors
  ensure_mic_permission (CGPreflight/RequestScreenCaptureAccess). NON-FATAL: on
  deny / <macOS 13 / SCStream start error, degrade to mic-only.
- audio_mix.rs: pure, unit-tested DSP — downmix_to_mono, resample_linear,
  mix_sum_limit (sum + soft-clip), to_i16_with_peak.
- recorder.rs: mic callback now pushes into a ring; a mixer/writer loop drains
  both rings, sums+limits, writes the mono WAV and feeds CAPTURE_PEAK from the
  MIXED signal. Ordered teardown stops SCStream then mic then finalizes.
- recorder_capture_mode() command + a subtle "Mic only" note in the toolbar so
  the user knows when the far end isn't captured.
- Mic selection/permission logic, silence guard, and pill are untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V2pJsw7KBLViKjXGGo4Nwg
liorrutenberg added a commit that referenced this pull request Aug 7, 2026
…it, dead code removal

P1 #1 DOUBLE IDENTITY on interactive runs: prompt.ts now composes contract.md + skill only — identity.md is delivered by the context engine at spawn via details.app (verified via transport.rs scope delivery).

P1 #2 build-context-manifest.mjs: empty details {} now fails explicitly — "declare app or agent, or remove details entirely".

P1 #3 restore dropped identity.md behavior: "Cycle, not library" and "Situational" sections now live in identity.md.

P1 #4 README: document all four runtime composition points (agents.ts, triggers.ts, panels.svelte.ts, prompt.ts).

P1 #5 tab-strip state split: layout owns which tab is open (layout/tabs.svelte.ts → exoTabState), runtime owns what each tab has read (runtime/panels.svelte.ts → exoReadLedger).

P2 #6 build-context-manifest.mjs: array elements validated as typeof string.

P2 #7 dead boardSkill remnants removed from contract.md.

P2 #8 dead AGENT_KIND exports removed from all four persona/cycle files; dead IrisData/MoData re-exports removed from personas/index.ts.

P2 #9 stale references: panel-reads.ts → reads.ts, skills/contract.md → contract.md.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
liorrutenberg added a commit that referenced this pull request Aug 7, 2026
…it, dead code removal

P1 #1 DOUBLE IDENTITY on interactive runs: prompt.ts now composes contract.md + skill only — identity.md is delivered by the context engine at spawn via details.app (verified via transport.rs scope delivery).

P1 #2 build-context-manifest.mjs: empty details {} now fails explicitly — "declare app or agent, or remove details entirely".

P1 #3 restore dropped identity.md behavior: "Cycle, not library" and "Situational" sections now live in identity.md.

P1 #4 README: document all four runtime composition points (agents.ts, triggers.ts, panels.svelte.ts, prompt.ts).

P1 #5 tab-strip state split: layout owns which tab is open (layout/tabs.svelte.ts → exoTabState), runtime owns what each tab has read (runtime/panels.svelte.ts → exoReadLedger).

P2 #6 build-context-manifest.mjs: array elements validated as typeof string.

P2 #7 dead boardSkill remnants removed from contract.md.

P2 #8 dead AGENT_KIND exports removed from all four persona/cycle files; dead IrisData/MoData re-exports removed from personas/index.ts.

P2 #9 stale references: panel-reads.ts → reads.ts, skills/contract.md → contract.md.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sebastian-ssvlabs added a commit that referenced this pull request Aug 11, 2026
#1 docs/CHROMIUM.md described the architecture this PR deliberately did not ship.
The MCP section, the tool table and the whole Layout table named files that do not
exist. Rewritten against the code: the three IPC commands that replaced the
server, the real file map, and why each command takes a `Window`. The crash
forensics and the pump section are kept — they were the parts worth having.

#2 The authorization ledger guarded nothing. `attach`/`is_attached` shipped with
tests and a doc calling itself the gate agent tools must consult, while nothing
called any of it — worse than no gate, because it reads as a control in review and
answers "allowed" at runtime. Deleted. What actually keeps the CDP commands
app-only is now real: each takes a `Window`, which the op table classifies as
opaque and REFUSES over the remote WS transport. That was the live hole — arbitrary
JavaScript in a signed-in Google session, reachable from a socket.

#3 cef-fetch fell back to an unpinned CEF on a warning nobody reads inside a
gigabyte of clone output, producing exactly the framework/bindings mismatch its own
header calls undefined behaviour. Hard-fails now, naming the tag and the crate
version it must match.

#4 CI compiled none of it. `cargo test --workspace` (the engine suites, including
report.rs's — kept outside the feature gate precisely so CI would run them, which
it then didn't) and `cargo check -p brains-browser --features chromium`, which is
what keeps ~2,000 gated lines from rotting.

#5 `page_target_for_url` could resolve to the wrong tab — two Gmail profiles are
two page targets on one origin, and `find` took whichever came first, putting one
account's mail into a conversation about the other. Targets are now pinned per tab
on first resolution and looked up by id after; resolution skips ids another tab
already owns.

#6 One OS thread per delayed pump request, spawned continuously during load and
input. Replaced with the single timer the safety pump already owned, waiting on a
condvar with a next-deadline slot.

#7 `set_visible(true)` never restored the container's frame, so a view whose panel
had not re-measured stayed parked off-screen while nominally visible. The frame is
remembered on hide and restored on show — the OS-webview backend has no such
asymmetry, and the panel is written not to have to know.

#8 `views::open` registered the tab after dispatching the closure whose failure
path removes it. Inserted first.

#9 Same-site now also requires a USER GESTURE before navigating in place: the panel
has no address bar, and a script-initiated hop needs none. `TWO_LABEL_SUFFIXES`
gains the user-content hosts, which matter more than the country ones — anyone can
take a label under `github.io`, and treating two of them as one site is the actual
risk. Test added.

#10/#11 The unauthenticated CDP port and the `--use-mock-keychain` + `no_sandbox`
pair are now named on a release-blocker list in the doc, with what has to change.

#12 The dev-token file was created at the umask and chmod'd afterwards, leaving it
world-readable in between. Opened 0600, directory 0700.

#15 One `reqwest::Client` for the process rather than one per call; the sentinel
parse drops a fragment; "flipped bottom-left" corrected (in AppKit flipped means
TOP-left) — the arithmetic was right, the word inverted it; and `init` latches on
ATTEMPTED so a failed attempt cannot re-enter CefInitialize.

Not done, deliberately: #13 (splitting the CI commit) is the owner's call and the
offer stands; #14's unused CDP driving surface is named in the doc's Next section
rather than trimmed, since the agent bridge is what lights it up.

Gates: fmt 0, clippy 0, workspace tests green, chromium check clean, svelte-check
725/0/0, 1659 frontend tests, all five lints.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
];
// Wait (bounded) for stop to acknowledge so no more sample callbacks fire
// before we free the context.
let _ = srx.recv_timeout(Duration::from_secs(5));

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.

🟡 stop() frees SinkCtx via Box::from_raw even when stopCaptureWithCompletionHandler times out after 5s, and the brains_ctx ivar clear is not synchronized with the sample-handler queue — an in-flight on_sample_buffer that loaded the pointer before the clear dereferences freed memory (use-after-free). Synchronize by dispatch_sync-ing an empty block on the sample queue before freeing, and leak the ctx instead of freeing it on the timeout path.

} else {
Vec::new()
};
if !mic.is_empty() {

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.

🟡 Mixer loop writes nothing when mic_ring is empty (if !mic.is_empty()), and drain_ring_up_to(&sys_ring, mic.len()) ties far-end consumption to mic delivery — if the mic stream stalls or errors mid-recording (err_fn only logs), captured system audio is silently discarded, sys_ring backlogs to its 30s cap and drops far-end samples, and the WAV stops growing. On mic underrun, pad the mic slice with silence and keep draining/writing sys_ring.

Comment thread src/routes/+page.svelte
try {
recWav = await getTransport().invoke<string>("recorder_start");
beginRecordingTimer();
void refreshCaptureMode();

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.

🟡 refreshCaptureMode() is invoked once, immediately after recorder_start resolves, but CAPTURE_MODE is reset to MODE_UNKNOWN in recorder_start and only finalized on the capture thread after device probing plus SCStream startup (up to two 10s timeouts) — the one-shot fetch almost always reads "unknown", so the Mic only note effectively never renders. Poll until the mode is not "unknown" (or emit an event from the capture thread).

Comment thread src/routes/+page.svelte
style="color:#9A6A00;font-weight:700;"
title="No audio detected — check your mic">⚠ No audio</span
>
{:else if recCaptureMode.startsWith("mic-only")}

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.

🟡 New user-facing UI text ("Mic only ⓘ" plus both tooltip strings) is hardcoded; CONTRIBUTING.md i18n section requires new UI text to be added to both messages/en.json and messages/zh-CN.json, and neither file is touched by this PR.

// 6. Register the audio output on a dedicated serial dispatch queue.
// SCStreamOutputTypeAudio == 1.
let queue_label = std::ffi::CString::new("ai.mybrains.scaudio").unwrap();
let queue: id = dispatch_queue_create(queue_label.as_ptr(), std::ptr::null_mut());

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.

🟡 The queue from dispatch_queue_create is never released — not in stop(), not on the addStreamOutput/startCapture failure paths — leaking one dispatch queue per capture start/stop cycle (auto-record cycles these per meeting). Store it in SystemAudioCapture and dispatch_release it during teardown.

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