Skip to content

perf(recording): drain macOS autorelease pools and reuse video frames in-place - #3625

Open
duongynhi000005-oss wants to merge 1 commit into
CapSoftware:mainfrom
duongynhi000005-oss:perf/issue-109-recording-pipeline-cpu-memory
Open

duongynhi000005-oss wants to merge 1 commit into
CapSoftware:mainfrom
duongynhi000005-oss:perf/issue-109-recording-pipeline-cpu-memory

Conversation

@duongynhi000005-oss

@duongynhi000005-oss duongynhi000005-oss commented Nov 1, 2026 •

Copy link
Copy Markdown

Fixes #109

Summary

This PR addresses the three primary sources of sustained CPU and memory growth in the macOS recording pipeline reported in #109:

  1. Drain Objective-C autorelease pools on per-frame worker threads and AVFoundation camera capture queues (crates/camera/src/macos.rs, crates/recording/src/output_pipeline/macos.rs, macos_fragmented_m4s.rs, oop_fragmented_m4s.rs):

    • In crates/camera/src/macos.rs, start_capturing_impl previously created the AVFoundation sample buffer delegate queue via dispatch::Queue::new() (default DISPATCH_QUEUE_SERIAL without DISPATCH_AUTORELEASE_FREQUENCY_WORK_ITEM), and CameraInfo::formats_impl enumerated AVCaptureDeviceFormat / NSString descriptions without an autorelease pool. Switching the capture queue to dispatch::Queue::serial_with_ar_pool() and wrapping ormats_impl in objc::ar_pool ensures transient Objective-C objects are drained immediately.
    • In macos.rs, macos_fragmented_m4s.rs, and oop_fragmented_m4s.rs, the long-lived std::thread::Builder encoder worker loops (mp4-video-encoder, mp4-audio-encoder, mp4-camera-encoder, mp4-camera-audio-encoder, m4s-segment-encoder, m4s-camera-segment-encoder, and oop-m4s-segment-encoder) run without a RunLoop, so transient CMSampleBuffer / AVAssetWriter Objective-C objects autoreleased during each frame or NotReadyForMore retry previously accumulated until recording stopped. Wrapping each frame and retry iteration in cidre::objc::ar_pool drains autoreleased objects per frame.
  2. Reuse AVFrame backing buffers in-place across the software M4S and OOP M4S pipelines (crates/recording/src/output_pipeline/macos_frame_convert.rs, macos_fragmented_m4s.rs, oop_fragmented_m4s.rs, crates/enc-ffmpeg/src/video/h264.rs, h264_packet.rs, crates/enc-ffmpeg/src/mux/segmented_stream.rs):

    • Previously, FramePool::take_frame() moved the fmpeg::frame::Video out of self.frame on every frame without returning it to the pool, causing rame_pool.get_frame() to allocate a brand-new full-resolution AVFrame buffer on every single frame (60fps).
    • Updated FramePool::get_frame() to check �v_frame_make_writable on the pooled frame (reusing the backing buffer in-place when uniquely owned and detaching cleanly if retained downstream), added H264Encoder::queue_frame_to_reusable / SegmentedVideoEncoder::queue_frame_reusable with �v_frame_make_writable guards on converted_frame_pool, and wired MacOSFragmentedM4SMuxer, MacOSFragmentedM4SCameraMuxer, and OutOfProcessFragmentedM4SMuxer (encode_frame_reusable) to encode directly from the reusable pooled frame without per-frame heap reallocation.

Testing

  • Added unit test rame_pool_reuses_backing_buffer_when_writable_and_detaches_when_shared in crates/recording/src/output_pipeline/macos_frame_convert.rs.

RetriggerView in GreptileConfidence Score: 4/5

Fix the converted frame's lifetime across encoder rebuilds and satisfy the required Rust condition style before merging.

Findings

  1. P1 Recovery keeps the wrong buffer ▶
  2. P2 Failed buffer checks are ignored ▶
  3. P2 Nested conditions break repository rules ▶
Fix with agent prompt
### Issue 1
crates/recording/src/output_pipeline/oop_fragmented_m4s.rs:438
`converted_frame` now survives an encoder rebuild, but `build_standalone()` can fall back to another codec with a different output pixel format. A scaled BGRA recording can switch from VideoToolbox to `libx264`, which requires YUV420P. `encode_frame_reusable()` keeps the old buffer because it is already `Some`, so later conversions can fail and drop every remaining frame.

Clear `converted_frame` when replacing the encoder, or recreate it when the new encoder needs a different format.

### Issue 2
crates/enc-ffmpeg/src/video/h264.rs:1038-1040
The new `av_frame_make_writable()` calls discard their return values. If FFmpeg cannot allocate a private buffer, conversion still writes into the old buffer, which may remain shared with the encoder. Under memory pressure, this can overwrite a frame the encoder still uses.

Return the error before calling `convert_or_scale()`. Apply this to the added checks in both `h264.rs` and `h264_packet.rs`.

### Issue 3
crates/recording/src/output_pipeline/macos_frame_convert.rs:57-61
`get_frame()` adds a nested `if let` and `if`. The repository guide requires combining nested conditions with `&&` and lists `collapsible_if` among the denied checks. Use an `if let` chain here. This repository requirement must be satisfied before merging.

```suggestion
        if let Some(ref mut frame) = self.frame
            && unsafe { ffmpeg::ffi::av_frame_make_writable(frame.as_mut_ptr()) } < 0
        {
            self.frame = None;
        }
```

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!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR reduces repeated allocations and drains temporary Objective-C objects during macOS recording.

  • macOS capture and recording work drains temporary Objective-C objects regularly.
  • Software fragmented recording reuses video-frame buffers between samples.

Reviews (1) · Last reviewed commit: "perf(recording): drain macOS autorelease..." · Reviewed by Greptile

@vercel

vercel Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

@duongynhi000005-oss is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

let keyframe_emitted_ref = &mut keyframe_emitted;
let encode_result = encoder.encode_frame_reusable(
frame,
&mut converted_frame,

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.

P1 Recovery keeps the wrong buffer

converted_frame now survives an encoder rebuild, but build_standalone() can fall back to another codec with a different output pixel format. A scaled BGRA recording can switch from VideoToolbox to libx264, which requires YUV420P. encode_frame_reusable() keeps the old buffer because it is already Some, so later conversions can fail and drop every remaining frame.

Clear converted_frame when replacing the encoder, or recreate it when the new encoder needs a different format.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/oop_fragmented_m4s.rs
Line: 438

Comment:
**Recovery keeps the wrong buffer**

`converted_frame` now survives an encoder rebuild, but `build_standalone()` can fall back to another codec with a different output pixel format. A scaled BGRA recording can switch from VideoToolbox to `libx264`, which requires YUV420P. `encode_frame_reusable()` keeps the old buffer because it is already `Some`, so later conversions can fail and drop every remaining frame.

Clear `converted_frame` when replacing the encoder, or recreate it when the new encoder needs a different format.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines +1038 to +1040
unsafe {
ffmpeg::ffi::av_frame_make_writable(converted.as_mut_ptr());
}

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.

P2 Failed buffer checks are ignored

The new av_frame_make_writable() calls discard their return values. If FFmpeg cannot allocate a private buffer, conversion still writes into the old buffer, which may remain shared with the encoder. Under memory pressure, this can overwrite a frame the encoder still uses.

Return the error before calling convert_or_scale(). Apply this to the added checks in both h264.rs and h264_packet.rs.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/enc-ffmpeg/src/video/h264.rs
Line: 1038-1040

Comment:
**Failed buffer checks are ignored**

The new `av_frame_make_writable()` calls discard their return values. If FFmpeg cannot allocate a private buffer, conversion still writes into the old buffer, which may remain shared with the encoder. Under memory pressure, this can overwrite a frame the encoder still uses.

Return the error before calling `convert_or_scale()`. Apply this to the added checks in both `h264.rs` and `h264_packet.rs`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines +57 to +61
if let Some(ref mut frame) = self.frame {
if unsafe { ffmpeg::ffi::av_frame_make_writable(frame.as_mut_ptr()) } < 0 {
self.frame = None;
}
}

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.

P2 Nested conditions break repository rules

get_frame() adds a nested if let and if. The repository guide requires combining nested conditions with && and lists collapsible_if among the denied checks. Use an if let chain here. This repository requirement must be satisfied before merging.

Suggested change
if let Some(ref mut frame) = self.frame {
if unsafe { ffmpeg::ffi::av_frame_make_writable(frame.as_mut_ptr()) } < 0 {
self.frame = None;
}
}
if let Some(ref mut frame) = self.frame
&& unsafe { ffmpeg::ffi::av_frame_make_writable(frame.as_mut_ptr()) } < 0
{
self.frame = None;
}

Context Used: AGENTS.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/macos_frame_convert.rs
Line: 57-61

Comment:
**Nested conditions break repository rules**

`get_frame()` adds a nested `if let` and `if`. The repository guide requires combining nested conditions with `&&` and lists `collapsible_if` among the denied checks. Use an `if let` chain here. This repository requirement must be satisfied before merging.

```suggestion
        if let Some(ref mut frame) = self.frame
            && unsafe { ffmpeg::ffi::av_frame_make_writable(frame.as_mut_ptr()) } < 0
        {
            self.frame = None;
        }
```

**Context Used:** AGENTS.md ([source](https://github.com/capsoftware/cap/blob/main/AGENTS.md))

---

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!

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

1 participant