Repository navigation
fix: CI green again — Swift 6.2 type-checker limits, duplicated subtitle cues, AV1 test on VMs - #41
Merged
Merged
Conversation
CI has been red since 069a818, and #39/#40 added two more failures of the same kind. The workflow asks for Xcode_26.app, which the macos-15 image no longer has; the fallback selects Xcode 26.3 — Swift 6.2 — while local development runs Xcode 27 (Swift 6.4). Code that 6.4 accepts at once, 6.2 rejects: - ChannelDrain.value: a bare `result` inside the initializer of the local `result` binds to that local on 6.2 ("initializer for conditional binding must have Optional type"). Now `self.result`. - Matrix3.multiply/inverse: nested `reduce(0)` closures over untyped literals exceed 6.2's solver ("unable to type-check this expression in reasonable time"). Plain loops with explicit types. - DolbyVisionTests.extraction: `#expect(pieces == [.polynomial(...)])` took 6.4 2.4 s to type-check; the expected values are now typed locals. HEVCBaseLayerFilterTests' hvcC array builder (0.1 s) is a loop. Found by compiling with -warn-long-expression-type-checking=25 on 6.4: nothing is above 40 ms any more, the level the existing tests already passed CI at. Behaviour is unchanged. The suite passes apart from the intermittent SRT cue test, a real duplicate-cue bug fixed in the next commit.
"embedded SRT track produces timed cues" failed intermittently — on clean main as well, one run in three to nine. Printing the store on a failing run showed every cue twice (ids 0/2 and 1/3), and the bug is not the test's. SubtitleStore is a timeline that deliberately survives seeks, but the decoder inserted every cue it decoded. So whenever the demuxer re-read subtitle packets it had already delivered, the cues were stored again: - on every backward seek — users saw the lines of that range doubled; - in the backfill seek of a track selected after the demuxer had read past it (selectSubtitleTrack), which is the test's path: when live delivery won the race against that seek, both copies landed. insert() now drops a cue identical to one it holds (same start, end, and trimmed text); cues sharing a start but differing otherwise both stay. The same-start run sits right before the insertion point, so the check costs nothing extra. New tests: a store unit test, and "seeking back does not duplicate cues", which reproduces the doubling deterministically — both fail without the fix. The full suite passed six runs out of six afterwards.
With the engine fixed, CI's Xcode 26.3 got as far as the tests and
rejected three more expressions that Swift 6.4 checks in a few
milliseconds — so -warn-long-expression-type-checking on 6.4 is no
guide to what 6.2 accepts:
- DolbyVisionTests' Reference.multiply/apply: the same nested
`map { reduce(0) { … } }` shape Matrix3 had; now loops. Its MMR sum,
the same shape, goes with them.
- HEVCBaseLayerFilterTests.hvcC: long `header + [6] + array(…) + …`
chains over untyped literals; now joined through a typed variadic.
On CI the AV1 test failed with 72 of ~96 frames, all from software, although VTIsHardwareDecodeSupported(AV1) said true. The runner is a virtualized Mac whose VideoToolbox claims AV1 and then fails it; the engine did what its recovery policy says — rebuilt on dav1d, emitted .downgradedToSoftware, resumed at the next keyframe, one 24-frame GOP later. The test assumed the claim always holds. It now tells the cases apart. Without claimed AV1 hardware — the case that played black (Apple TV 4K, M1) — dav1d must decode from the first frame, with no hardware attempt and nothing lost. With claimed hardware it accepts either hardware frames, or a reported downgrade that loses at most one GOP; never a silent failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CI on
mainhas been red since069a818, and #39/#40 added more failures. None of it was a logic bug in those PRs. Three separate problems:1.
fix(ci): keep the sources within Swift 6.2's type checkerThe workflow asks for
Xcode_26.app, which themacos-15image no longer has, so the fallback selects Xcode 26.3 (Swift 6.2). Local development uses Xcode 27 (Swift 6.4), and code that 6.4 accepts instantly, 6.2 rejects:Tests/.../Support/ChannelDrain.swift:88(onmainsince069a818)result, a bareresultbinds to that localif let stored = self.resultDolbyVisionMapping.swift:174Matrix3(#39)reduce(0)closures over untyped literalsDolbyVisionTests.extraction(#39)#expect(pieces == [.polynomial(…)])HEVCBaseLayerFilterTestshvcC builder (#40)I found the first slow spots by compiling with
-warn-long-expression-type-checking=25on 6.4. But 6.4's timings turned out to be a poor guide: CI's first run on this PR rejected three more test expressions that 6.4 checks in milliseconds. They're fixed by84f8466:DolbyVisionTests'Reference.multiply/apply, the samereduce(0)shape;HEVCBaseLayerFilterTests+chains over untyped literals.Swift 6.2 isn't installed locally, so CI is the real check.
3.
fix(test): don't take VideoToolbox's word on AV1 hardwareOnce everything compiled, the AV1 test failed on CI: 72 of ~96 frames, all from software, although
VTIsHardwareDecodeSupported(AV1)returned true. The runner is a virtualized Mac whose VideoToolbox claims AV1 and then fails it. The engine did what its recovery policy says:.downgradedToSoftware,The test now distinguishes three cases:
2.
fix(subtitle): store a re-read cue once, not again"embedded SRT track produces timed cues" failed intermittently, including on clean
main: one run in 3 to 9 under full-suite load. Printing the store on a failing run showed every cue stored twice. That's an engine bug, not a test problem.SubtitleStoreis a timeline that deliberately survives seeks, but the decoder inserted every cue it decoded. Whenever the demuxer re-read subtitle packets it had already delivered, the cues were stored again:selectSubtitleTrack), the test's path: when live delivery won the race against that seek, both copies landed.insert()now drops a cue identical to one it already holds (same start, end and trimmed text). Different cues that share a start both stay.Type of change
fix— bug fixfeat— new capabilityperf— performance improvementrefactor— neither fixes a bug nor adds a featuredocs— documentation onlytest— adding or fixing testsbuild/chore— build pipeline, tooling, or maintenanceInvariants
None affected: the only data-plane change is the dedup inside
SubtitleStore.insert, under its existing lock.Channel.TimestampUnwrapper.AVSampleBufferRenderSynchronizer(no new heuristics or clocks).as!,fatalError, orassertionFailurein engine paths; failures are typed.PlayerConfiguration.av_logstrings.Verification
swift buildis warning-free. Nothing new warns;Render/SystemRenderer.swift:63-64still has its two existing main-actor isolation warnings.swift testis green, six runs out of six (Swift 6.4, local dav1d xcframework):storeDeduplicates: a store unit test.seekBackKeepsCuesUnique: reproduces the doubling deterministically.Platforms verified
Not in this PR
Xcode_26.appand silently falls back to the newest Xcode on the image, so the CI toolchain can change without notice. It may be worth pinning it explicitly, e.g. toXcode_26.3.app, or to whatever minimum toolchain the package should support.