Skip to content

Commit a9b430e

Browse files
committed
Harden CineScreen: crash/data-loss fixes, correctness, perf, tests, CI
One squashed commit covering a full-repo hardening pass. Recording safety - Stop hotkey is ⌥⎋, not bare ESC (a global unmodified-ESC monitor silently ended recordings whenever ESC was pressed in the recorded app). - Stream death mid-recording (display disconnect, permission revoked) is surfaced: the session salvages captured frames via the normal stop path and the HUD swaps to a failure notice instead of counting over a dead stream. - Quit guard stops-and-saves before terminating; automatic termination is disabled (the OS could reclaim the app mid-recording while its windows were hidden). ⌘N is disabled while a session is active. - Cancelled/failed recordings no longer leave permanent "Incomplete" tiles; a metadata-write failure no longer deletes the captured video. Data safety - Project delete moves to the Trash behind a confirmation, and only folders with CineScreen artifacts are listed (pointing the library at a general directory used to make arbitrary folders one misclick from permanent deletion). - Editor edits autosave (debounced) and flush on window close — the Save button was the only write path and closing the window lost everything. Export - Failure-safe export session: resume-once teardown (double-resumed continuations crashed the app on e.g. disk-full), shared stop flags (a dead video loop hung the audio loop forever), temp file + atomic promote (failures destroyed existing files and left broken partials), Cancel button, and decoder failures fail loudly instead of freeze-framing a "successful" export. Ordered progress stream. - Webcam overlay is time-aligned: the camera warm-up offset (webcamFirstPTS − screenFirstPTS, both host-clock) is persisted as metadata.webcamOffsetMs and applied in editor playback and export. Capture correctness - Cursor coordinates are correct for window/region/secondary-display capture: mapping is (global − contentRect.origin) × px/pt against the captured content's rect, not full-display bounds; the seed sample converts Cocoa→CG via the primary screen. - The Low/Medium/High quality picker actually applies its bitrate; duration derives from the last video frame's PTS instead of a wall clock that included startup latency; frame counter race fixed. Editor correctness + perf - Adaptive cursor smoothing: speed + lag urgency, mouse-down-only click window (binary-searched), validated by offline replay (click lag 21-149px → 0.3-1.9px; hover settle 467ms → 150ms). - RenderSnapshot cached on the VM (it was rebuilt 3× per frame including 240Hz pan-track integration — preview stutter scaled with zoom coverage); invalidation centralised in metadata.didSet. - Zoom sections can no longer overlap, cross, or collapse; persisted order is repaired on load; pan table sorted defensively. - Preview canvas locks to the export aspect (webcam placement, padding, and shadow used to silently diverge from the exported file). - Cancel-safe gestures via @GestureState; pinch zoom no longer compounds exponentially; webcam drag/resize inverts the exact forward mapping. Foundations - New CineScreenTests target (19 tests over smoothing, snapshot policy, metadata codec, zoom generation, projects library) + make test. - CI builds and tests every push to main (previously PR-only, so main was never verified). Releases gate on tag == MARKETING_VERSION, Sparkle is pinned to exactly 2.6.4, release concurrency serialised, broken workflow_dispatch removed, artifacts named -universal. - Dead code removed (legacy window-picker plumbing and its UI, SimplePlayerView, fossil scripts, write-only fields); control-bar frame-rate/quality settings persist; README inaccuracies and committed tool-call residue fixed; Info.plist pseudo-keys dropped. - .claude/skills/ added: dev-loop, debug-recording, pitfalls, release.
1 parent 359f0db commit a9b430e

43 files changed

Lines changed: 1799 additions & 508 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
---
2+
name: debug-recording
3+
description: Debug CineScreen capture, cursor, playback, and export issues — logs, recording artifacts on disk, the ffmpeg frame-extraction validation method, and the offline smoother-replay harness. Use when investigating "cursor is off", A/V desync, export problems, or any recording misbehaviour.
4+
---
5+
6+
# Debugging recordings
7+
8+
## Logs
9+
10+
Everything logs through `os.Logger` via `Log.*` (CineScreen/Util/Logger.swift):
11+
subsystems `capture`, `session`, `mouse`, `editor`, `app`. Stream live:
12+
13+
```bash
14+
log stream --predicate 'subsystem == "com.cinescreen.app"' --level debug
15+
```
16+
17+
(Adjust the subsystem string to what Logger.swift declares — check it first.)
18+
19+
## Recording artifacts on disk
20+
21+
Projects live at `~/Documents/CineScreen/Recording YYYY-MM-DD HH-mm-ss/`:
22+
23+
- `recording.mp4` — the capture (H.264, PTS rebased to start at 0)
24+
- `recording.json` — sidecar metadata: cursor keyframes + clicks in
25+
**video-pixel space, top-left origin**; zoom sections; trim; canvas;
26+
`webcamOffsetMs` (screenT = webcamT + offset)
27+
- `webcam.mp4` — optional webcam track (its t=0 lags the screen's by
28+
`webcamOffsetMs` — camera warm-up)
29+
- `project.json` — descriptor (this file is what makes a folder count as a
30+
project in the library)
31+
32+
Inspect metadata quickly:
33+
34+
```bash
35+
python3 -c "import json;m=json.load(open('recording.json'));print(m['video'],len(m['cursor']['keyframes']),'kf',len(m['clicks']),'clicks')"
36+
```
37+
38+
## "Cursor position is off" — validation method (proven)
39+
40+
The rendered cursor is a **synthetic sprite** (the capture hides the real
41+
one, `showsCursor=false`), positioned from `recording.json` + spring
42+
smoothing. Two distinct root causes exist; identify which before fixing:
43+
44+
1. **Smoothing lag** (display recordings): raw coordinates are correct; the
45+
sprite trails because of `SmoothPosition2D` glide. Validate: extract the
46+
frame at a click timestamp and check the raw (x,y) lands on the clicked
47+
UI element —
48+
49+
```bash
50+
ffmpeg -ss <t_seconds> -i recording.mp4 -frames:v 1 click.png
51+
```
52+
53+
then overlay a crosshair at the click's (x,y) from recording.json.
54+
2. **Capture geometry** (window/region/secondary-display recordings): the
55+
mapping is `(globalPoint − contentRectPoints.origin) × pixelsPerPoint`
56+
(MouseTrackingService). If raw coordinates themselves miss the target,
57+
the geometry (CaptureInfo.contentRectPoints) is wrong for that mode.
58+
59+
## Offline smoother replay (no GUI needed)
60+
61+
To evaluate smoothing changes numerically, write a standalone Swift script
62+
that mirrors `Spring.swift` + the relevant `RenderSnapshot` math, replays
63+
`recording.json` at a 60fps grid, and reports sprite↔raw lag at mouse-downs
64+
plus settle time after moves. This pattern validated the adaptive-smoothing
65+
work (click lag 21–149px → 0.3–1.9px). Keep the script out of the repo
66+
(/tmp) — the real math lives in unit tests (Tests/CineScreenTests).
67+
68+
## Preview vs export divergence
69+
70+
Preview (`MetalRenderer`) and export (`ExportCompositor`) share Shaders.metal
71+
and `RenderSnapshot`, but have **duplicated pipeline setup and uniform struct
72+
definitions** ("must match" comments). If a visual effect differs between
73+
preview and exported file, diff those two files first — and remember the
74+
preview canvas is aspect-locked to the video (EditorView) precisely so
75+
canvas-relative placement matches the export.

.claude/skills/dev-loop/SKILL.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
name: dev-loop
3+
description: Build, test, and iterate on CineScreen — XcodeGen workflow, make targets, stale-module fixes, and verification discipline. Use when building, running, or testing this repo, or when the build fails mysteriously.
4+
---
5+
6+
# CineScreen development loop
7+
8+
## Project generation — edit project.yml, never the pbxproj
9+
10+
`CineScreen.xcodeproj` is **gitignored and generated**. Adding files, targets,
11+
settings, or SPM packages happens in `project.yml`, then:
12+
13+
```bash
14+
make project # xcodegen generate
15+
```
16+
17+
New Swift files under `CineScreen/` or `Tests/CineScreenTests/` are picked up
18+
by the source globs automatically — but the xcodeproj must be regenerated to
19+
see them.
20+
21+
## Build / test / run
22+
23+
```bash
24+
make build # Debug build (the type-check gate — run before claiming any change works)
25+
make test # unit tests (CineScreenTests, app-hosted, ~20s incremental)
26+
make open # open in Xcode
27+
```
28+
29+
Verification discipline: a change is not done until `make build` (or
30+
`make test` when logic changed) passes. There is no standalone type-checker —
31+
the build IS the check.
32+
33+
## Known failure modes
34+
35+
- **SourceKit/IDE diagnostics say "Cannot find type X in scope" everywhere**
36+
for types that clearly exist (CTheme, RecordingMetadata, Log, …): false
37+
positives when the xcodeproj hasn't been generated or is stale. Ignore
38+
them; trust `make build`. Run `make project` to quiet the IDE.
39+
- **"file has been modified since the module file was built"** (usually a
40+
Sparkle header): stale precompiled modules after a dependency version
41+
change. Fix: `rm -rf build/derived` and rebuild.
42+
- **Sparkle is pinned to exactly 2.6.4** in project.yml. Don't float it —
43+
Package.resolved lives inside the gitignored xcodeproj and can't pin
44+
anything, and CI pins the 2.6.4 appcast CLI tools to match.
45+
46+
## CI
47+
48+
`.github/workflows/build.yml` builds + tests every PR **and every push to
49+
main** (unsigned Debug). Releases run `release.yml` on `v*.*.*` tags only —
50+
see the `release` skill.

.claude/skills/pitfalls/SKILL.md

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
---
2+
name: pitfalls
3+
description: CineScreen's sharp edges and invariants — metadata invalidation, zoom-section rules, coordinate spaces, PTS rebasing, AVFoundation callback traps, SwiftUI gesture state, autosave suppression. Read before modifying capture, editor, or export code.
4+
---
5+
6+
# Common pitfalls & invariants
7+
8+
## Editor state
9+
10+
- **`EditorViewModel.metadata.didSet` is the single invalidation choke
11+
point**: it clears `cachedSnapshot` + `cachedZoomSections` and schedules
12+
the debounced autosave. Mutate metadata *through the property* (value-type
13+
write-back does this automatically, e.g. `metadata?.zoom.sections = …`).
14+
Never cache derived render state anywhere else.
15+
- **`suppressAutosave` must wrap any code that loads state INTO the VM**
16+
(loadMetadata, derived defaults) — otherwise merely opening a recording
17+
rewrites its sidecar.
18+
- **Trim lives twice** (vm.trimStartMs/EndMs ↔ metadata.trim) and is synced
19+
by the trim didSets. Don't add a third copy.
20+
- **Zoom sections invariant: sorted by startTime, non-overlapping, ≥100ms.**
21+
`updateZoomSection` enforces it by clamping to neighbours (which is also
22+
why no mid-drag re-sort is needed — a drag can't cross a neighbour).
23+
`RenderSnapshot.init` sorts defensively; the pan table is binary-searched
24+
by time and breaks on non-monotonic input.
25+
- **Drag gesture baselines must be `@GestureState`, not `@State`** — SwiftUI
26+
never calls `onEnded` for a *cancelled* gesture, and stale `@State` wedged
27+
scrubbing/drags before. Same for pinch: `MagnifyGesture.magnification` is
28+
cumulative from gesture start; scale a gesture-start baseline or it
29+
compounds exponentially.
30+
31+
## Coordinate spaces (four of them — never mix)
32+
33+
1. **CGEvent global points**: top-left origin of the primary display. This is
34+
what the mouse tap yields. (NSEvent.mouseLocation is bottom-left Cocoa —
35+
convert once using the *primary* screen, not NSScreen.main.)
36+
2. **Recorded-file pixels**: top-left; metadata keyframes/clicks live here.
37+
Mapping: `(global − CaptureInfo.contentRectPoints.origin) × px/pt`.
38+
3. **UV [0,1]²** in shaders (video space), then **NDC** with y-up; canvas
39+
passes multiply `aspectScale` then `canvas.contentScale` — every overlay
40+
pass (video, cursor, clicks) must apply both or it drifts under padding.
41+
4. **Webcam layout norms** are relative to the *padded content rect*:
42+
on-screen px = norm × contentScale × viewSize. Invert exactly.
43+
44+
## Timing
45+
46+
- All capture PTS are **rebased so the file starts at 0** (first screen
47+
frame is the base; mic + system audio rebase against it and drop
48+
negative-PTS samples).
49+
- **Webcam**: screenT = webcamT + `metadata.webcamOffsetMs` (camera warm-up).
50+
Editor seeks and export reads must apply the mapping; playback defers the
51+
webcam start inside the warm-up gap.
52+
- Recorded duration comes from the **last video frame's rebased PTS**, not
53+
wall clock (wall clock includes ~0.3–1s of startup latency).
54+
55+
## AVFoundation traps
56+
57+
- `requestMediaDataWhenReady` blocks are **re-invoked after failures** (a
58+
failed writer forces `isReadyForMoreMediaData=true` so you can poll the
59+
error). Any continuation resumed from such a block needs a resume-once
60+
guard + `markAsFinished()` — see ExportPipeline's `finish(_:)` pattern.
61+
Double-resume = runtime trap.
62+
- With multiple writer inputs, the writer **interleaves**: a stalled/failed
63+
input blocks the others forever. Mark the failed input finished so
64+
siblings' callbacks fire and can exit (shared `ExportSessionState`).
65+
- `alwaysCopiesSampleData = true` on reader outputs feeding
66+
CVMetalTextureCache is load-bearing: cached textures pin decoder pool
67+
buffers; without copies the decoder stalls (~2s in, export "freezes").
68+
- Export writes to a hidden temp file and promotes atomically on success —
69+
never write directly to the user's chosen path.
70+
71+
## Capture
72+
73+
- `ScreenCaptureService.stop()` tolerates an already-dead stream — stream
74+
death mid-recording routes through `onRuntimeFailure` → RecordingSession
75+
salvages via the normal stop path.
76+
- The stop hotkey is **⌥⎋ exactly** (caps-lock tolerated). Never rebind to
77+
unmodified ESC (it silently ended recordings from the recorded app) and
78+
never match ⌘⌥⎋ (Force Quit).
79+
- After granting Screen Recording, macOS requires an **app relaunch** before
80+
`CGPreflightScreenCaptureAccess()` returns true — a permission that "won't
81+
turn green" in-process is expected, not a bug.
82+
- `_CGSCurrentCursorSeed` is private SPI (cursor-shape detection). It can
83+
vanish in an OS update; if cursor-shape code crashes at launch, look here.
84+
85+
## Process & release
86+
87+
- Each improvement = its own commit, pushed (user preference).
88+
- `NSSupportsAutomaticTermination` must stay **false** — a recorder with all
89+
regular windows hidden must not be reclaimable mid-capture.
90+
- CFBundleVersion = `git rev-list --count HEAD` (Sparkle monotonicity).
91+
History rewrites that reduce the commit count below the last release's
92+
would break auto-update — check before squashing/rebasing main.
93+
- The release tag must equal project.yml's MARKETING_VERSION (CI preflight
94+
enforces it).

.claude/skills/release/SKILL.md

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
name: release
3+
description: Cut a CineScreen release — version bump, tag, CI pipeline, Sparkle appcast, and the invariants that keep auto-update working. Use when releasing a new version or debugging the release/update pipeline.
4+
---
5+
6+
# Cutting a release
7+
8+
## The happy path
9+
10+
1. Bump `MARKETING_VERSION` in **project.yml** (nowhere else).
11+
2. Commit and push to main; wait for CI green.
12+
3. Tag and push — the tag must match the version exactly:
13+
14+
```bash
15+
git tag v2.6.0 && git push origin v2.6.0
16+
```
17+
18+
4. `release.yml` takes over: preflight asserts tag == MARKETING_VERSION,
19+
then the reusable build workflow signs with Developer ID (secrets),
20+
notarizes the app ZIP + DMG, staples, EdDSA-signs the ZIP, generates
21+
`appcast.xml`, publishes a GitHub Release, and deploys the appcast to
22+
`gh-pages`. Existing users auto-update via Sparkle.
23+
24+
## Invariants — break these and auto-update breaks
25+
26+
- **Tag == MARKETING_VERSION.** CI preflight fails the release otherwise.
27+
Without it, Sparkle would still push mismatched assets to every user,
28+
because…
29+
- **CFBundleVersion = `git rev-list --count HEAD`** (set by
30+
scripts/make_release.sh). It must increase every release. Never rewrite
31+
main's history in a way that drops the commit count below the previous
32+
release's count.
33+
- **Sparkle framework is pinned to exactly 2.6.4** in project.yml, matching
34+
the `SPARKLE_VERSION` CLI tools pinned in build-workflow.yml. Bump both
35+
together.
36+
- **`SUPublicEDKey` in Info.plist is the real production key.** Updates are
37+
signature-verified against it; the matching private key is the
38+
`SPARKLE_PRIVATE_KEY` repo secret.
39+
- In-place updates preserve TCC grants (Screen Recording/Accessibility)
40+
because the app is replaced at its existing path with the same signing
41+
identity — don't change the bundle id or signing identity casually.
42+
43+
## Artifacts
44+
45+
`CineScreen-<version>-universal.{dmg,zip}` — the archive is universal
46+
(ARCHS_STANDARD, ONLY_ACTIVE_ARCH=NO). The ZIP is what the appcast
47+
enclosures point at; the DMG is the human download.
48+
49+
## Local / debugging
50+
51+
- `bash scripts/make_release.sh` is the real pipeline (the Makefile's
52+
archive/export/dmg targets are a simplified local approximation).
53+
- `--archive-only` stops after archiving.
54+
- Notarization credentials: `xcrun notarytool store-credentials
55+
cinescreen-notary …` (see README).
56+
- There is deliberately **no workflow_dispatch** on release.yml — running it
57+
from a branch baked broken enclosure URLs into the appcast. Tag pushes
58+
only.

.github/workflows/build-workflow.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ jobs:
7070
sed -i '' 's/objectVersion = 77;/objectVersion = 70;/' CineScreen.xcodeproj/project.pbxproj
7171
grep -m1 "objectVersion" CineScreen.xcodeproj/project.pbxproj
7272
73-
- name: Build (Debug, no signing)
73+
- name: Build & test (Debug, no signing)
7474
if: '!inputs.release'
7575
run: |
7676
set -o pipefail
@@ -83,7 +83,7 @@ jobs:
8383
-destination 'platform=macOS' \
8484
CODE_SIGN_IDENTITY="-" \
8585
CODE_SIGNING_REQUIRED=NO \
86-
build | tee build/build.log
86+
build test | tee build/build.log
8787
8888
# ----- Code signing setup (release only) ------------------------------
8989
- name: Import signing certificate

.github/workflows/build.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
name: CI
22

3+
# PRs and pushes to main both build + test. Commits land directly on main in
4+
# this repo, so a PR-only trigger left main permanently unverified.
35
on:
46
pull_request:
7+
push:
8+
branches: [main]
59

610
jobs:
711
build:

.github/workflows/release.yml

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,41 @@
11
name: Release
22

3+
# Tag-push only. workflow_dispatch was removed deliberately: run from a
4+
# branch, github.ref_name is the branch name, which baked broken enclosure
5+
# URLs into the appcast and broke the release step's tag lookup.
36
on:
47
push:
58
tags:
69
- 'v*.*.*'
7-
workflow_dispatch:
810

911
permissions:
1012
contents: write
1113

14+
# One release at a time — concurrent tag pushes raced each other publishing
15+
# to gh-pages.
16+
concurrency:
17+
group: release
18+
cancel-in-progress: false
19+
1220
jobs:
21+
preflight:
22+
runs-on: ubuntu-latest
23+
steps:
24+
- uses: actions/checkout@v4
25+
- name: Assert tag matches MARKETING_VERSION
26+
run: |
27+
TAG="${GITHUB_REF_NAME#v}"
28+
VERSION="$(grep -E '^\s*MARKETING_VERSION:' project.yml | head -1 | sed 's/.*"\(.*\)".*/\1/')"
29+
if [ "$TAG" != "$VERSION" ]; then
30+
echo "Tag v$TAG does not match project.yml MARKETING_VERSION=$VERSION." >&2
31+
echo "Bump MARKETING_VERSION and commit before tagging — Sparkle's" >&2
32+
echo "CFBundleVersion is the commit count, so a mismatched tag would" >&2
33+
echo "still auto-update every user to wrongly-versioned assets." >&2
34+
exit 1
35+
fi
36+
1337
build:
38+
needs: preflight
1439
uses: ./.github/workflows/build-workflow.yml
1540
with:
1641
upload-artifacts: true

CineScreen/App/AppState.swift

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@ final class AppState {
2424
var captureCamera: Bool = false
2525
/// AVCaptureDevice uniqueID for the selected webcam. nil = system default.
2626
var selectedCameraID: String? = nil
27-
/// `nil` = capture the entire display. Otherwise the chosen on-screen window.
28-
var selectedWindowID: CGWindowID? = nil
29-
var availableWindows: [CaptureWindow] = []
3027

3128
// Permissions snapshot (refreshed on a timer and after each request)
3229
var permissions: PermissionStatus
@@ -124,16 +121,6 @@ final class AppState {
124121
permissions = Permissions.currentStatus()
125122
}
126123

127-
// MARK: - Windows
128-
129-
func refreshAvailableWindows() async {
130-
do {
131-
availableWindows = try await ScreenCaptureService.availableWindows()
132-
} catch {
133-
Log.app.error("Failed to enumerate windows: \(error.localizedDescription)")
134-
}
135-
}
136-
137124
// MARK: - Projects
138125

139126
func refreshProjects() {
@@ -142,7 +129,7 @@ final class AppState {
142129

143130
/// Creates a new project in the configured library and selects it as the
144131
/// recording target. The video will be written to
145-
/// `<projectFolder>/recording.mov`.
132+
/// `<projectFolder>/recording.mp4`.
146133
func beginNewProject() -> Project? {
147134
do {
148135
let project = try ProjectsLibrary.createNew(in: projectsDirectory)

0 commit comments

Comments
 (0)