From 156588a2f33fb3133c1d8cf2044d4e803a2e303d Mon Sep 17 00:00:00 2001 From: orut34iop Date: Sun, 18 Oct 2026 11:58:40 +0800 Subject: [PATCH] fix(subtitles): preserve secondary track identity across rebuilds --- CHANGELOG.md | 4 + .../AetherEngine/AetherEngine+Loading.swift | 10 +- .../AetherEngine/AetherEngine+Subtitles.swift | 23 ++++- Sources/AetherEngine/AetherEngine.swift | 1 + .../ExternalSubtitleSelectionTests.swift | 94 +++++++++++++++++++ docs/api.md | 10 ++ 6 files changed, 134 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84f69ff13..9ef98aebf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ the public-API contract. ## [Unreleased] +### Fixed + +- Audio-switch rebuilds restore a registered secondary subtitle through its track id, preserving its explicit headers and container stream index. Embedded selection clears stale sidecar state, internal teardown clears the active external id, and late secondary decode completions cannot publish into another load generation. No subtitle capability or primary/secondary selection restrictions change. + ### Changed - **The documentation no longer says TrueHD Atmos objects are always lost.** The Known limitations diff --git a/Sources/AetherEngine/AetherEngine+Loading.swift b/Sources/AetherEngine/AetherEngine+Loading.swift index 660e7620b..b43f7836e 100644 --- a/Sources/AetherEngine/AetherEngine+Loading.swift +++ b/Sources/AetherEngine/AetherEngine+Loading.swift @@ -2400,6 +2400,7 @@ extension AetherEngine { // playhead, so this is the correct anchor for the reader re-arm and the preserved-cue snapshot. let preSwitchSourceTime = PresentationAxis.source(displayTime: resumeAt, origin: sourcePresentationOrigin) let preservedActiveImageCues = Self.activeImageCues(in: subtitleCues, at: preSwitchSourceTime) + let secondaryExternalToResume = activeSecondaryExternalSubtitleTrackID let secondaryEmbeddedToResume: Int32 = activeSecondaryEmbeddedSubtitleStreamIndex let secondarySidecarToResume: URL? = isSecondarySubtitleActive && activeSecondaryEmbeddedSubtitleStreamIndex < 0 ? loadedSecondarySidecarURL @@ -2776,12 +2777,9 @@ extension AetherEngine { subtitleCues = preservedActiveImageCues } } - if let secondarySidecar = secondarySidecarToResume { - selectSecondarySidecarSubtitle(url: secondarySidecar) - } else if secondaryEmbeddedToResume >= 0 { - // #112 (audio-switch reanchor): same collapsed-sourceTime slip on the secondary channel. - selectSecondarySubtitleTrack(index: Int(secondaryEmbeddedToResume), startAt: preSwitchSourceTime) - } + restoreSecondarySubtitleAfterRebuild( + externalID: secondaryExternalToResume, sidecarURL: secondarySidecarToResume, + embeddedIndex: secondaryEmbeddedToResume, startAt: preSwitchSourceTime) // #170: replay the native-rendition pick onto the rebuilt item, the way the #65 recovery // does; the table was rebuilt from the preserved track list, so a rendering-derived // ordinal recomputes to the same rendition and a host-positional one replays as-is. diff --git a/Sources/AetherEngine/AetherEngine+Subtitles.swift b/Sources/AetherEngine/AetherEngine+Subtitles.swift index 02ab63cbf..c1b08fe27 100644 --- a/Sources/AetherEngine/AetherEngine+Subtitles.swift +++ b/Sources/AetherEngine/AetherEngine+Subtitles.swift @@ -191,6 +191,7 @@ extension AetherEngine { activeSecondaryExternalSubtitleTrackID = nil guard loadedURL != nil else { return } cancelSidecarTask(channel: .secondary) + loadedSecondarySidecarURL = nil // #112 rework: secondary embedded tracks ride the same packet-store drainer on their // own channel; the immediate tick backfills it. @@ -1603,6 +1604,21 @@ extension AetherEngine { startSecondarySidecarDecode(url: url, httpHeaders: httpHeaders) } + /// Audio rebuilds keep the registry. Restore through its id so headers and a container's + /// selected subtitle stream survive; the URL-only route is only for one-shot sidecars. + func restoreSecondarySubtitleAfterRebuild(externalID: Int?, sidecarURL: URL?, + embeddedIndex: Int32, startAt: Double) { + if let sidecarURL { + if let externalID, externalSubtitleRegistry[externalID] != nil { + selectSecondarySubtitleTrack(index: externalID, startAt: startAt) + } else { + selectSecondarySidecarSubtitle(url: sidecarURL) + } + } else if embeddedIndex >= 0 { + selectSecondarySubtitleTrack(index: Int(embeddedIndex), startAt: startAt) + } + } + /// Shared secondary sidecar-decode start (#88): the pre-#88 selectSecondarySidecarSubtitle body. func startSecondarySidecarDecode(url: URL, httpHeaders: [String: String]?, sourceStreamIndex: Int32? = nil) { @@ -1613,6 +1629,7 @@ extension AetherEngine { isLoadingSecondarySubtitles = true let effectiveHeaders = httpHeaders ?? loadedOptions.httpHeaders + let sessionGeneration = loadGeneration secondarySidecarTask = Task { [weak self] in let result: SidecarDecodeResult do { @@ -1622,13 +1639,15 @@ extension AetherEngine { } catch { EngineLog.emit("[AetherEngine] secondary sidecar decode failed: \(error)", category: .engine) await MainActor.run { - guard !Task.isCancelled, let self = self else { return } + guard !Task.isCancelled, let self = self, + self.loadGeneration == sessionGeneration else { return } if self.isSecondarySubtitleActive { self.isLoadingSecondarySubtitles = false } } return } await MainActor.run { - guard !Task.isCancelled, let self = self else { return } + guard !Task.isCancelled, let self = self, + self.loadGeneration == sessionGeneration else { return } guard self.isSecondarySubtitleActive else { return } self.secondarySubtitleCues = result.cues self.isLoadingSecondarySubtitles = false diff --git a/Sources/AetherEngine/AetherEngine.swift b/Sources/AetherEngine/AetherEngine.swift index bdbf1a191..9375e96a7 100644 --- a/Sources/AetherEngine/AetherEngine.swift +++ b/Sources/AetherEngine/AetherEngine.swift @@ -7828,6 +7828,7 @@ public final class AetherEngine: ObservableObject { nativeSubtitleRenditionAvailable = false cancelSidecarTask(channel: .secondary) activeSecondaryEmbeddedSubtitleStreamIndex = -1 + activeSecondaryExternalSubtitleTrackID = nil loadedSecondarySidecarURL = nil isSecondarySubtitleActive = false secondarySubtitleCues = [] diff --git a/Tests/AetherEngineTests/ExternalSubtitleSelectionTests.swift b/Tests/AetherEngineTests/ExternalSubtitleSelectionTests.swift index 5adc8dc75..19f91f369 100644 --- a/Tests/AetherEngineTests/ExternalSubtitleSelectionTests.swift +++ b/Tests/AetherEngineTests/ExternalSubtitleSelectionTests.swift @@ -67,3 +67,97 @@ struct ExternalSubtitleSelectionTests { #expect(engine.isSubtitleActive) } } + +extension ExternalSubtitleSelectionTests { + @Test("Internal teardown clears active identity but preserves the rebuild registry", .timeLimit(.minutes(1))) + func teardownIdentity() throws { + let engine = try AetherEngine() + let url = URL(fileURLWithPath: "/nonexistent/subtitle.srt") + let track = engine.addExternalSubtitleTrack(.init(url: url, name: "external")) + engine.activeSecondaryExternalSubtitleTrackID = track.id + engine.isSecondarySubtitleActive = true + engine.stopInternal(resetDisplayCriteria: false) + #expect(engine.activeSecondaryExternalSubtitleTrackID == nil) + #expect(engine.externalSubtitleRegistry[track.id] != nil) + #expect(!engine.isSecondarySubtitleActive) + engine.stop() + } + + @Test("Embedded selection removes the old one-shot sidecar URL", .timeLimit(.minutes(1))) + func embeddedSelectionClearsSidecar() throws { + let engine = try AetherEngine() + defer { engine.stop() } + engine.loadedURL = URL(fileURLWithPath: "/nonexistent/movie.mkv") + engine.loadedSecondarySidecarURL = URL(fileURLWithPath: "/nonexistent/subtitle.srt") + engine.selectSecondarySubtitleTrack(index: 3) + #expect(engine.loadedSecondarySidecarURL == nil) + #expect(engine.activeSecondaryEmbeddedSubtitleStreamIndex == 3) + } + + @Test("A rebuilt external companion retains its selected container stream", .timeLimit(.minutes(1))) + func registryStreamRestoration() async throws { + let url = try MultiSubtitleContainerFixture.write() + defer { try? FileManager.default.removeItem(at: url) } + let engine = try AetherEngine() + defer { engine.stop() } + let info = engine.addExternalSubtitleTrack(.init( + url: url, name: "Spanish", language: "es", httpHeaders: ["X-Test": "track"], + sourceStreamIndex: MultiSubtitleContainerFixture.spanishStreamIndex)) + engine.selectSecondarySubtitleTrack(index: info.id) + await engine.secondarySidecarTask?.value + #expect(engine.secondarySubtitleCues.compactMap(\.text) == MultiSubtitleContainerFixture.spanishLines) + let savedID = engine.activeSecondaryExternalSubtitleTrackID + let savedURL = engine.loadedSecondarySidecarURL + engine.stopInternal(resetDisplayCriteria: false) + engine.restoreSecondarySubtitleAfterRebuild(externalID: savedID, sidecarURL: savedURL, + embeddedIndex: -1, startAt: 0) + await engine.secondarySidecarTask?.value + #expect(engine.activeSecondaryExternalSubtitleTrackID == info.id) + #expect(engine.secondarySubtitleCues.compactMap(\.text) == MultiSubtitleContainerFixture.spanishLines) + #expect(engine.externalSubtitleRegistry[info.id]?.httpHeaders == ["X-Test": "track"]) + engine.removeExternalSubtitleTrack(id: info.id) + #expect(!engine.isSecondarySubtitleActive) + #expect(engine.secondarySubtitleCues.isEmpty) + } +} + +extension ExternalSubtitleSelectionTests { + @Test("A prior load's decode cannot change current cues or loading state", .timeLimit(.minutes(1))) + func staleDecodeDoesNotPublish() async throws { + let url = try MultiSubtitleContainerFixture.write() + defer { try? FileManager.default.removeItem(at: url) } + let engine = try AetherEngine() + defer { engine.stop() } + let info = engine.addExternalSubtitleTrack(.init( + url: url, name: "Spanish", sourceStreamIndex: MultiSubtitleContainerFixture.spanishStreamIndex)) + engine.selectSecondarySubtitleTrack(index: info.id) + let oldTask = engine.secondarySidecarTask + // Advance ownership without cancellation to exercise the independent generation fence. + engine.loadGeneration &+= 1 + engine.secondarySubtitleCues = [] + engine.isLoadingSecondarySubtitles = true + await oldTask?.value + #expect(engine.secondarySubtitleCues.isEmpty) + #expect(engine.isLoadingSecondarySubtitles) + } +} + +extension ExternalSubtitleSelectionTests { + @Test("The same registered track remains selectable in both channels", .timeLimit(.minutes(1))) + func noDownstreamExclusivityRule() async throws { + let url = try MultiSubtitleContainerFixture.write() + defer { try? FileManager.default.removeItem(at: url) } + let engine = try AetherEngine() + defer { engine.stop() } + let info = engine.addExternalSubtitleTrack(.init( + url: url, name: "Spanish", sourceStreamIndex: MultiSubtitleContainerFixture.spanishStreamIndex)) + engine.selectSubtitleTrack(index: info.id) + engine.selectSecondarySubtitleTrack(index: info.id) + await engine.sidecarTask?.value + await engine.secondarySidecarTask?.value + #expect(engine.activeSubtitleTrackIndex == info.id) + #expect(engine.activeSecondaryExternalSubtitleTrackID == info.id) + #expect(engine.subtitleCues.compactMap(\.text) == engine.secondarySubtitleCues.compactMap(\.text)) + #expect(!engine.secondarySubtitleCues.isEmpty) + } +} diff --git a/docs/api.md b/docs/api.md index 16362779c..4e8d7da4e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1378,3 +1378,13 @@ Public for the CLI, the test suite, or a diagnostic overlay, and outside the sha - **Dolby Vision Profile 7 FEL tooling** (`@_spi(Diagnostics)`): `FelProbeResult` / `felDecodeProbe` (`aetherctl feltest`) and `felResidualEnabled` (`aetherctl play --cycle-el`), which exist only with the `DualLayerDolbyVision` trait, `DoviRpuSurvey` / `DoviRpuSurveyResult` / `DoviRpuFrameSurvey` / `doviRpuSurvey` (`aetherctl dvsurvey`), and, in `DEBUG` builds with the `DualLayerDolbyVision` trait only, `armDolbyVisionPinTest` / `dolbyVisionPinIsArmed` / `dolbyVisionPinIsActive` (`aetherctl play --dv-pin`). Hosts import none of them; the host surface of the FEL route is `LoadOptions.preferDualLayerDolbyVision` and, in `SessionReloadRefusal`, the new case `.dualLayerRouteNeedsFreshLoad`. See [Dolby Vision FEL](dolby-vision-fel.md) and [CLI](cli.md). - **`HLSLiveIngestReader`'s internals** (`terminalError`, `upstreamTargetDuration`, `observedLiveCadenceSeconds`, `closedLiveCadenceSeconds`, `upstreamSegmentDurationSeconds`, `companionAudioReader`): fixture and diagnostic reads. The last two are the closed evidence the served TARGETDURATION is sealed from (AE#447); `upstreamTargetDuration` is the upstream's own claim, reported in the seal line and derived from nowhere. - **`SubtitleChannel`**: the primary / secondary selector on the engine's internal subtitle routing. No public signature takes one; a host picks the channel by calling the primary or the secondary method. + +### Secondary subtitles across audio rebuilds + +An active registered secondary subtitle survives an audio-track rebuild through its original +registry id, including `ExternalSubtitleTrack.httpHeaders` and `sourceStreamIndex`. A one-shot +secondary sidecar still restores through its URL. Switching to an embedded secondary track +clears the previous sidecar URL. Internal teardown clears active secondary identity while the +rebuild's saved selection can restore it; a decode result from an older load cannot republish +cues or loading state in the new one. This does not add bitmap/ASS capabilities or prohibit +selecting the same track in both channels.