Skip to content

feat: add Windows native capture and cursor pipeline#217

Open
EtienneLescot wants to merge 38 commits intosiddharthvaddem:mainfrom
EtienneLescot:feat/cursor-pipeline
Open

feat: add Windows native capture and cursor pipeline#217
EtienneLescot wants to merge 38 commits intosiddharthvaddem:mainfrom
EtienneLescot:feat/cursor-pipeline

Conversation

@EtienneLescot
Copy link
Copy Markdown
Contributor

@EtienneLescot EtienneLescot commented Mar 15, 2026

Description

This PR adds a Windows-only native recording path for OpenScreen. The goal is to avoid the preview/export cursor drift caused by splitting capture, cursor telemetry, audio, webcam, and muxing across Electron/browser APIs.

On Windows, recordings are routed through a bundled native Windows Graphics Capture helper. macOS and Linux behavior is intentionally unchanged in this PR.

What changed

  • Adds a redistributable Windows WGC helper at electron/native/bin/win32-x64/wgc-capture.exe.
  • Captures full display and app windows through Windows Graphics Capture.
  • Captures native Windows cursor telemetry and reconstructs a high-quality scalable cursor overlay in preview/export.
  • Adds Windows cursor settings only when the active platform is Windows.
  • Captures system audio through WASAPI loopback.
  • Captures microphone audio through WASAPI and mixes mic + system audio into the MP4.
  • Captures webcam natively through Media Foundation, with DirectShow fallback for virtual cameras such as NVIDIA Broadcast.
  • Composes webcam picture-in-picture in the helper-owned MP4 during the Windows-native migration.
  • Packages the native helper with Windows builds through asarUnpack.
  • Adds/updates fast Windows smoke-test scripts for native cursor, WGC display/window capture, audio, mic, mixed audio, and webcam.
  • Adds a focused Playwright Electron smoke test for the maintainer launch/editor checklist paths.
  • Documents the Windows native recorder architecture and testing pipeline.

Windows-only scope

This PR does not add native capture for macOS or Linux. Windows-only cursor/capture controls are hidden outside Windows where applicable.

Linux remains on the existing Electron capture path. macOS behavior is not changed by this PR.

Validation run

Passed locally on Windows:

  • npm run build-vite
  • npm run build:win
  • npm run test:e2e:windows-native-checklist
  • npm run test:cursor-native:win
  • npm run test:wgc-helper:win
  • npm run test:wgc-window:win
  • npm run test:wgc-audio:win
  • npm run test:wgc-mic:win
  • npm run test:wgc-mixed-audio:win
  • npm run test:wgc-webcam:win
  • OPENSCREEN_WGC_TEST_WEBCAM_DEVICE_NAME="NVIDIA Broadcast" npm run test:wgc-webcam:win
  • OPENSCREEN_WGC_TEST_WEBCAM_DEVICE_NAME="NVIDIA Broadcast" npm run test:wgc-full:win
  • npx vitest --run src/lib/cursorTelemetryBuffer.test.ts

Known repo-level checks still needing separate cleanup:

  • npm run lint currently reports broad existing Biome formatting/line-ending issues outside the native-capture work.
  • npm test has existing failures in blur effects, tutorial translations, and browser exporter fixture/canvas handling.
  • npm run test:e2e currently times out in tests/e2e/gif-export.spec.ts.

Maintainer test checklist

Capture + Launch

  • pre commit check
  • Source selector opens and lists screens/windows with thumbnails
  • Selecting a source works for full screen and app window
  • Record start/stop works from launch window
  • Tray “Stop Recording” works while recording
  • Opening existing video file works from launch window
  • Opening existing project works from launch window

Audio

  • Mic toggle on/off works before recording
  • Mic device selection works
  • System audio toggle works across recordings
  • Mic-only recording works
  • System-audio-only recording works
  • Mic + system audio mix works and levels are balanced

Editor Load + Playback

  • Playback, pause, seek works.
  • Cursor telemetry overlays correctly

Timeline

  • Add/edit/remove different elements (zoom/ annotation/ trim/ speed), see how they are in playback and if they are 1:1 with exported video
  • Region drag/resize snaps and persists correctly
  • No overlap/ordering bugs on timeline items (apart from annotations)

Project Persistence

  • save/ load works

Other

  • Tweaking sliders/ wallpapers/ other effects.

Notes for reviewers

The checked items above are the paths covered by local Windows automation and manual iteration on this branch. The unchecked items are still useful manual QA targets before considering the PR ready to merge.

Summary by CodeRabbit

  • New Features

    • Windows native recording (WGC) with system audio, microphone and webcam; integrated native cursor capture, preview and export; native bridge APIs for project/video flows and cursor telemetry.
    • Cursor rendering controls (visibility, size, smoothing, motion blur, click bounce) with preview and export support.
  • Documentation

    • Native bridge architecture, Windows recorder roadmap, native helper build/run docs and Windows cursor test workflow; README developer notes link added.
  • Tests

    • Windows-native smoke scripts and Playwright E2E checklist.
  • Chores

    • Expanded .gitignore, packaging unpack rules, Windows native build/test scripts, and audio export switched to AAC (MP4).

@EtienneLescot EtienneLescot changed the title feat: add cursor overlay pipeline feat: add cursor overlay pipeline (depends on #207) Mar 15, 2026
@siddharthvaddem
Copy link
Copy Markdown
Owner

siddharthvaddem commented Mar 17, 2026

Just wanted to thank you for working on this and helping contribute to the project 🙏

I know this is a draft, but wanted to reiterate that I was never against Native Capture. We can have the cursor related settings hidden for Linux and Windows. If that helps. Let me know if there's any further clarification needed. Sorry if this feels like going back and forth

Had a couple of folks on X reach out and say they really wanted this for macOS.

No rush on this at all. Don't feel obliged.

This is a pretty large change which would require a lot of testing to avoid bugs. This is what usually helps me (might have missed something):

Capture + Launch

  • pre commit check
  • Source selector opens and lists screens/windows with thumbnails
  • Selecting a source works for full screen and app window
  • Record start/stop works from launch window
  • Tray “Stop Recording” works while recording
  • Opening existing video file works from launch window
  • Opening existing project works from launch window

Audio

  • Mic toggle on/off works before recording
  • Mic device selection works
  • System audio (toggle these across 2 recordings to see if both paths work
  • Mic-only recording works
  • System-audio-only recording works
  • Mic + system audio mix works and levels are balanced

Editor Load + Playback

  • Playback, pause, seek works.
  • Cursor telemetry overlays correctly

Timeline

  • Add/edit/remove different elements (zoom/ annotation/ trim/ speed), see how they are in playback and if they are 1:1 with exported video
  • Region drag/resize snaps and persists correctly
  • No overlap/ordering bugs on timeline items (apart from annotations)

Project Persistence

  • save/ load works

Other

  • Tweaking sliders/ wallpapers/ other effects.

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

EtienneLescot commented Mar 23, 2026

@siddharthvaddem
Thank you for the testing procedure.
I lack a little bit of time this week but I will work on this as soon as possible.

@siddharthvaddem
Copy link
Copy Markdown
Owner

@siddharthvaddem Thank you for the testing procedure. I like a little bit of time this week that I will work on this as soon as possible.

take your time 🙏 no rush. want to make sure it is implemented correctly even if it takes time.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 26, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a typed native bridge and Windows-native recording pipeline: bridge contracts, state store, services, IPC/preload wiring, renderer client and hooks, cursor recording sessions/adapters, a native wgc-capture helper (C++), audio/video/webcam capture + encoder, exporter and cursor-rendering integration, build/test scripts, and documentation.

Changes

Native bridge + Windows-native recording (single DAG)

Layer / File(s) Summary
Data Shape / Contracts
src/native/contracts.ts, src/lib/nativeWindowsRecording.ts, electron/electron-env.d.ts
Adds bridge constants/version, typed request/response envelopes, cursor telemetry/recording types, project/system result shapes, Windows recording request/result types, and extends preload typings for new electronAPI methods.
Runtime State
electron/native-bridge/store.ts
New in-memory NativeBridgeState and NativeBridgeStateStore with setters for system/project/cursor state and telemetry load metadata.
Service Layer
electron/native-bridge/services/*.ts
Adds SystemService, ProjectService, CursorService to wrap adapters/state and expose platform/project/video/cursor operations.
Adapter / Cursor Session Interfaces
electron/native-bridge/cursor/adapter.ts, .../recording/session.ts
Declare CursorNativeAdapter, CursorTelemetryLoadResult, and CursorRecordingSession interfaces.
Cursor Session Implementations
electron/native-bridge/cursor/recording/*.ts
Add TelemetryRecordingSession (timed telemetry) and WindowsNativeRecordingSession (PowerShell helper spawn, NDJSON parsing), types, and PowerShell script builder.
Native Bridge IPC Handler
electron/ipc/nativeBridge.ts, electron/ipc/handlers.ts
Register native-bridge:invoke, validate requests, create NativeBridgeStateStore, wire services; update IPC handlers to coordinate cursor sessions, native Windows capture start/stop, project/video path management, file approvals, and persisted session manifests.
Preload / Renderer Client / Hooks
electron/preload.ts, src/native/client.ts, src/native/hooks/*, src/native/index.ts
Expose invokeNativeBridge and native recording APIs in preload; add nativeBridgeClient wrapper and helpers; add hooks useCursorTelemetry and useCursorRecordingData.
Windows Native Helper (C++ build)
electron/native/wgc-capture/CMakeLists.txt, electron/native/wgc-capture/src/*
Adds wgc-capture executable: WGC session, MF encoder (H.264 + AAC), WASAPI loopback & mic capture, audio mixer, webcam capture (MF + DirectShow), monitor utils, main orchestration, and NDJSON/stdin contract.
Audio/Encoder Utilities
electron/native/wgc-capture/src/audio_sample_utils.*, mf_encoder.*
New audio conversion/mixing utilities and AudioMixer worker; Media Foundation encoder with staging-texture readback and audio stream handling.
Build & Packaging
scripts/build-windows-wgc-helper.mjs, package.json, electron-builder.json5, .gitignore
Windows build script using vcvarsall/CMake/Ninja, build:native:win script, add electron/native/bin/** to asarUnpack, and ignore native build outputs in .gitignore.
Developer Tools & Tests
scripts/test-windows-native-cursor.mjs, scripts/test-windows-wgc-helper.mjs, scripts/capture-openscreen-preview.mjs, tests/e2e/windows-native-checklist.spec.ts
Add Windows diagnostics and smoke tests: PowerShell cursor sampler, helper smoke tests (ffprobe/ffmpeg checks), OpenScreen preview automation, and Playwright E2E checklist.
Renderer integration: UI, playback, exporters
src/hooks/useScreenRecorder.ts, src/native/*, src/components/video-editor/*, src/lib/exporter/*, src/lib/cursor/nativeCursor.ts
Wire native-recording flow into recording hook; add native cursor utilities, smoothing, asset preload, Pixi overlay + DOM preview, and propagate cursorRecordingData and tuning through FrameRenderer, VideoExporter, and GifExporter.
UI tweaks & testids
src/components/launch/*, src/components/video-editor/SettingsPanel.tsx, src/components/launch/SourceSelector.tsx, src/components/video-editor/timeline/TimelineEditor.tsx
Add cursor settings UI, crop dropdown behavior, pointer-down counters, webcam/mic device name sync, data-testid attributes, and hasVideoSource prop for timeline empty state.
Docs / READMEs / Roadmap
docs/*, electron/native/README.md, README.md
Add native bridge architecture doc, Windows native recorder roadmap, Windows cursor testing guide, native helper README, and README developer note link.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested reviewers

  • siddharthvaddem
  • FabLrc

"a midnight patch for native capture"
powershell hums and the helper spits ndjson,
dshow and mf argue in c++ while audio threads mix,
pixi smooths ghost cursors, exporters bake h264+aac,
nit: build paths are kinda cursed but lowkey clever,
ship when tests stop complaining and windows stops surprising.

@siddharthvaddem
Copy link
Copy Markdown
Owner

@EtienneLescot please close the PR if there are no plans on continuing work here. Appreciate all the work and help put in 🙏

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

@EtienneLescot please close the PR if there are no plans on continuing work here. Appreciate all the work and help put in 🙏

@siddharthvaddem my goodness, time goes by so quickly!
Let me see if I can find time on this, tuesday or wednesday.
If not I will close it to reopen it when possible ;-)

@EtienneLescot EtienneLescot force-pushed the feat/cursor-pipeline branch from bc200cd to ac41399 Compare May 5, 2026 07:09
…and playback

- Implement native bridge for Windows cursor capture via PowerShell/C#
- Add cursor-free capture using getDisplayMedia with setDisplayMediaRequestHandler
- Update video player and exporters to support native cursor telemetry
- Enable system audio capture on Windows via WASAPI loopback
- Add interpolation for smoother cursor movement in playback and export
- Improve cursor scaling and visibility handling in editor and playback
@EtienneLescot EtienneLescot force-pushed the feat/cursor-pipeline branch from 00b6e83 to e48dd9c Compare May 5, 2026 08:41
@EtienneLescot
Copy link
Copy Markdown
Contributor Author

@siddharthvaddem I am working on it today ;-)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/video-editor/VideoEditor.tsx (1)

1934-1954: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

preview is ignoring the saved cursor highlight settings now.

VideoPlayback still falls back to DEFAULT_CURSOR_HIGHLIGHT and [] when the parent omits cursorHighlight / cursorClickTimestamps. Since this call site stopped passing both props, preview no longer reflects the editor state even though export still uses effectiveCursorHighlight.

small wiring fix
 											cursorRecordingData={cursorRecordingData}
 											trimRegions={trimRegions}
 											speedRegions={speedRegions}
 											annotationRegions={annotationOnlyRegions}
@@
 											onBlurDataChange={handleBlurDataPreviewChange}
 											onBlurDataCommit={commitState}
 											cursorTelemetry={cursorTelemetry}
+											cursorHighlight={effectiveCursorHighlight}
+											cursorClickTimestamps={cursorClickTimestamps}
 											showCursor={showCursor}
 											cursorSize={cursorSize}
 											cursorSmoothing={cursorSmoothing}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/video-editor/VideoEditor.tsx` around lines 1934 - 1954, The
preview stopped reflecting saved cursor highlight because VideoPlayback is no
longer being passed the cursorHighlight and cursorClickTimestamps props; update
the VideoPlayback call (the component receiving props like cursorRecordingData,
trimRegions, etc.) to pass cursorHighlight={effectiveCursorHighlight} and
cursorClickTimestamps={cursorClickTimestamps} (or the corresponding state/prop
names used in this file) so the preview uses the editor's effective settings
rather than falling back to DEFAULT_CURSOR_HIGHLIGHT/[].
♻️ Duplicate comments (1)
electron/native-bridge/cursor/recording/windowsNativeRecordingSession.ts (1)

106-107: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Readiness failure can still orphan the helper process.

lowkey risky: start() awaits readiness, but timeout/rejection only rejects the promise and doesn’t guarantee teardown. If caller drops the session on error, the helper can keep running.

Suggested minimal fix
 async start(): Promise<void> {
@@
-		await this.waitUntilReady();
+		try {
+			await this.waitUntilReady();
+		} catch (error) {
+			const normalizedError =
+				error instanceof Error ? error : new Error(String(error));
+			this.failHelper(normalizedError);
+			throw normalizedError;
+		}
 	}
@@
 	private waitUntilReady() {
 		return new Promise<void>((resolve, reject) => {
 			this.readyResolve = resolve;
 			this.readyReject = reject;
 			this.readyTimer = setTimeout(() => {
-				this.rejectReady(new Error("Timed out waiting for Windows cursor helper readiness"));
+				this.failHelper(
+					new Error("Timed out waiting for Windows cursor helper readiness"),
+				);
 			}, READY_TIMEOUT_MS);
 		});
 	}

Also applies to: 249-255

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/native-bridge/cursor/recording/windowsNativeRecordingSession.ts`
around lines 106 - 107, start() currently awaits waitUntilReady() but if that
promise times out or rejects the helper process can be orphaned; change start()
(and the other call site that awaits waitUntilReady around lines 249-255) to
ensure cleanup on failure by wrapping the await in try/catch/finally (or using
Promise.race/AbortController) and calling the session teardown method (e.g.,
this.stop(), this.dispose(), or the helper-kill routine) in the catch/finally
path so the helper process is always terminated when readiness fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 154-159: cursorClickTimestamps has been hardcoded to an empty
array which disables click-driven emphasis on macOS; change it to derive
timestamps conditionally instead: keep the same inputs (videoSourcePath,
useCursorTelemetry, useCursorRecordingData) and compute cursorClickTimestamps
based on available data (prefer cursorRecordingData if present, otherwise
extract click events from cursorTelemetry), and only fall back to an empty array
when neither source provides click timestamps; update the derivation referenced
by effectiveCursorHighlight and preview/export paths so mac projects get the
platform-specific fallback rather than a hardcoded [] (use the symbols
cursorClickTimestamps, useCursorTelemetry, useCursorRecordingData,
cursorRecordingData, cursorTelemetry, videoSourcePath/fromFileUrl, and
effectiveCursorHighlight to locate and change the logic).

In `@src/components/video-editor/VideoPlayback.tsx`:
- Around line 382-390: The fallback that treats any positive video.currentTime
as the full duration in resolveCurrentDuration is unsafe (forceResolveDuration
sets currentTime to 0.1); change the check so we only accept currentTime as the
duration when playback has actually reached the end: i.e., require video.ended
=== true OR that video.duration is finite and video.currentTime >=
video.duration - EPSILON (or matches seekable end), then call onDurationChange;
otherwise ignore small positive currentTime values. Update
resolveCurrentDuration (and related logic that relies on
lastResolvedDurationRef/onDurationChange) to use this stricter check.

In `@src/lib/cursor/nativeCursor.ts`:
- Around line 291-315: getNativeCursorClickBounceProgress (and the similar
helper at lines 429-448) currently reverse-scans recordingData.samples on every
frame which causes O(samples × frames) perf; replace the per-frame linear tail
scan with a binary-search helper that finds the last sample with sample.timeMs
<= timeMs (or maintain a monotonic cursor index when iterating sequential
frames) and use that index to compute ageMs and the click-bounce value; update
resolveInterpolatedNativeCursorFrame and getNativeCursorClickBounceProgress to
call the new binary-search (or use the monotonic index) so semantics (handling
null/undefined recordingData, NATIVE_CURSOR_CLICK_ANIMATION_MS threshold, and
interactionType === "click") remain the same while avoiding repeated full
reverse scans called by drawNativeCursor and the VideoPlayback ticker.

In `@src/lib/exporter/frameRenderer.ts`:
- Around line 599-606: When projectNativeCursorToLocal(...) returns null we only
reset motion blur; also reset the cursor smoothing state so exports don't ease
in from stale off-screen positions. In the same branch where
resetNativeCursorMotionBlurState(this.nativeCursorMotionBlurState) is called,
also clear the smoothing state by calling the smoothing reset used elsewhere
(e.g. resetNativeCursorSmoothingState(this.nativeCursorSmoothingState) or
otherwise clear the smoothing buffer/last-position stored on
this.nativeCursorSmoothingState) so both motion blur and smoothing are reset
when the cursor falls outside the crop.

---

Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1934-1954: The preview stopped reflecting saved cursor highlight
because VideoPlayback is no longer being passed the cursorHighlight and
cursorClickTimestamps props; update the VideoPlayback call (the component
receiving props like cursorRecordingData, trimRegions, etc.) to pass
cursorHighlight={effectiveCursorHighlight} and
cursorClickTimestamps={cursorClickTimestamps} (or the corresponding state/prop
names used in this file) so the preview uses the editor's effective settings
rather than falling back to DEFAULT_CURSOR_HIGHLIGHT/[].

---

Duplicate comments:
In `@electron/native-bridge/cursor/recording/windowsNativeRecordingSession.ts`:
- Around line 106-107: start() currently awaits waitUntilReady() but if that
promise times out or rejects the helper process can be orphaned; change start()
(and the other call site that awaits waitUntilReady around lines 249-255) to
ensure cleanup on failure by wrapping the await in try/catch/finally (or using
Promise.race/AbortController) and calling the session teardown method (e.g.,
this.stop(), this.dispose(), or the helper-kill routine) in the catch/finally
path so the helper process is always terminated when readiness fails.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 99cef220-8320-4f1c-a307-ab49414c1eee

📥 Commits

Reviewing files that changed from the base of the PR and between d076a9b and 1c92110.

📒 Files selected for processing (11)
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.script.ts
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.ts
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.types.ts
  • scripts/test-windows-native-cursor.mjs
  • src/components/video-editor/VideoEditor.tsx
  • src/components/video-editor/VideoPlayback.tsx
  • src/lib/cursor/nativeCursor.ts
  • src/lib/exporter/frameRenderer.ts
  • src/lib/exporter/gifExporter.ts
  • src/lib/exporter/videoExporter.ts
  • src/native/contracts.ts
✅ Files skipped from review due to trivial changes (1)
  • src/native/contracts.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/exporter/videoExporter.ts
  • scripts/test-windows-native-cursor.mjs
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.script.ts

Comment thread src/components/video-editor/VideoEditor.tsx Outdated
Comment thread src/components/video-editor/VideoPlayback.tsx
Comment thread src/lib/cursor/nativeCursor.ts
Comment thread src/lib/exporter/frameRenderer.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (3)
scripts/test-windows-native-cursor.mjs (2)

6-12: 💤 Low value

Number(process.env.X ?? default) will happily produce NaN and bake it into PowerShell.

If anyone sets e.g. CURSOR_TEST_SAMPLE_INTERVAL_MS=fast in CI, you get NaN, and then Start-Sleep -Milliseconds NaN (line 322/469) fails inside PS, which throws under $ErrorActionPreference = 'Stop' and the sampler dies immediately with a not-super-obvious error. A tiny clamp/validate at the top would save someone a 2am debugging session.

diff
-const SAMPLE_INTERVAL_MS = Number(process.env.CURSOR_TEST_SAMPLE_INTERVAL_MS ?? 25);
-const DURATION_MS = Number(process.env.CURSOR_TEST_DURATION_MS ?? 1800);
-const SCREEN_FRAME_INTERVAL_MS = Number(process.env.CURSOR_TEST_SCREEN_FRAME_INTERVAL_MS ?? 100);
+function readPositiveIntEnv(name, fallback) {
+	const raw = process.env[name];
+	if (raw === undefined) return fallback;
+	const parsed = Number(raw);
+	if (!Number.isFinite(parsed) || parsed <= 0) {
+		throw new Error(`Invalid ${name}=${raw}; expected positive number`);
+	}
+	return Math.floor(parsed);
+}
+
+const SAMPLE_INTERVAL_MS = readPositiveIntEnv("CURSOR_TEST_SAMPLE_INTERVAL_MS", 25);
+const DURATION_MS = readPositiveIntEnv("CURSOR_TEST_DURATION_MS", 1800);
+const SCREEN_FRAME_INTERVAL_MS = readPositiveIntEnv("CURSOR_TEST_SCREEN_FRAME_INTERVAL_MS", 100);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/test-windows-native-cursor.mjs` around lines 6 - 12, The numeric
environment parsing for SAMPLE_INTERVAL_MS, DURATION_MS,
SCREEN_FRAME_INTERVAL_MS (and READY_TIMEOUT_MS if desired) can yield NaN when
env vars are non-numeric; validate and clamp each parsed value immediately after
creation (e.g., check Number.isFinite(value) and fall back to the intended
default or a safe minimum/maximum) and emit a warning if an env value was
invalid so downstream PowerShell calls (Start-Sleep) never receive NaN; update
the initialization around SAMPLE_INTERVAL_MS, DURATION_MS,
SCREEN_FRAME_INTERVAL_MS to perform these checks and fallback logic and ensure
OUTPUT_DIR handling remains unchanged.

476-492: 💤 Low value

waitForReady polls instead of resolving from the stream; nit: cleaner.

The current 25ms setInterval over a shared events array works, but it's a polling loop tied to a buffer that's mutated elsewhere. A small "ready" promise resolved directly from onStdout would remove the timer/race-y interplay and make the timeout the only wall-clock concern. Totally optional — current code is correct, just kinda cursed at 2am.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/test-windows-native-cursor.mjs` around lines 476 - 492, Replace the
polling loop in waitForReady with a promise that is resolved directly when the
stdout handler pushes a "ready" event: change waitForReady to return a new
Promise that sets a single timeout (setTimeout) to reject after READY_TIMEOUT_MS
and exposes a local resolve function; remove setInterval. In the stdout handler
(the function that currently mutates events, e.g., onStdout or wherever
events.push(...) happens) call that resolve when you detect an event.type ===
"ready" so the promise resolves immediately; ensure the timeout is cleared on
resolve to avoid leaks. Keep the existing rejection message and behavior but
switch polling to this direct-resolution pattern.
electron/native-bridge/cursor/recording/windowsNativeRecordingSession.types.ts (1)

4-21: 💤 Low value

nit: asset should be WindowsCursorAssetPayload | null for consistency.

The PowerShell sampler emits asset = $null on every sample where no asset is captured (see scripts/test-windows-native-cursor.mjs line 332/360), so JSON.parse will produce asset: null rather than absent. bounds on line 19 already correctly allows | null, but asset on line 20 is just optional. Consumers happen to use optional chaining (payload.asset?.id) so it works, but the type lowkey lies about what comes off the wire.

diff
-	asset?: WindowsCursorAssetPayload;
+	asset?: WindowsCursorAssetPayload | null;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@electron/native-bridge/cursor/recording/windowsNativeRecordingSession.types.ts`
around lines 4 - 21, The WindowsCursorSampleEvent type's asset field is declared
optional but should explicitly allow null; update the WindowsCursorSampleEvent
interface to change the asset property from optional (asset?) to a non-optional
field typed as WindowsCursorAssetPayload | null (i.e., asset:
WindowsCursorAssetPayload | null) so it matches incoming JSON from the
PowerShell sampler, and then grep for uses of
WindowsCursorSampleEvent.payload.asset to ensure callers handle null (or adjust
code to use optional chaining if needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/test-windows-native-cursor.mjs`:
- Around line 1025-1044: The current findPlaywrightChromiumExecutable uses
lexicographic sort on chromium-* directory names which misorders numeric
revisions; change the candidate selection to extract the numeric revision from
each entry.name (the suffix after "chromium-"), convert to integer (handle
non-numeric safely), sort candidates by that numeric revision descending, then
pick the highest numeric candidate (falling back to defaultPath). Update the
mapping/filtration logic around candidates in findPlaywrightChromiumExecutable
so it pairs each path with its parsed revision number, sorts by that number (not
by string), and returns the highest-version path or defaultPath if none valid.
- Around line 1100-1118: The handler that parses stdout lines calls
JSON.parse(trimmed) directly which can throw and crash the sampler; wrap that
parse in a try/catch (or otherwise validate the trimmed string is valid JSON)
inside the stdout data handler so malformed or non-JSON lines are skipped (and
optionally logged with the `[cursor-native-test]` prefix), and only push to
events/update assets when parsing succeeds — reference the JSON.parse(trimmed)
call, the events array, and the assets Map in the stdout handler and mirror the
defensive filtering used in onStderr for non-JSON/CLIXML lines.

---

Nitpick comments:
In
`@electron/native-bridge/cursor/recording/windowsNativeRecordingSession.types.ts`:
- Around line 4-21: The WindowsCursorSampleEvent type's asset field is declared
optional but should explicitly allow null; update the WindowsCursorSampleEvent
interface to change the asset property from optional (asset?) to a non-optional
field typed as WindowsCursorAssetPayload | null (i.e., asset:
WindowsCursorAssetPayload | null) so it matches incoming JSON from the
PowerShell sampler, and then grep for uses of
WindowsCursorSampleEvent.payload.asset to ensure callers handle null (or adjust
code to use optional chaining if needed).

In `@scripts/test-windows-native-cursor.mjs`:
- Around line 6-12: The numeric environment parsing for SAMPLE_INTERVAL_MS,
DURATION_MS, SCREEN_FRAME_INTERVAL_MS (and READY_TIMEOUT_MS if desired) can
yield NaN when env vars are non-numeric; validate and clamp each parsed value
immediately after creation (e.g., check Number.isFinite(value) and fall back to
the intended default or a safe minimum/maximum) and emit a warning if an env
value was invalid so downstream PowerShell calls (Start-Sleep) never receive
NaN; update the initialization around SAMPLE_INTERVAL_MS, DURATION_MS,
SCREEN_FRAME_INTERVAL_MS to perform these checks and fallback logic and ensure
OUTPUT_DIR handling remains unchanged.
- Around line 476-492: Replace the polling loop in waitForReady with a promise
that is resolved directly when the stdout handler pushes a "ready" event: change
waitForReady to return a new Promise that sets a single timeout (setTimeout) to
reject after READY_TIMEOUT_MS and exposes a local resolve function; remove
setInterval. In the stdout handler (the function that currently mutates events,
e.g., onStdout or wherever events.push(...) happens) call that resolve when you
detect an event.type === "ready" so the promise resolves immediately; ensure the
timeout is cleared on resolve to avoid leaks. Keep the existing rejection
message and behavior but switch polling to this direct-resolution pattern.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 42c3641f-1ebe-441e-a7cf-1ec0a2f42a1e

📥 Commits

Reviewing files that changed from the base of the PR and between 1c92110 and 0f26eac.

📒 Files selected for processing (4)
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.script.ts
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.ts
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.types.ts
  • scripts/test-windows-native-cursor.mjs
✅ Files skipped from review due to trivial changes (1)
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • electron/native-bridge/cursor/recording/windowsNativeRecordingSession.script.ts

Comment thread scripts/test-windows-native-cursor.mjs
Comment thread scripts/test-windows-native-cursor.mjs
@EtienneLescot
Copy link
Copy Markdown
Contributor Author

Addressed the non-inline nitpicks from the latest CodeRabbit summary in 159b24e:

  • validated numeric cursor diagnostic env vars with fallback warnings;
  • replaced waitForReady polling with a ready promise resolved directly from stdout parsing;
  • corrected the Windows cursor sample wire type to asset: WindowsCursorAssetPayload | null.

Validation run:

  • npx biome check scripts\test-windows-native-cursor.mjs electron\native-bridge\cursor\recording\windowsNativeRecordingSession.types.ts
  • npm run test:cursor-native:win
  • npm run build-vite

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

Addressed the two latest non-inline CodeRabbit notes in f609611 as well:

  • VideoPlayback now receives cursorHighlight={effectiveCursorHighlight} and cursorClickTimestamps={cursorClickTimestamps}, so preview and export use the same effective cursor highlight settings again.
  • WindowsNativeRecordingSession.start() now kills the PowerShell helper and cleans up the temporary script if readiness fails or times out, preventing an orphaned cursor sampler.

Validation run:

  • npx biome check src\components\video-editor\VideoEditor.tsx src\components\video-editor\VideoPlayback.tsx src\lib\cursor\nativeCursor.ts src\lib\exporter\frameRenderer.ts electron\native-bridge\cursor\recording\windowsNativeRecordingSession.ts
  • npm run build-vite
  • npm run test:cursor-native:win

npm test was also run, but the full suite is still failing on pre-existing unrelated failures: blur overlay expected value, tutorial locale data, and browser exporter tests failing on relative fixture URLs / missing canvas implementation in the current Vitest environment.

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

Addressed the older unresolved CodeRabbit thread group in eab205a:

  • re-validates files selected by open-video-file-picker before approval;
  • requires approved video paths for native-bridge cursor sidecar reads;
  • removes unused DirectShow registry matching helpers;
  • makes DirectShow webcam thread startup exception-safe;
  • keeps the native recording handle retryable until stop succeeds;
  • allows non-Windows / unsupported OS paths to continue to the browser recorder while intentionally keeping missing Windows helper as a hard native-recorder error.

One thread was verified as not applicable: the E2E native bridge mock for cursor.getTelemetry should continue returning CursorTelemetryPoint[], because CursorService.getTelemetry() unwraps the internal CursorTelemetryLoadResult before data reaches the renderer-facing bridge response.

Validation run:

  • npx biome check electron\ipc\handlers.ts src\hooks\useScreenRecorder.ts tests\e2e\windows-native-checklist.spec.ts
  • npm run build-vite
  • npm run build:native:win

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

@siddharthvaddem all tests ok on my side on windows.

Comment thread README.md
Comment on lines +104 to +106
Developer notes:
- [Windows native cursor test pipeline](docs/testing/windows-native-cursor.md)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

can we remove this

@siddharthvaddem
Copy link
Copy Markdown
Owner

madlad 🐐 taking a look

@siddharthvaddem
Copy link
Copy Markdown
Owner

@EtienneLescot few regressions I noticed that affect macos and Linux

I also see you replaced getDisplayMedia() for all platforms. On macOS, getDisplayMedia had no handler so throws NotSupportedError immediately and folks will not be able to record at all. You need to move the platform check inside.

image

It adds an additional cursor on macOS - should this not be gated to just Windows?

@siddharthvaddem
Copy link
Copy Markdown
Owner

can we also restore the stream-copy fast path when sortedTrims.length === 0 and speedRegions.length === 0 (or whenever no per-sample manipulation is needed

@siddharthvaddem
Copy link
Copy Markdown
Owner

noticed you also switched audio codec from opus to aac with no fallback

@siddharthvaddem
Copy link
Copy Markdown
Owner

can we avoid native bridge also intializing on macos/linux unconditionally if this is only going to be used by windows

we also dont want the electron builder to ship windows native binaries into all packages

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

EtienneLescot commented May 6, 2026

@siddharthvaddem :

  • macOS/Linux regression from getDisplayMedia() + extra cursor: fixed in 249658d. Windows now keeps the getDisplayMedia cursor-free path, while macOS/Linux use the previous Electron getUserMedia(chromeMediaSource: "desktop") path again. Native cursor recording is now a no-op outside Windows.
  • Audio codec change: fixed in 249658d. Export now selects AAC when supported and falls back to Opus, with the muxer and encoder using the same codec.
  • Windows native binaries in non-Windows packages: fixed in 249658d. wgc-capture.exe is now packaged only through the Windows config. I verified a Linux --dir build does not include it, while a Windows --dir build still does.
  • Stream-copy fast path: agreed, still outstanding. I left this out of 249658d because it is a separate export-path change and should restore the no-reencode path only when no per-sample manipulation is needed.

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

EtienneLescot commented May 6, 2026

@siddharthvaddem Addressed the stream-copy fast path as well:

  • 6e8932a restores a conservative source-copy fast path for no-op MP4 exports, so we avoid the WebCodecs re-encode path when no per-frame manipulation is needed.
  • fbcbf8f adds a native-cursor guard: if native cursor data is present, or still loading while cursor rendering is enabled, the fast path is disabled so cursor overlays/effects still go through the renderer.

Validated with targeted Biome checks, the new videoExporter unit tests, and npm run build-vite.

EDIT: It breaks the cursor feature, so I will take a deeper look.

@EtienneLescot
Copy link
Copy Markdown
Contributor Author

@siddharthvaddem I restored the source-copy fast path for no-op MP4 exports.

To make this compatible with the Windows native cursor work, I added a pre-recording cursor mode toggle in the launch HUD. This button is currently Windows-only.

The reason is that the fast path copies the original MP4 without re-encoding, so it cannot be used when we need to inject the editable/magnified cursor overlay during export. On Windows, users can now choose before recording:

  • editable cursor mode: the native cursor is excluded from the recording and rendered later as an editable overlay;
  • system cursor mode: the OS cursor is captured directly into the source video.

When system cursor mode is used, and the project remains eligible for source-copy export, for example full-screen/source-size export, no padding, no crop, no webcam overlay, no trims/zoom/speed/annotations/effects, the export can use the fast path again.

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