Skip to content

Commit 2c3e5c4

Browse files
Extract RecordingPipeline from CameraViewController (UI modernization PR 9) (#132)
* Extract RecordingPipeline from CameraViewController (UI modernization PR 9) Mechanical move, no behavior/threading changes: RecordingPipeline (non-UI) now owns the asset writer and its inputs, the six-boolean recording state machine, the writing queue, per-frame write/crop, and saving/sending the finished movie. The VC stays the sample-buffer delegate (captureOutput fans out to the frame streamers, which are live preview, not recording) and keeps the microphone-permission flow, forwarding recording frames to the pipeline. Seams: sendMessage (actor relay for the start ack, stop response and video resource — captures the session ref, not the VC), onRecordingStarted/Stopped (timer overlay), onModeChanged (idle vs recording chrome), onError, onPhotosAccessDenied. CameraViewController: 962 -> 632 lines. New device-free tests cover asset writer input setup including the even-rounded 4:3 crop rect and the stop-while-idle guard. 385/385 tests green (381 + 4 new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Apply review fixes to the RecordingPipeline extraction New-code cleanups from the senior Swift review: tighten access control on the pipeline (assetWriter/videoCropContext/writeSampleBuffer private, pixelBufferAdaptor/cachedVideoCropRect private(set)), document that onError fires on the writing queue, name the idle condition at both onModeChanged call sites, and use XCTUnwrap instead of force-unwraps in the tests. Also three pre-existing one-liners the extraction surfaced: guard the missing-microphone case so it reaches the record-without-audio fallback instead of crashing on audioDevice!, wire the silent startWriting() failure to onError, and drop two DEBUG prints from production paths. 385/385 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 70cd5ee commit 2c3e5c4

6 files changed

Lines changed: 612 additions & 377 deletions

File tree

Docs/UI_MODERNIZATION.md

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ the loopback harness run the full two-device photo capture (become camera →
140140
become monitor → TakePic → OnPicture → Ack + Resp with bytes → both sides back
141141
to steady state) in CI for the first time.
142142

143-
### PR 8: Extract CaptureEngine (configuration + stills)**this PR**
143+
### PR 8: Extract CaptureEngine (configuration + stills)
144144
Mechanical extraction, no behavior/threading changes: `CaptureEngine` (non-UI,
145145
`NSObject`, the photo-capture delegate) now owns the capture session, device/lens/zoom
146146
discovery, capabilities, flash/torch intent, quality/format/aspect config, and still
@@ -152,9 +152,24 @@ Still queued for follow-ups: single owned serial queue (threading fix),
152152
`RotationCoordinator` (iOS 17) and `maxPhotoDimensions` (iOS 16) migrations,
153153
recording pipeline extraction.
154154

155-
### PR 9+: Camera SwiftUI chrome
156-
With the engine out, the VC is ~400 lines of real UI: SwiftUI chrome around a
157-
`UIViewRepresentable` preview layer. `WatchRemoteCameraController` follows.
155+
### PR 9: Extract RecordingPipeline (video recording) — **this PR**
156+
Same playbook as PR 8 — mechanical move, no behavior/threading changes:
157+
`RecordingPipeline` (non-UI) owns the asset writer and its inputs, the
158+
recording state machine, the writing queue, per-frame write/crop, and
159+
saving/sending the finished movie. The VC stays the sample-buffer delegate
160+
(the frame streamers are live preview, not recording) and keeps the
161+
microphone-permission flow, forwarding recording frames to
162+
`pipeline.processFrame`. Pipeline↔UI seams: `sendMessage` (actor relay for
163+
the start ack, stop response and video resource), `onRecordingStarted`/
164+
`onRecordingStopped` (timer overlay), `onModeChanged` (idle vs recording
165+
chrome), `onError`, `onPhotosAccessDenied`. Still queued for follow-ups:
166+
single owned serial queue (threading fix), `RotationCoordinator` (iOS 17)
167+
and `maxPhotoDimensions` (iOS 16) migrations, frame-streaming extraction.
168+
169+
### PR 10+: Camera SwiftUI chrome
170+
With capture and recording out, the VC is real UI plus the frame streamers:
171+
SwiftUI chrome around a `UIViewRepresentable` preview layer.
172+
`WatchRemoteCameraController` follows.
158173

159174
## Worklog (blog raw material)
160175

@@ -283,3 +298,23 @@ With the engine out, the VC is ~400 lines of real UI: SwiftUI chrome around a
283298
why nobody noticed. Fix: one forwarding case in `cameraShootingVideo`.
284299
The loopback harness caught in an afternoon what shipped unnoticed for years —
285300
because nobody ever watched both phones' timers at once.
301+
302+
### PR 9 — RecordingPipeline extraction
303+
- The smaller sibling of PR 8: 962 → 632 VC lines; `RecordingPipeline` is 407
304+
lines with no UIKit at all. The six-boolean recording state machine moved
305+
intact — extraction first, redesign later (still queued behind the
306+
single-queue threading fix).
307+
- The delegate split was the only real design decision: the VC stays the
308+
sample-buffer delegate because `captureOutput` fans out to *two* consumers —
309+
frame streaming (live preview, stays) and recording (moves). The pipeline
310+
gets frames forwarded, mirroring how the audio delegate is passed into
311+
`startRecording` the same way PR 8 passed it into `setupCamera`.
312+
- One seam replaced three actor touchpoints: `sendMessage` relays the start
313+
ack, stop response and video resource without the pipeline knowing the actor
314+
system — which also made the no-op guards assertable in tests (inject a
315+
recorder, assert nothing was sent).
316+
- New device-free coverage: asset-writer input setup including the 4:3
317+
even-rounded crop rect (1920×1080 → 1440×1080 at x=240) — the "must be even
318+
for the codec" rounding had never been asserted anywhere.
319+
- 385/385 green (381 + 4 pipeline tests); the loopback video round-trips —
320+
including the 3-step stop protocol — passed unchanged across the move.

0 commit comments

Comments
 (0)