Skip to content

Commit 543ffc5

Browse files
fix(ios): canonical AVKit signals + sink-time payload snapshot (#385)
Two coordinated fixes to iOS metrics emission. Closes #349 and #350. ## Summary Both issues root in the same late-evaluation pattern: payloads were built at Task-execution time after queuing through `metricsTaskTail`, so `Date()` and `diagnostics.state` were stamped well after the underlying event fired. Plus `video_first_frame` was synthesized from `currentTime` ticks instead of the canonical `AVPlayerLayer.isReadyForDisplay` signal. ### #350 — `video_first_frame` 2-3× burst - New `Coordinator` on the `PlayerView` `UIViewControllerRepresentable` walks `AVPlayerViewController.view.layer` to find the embedded `AVPlayerLayer`, KVO-observes `isReadyForDisplay`, and calls `markFirstFrameRendered(at:)` on the flip. - iOS doesn't reliably expose the layer on initial mount (Apple's private rendering path), so the `currentTime > 0 && state == "playing"` synthesis is kept as a **fallback**. Shared `firstFrameReported` latch ensures exactly one event per playback regardless of which signal wins. - `video_start_time` migrated to the same canonical pattern via `timeControlStatus == .playing` KVO + `markPlayingStarted(at:)`. ### #349 — `state_change` burst with frozen `last_event_at` - Every metrics emit site now snapshots `Date()` AND builds the payload synchronously at the firing context, then hands the immutable dict to a Task that does only HTTP. - New `sendPlayerMetrics(payload:)` overload. Existing `sendPlayerMetrics(event:at:extra:)` kept as convenience that builds + forwards. - Sites converted (no caller deferred): every Combine sink in `bindMetricsReporting` and `bindDiagnosticsLogging`, `timeJumpSubject`, `handleRenditionShift`, heartbeat timer, `loadStream` (`playing`), `mark911`, both `restart` paths, plus the canonical-signal callbacks. - All Combine sinks receive on `DispatchQueue.main` for serialization safety. ### Side fixes - **Heartbeat restart on each playback period.** `startMetricsHeartbeat` moved out of `init()` into `loadStream` so the first tick lands exactly 1 s after `playing`. No more pre-playback heartbeats during app idle. - **Single `Date()` capture in `loadStream`** — `playbackStartAt` and the `playing` event's `at:` stamp share one capture so elapsed-since-start is exactly 0. - **`markPlayingStarted(at:)` captures `firedAt` on the KVO thread** before the `Task @MainActor` hands off, so the timestamp reflects the true `timeControlStatus` transition instant (caught in pre-PR review). ### Observability additions (kept intentionally) - `[METRICS event=… ts=… state=… pos=… from=… to=…]` — one print per PATCH; greppable in `idevicesyslog` / Console.app for spotting future bursts. - `[FIRSTFRAME] attach: …` — prints which path won the AVPlayerLayer discovery on each attach pass; useful for tracking iOS version regressions. ## Guiding principle Preserve every real state transition; only suppress true duplicates. AVPlayer's startup flicker through paused → buffering → stalled → buffering → playing now stamps distinct timestamps, distinct `state_from`/`state_to`, distinct `player_state` snapshots. Each row carries enough data to be unambiguous on the wire. ## Test plan - [x] iOS / iPadOS simulator builds green - [x] tvOS simulator builds green - [x] iPad sim cold-start: one `video_first_frame` (synthesis fallback), one `video_start_time`, three distinct `state_change` rows with walking from/to pairs - [x] iPad sim Reload: one `video_first_frame` (KVO won, fires before `state == playing`), one `video_start_time` - [x] Heartbeat starts 1 s after `playing`, no pre-playback heartbeats - [x] Pre-PR Sonnet review run; correctness items addressed - [ ] Apple TV physical device verification - [ ] Soak run — confirm no regression on `rate_shift_*`, `frozen`, `segment_stall`, `error` event flows ## Follow-ups (not in this PR) - Investigate `player_metrics_event_time` vs `player_metrics_last_event_at` redundancy (both fields carry the identical timestamp literal today). - Optional: wire `AVPlayerItem.isPlaybackLikelyToKeepUp` as a richer startup-quality signal. - Maybe rename `playing` → `playback_initiated` since it fires at `loadStream` start, not when video shows. - Consider gating heartbeat on active playback only (currently runs whenever a `loadStream` has happened, even after stop). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 917ff74 commit 543ffc5

3 files changed

Lines changed: 364 additions & 120 deletions

File tree

apple/InfiniteStreamPlayer/InfiniteStreamPlayer/PlaybackScreen.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ struct PlaybackScreen: View {
2222
onRetry: { vm.retry() },
2323
onReload: { vm.reload() },
2424
onMark911: { vm.mark911() },
25-
onOpenSettings: { vm.setSettingsOpen(true) }
25+
onOpenSettings: { vm.setSettingsOpen(true) },
26+
onFirstFrame: { at in vm.markFirstFrameRendered(at: at) }
2627
)
2728
.id(vm.playerEpoch)
2829
.ignoresSafeArea()

apple/InfiniteStreamPlayer/InfiniteStreamPlayer/PlayerView.swift

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ struct PlayerView: UIViewControllerRepresentable {
1717
var onReload: (() -> Void)? = nil
1818
var onMark911: (() -> Void)? = nil
1919
var onOpenSettings: (() -> Void)? = nil
20+
/// Fired exactly once per playback when the embedded `AVPlayerLayer`
21+
/// reports `isReadyForDisplay = true`. The closure receives the
22+
/// instant the flip was observed; the consumer (PlayerViewModel)
23+
/// is responsible for idempotency (resetting on item replace).
24+
var onFirstFrame: ((Date) -> Void)? = nil
25+
26+
func makeCoordinator() -> Coordinator {
27+
Coordinator()
28+
}
2029

2130
func makeUIViewController(context: Context) -> AVPlayerViewController {
2231
let controller = AVPlayerViewController()
@@ -27,13 +36,21 @@ struct PlayerView: UIViewControllerRepresentable {
2736
#else
2837
controller.showsPlaybackControls = false
2938
#endif
39+
context.coordinator.onFirstFrame = onFirstFrame
40+
context.coordinator.attach(to: controller, player: player)
3041
return controller
3142
}
3243

44+
static func dismantleUIViewController(_ uiViewController: AVPlayerViewController, coordinator: Coordinator) {
45+
coordinator.detach()
46+
}
47+
3348
func updateUIViewController(_ uiViewController: AVPlayerViewController, context: Context) {
3449
if uiViewController.player !== player {
3550
uiViewController.player = player
3651
}
52+
context.coordinator.onFirstFrame = onFirstFrame
53+
context.coordinator.attach(to: uiViewController, player: player)
3754
#if os(tvOS)
3855
// Rebuild every update so each UIAction closure captures the
3956
// freshest callback (the parent passes closures that reference
@@ -69,5 +86,112 @@ struct PlayerView: UIViewControllerRepresentable {
6986
uiViewController.transportBarCustomMenuItems = actions
7087
#endif
7188
}
89+
90+
/// Holds the AVPlayerLayer KVO state across SwiftUI re-renders.
91+
/// AVPlayerViewController doesn't expose its embedded `AVPlayerLayer`
92+
/// directly, so we walk `view.layer.sublayers` to find it. The walk
93+
/// re-runs on player swap (Reload / Retry recreates AVPlayer) and on
94+
/// item replace, so the observer always tracks the live layer.
95+
final class Coordinator: NSObject {
96+
var onFirstFrame: ((Date) -> Void)?
97+
98+
private weak var observedPlayer: AVPlayer?
99+
private weak var observedLayer: AVPlayerLayer?
100+
private var readyObservation: NSKeyValueObservation?
101+
private var sublayerObservation: NSKeyValueObservation?
102+
private var didReportForCurrentLayer: Bool = false
103+
104+
func attach(to controller: AVPlayerViewController, player: AVPlayer) {
105+
// Player swap = brand-new AVPlayerLayer underneath.
106+
// Tear down the old observer and start fresh.
107+
if observedPlayer !== player {
108+
detachLayer()
109+
observedPlayer = player
110+
}
111+
// The AVPlayerLayer can be added to the controller's view
112+
// hierarchy after `makeUIViewController` returns. If we
113+
// can't find it yet, install a one-shot KVO on the parent
114+
// layer's `sublayers` and retry when it appears.
115+
// AVPlayerViewController on iOS doesn't reliably expose
116+
// its embedded AVPlayerLayer as a sublayer of view.layer
117+
// on initial mount — Apple uses a private rendering path.
118+
// After Reload (when the controller's view tree has
119+
// settled), the layer is sometimes findable. The fallback
120+
// synthesis in PlayerViewModel's $currentTime sink covers
121+
// cold-start where this KVO never fires. Debug prints
122+
// tagged `[FIRSTFRAME]` show which path won for each
123+
// mount — useful for spotting regressions / OS changes.
124+
if let layer = findAVPlayerLayer(in: controller.view.layer) {
125+
print("[FIRSTFRAME] attach: found AVPlayerLayer immediately depth=\(layerDepth(of: layer, in: controller.view.layer)) ready=\(layer.isReadyForDisplay)")
126+
installReadyObserver(on: layer)
127+
} else if sublayerObservation == nil {
128+
let topSubs = controller.view.layer.sublayers?.count ?? 0
129+
print("[FIRSTFRAME] attach: no AVPlayerLayer yet — topSubs=\(topSubs) — installing sublayer KVO")
130+
let parent = controller.view.layer
131+
sublayerObservation = parent.observe(\.sublayers, options: [.new]) { [weak self, weak controller] _, _ in
132+
guard let self, let controller else { return }
133+
if let layer = self.findAVPlayerLayer(in: controller.view.layer) {
134+
print("[FIRSTFRAME] sublayer KVO fired: AVPlayerLayer appeared, ready=\(layer.isReadyForDisplay)")
135+
self.installReadyObserver(on: layer)
136+
self.sublayerObservation?.invalidate()
137+
self.sublayerObservation = nil
138+
}
139+
}
140+
}
141+
}
142+
143+
private func layerDepth(of target: CALayer, in root: CALayer, depth: Int = 0) -> Int {
144+
if root === target { return depth }
145+
for sub in root.sublayers ?? [] {
146+
let d = layerDepth(of: target, in: sub, depth: depth + 1)
147+
if d >= 0 { return d }
148+
}
149+
return -1
150+
}
151+
152+
func detach() {
153+
detachLayer()
154+
observedPlayer = nil
155+
}
156+
157+
private func detachLayer() {
158+
readyObservation?.invalidate()
159+
readyObservation = nil
160+
sublayerObservation?.invalidate()
161+
sublayerObservation = nil
162+
observedLayer = nil
163+
didReportForCurrentLayer = false
164+
}
165+
166+
private func installReadyObserver(on layer: AVPlayerLayer) {
167+
if observedLayer === layer { return }
168+
readyObservation?.invalidate()
169+
observedLayer = layer
170+
didReportForCurrentLayer = false
171+
// `.initial` lets us fire immediately if the layer is
172+
// already ready (e.g. SwiftUI re-rendered after first frame
173+
// already landed).
174+
readyObservation = layer.observe(\.isReadyForDisplay, options: [.new, .initial]) { [weak self] observed, _ in
175+
guard let self, observed.isReadyForDisplay, !self.didReportForCurrentLayer else { return }
176+
self.didReportForCurrentLayer = true
177+
let now = Date()
178+
if Thread.isMainThread {
179+
self.onFirstFrame?(now)
180+
} else {
181+
DispatchQueue.main.async { [weak self] in
182+
self?.onFirstFrame?(now)
183+
}
184+
}
185+
}
186+
}
187+
188+
private func findAVPlayerLayer(in layer: CALayer) -> AVPlayerLayer? {
189+
if let avLayer = layer as? AVPlayerLayer { return avLayer }
190+
for sub in layer.sublayers ?? [] {
191+
if let found = findAVPlayerLayer(in: sub) { return found }
192+
}
193+
return nil
194+
}
195+
}
72196
}
73197

0 commit comments

Comments
 (0)