Skip to content

sharper, smoother window recordings on macos - #1089

Open
Kevin-Liu-01 wants to merge 2 commits into
webadderallorg:mainfrom
Kevin-Liu-01:fix/macos-window-capture
Open

Kevin-Liu-01 wants to merge 2 commits into
webadderallorg:mainfrom
Kevin-Liu-01:fix/macos-window-capture

Conversation

@Kevin-Liu-01

@Kevin-Liu-01 Kevin-Liu-01 commented Oct 3, 2026 •

Copy link
Copy Markdown

window recordings were slower and blurrier than they needed to be.

the same window recorded by each build
the same 401×301 window recorded on a 1x display. upstream squeezes 401 pixels into 400 and smears the one-pixel detail. this branch keeps every pixel.

what was going on

to record a window, recordly captured the whole display and cut the window out of every frame with core image. that cut took up to 210 ms per frame on my macbook, so a 1440×960 window recorded at 45.8 fps, and it ran on the same queue as the audio.

the cut also rounded the window down to an even number of pixels and scaled the window into that. on a 1x display, any window with an odd width or height came out blurry.

the bitrate came from settings meant for 30 fps, but recordly records at 60, so text broke up while scrolling.

one frame of fast scrolling at each bitrate
one frame of fast scrolling. at upstream's 25 mbps the text breaks up, and at 43 mbps it stays close to the original. the bottom row shows how far each frame is from the original, brightened 8×.

what this changes

  • ScreenCaptureKit crops the display itself with sourceRect, so there's no per-frame cut anymore. the same window now records at 56.9 fps
  • the crop lands on whole pixels and trims to an even size, so nothing gets scaled
  • the bitrate scales with the frame rate, with one keyframe per second
  • window tracking moves the crop when the window moves or resizes
  • a window that isn't changing sends a single frame when capture starts. that frame could arrive before the writer was ready, and then the recording never started. the helper now holds on to it and writes it once the writer is ready

how to check

  • npx vitest run electron/native
  • npm run build:native-helpers builds the helper. the binaries aren't in this pr
  • record a window, then move and resize it while recording
  • record a window that isn't changing. the recording should start right away
  • on a 1x display, record a window with an odd width and zoom into the text

this pr and #1088 both touch ScreenCaptureKitRecorder.swift, so i'll rebase whichever lands second.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Window recordings now keep the selected window in frame as it moves, resizes, or shifts between displays.
    • Improved recording startup reliability by allowing the first video frame time to become ready.
    • Improved consistency of window and display recordings, with video encoding settings adjusted to the selected frame rate.
    • Recording finalization now better preserves the video through its final moments.

Kevin-Liu-01 and others added 2 commits October 3, 2026 10:20
Window recordings captured the whole display and cropped every frame to
the window with a CoreImage render on the sample queue. At display
resolution that render took up to 200 ms per frame, which dropped video
frames and delayed everything else on the queue. ScreenCaptureKit now
crops the display to the window with sourceRect, and window tracking
updates that rectangle as the window moves or resizes.

A window that is not changing sends one complete frame when capture
starts, and it could arrive before the writer was ready. The recording
then never started. The newest frame is now kept until the writer can
take it, and it starts the timeline when it is written.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The crop rectangle followed the window frame in points, and the output
size was rounded down to an even number of pixels. On a 1x display a
window with an odd width or height was therefore scaled by a fraction of
a pixel, which turned one-pixel detail into gray. The crop now snaps to
whole pixels and trims to an even size, so the recording holds the
screen's own pixels.

AVOutputSettingsAssistant sizes its bitrate and keyframe interval for
30 fps, but recordings run at 60. Scale the bitrate to the capture rate
and keep one keyframe per second, so text stays sharp while the screen
scrolls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: webadderallorg/Recordly/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 413bd85b-7e80-4b66-9927-ce07a6fd975e
📥 Commits

Reviewing files that changed from the base of the PR and between f24ce5c and cc2a812.

📒 Files selected for processing (2)
  • electron/native/ScreenCaptureKitRecorder.swift
  • electron/native/ScreenCaptureKitRecorder.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Window capture now uses ScreenCaptureKit’s native crop and updates it as the window moves or changes displays. The recorder also retries a pending first frame until the writer is ready, and configures encoding parameters from the requested frame rate.

Changes

Screen Capture Recording

Layer / File(s) Summary
Capture configuration
electron/native/ScreenCaptureKitRecorder.swift, electron/native/ScreenCaptureKitRecorder.test.ts
The recorder configures native sourceRect cropping and uses the bi-planar video format for window and display capture. It sets output dimensions and encoding parameters from the crop and requested frame rate. Tests cover crop dimensions and bitrate configuration.
Window and display tracking
electron/native/ScreenCaptureKitRecorder.swift, electron/native/ScreenCaptureKitRecorder.test.ts
The recorder updates the crop rectangle from window-frame changes and updates the stream filter when the window changes displays. Tests check stream configuration updates during window movement.
First-frame handling and finalization
electron/native/ScreenCaptureKitRecorder.swift, electron/native/ScreenCaptureKitRecorder.test.ts
The recorder saves and retries a pending first frame until the writer is ready, appends accepted frames with adjusted timestamps, and retimes the last sample during finalization. Tests check the pending-frame retry behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ScreenCaptureKit
  participant Recorder
  participant AVAssetWriter
  ScreenCaptureKit->>Recorder: Deliver screen sample
  Recorder->>Recorder: Save pending first frame and schedule retries
  Recorder->>AVAssetWriter: Append retimed frame when writer is ready
  AVAssetWriter-->>Recorder: Report append result
  Recorder->>Recorder: Mark first accepted frame as ready
Loading

Suggested reviewers: webadderall

Merge Risk: ⚪ Minimal · up to cc2a8

No actionable merge-blocking regression was established; the change is mergeable after the normal build and recording checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to cc2a8

Failed crop updates can leave recording active with stale geometry, particularly when a window changes displays. This could capture unintended screen content. Actual frame delivery during that state remains unverified, and the existing caller authority is not expanded.

Retained concerns

  • Medium · security · inferred: Native crop-update failure can prolong stale capture geometry while frames remain admissible. After a successful display-filter switch, a failed sourceRect update has no rollback or admission barrier; repeated failures can retain the mismatch. Unlike the base software-crop publication, this introduces another fallible operation before geometry becomes consistent. Unintended pixels reaching the recording depend on unverified ScreenCaptureKit behavior.
Security review details

Security Blast Radius

  • inferred — The identified privacy risk concerns pixels on the active local capture display reaching the recording file. It requires an existing recording and a geometry transition with unsuccessful native reconfiguration; the inspected changes do not establish additional remote reachability or helper privileges.

Security Findings and Attack Paths

  • inferred — If the stream continues emitting complete frames after a failed crop update, stale geometry can select content outside the window's current location and the writer can persist it. This is a conditional privacy path, not verified disclosure. Base already had a transient update gap; the introduced concern is failure-dependent persistence of inconsistent geometry.

Trust Boundaries and Controls

  • observed — Initial geometry is intersected with the selected display, tracked geometry follows the initial capture frame using window-frame deltas, and application exclusions remain enforced by the display filter. Writer checks enforce lifecycle and readiness, but do not verify that filter and crop belong to the same completed geometry transition.

Resilience and Maintainability Implications

  • observed — The tracking loop retries after update errors on subsequent polling cycles. This provides recovery attempts but no explicit privacy containment while geometry is unresolved. The inspected tests check that update calls exist, not whether failed or intermediate updates suppress output.

Hardening Proposals

  • proposed — Treat display identity and crop geometry as one capture-policy transition. Establish frame-application guarantees, and suspend admission during unresolved transitions or failed updates until a matching configuration is confirmed; otherwise terminate capture safely.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title identifies the main user-visible change: sharper, smoother macOS window recordings. It is concise and relevant, though it does not mention native cropping.
Description check ✅ Passed The description explains the problem, motivation, changes, and testing steps. It includes screenshots and video-style evidence. The type-of-change, related-issue, and checklist sections from the templ…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Kevin-Liu-01 Kevin-Liu-01 changed the title Crop macOS window recordings natively and keep them sharp sharper, smoother window recordings on macos Oct 5, 2026
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.

1 participant