[jonluca]

Summary

The feeder's post-arm rate correction can run between pause() stopping the synchronizer and publishing isPlaying = false. It then restarts the clock behind paused decode loops, which can make video catch up rapidly when playback resumes.

What changed

• Keep the armed latch synchronous, but reconcile transport intent on the main actor alongside play() and pause().
• Reject retired outputs and seek generations. Atomically compare an output clock-command revision before applying a correction, so a newer rebuffer pause, seek, or speed change wins.
• Cancel pending preroll starts under paused intent and preserve EOF audio-tail playback unless the host explicitly paused.
• Add deterministic regression coverage using the real host callback and AudioOutput, plus changelog and architecture notes. No public API or dependency changes.

Test plan

• Device / OS: Apple M4 Max, macOS 27.0.1, Xcode 27.1.
• Source media: none needed for the reproducer; it uses the real AVSampleBufferRenderSynchronizer and forces clock arming into the middle of a real pause() through its synchronous published-rate notification.
• The new pause regression fails with the original callback: transport is paused while the output rate remains 1. It passes with the correction serialized on the main actor.
• Current branch: 32 focused Swift Testing tests and 4 API-documentation XCTest cases pass, covering clock resume/timeline behavior, pre-arm pause, EOF, preroll, replacement, rebuffering, and invalidation during preroll preflight. The pause regression checks transport-command ordering inside the published pause notification and retains both final synchronizer-rate assertions; its 20-test suite also passes three consecutive reruns.
• Broader validation on the original patch before the upstream rebase: 4,995 Swift Testing tests pass; the two XCTest bundles report 783 and 17 tests with no failures (one XCTest skip). Command: swift test --disable-automatic-resolution --skip-build --skip lanPeersLeaveLoopbackSlots with an isolated scratch directory.
• The unfiltered run hit a 120-second timeout in HLSLocalServerSlowPeerTests.lanPeersLeaveLoopbackSlots; the successful rerun excludes only that networking test.
• swift build --target AetherEngine -Xswiftc -warnings-as-errors passes with the resolved dependency pins. Documentation links and git diff --check also pass.

Checklist

• CHANGELOG.md updated
• Commit messages follow Conventional Commits
• The fix lives in the engine
• Public API changes are intentional and documented (none introduced)

[Copilot]

🔵 Needs a closer look

Cross-thread clock, preroll, seek, and end-of-media interactions warrant final human review despite the focused regression coverage.

No open issues 🎉

What changed in this PR

Fixes a software playback race where first-frame clock arming could restart the synchronizer during a pause, causing video to catch up rapidly on resume.

Changes:

• Serializes post-arm reconciliation with transport changes on the main actor.
• Adds atomic clock-command revision checks while preserving preroll cancellation and end-of-media audio tails.
• Adds deterministic regression tests and documents the synchronization behavior.

File	Description
Tests/​AetherEngineTests/​Sodalite104PausedFirstFrameTests.swift	Adds regression coverage for pause races, stale corrections, preroll, and EOF.
Sources/​AetherEngine/​Native/​SoftwarePlaybackHost.swift	Moves arm reconciliation to the main actor with session and transport guards.
Sources/​AetherEngine/​Audio/​AudioOutput.swift	Applies revision checks and clock commands under the same lock.
docs/​architecture.md	Documents clock-arm ordering and correction ownership.
CHANGELOG.md	Records the playback race fix.

_{Review effort: Balanced}

[github-actions]

Automated review

This moves the software host's post-arm rate correction off the feeder and onto the main actor, next to play() and pause(). It also adds an AudioOutput clock-command revision so that a newer pause, seek, flush or rate change wins over a queued correction. The fix is in the engine and addresses the cause (the feeder could restart the clock between pause() stopping it and isPlaying = false), so the direction is right.

I traced the arm sites (seekClock then markClockArmed, so the revision is captured after the arming command), the play()/pause()/setRate paths that gate on clockArmed, seek (flush and the re-anchor both bump the revision, and admitsRead drops corrections from an old generation), the EOF park and stop(). I found no correctness defect. Moving synchronizer.setRate under lock in setRate/pause matches what seekClock already did. The preroll gate is still asked before the lock is taken.

Findings

• Nit: in the playing branch, rateCorrectionAtArm is passed self.isPlaying and self.pausedBeforeFirstFrame even though the if above has already fixed them to true and false. Reading each flag again under its lock can also return a different value than the branch saw. Passing the literals, or comparing aOut.synchronizer.rate against lastRate directly, would make the intent clearer (Sources/AetherEngine/Native/SoftwarePlaybackHost.swift:1713).

Checklist: CHANGELOG yes, docs yes, tests yes, traits both ways n.a.

Needs a device run: The correction now waits for a main-actor hop instead of running inline. In the narrow case where a pause lands after the feeder read its arming rate, the clock (and audio) can run briefly until that hop runs. On a busy main actor (the #694 class of delay) this could be audible. A Sodalite pause-right-after-load check on Apple TV would settle whether the window matters in practice.

Thanks for the careful write-up and the deterministic reproducer.

_{Automated review by Claude of 306a53e. It reads the code, it does not build or run it; a maintainer reviews and merges.}

[github-actions]

Nit: this branch only runs when isPlaying is true and pausedBeforeFirstFrame is false, but both flags are read again here, each under its own lock. A main-actor write can't land in between, so it is harmless. It does make a reader wonder whether the zero-target branch of rateCorrectionAtArm can fire here, and it cannot. Passing transportPlaying: true, pausedBeforeFirstFrame: false (or comparing aOut.synchronizer.rate with lastRate inline) would say what the code means.

[github-actions]

Automated review

Checked the new head, 3da8803. The direction is unchanged and still right: the post-arm rate correction runs on the main actor, and the AudioOutput revision check lets a newer pause, seek, flush or rate command win.

Findings

• The earlier nit is fixed. The playing branch now passes transportPlaying: true, pausedBeforeFirstFrame: false (Sources/AetherEngine/Native/SoftwarePlaybackHost.swift:1714).
• I found no new findings. I traced the preflight case in setRate(_:ifRevision:), where the gate runs outside the lock, a pause lands, and then both the seekClock branch and the direct branch reject the stale revision. The new test pins both branches.

Checklist: CHANGELOG yes, docs yes, tests yes, traits both ways n.a.

Needs a device run: Same as before. A pause that lands after the feeder read its arming rate leaves the clock running until the main-actor hop. A pause right after load in Sodalite on Apple TV would show whether that window can be heard.

Thanks for the quick follow-up.

_{Automated review by Claude of 3da8803. It reads the code, it does not build or run it; a maintainer reviews and merges.}