From bc617eb1e75b76276fc185d65649b4170d59be76 Mon Sep 17 00:00:00 2001 From: Christian Pillsbury Date: Mon, 22 Jun 2026 09:41:25 -0700 Subject: [PATCH] refactor(spf): inline the resolve task, keep resolve-track close to main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The createResolveTask factory was a single-call hoist (the RecurringRunner clones the task itself), so inline the task back into runner.schedule(...) as on main, and inline the shouldLoadTrack gate the same way. Restore the surrounding comments and formatting to main's, deviating only where the refactor changed meaning: the gate now reloads resolved-but-incomplete (live) windows (resolved→complete), and the obsolete scheduler/commit-time-id-check notes are dropped. Net diff against main: 131+/70- down to 33+/18-. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/playback/behaviors/resolve-track.ts | 202 +++++++----------- 1 file changed, 78 insertions(+), 124 deletions(-) diff --git a/packages/spf/src/playback/behaviors/resolve-track.ts b/packages/spf/src/playback/behaviors/resolve-track.ts index eb91b213..28bd375d 100644 --- a/packages/spf/src/playback/behaviors/resolve-track.ts +++ b/packages/spf/src/playback/behaviors/resolve-track.ts @@ -17,27 +17,9 @@ import { AUDIO_TYPE_CONFIG, TEXT_TYPE_CONFIG, VIDEO_TYPE_CONFIG } from '../primi // `setupTrackResolution` has the same shape as a Behavior `setup` function: // `({ state, config }) => cleanup`. Each `resolveXTrack` export below calls it // from inside its own `defineBehavior` setup, supplying its per-type config -// inline. The orchestration — gate on a selection, schedule the fetch+parse, -// and patch the resolved track back into `state.presentation` (carrying the -// prior snapshot's timeline forward) — is shared. -// -// This behavior is category [1] "content snapshot" from -// [live-presentation-modeling.md](../../../internal/design/spf/live-presentation-modeling.md): -// it produces the windowed segment list. *Whether* to (re)load is the `shouldLoadTrack` -// gate (resolve an unresolved track; reload a resolved-but-incomplete one — a live -// window may have slid past the playhead; reuse a complete one). *When* to reload -// (category [3] "refetch policy") is owned not by this behavior but by the -// `RecurringRunner` it schedules on: given a `reschedule` function (injected by -// the engine, which composes the playlist's target-duration cadence with a -// sleep), the runner re-runs the resolve task whenever `reschedule` resolves, -// until it returns `null` (the playlist completed). With no `reschedule` it runs -// once (VoD / non-live). The recurrence loop lives in the runner, which knows -// nothing about time — so the behavior stays free of timers and reload signals. -// -// The runner is single-slot: a selection change re-schedules (abort-and-replace), -// and the reactor's `presentation-resolved` exit aborts it on source change. The -// post-resolve presentation write is read with `peek`, so it never re-fires the -// effect. +// inline. The orchestration — gate on a selection, short-circuit when the +// track is already complete or missing, schedule the fetch+parse, and patch +// the resolved track back into `state.presentation` — is shared. // ============================================================================ /** @@ -66,18 +48,13 @@ interface TrackResolutionConfig { ) => PartiallyResolvedTrack | ResolvedTrack | undefined; /** Fetch a track's media-playlist text — already failover-decorated by the behavior. */ fetchResolvableText?: FetchText; - /** - * Re-run policy handed to the `RecurringRunner`: returns a promise that resolves - * when the live track's playlist should reload, or `null` to stop. Absent → - * resolve once (VoD / non-live). Injected by the engine (which composes the - * target-duration cadence with a sleep); the behavior stays free of timers and - * reload signals. - */ + /** Live re-run policy for the `RecurringRunner`; absent → resolve once (VoD). */ reschedule?: Reschedule; } /** - * Engine-config slice each `resolve*` behavior reads. + * Engine-config slice each `resolve*` behavior reads to build its failover- + * decorated playlist fetch. */ interface ResolveTrackConfig { /** CDN-id derivation for the failover trip; defaults to origin-based `getCdnId`. */ @@ -86,19 +63,6 @@ interface ResolveTrackConfig { reschedule?: Reschedule; } -/** - * Whether the loader should (re)load this track now: yes if it's unresolved (the - * initial resolve, or a retry of a failed one), or resolved-but-incomplete (a - * live window that may have slid past the playhead — reuse risks a stall); no if - * resolved + complete (VoD, or live that hit `#EXT-X-ENDLIST` — a complete - * playlist can never go stale). Completeness keys off `Track.duration`, the - * single completeness source of truth. This gate governs only whether to - * (re)*start* loading; the live reload *cadence* is the `RecurringRunner`'s job. - */ -function shouldLoadTrack(track: PartiallyResolvedTrack | ResolvedTrack): boolean { - return !isResolvedTrack(track) || !Number.isFinite(track.duration); -} - function setupTrackResolution({ state, config: { selectedKey, findTrackToResolve, fetchResolvableText = defaultFetchResolvableText, reschedule }, @@ -106,71 +70,18 @@ function setupTrackResolution({ state: ResolveTrackStateMap; config: TrackResolutionConfig; }) { - // The runner owns recurrence: with a `reschedule` (live) it re-runs the resolve - // task whenever reschedule resolves true, until it returns false; with none - // configured (VoD) `runOnce` makes it resolve exactly once. Single-slot — a - // selection change re-schedules (abort-and-replace), and the reactor's - // state-exit aborts it on source change. + // Recurrence lives in the runner: with a `reschedule` (live) it re-runs the + // task until the policy stops; `runOnce` (VoD) runs it exactly once. Single- + // slot — a selection change re-schedules (abort-and-replace). const runner = new RecurringRunner(reschedule ?? runOnce); - // The resolve task for `trackId`. The `RecurringRunner` re-runs this same task - // each reload cycle, so its run fn re-reads the current snapshot each time — - // carrying the prior window's timeline forward — then fetches+parses, patches - // `state.presentation`, and returns the resolved track (the runner's - // `reschedule` reads its metadata to decide the next cadence). - const createResolveTask = (trackId: string): Task => - new Task( - async (signal) => { - const presentation = peek(state.presentation); - const track = presentation ? findTrackToResolve(presentation, trackId) : undefined; - // The recurrence is aborted on source change, so a missing track here is - // a transient race; surfacing it as an error lets the policy retry. - if (!track) throw new Error('resolve-track: selected track not found'); - - // `fetchResolvableText` is the behavior's failover-decorated fetch: it - // trips the CDN on a failed fetch (network error or non-OK status). A - // parse failure is a content issue, not a CDN-availability one, so it - // doesn't trip. `track` is the prior snapshot (the unresolved shell on the - // first pass, the last resolved window on a live reload); the parser - // carries its timeline forward. - const text = await fetchResolvableText(track, { signal }); - const mediaTrack = parseMediaPlaylist(text, track); - - // Updater handles undefined inputs by returning current unchanged; - // isResolvedPresentation narrows for the patch. State-exit on - // resolving→unresolved fires runner.abortAll before any URL change - // settles, and per the Fetch spec the signal abort cancels in-flight body - // reads — so by the time we reach this point the presentation we resolved - // against is the live one. - update(state.presentation, (current) => { - if (!isResolvedPresentation(current)) return current; - const patched = updateTrackInPresentation(current, mediaTrack); - // Container is uniform within a type (an ABR ladder shares its - // container), so a detected non-fMP4 rendition (TS, raw AAC) implies - // every rendition of *this* type matches — relabel them all from one - // resolved playlist instead of fetching each. Scoped to this track's own - // type: never cross audio↔video (mixed-container sources exist, e.g. - // muxed-TS video + raw-.aac audio), which also keeps per-type - // resolutions' writes disjoint (no race). - const relabeled = NON_FMP4_CONTAINER_MIMES.has(mediaTrack.mimeType) - ? applyContainerMimeType(patched, mediaTrack.type, mediaTrack.mimeType) - : patched; - // Stream nature (category [2a]) — stable once a media playlist is - // parsed; recomputing each reload is harmless. - return { ...relabeled, streamType: deriveStreamType(getMediaPlaylistMetadata(mediaTrack)) }; - }); - return mediaTrack; - }, - { id: trackId } - ); - // Reactor states model the FSM the previous effect-based body was // hand-rolling. 'presentation-resolved' is entered when the // presentation is fully parsed (has a Ham id + selectionSets); leaving - // it (presentation cleared or reset to an unresolved value) aborts the - // in-flight + scheduled reload via the entry-cleanup. Most URL changes go - // through 'presentation-unresolved' naturally (set undefined → set new - // partial → re-parse), so the common case is covered by state-exit alone. + // it (presentation cleared or reset to an unresolved value) aborts all + // in-flight tasks via the entry-cleanup. Most URL changes go through + // 'presentation-unresolved' naturally (set undefined → set new partial + // → re-parse), so the common case is covered by state-exit alone. const derivedStateSignal = computed(() => isResolvedPresentation(state.presentation.get()) ? ('presentation-resolved' as const) @@ -185,32 +96,76 @@ function setupTrackResolution({ 'presentation-resolved': { // `entry` runs on state entry; the function it returns is the // state-exit cleanup. Returning `() => runner.abortAll()` binds - // abort-of-in-flight-resolution (and any pending reload) to leaving - // 'presentation-resolved' (presentation cleared/reset, or behavior - // destroyed) — source-change cancellation expressed structurally - // through the state machine. + // abort-of-in-flight-resolutions to leaving 'presentation-resolved' + // (presentation cleared/reset, or behavior destroyed) — + // source-change cancellation expressed structurally through the + // state machine. entry: () => () => runner.abortAll(), effects: [ () => { - // `selectedKey` is read tracked so a selection change re-fires this - // and re-schedules (the single-slot runner aborts+replaces the prior - // recurrence). Presentation is peeked (untracked) so the post-resolve - // write — and the runner's own reload cycles — don't re-fire it. - const trackId = state[selectedKey].get(); + // The reactor's state transitions handle relevant presentation + // changes (presentation-resolved ↔ presentation-unresolved); + // within 'presentation-resolved' we peek (untracked read) so + // internal updates (the post-resolve write, the runner's own + // reload cycles) don't re-fire the effect. const presentation = peek(state.presentation); + const trackId = state[selectedKey].get(); if (!presentation || !trackId) return; const track = findTrackToResolve(presentation, trackId); - // Gate: schedule only when there's loading to do (unresolved, or a - // resolved-but-incomplete live window). Recurrence past the first run - // is the runner's job, driven by `reschedule`. - if (!track || !shouldLoadTrack(track)) return; + // Skip a complete or missing track; a resolved-but-incomplete (live) + // window still reloads — its window may have slid past the playhead. + if (!track || (isResolvedTrack(track) && Number.isFinite(track.duration))) return; - // Abort (selection/source change, via `abortAll`) settles quietly; - // `schedule` rejects only on a genuine resolve failure. We don't - // surface those to state yet, so end quietly. - // TODO: surface unrecoverable resolve errors. - runner.schedule(createResolveTask(trackId)).catch(() => {}); + // Abort (selection/source change) settles quietly; a genuine resolve + // failure rejects — swallowed for now (TODO: surface to state). + const scheduled = runner.schedule( + // Re-runs (clones) on each reload, so the body re-reads the live + // snapshot (via `trackId`) rather than capturing the gate-time + // `track` — a reload carries the prior window's timeline forward. + new Task( + async (signal) => { + const snapshot = peek(state.presentation); + const track = snapshot ? findTrackToResolve(snapshot, trackId) : undefined; + if (!track) throw new Error('resolve-track: selected track not found'); + + // `fetchResolvableText` is the behavior's failover-decorated + // fetch: it trips the CDN on a failed fetch (network error or + // non-OK status). A parse failure is a content issue, not a + // CDN-availability one, so it doesn't trip. + const text = await fetchResolvableText(track, { signal }); + const mediaTrack = parseMediaPlaylist(text, track); + + // Updater handles undefined inputs by returning current + // unchanged; isResolvedPresentation narrows for the patch. + // State-exit on resolving→unresolved fires runner.abortAll + // before any URL change settles, and per the Fetch spec the + // signal abort cancels in-flight body reads — so by the + // time we reach this point the presentation we resolved + // against is the live one. + update(state.presentation, (current) => { + if (!isResolvedPresentation(current)) return current; + const patched = updateTrackInPresentation(current, mediaTrack); + // Container is uniform within a type (an ABR ladder shares + // its container), so a detected non-fMP4 rendition (TS, + // raw AAC) implies every rendition of *this* type matches — + // relabel them all from one resolved playlist instead of + // fetching each. Scoped to this track's own type: never cross + // audio↔video (mixed-container sources exist, e.g. muxed-TS + // video + raw-.aac audio), which also keeps per-type + // resolutions' writes disjoint (no race). + const relabeled = NON_FMP4_CONTAINER_MIMES.has(mediaTrack.mimeType) + ? applyContainerMimeType(patched, mediaTrack.type, mediaTrack.mimeType) + : patched; + // Stream nature (category [2a]) — stable once parsed. + return { ...relabeled, streamType: deriveStreamType(getMediaPlaylistMetadata(mediaTrack)) }; + }); + return mediaTrack; + }, + { id: track.id } + ) + ); + scheduled.catch(() => {}); }, ], }, @@ -261,10 +216,9 @@ export const resolveVideoTrack = defineBehavior({ }) => { // Engine `config` layers over the per-type defaults (mirrors the other // per-type variants, see track-types.ts); `failoverFetch` reads its - // `selectedKey` + `getCdnId` from the merged result, and `reschedule` - // rides through to the runner. `fetchResolvableText` is placed AFTER the - // spread so the failover-decorated fetch wins — unlike segments, playlists - // expose no overridable per-type fetch. + // `selectedKey` + `getCdnId` from the merged result. `fetchResolvableText` + // is then placed AFTER the spread so the failover-decorated fetch wins — + // unlike segments, playlists expose no overridable per-type fetch. const trackConfig = { ...VIDEO_TRACK_RESOLUTION_CONFIG, ...config }; return setupTrackResolution({ state,