perf(recording): drain macOS encoder autorelease pools and recycle video frames by ryder777777 · Pull Request #3084 · CapSoftware/Cap

ryder777777 · 2026-10-24T10:20:57Z

/claim #109
Closes #109

Summary

Narrows the fix to the macOS recording paths that run in Studio and Instant recordings (with and without camera):

Drain Objective-C autorelease pools on pool-less capture & encoder worker threads:
- Wraps per-frame and busy-retry Objective-C/AVFoundation work in cidre::objc::ar_pool across AVFoundationMp4Muxer and AVFoundationCameraMuxer (crates/recording/src/output_pipeline/macos.rs), MacOSFragmentedM4SMuxer and MacOSFragmentedM4SCameraMuxer (macos_fragmented_m4s.rs), and OutOfProcessFragmentedM4SMuxer (oop_fragmented_m4s.rs).
- Switches AVCaptureVideoDataOutput's sample-buffer delegate queue in crates/camera/src/macos.rs from dispatch::Queue::new() (DISPATCH_AUTORELEASE_FREQUENCY_INHERIT) to dispatch::Queue::serial_with_ar_pool() (matching scap-screencapturekit) and wraps CameraInfo::formats_impl in objc::ar_pool.
Reuse ffmpeg::frame::Video plane buffers in-place across frames:
- Switches MacOSFragmentedM4SMuxer, MacOSFragmentedM4SCameraMuxer, and OutOfProcessFragmentedM4SMuxer from frame_pool.take_frame() (which forced a fresh AVFrame buffer allocation every frame) to frame_pool.get_frame() paired with SegmentedVideoEncoder::queue_frame_reusable / H264PacketEncoder::encode_frame_reusable.
- Guards FramePool::get_frame, H264Encoder::queue_frame_to_reusable, H264Encoder::queue_frame_reusable, and H264PacketEncoder::encode_frame_reusable with av_frame_make_writable so retained encoder reference buffers detach safely without overwriting in-flight frames.

Verification

cargo fmt --check (Rust 1.88.0)
cargo test --locked -p cap-enc-ffmpeg (212 passed, including reusable_queue_and_encode_preserve_plane_backing_and_detach_when_retained)
cargo clippy --locked -p cap-enc-ffmpeg -p cap-camera --all-targets -- -D warnings

vercel · 2026-10-24T10:21:04Z

@ryder777777 is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

greptile-apps · 2026-10-24T10:25:08Z

+        if reorder_buf.len() > reorder_window
+            && let Some((seq, oldest)) = reorder_buf.pop_first()
+        {
+            *next_encode_seq = seq.saturating_add(1);
+            if emit(oldest, frame_pool)? {


Delayed camera frames disappear

With two workers, one can hold sequence 0 while the other finishes 1, 2, and 3. This branch then exceeds reorder_window, encodes 1–3, and advances next_encode_seq. When 0 arrives, encode_converted_frame rejects its older timestamp, losing a valid frame even though neither queue dropped it.

Worker count does not limit how many frames another worker can finish first. Track which sequences were actually dropped, or limit workers from running ahead, rather than treating a full buffer as proof that a missing frame was dropped.

Knowledge Base Used: Capture and recording pipeline

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/recording/src/output_pipeline/async_camera.rs Line: 151-155 Comment: **Delayed camera frames disappear** With two workers, one can hold sequence 0 while the other finishes 1, 2, and 3. This branch then exceeds `reorder_window`, encodes 1–3, and advances `next_encode_seq`. When 0 arrives, `encode_converted_frame` rejects its older timestamp, losing a valid frame even though neither queue dropped it. Worker count does not limit how many frames another worker can finish first. Track which sequences were actually dropped, or limit workers from running ahead, rather than treating a full buffer as proof that a missing frame was dropped. **Knowledge Base Used:** [Capture and recording pipeline](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/capture-and-recording-pipeline.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fixed in 311332b — try_submit now only assigns sequence numbers when input_tx accepts the frame, worker-side conversion errors and full-output_tx drops explicitly record skipped sequences (advance_past_skipped), and worker run-ahead is bounded before submission instead of evicting delayed in-flight sequences from reorder_buf.

greptile-apps · 2026-10-24T10:25:08Z

+            let stats = pool.stats();
+            let in_flight = stats
+                .frames_received
+                .saturating_sub(stats.frames_converted + stats.frames_dropped);
            if encoded_this_call == 0
-                && backlog > 10
+                && in_flight > (self.reorder_window as u64).saturating_add(4)


Backlog counts overlap

in_flight subtracts counters that overlap. A frame increments frames_converted before delivery, then also increments frames_dropped if the output queue is full. Enough output drops make this calculation report zero despite queued work, disabling the intended short wait under load.

Conversion errors have the opposite effect: neither counter advances, leaving a permanent apparent backlog. Count unfinished conversions separately from dropped output frames, and count conversion failures as finished work.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/recording/src/output_pipeline/async_camera.rs Line: 416-421 Comment: **Backlog counts overlap** `in_flight` subtracts counters that overlap. A frame increments `frames_converted` before delivery, then also increments `frames_dropped` if the output queue is full. Enough output drops make this calculation report zero despite queued work, disabling the intended short wait under load. Conversion errors have the opposite effect: neither counter advances, leaving a permanent apparent backlog. Count unfinished conversions separately from dropped output frames, and count conversion failures as finished work. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fixed in 311332b — added an explicit in_flight atomic counter on AsyncConverterPool (incremented on accepted try_submit and decremented via RAII guard when a worker finishes converting, dropping, or erroring on each item), and updated both AsyncCameraMp4Muxer and drain_with_timeout to read pool.in_flight().

greptile-apps · 2026-10-24T10:25:08Z

+    let twenty_sends_elapsed = paced_start.elapsed();
+    // Previously, 20 calls after >10 drops slept 20 * 5ms = >=100ms unconditionally.
+    assert!(
+        twenty_sends_elapsed < Duration::from_millis(80),
+        "send_video_frame after burst drops took {twenty_sends_elapsed:?} (expected non-blocking in_flight tracking)"


Busy machines fail the test

The 80 ms limit measures frame allocation, encoding, and thread scheduling as well as the unwanted waits. A slow or busy machine can exceed it with correct code. The burst also does not establish that more than ten frames were dropped.

Test the wait decision with controlled counters and a fake receiver instead. Keep elapsed-time measurements in the ignored benchmark.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/recording/tests/recording_memory_pressure.rs Line: 168-172 Comment: **Busy machines fail the test** The 80 ms limit measures frame allocation, encoding, and thread scheduling as well as the unwanted waits. A slow or busy machine can exceed it with correct code. The burst also does not establish that more than ten frames were dropped. Test the wait decision with controlled counters and a fake receiver instead. Keep elapsed-time measurements in the ignored benchmark. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fixed in 311332b — removed wall-clock timing assertions from the CI test suite and replaced them with deterministic barrier/gate tests covering delayed peer workers (0..=4 preserved in exact order) plus >10 input drops, >10 output drops, and >10 conversion errors (pool.in_flight() == 0).

greptile-apps · 2026-10-24T10:39:01Z

+        "frames 1..=4 must wait for delayed sequence 0 rather than skipping it: {emitted_pts:?}"
+    );
+    assert_eq!(reorder_buf.len(), 4);
+    assert_eq!(pool.in_flight(), 1);


Counter check can hang tests

worker_loop sends each result before InFlightGuard lowers in_flight. Receiving the fourth peer frame therefore does not guarantee this counter is 1; it can still be 2.

If this assertion fails, sequence 0 is still blocked on release_rx.recv(). Dropping pool then waits forever for that worker before release_tx is dropped. Wait for worker completion before checking the counter, and release the gate during failure cleanup. The counter assertion at line 200 has the same race.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/recording/tests/recording_memory_pressure.rs Line: 179 Comment: **Counter check can hang tests** `worker_loop` sends each result before `InFlightGuard` lowers `in_flight`. Receiving the fourth peer frame therefore does not guarantee this counter is 1; it can still be 2. If this assertion fails, sequence 0 is still blocked on `release_rx.recv()`. Dropping `pool` then waits forever for that worker before `release_tx` is dropped. Wait for worker completion before checking the counter, and release the gate during failure cleanup. The counter assertion at line 200 has the same race. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fixed in 4d42b01 — bounded HoldFirstFrameConverter/GateAndFailConverter waits so panics can never hang join(), and waited for worker B's InFlightGuard to settle after its output_tx send before checking pool.in_flight() == 1.

Good catch on the second in_flight == 0 check and drop order — fixed in 50efb92 (ReleaseOnDrop declared after pool so unwinding always releases worker A first, and drain_with_timeout joins workers before asserting pool.in_flight() == 0).

greptile-apps · 2026-10-24T10:39:01Z

+        })
+        .unwrap();
+    }
+    assert!(pool.try_submit(tail, next_submit_seq).unwrap());


Tail submission can fail randomly

Emptying the output queue does not mean the input queue has room. The worker may still have two negative-PTS frames queued after the preceding submission loop. If it is paused there, this one-shot try_submit returns Ok(false) and fails the test even though the pool is working correctly.

Wait for the earlier work to finish or retry the tail submission, as the preceding loops do.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/recording/tests/recording_memory_pressure.rs Line: 299 Comment: **Tail submission can fail randomly** Emptying the output queue does not mean the input queue has room. The worker may still have two negative-PTS frames queued after the preceding submission loop. If it is paused there, this one-shot `try_submit` returns `Ok(false)` and fails the test even though the pool is working correctly. Wait for the earlier work to finish or retry the tail submission, as the preceding loops do. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fixed in 4d42b01 — wrapped the final valid frame submission in a bounded retry loop while draining output_rx so residual queued error frames on input_tx cannot cause a transient Ok(false).

richiemcilroy · 2026-10-24T12:21:55Z

Thanks for digging into this. Before we can take it, a few things need to change:

AsyncCameraMp4Muxer isn't used by the app. Outside its own file it's only reachable through pub use, and AsyncConverterPool is only used by the benchmark examples. So the reorder buffer, skipped-sequence tracking, in_flight counter, lazy VideoFramePool, the new CAP_CAMERA_MUXER_* env vars, the 4/120/90 to 2/16/16 default change and most of tests/recording_memory_pressure.rs don't affect real recordings. It also means the Linux numbers in the description measure a muxer the app never runs. Please drop that part from this PR (or open it separately if you think the muxer should be wired up).
If the ordered path stays anywhere, it can stall. If a sequence never comes back, for example after a converter worker panics, next_encode_seq stops advancing. After that, the run-ahead guard drops every later frame for the rest of the recording. Today the unordered path just loses one frame.
The changes that do reach users are the ar_pool wrapping in macos.rs, macos_fragmented_m4s.rs, oop_fragmented_m4s.rs and camera/src/macos.rs, plus in-place frame reuse through queue_frame_reusable/encode_frame_reusable. That code is macOS only and hasn't been compiled or run yet, because CI hasn't run on this branch. Please keep the PR to that part. Then we can run macOS CI and compare memory before and after on real Macs (Studio and Instant, with and without camera, 1080p and 4K) against current main.

Once it's narrowed down, we'll approve the workflows and test it.

… in-place - Wrap per-frame and retry AVFoundation Objective-C work in cidre::objc::ar_pool across AVFoundationMp4Muxer, AVFoundationCameraMuxer, MacOSFragmentedM4SMuxer, MacOSFragmentedM4SCameraMuxer, and OutOfProcessFragmentedM4SMuxer. - Switch camera capture output dispatch queue to dispatch::Queue::serial_with_ar_pool() and wrap CameraInfo::formats_impl in objc::ar_pool. - Reuse FramePool's backing ffmpeg::frame::Video in-place via queue_frame_reusable / encode_frame_reusable with av_frame_make_writable copy-on-write safety guards.

ryder777777 · 2026-10-24T13:09:16Z

Good catch on AsyncCameraMp4Muxer not being wired into the live app paths — dropped all of the async_camera / frame-converter / memory_profiling changes and narrowed this down to f55dc6d5 on top of current main:

cidre::objc::ar_pool around the per-frame and busy-retry loops in output_pipeline/macos.rs, macos_fragmented_m4s.rs, and oop_fragmented_m4s.rs
objc::ar_pool in CameraInfo::formats_impl plus switching the AVCaptureVideoDataOutput callback queue in camera/src/macos.rs from dispatch::Queue::new() to dispatch::Queue::serial_with_ar_pool() (matching scap-screencapturekit)
In-place FramePool::get_frame() reuse through queue_frame_reusable / encode_frame_reusable, with av_frame_make_writable guards on the reusable conversion buffers

Ready whenever you want to kick off macOS CI and compare on hardware.

algora-pbc Bot added the 🙋 Bounty claim label Oct 24, 2026

greptile-apps Bot reviewed Oct 24, 2026

View reviewed changes

ryder777777 force-pushed the perf/recording-memory-pressure-109 branch from 50efb92 to f55dc6d Compare October 24, 2026 13:07

Conversation

ryder777777 commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Uh oh!

vercel Bot commented Oct 24, 2026

Uh oh!

greptile-apps Bot Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

ryder777777 Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

ryder777777 Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

ryder777777 Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

ryder777777 Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

ryder777777 Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

ryder777777 Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

richiemcilroy commented Oct 24, 2026

Uh oh!

ryder777777 commented Oct 24, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ryder777777 commented Oct 24, 2026 •

edited

Loading