Skip to content

fix(verify): match Studio crash checks to fragment and encoder buffering - #2785

Open
jatmn wants to merge 1 commit into
CapSoftware:mainfrom
jatmn:fix/verify-studio-crash-xZXWHq6s
Open

jatmn wants to merge 1 commit into
CapSoftware:mainfrom
jatmn:fix/verify-studio-crash-xZXWHq6s

Conversation

@jatmn

@jatmn jatmn commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Crash verification can report failures on unchanged main because its harness differs from Studio and undercounts encoder buffering. This was observed in #2590 and #2772.

Match the in-process harness to Studio's timed-fragment configuration from #2646. A segment file can contain multiple complete moof/mdat fragments, including a recoverable prefix before an interrupted write; inspect those boundaries independently and decode cumulative prefixes to retain frame dependencies. Remove the two obsolete static-screen expected failures. Preserve the out-of-process EOF-drain contract from #2644 instead of applying the in-process timed-flush assertion to it.

Also account for the four frames retained by VideoToolbox before packet output, in addition to the three existing pipeline slots, for macOS in-process recorder loss only. The original three-frame allowance caused a separate random1 failure. In controlled tests, both old and Studio configurations lost exactly 65 frames when killed after frame 184, and recovered the next fragment when killed after frame 186. A control with unchanged production encoder/writer code reproduced both outcomes. Physical fragment accounting and production recovery agreed in every case.

The two-second fragment-span check, complete-frame preservation, timestamps, ordering, and OOP/non-macOS loss limits remain unchanged. No production recording code or CI scheduling changes.

Final validation passed on the exact candidate tree d8e0f4e8eb53eef28894e73a93ebe80efc0245dd:

  • Full crash suite: macOS 21/21, Linux 9/9, Windows 9/9; zero expected failures or unsupported cases.
  • Formatting, cargo check -p cap-verify --tests, and all three new regression tests passed on all platforms, using Rust 1.88 and locked dependencies.
  • Disabling timed flushing deliberately reproduced the static-screen failure, confirming that the fragment check still detects it.
  • Diagnostic runners: GitHub-hosted macOS 15 ARM64, Ubuntu 24.04, Windows Server 2022/VS2022. Upstream CI uses Blacksmith. GitHub's newer Windows 2025/VS2026 image failed in generated FFmpeg bindings before tests; the VS2022 run passed. No gate waiver used.

Reviewed commit: a7655b70c2cdaa895d48a9142a86e3df0b2020f3

Why main looked green: recent main pushes skipped engine suites for CLI-only changes. In the latest nightly, one static-screen case was an expected failure and the other did not exercise the old span assertion. See unchanged-main reproduction, packet trace, and control without encoder instrumentation.

@vercel

vercel Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

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

A member of the Team first needs to authorize it.

@jatmn
jatmn marked this pull request as ready for review October 21, 2026 00:00
@jatmn

jatmn commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

@jatmn

jatmn commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@richiemcilroy

Copy link
Copy Markdown
Member

Thanks @jatmn! We've reviewed this and it looks good to merge. This also explains the macOS crash_safety failures on main, nice find. Our automated reviewer (Greptile) is temporarily not posting reviews; as soon as it scores this 5/5 and CI is green on the branch updated with main, we'll merge it.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants