Repository navigation
fix(enc-ffmpeg): force recording keyframes every 2 s of capture time - #2641
Closed
richiemcilroy wants to merge 3 commits into
Closed
richiemcilroy wants to merge 3 commits into
richiemcilroy wants to merge 3 commits into
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Member
Author
|
hey @greptileai, please re-review the PR |
Member
Author
|
hey @greptileai, please re-review the PR |
richiemcilroy
force-pushed
the
fix/time-based-keyframes
branch
from
October 16, 2026 10:42
2c1c57b to
94ace03
Compare
Member
Author
|
Replaced by #2646 (merged): Studio now flushes a fragment every 2 s of capture time without forcing extra keyframes, so the encoded size of sparse recordings is unchanged. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Crash-recoverable recordings are written as 2 s fragments, but a fragment can only end at a keyframe, and every recording encoder places keyframes by frame count. When frames arrive slower than the nominal rate, a fragment covers far more than 2 s of capture, and a crash loses all of it.
flowchart LR A[ScreenCaptureKit / WGC<br/>delivers only changes] --> B[encoder<br/>g = 2 s x nominal fps frames] B --> C[DASH writer<br/>cuts at the first keyframe after 2 s] C --> D[fragment on disk] style B fill:#fddgisround(2 x nominal fps)frames for libx264, NVENC, QSV, AMF and Media Foundation (crates/enc-ffmpeg/src/video/h264.rs:1225); recording VideoToolbox keeps FFmpeg's defaultgop_sizeof 12 frames.KeyframeCadence(crates/enc-ffmpeg/src/video/keyframe_cadence.rs) sends a frame as a keyframe (pict_type = I) once 2 s of capture time passed since the last keyframe. The last keyframe is the newest one that was requested by the caller, forced here, or seen in the encoder's output packets; the encoder's own frame count (gop_size) is only a prediction until its packet confirms it, so a scene-cut keyframe or a skipped one cannot shift the timer. Frames at the nominal rate are keyframes exactly where the encoder puts them today.pict_type = Ion the input frame) are kept and restart the interval, also when the frame goes through the converter.H264EncoderandH264PacketEncoderbuilt without export settings); exports are unchanged. NVENC and QSV getforced-idr/forced_idrso a forced frame is an IDR the writer can cut at; libx264 (closed GOP) and VideoToolbox already turn a forced I frame into an IDR.Keyframes and crash loss
Keyframe times from the recording encoder (unit tests in
h264_packet.rs), 30 fps nominal:Engine-verify
crashsuite, static screen (3 frames per second at 30 fps nominal), recorder killed after 11 s, Blacksmith runners:Real capture on Blacksmith macOS (libx264 path, ScreenCaptureKit delivering about 33 fps): the Studio crash-recovery case lost 4.5 to 4.8 s of screen before (fragments of 3.68 s, keyframes every 120 frames). With this change and #2640 it stays inside its 3 s budget in 10 of 10 runs on macOS 15 and 26.


Confidence Score: 5/5
No new blocking issue was established.Summary
Recording H.264 encoders now request keyframes using capture time rather than frame count alone.
Both earlier threads were manually resolved without explanation. They were unnumbered, so neither receives a
previousFindingsentry. The VideoToolbox test now checks a minimum keyframe count.The full PR diff was used because the previous and current commits have different merge bases with the target commit. No tests were run.
Reviews (3) · Last reviewed commit: "test(enc-ffmpeg): require VideoToolbox k..." · Reviewed by Greptile