refactor(spf): Changed adapter code to recycle engine on src change (#1813)

internal/design/spf/features/engine-adapter-integration.md

@@ -35,7 +35,7 @@ the mixin.
 | Writable signal refs via `onSignalsReady` | `shareSignals` captures `Signal<T>` / `ReadonlySignal<T>` refs into a consumer-supplied callback at setup time. Generic over composition shape (`makeShareSignals<S, C>()`) | Per-slot read/write intent is expressed at the use site (callers type captured refs as `Signal<T>` or `ReadonlySignal<T>`). Composed last in the engine so initial state writes are visible to the consumer |
 | Mixin adapter pattern | `SimpleHlsMediaMixin` is the canonical consumer: function-of-base-class structure (mix into any base), captures refs once in `onSignalsReady`, exposes a WHATWG HTMLMediaElement-shaped API mapping each setter/method to engine writes | Downstream use: `class SimpleHlsMedia extends SimpleHlsMediaMixin(HTMLVideoElementHost) {}` in `packages/core/src/dom/media/simple-hls/` |
 | Media element binding | `attach(el)` writes `context.mediaElement`; `detach()` clears it. **Engine persists across attach/detach cycles** — only `src` reassignment or explicit `destroy()` tears it down | Re-attach to a different element is supported. The engine is the durable state holder; `mediaElement` is a context slot |
-| Source assignment via destroy + recreate | Adapter's `set src` destroys the current engine and creates a fresh one, re-applies any explicit preload, re-attaches `mediaElement` to the new engine, and writes the new `{ url }` | Bypasses the in-place source-replacement path. Rationale not documented in code — see Open questions and [source-replacement.md](./source-replacement.md) |
+| Source assignment via in-place recycling | Adapter's `set src` overwrites `state.presentation` on its single recycled engine (`{ url }`, or `undefined` for empty src). Media element + engine-wide preload persist; no engine recreation, no signal re-capture | Drives the engine's in-place source-replacement cascade — see [source-replacement.md](./source-replacement.md). (The adapter previously destroyed + recreated the engine per assignment.) |
 | Preload reflection | `set preload(value)` writes W3C values to `state.preload`; clearing (`preload = ''`) doesn't patch the current engine but is re-applied on the next src change. Pre-attach src + preload combinations are supported | Extended preload values flow through state but don't reach the DOM (per [`preload-modes`](./preload-modes.md)'s sticky-extended-values semantics) |
 | Programmatic `play()` with retry | `play()` writes `state.loadActivated = true` (co-writer with `trackLoadTriggers`'s DOM listener path) before invoking native play. **Defensive retry:** if native play rejects with "no supported sources" while src is pending, wait for `loadstart` (MSE attaches blob URL) and retry once | The retry handles MSE pipeline timing — adapter doesn't know exactly when MSE setup attaches the blob URL. Listener canceled on src change |
 
@@ -101,7 +101,7 @@ return createComposition(
 | `attach(el)` | `context.mediaElement.set(el)` |
 | `detach()` | `context.mediaElement.set(undefined)` |
 | `destroy()` | `engine.destroy()` |
-| `set src(value)` | `engine.destroy()` → new engine → `state.presentation.set({ url: value })` |
+| `set src(value)` | `state.presentation.set({ url: value })` on the recycled engine (`undefined` for empty src) |
 | `set preload(value)` | `state.preload.set(value)` (W3C values only; pre-empties stay engine-local) |
 | `play()` | `state.loadActivated.set(true)` → native `play()` with `loadstart` retry on "no supported sources" |
 
@@ -156,15 +156,15 @@ each `set src`).
 
 ## Open questions
 
-- **Destroy-recreate vs in-place source replacement.** The canonical
-  adapter destroys + recreates the engine on every `src` change, even
-  though the engine's behaviors support in-place
-  `state.presentation` overwrite (validated by
-  [`source-replacement`](./source-replacement.md)'s test). The
-  rationale isn't documented in the code or commit history. Possible
-  motivations: stricter isolation between sources; simpler reasoning
-  per-engine; guarding against latent cleanup-cascade bugs. Worth
-  resolving when the cost of either choice surfaces.
+- **Destroy-recreate vs in-place source replacement.** Resolved: the
+  canonical adapter now recycles a single engine and overwrites
+  `state.presentation` in place on every `src` change, driving the same
+  cascade validated by
+  [`source-replacement`](./source-replacement.md)'s test. This unifies
+  per-source teardown on one path and lets adapter-side projections
+  wire once at construction rather than re-wiring on every src change.
+  It also makes source-change behavior stable enough to build the
+  media-tracks mixin integration on top of.
 - **Callback timing semantics.** `shareSignals`'s JSDoc explicitly
   notes the callback fires while other behaviors are still in setup;
   reads inside the callback may yield only initial-seed values. The

internal/design/spf/features/source-replacement.md

@@ -15,12 +15,6 @@ load-bearing — every behavior that gates on `isResolvedPresentation`
 must honor the state-exit cleanup contract, or in-place replacement
 breaks silently.
 
-The canonical adapter (`SimpleHlsMediaMixin.src`) takes a different
-path: destroy the engine and create a fresh one on every assignment.
-Both paths work; the in-place path is the load-bearing one for
-*engine-internal* reasoning, since the adapter's destroy path bypasses
-the cascade entirely.
-
 This doc captures the **capability surface**, the **cleanup contract**
 new behaviors must honor, and the verification that pins the
 in-place path against regression.
@@ -49,7 +43,7 @@ distinct engine behavior observable from outside.
 | Initial source load | First source on a fresh engine: external write of `state.presentation = { url }` triggers resolve + full pipeline setup | The unresolved → resolved transition that bootstraps everything |
 | In-place source replacement | Overwrite `state.presentation` with a new `{ url }` while a previous source is resolved / playing. `resolvePresentation` routes back through `'resolving'`; downstream behaviors tear down via reactor state-exit; new source resolves and plays — *same engine instance* | Validated end-to-end. MediaSource + buffer actors are fresh instances; in-flight fetches aborted via state-bound `AbortController`s |
 | Source unset | Set `state.presentation` to `undefined`. All presentation-gated behaviors transition to `'preconditions-unmet'` and tear down. Engine is fresh-but-attached, ready for the next source | The "no source" steady state; reachable from any resolved state |
-| Destroy + recreate (canonical adapter path) | `SimpleHlsMediaMixin.src` destroys the current engine and creates a fresh one on every assignment. Re-attaches the media element to the new engine | Canonical *consumer-side* mechanism. Bypasses the in-place cascade entirely. Tested via `adapter.test.ts:115–150` |
+| Adapter-driven in-place replacement (canonical consumer path) | `SimpleHlsMediaMixin.src` overwrites `state.presentation` on its recycled engine (empty `src` → `undefined`, unsetting the source). Media element + engine-wide preload persist across the change | Canonical *consumer-side* mechanism. Rides the same in-place cascade as engine-internal replacement — no engine recreation. Tested via the recycling assertions in `adapter.test.ts` |
 | Per-source-identity slot lifecycle | `loadActivated` resets to `false` when source identity changes (URL or `mediaElement`); selected*TrackIds clear naturally on un-resolve (their pickers re-run against the new presentation); **`bandwidthState` is intentionally preserved** across source resets — sampling accumulates via the once-per-behavior `createTrackedFetch` | ABR resume: bandwidth estimate carries over so the first segment of a new source picks an appropriate quality based on observed throughput |
 
 ## What's not implemented
@@ -153,17 +147,17 @@ semantics — every replaced source runs through the same parser.
     captured identities differ from the new ones (proving teardown
     cascade ran)
   - `packages/spf/src/playback/engines/hls/tests/adapter.test.ts` →
-    `"creates a new engine when src is set"` /
-    `"destroys the old engine when src changes"` /
-    `"re-attaches the media element to the new engine when src changes"`
-    — validates the canonical adapter destroy-recreate path
+    `"reuses the same engine instance when src changes"` /
+    `"does not destroy the engine when src changes"` /
+    `"keeps the attached media element across src changes"`
+    — validates the canonical adapter's in-place recycling path
   - `packages/spf/src/playback/behaviors/dom/tests/track-load-triggers.test.ts`
     — `loadActivated` per-source-identity reset coverage
 - **Sandbox:**
   - `apps/sandbox/src/spf-segment-loading/` — exercises initial source
     load + manual rendition switching (in-track, not source change)
   - `apps/sandbox/src/simple-hls-html/` / `simple-hls-react/` — adapter
-    integration; src reassignment hits the destroy-recreate path
+    integration; src reassignment recycles the engine via the in-place path
 
 ## Open questions
 
@@ -172,12 +166,12 @@ semantics — every replaced source runs through the same parser.
   management)` for a state-error slot. The shape of this slot — single
   error vs per-source — affects how consumers respond to "source failed
   to load."
-- **Adapter rationale.** The canonical adapter destroys + recreates the
-  engine on every src change instead of using in-place replacement. The
-  reasoning (stricter isolation? simpler reasoning? guarding against
-  cleanup-cascade bugs?) isn't documented in the code or commit history.
-  If the in-place path is the engine's load-bearing capability for
-  internal reasoning, why doesn't the adapter use it?
+- **Adapter rationale.** Resolved: the canonical adapter
+  now recycles a single engine and drives source changes through in-place
+  `state.presentation` replacement — the same load-bearing cascade the
+  engine uses internally. Recycling was adopted so per-source teardown
+  routes through one path, and so adapter-side projections wire once at
+  construction instead of re-wiring on every src change.
 - **Per-source `bandwidthState` reset opt-in.** Preserving across
   sources is the right default for ABR resume, but a test / fresh-
   session escape hatch may earn its place when consumers start needing
@@ -200,10 +194,9 @@ semantics — every replaced source runs through the same parser.
   re-bootstrapping from `initialBandwidth`.
 - **subtitles** — text-track actors and selection clear on source
   un-resolve via the cleanup cascade.
-- **engine-adapter-integration** *(not yet documented, candidate)* —
-  `SimpleHlsMediaMixin`'s destroy-recreate path lives here. The
-  adapter's choice to bypass in-place replacement is the
-  feature-design decision to capture in that doc.
+- **engine-adapter-integration** — `SimpleHlsMediaMixin`'s source-
+  assignment path lives here. The adapter recycles a single engine and
+  drives source changes through this feature's in-place cascade.
 
 ## See also
 

packages/spf/src/playback/engines/hls/adapter.ts

@@ -32,10 +32,8 @@ export interface SimpleHlsMediaAPI extends SimpleHlsMediaProps {
  * Implements the src/play() contract per the WHATWG HTML spec so that SPF can
  * be used anywhere a media element API is expected.
  *
- * A new engine is created on every src assignment — this fully tears down all
- * state, SourceBuffers, and in-flight requests from the previous source before
- * the next one begins. The media element reference is preserved across src
- * changes and re-applied to the new engine automatically.
+ * A single engine instance is created at construction and recycled across src
+ * changes.
  *
  * @example
  * class SimpleHlsMedia extends SimpleHlsMediaMixin(HTMLVideoElementHost) {}
@@ -46,7 +44,7 @@ export interface SimpleHlsMediaAPI extends SimpleHlsMediaProps {
  */
 export function SimpleHlsMediaMixin<Base extends Constructor<any>>(BaseClass: Base) {
   class SimpleHlsMediaImpl extends BaseClass {
-    #engine: Composition<SimpleHlsEngineState, SimpleHlsEngineContext>;
+    readonly #engine: Composition<SimpleHlsEngineState, SimpleHlsEngineContext>;
     #config: SimpleHlsEngineConfig;
     #signals!: SimpleHlsEngineSignals;
     #preload: '' | 'none' | 'metadata' | 'auto' = simpleHlsMediaDefaultProps.preload;
@@ -106,15 +104,19 @@ export function SimpleHlsMediaMixin<Base extends Constructor<any>>(BaseClass: Ba
       if (value) {
         this.#signals.state.preload.set(value);
       }
-      // value = '' clears #preload (so the next engine recreation won't re-apply
-      // an explicit value) but does not patch current state — the existing preload
-      // stays in effect until the next src change creates a fresh engine.
+      // value = '' resets the IDL mirror (so `get preload` reflects '') but does
+      // not patch state — the engine keeps its current preload until an explicit
+      // W3C value replaces it.
     }
 
     // -------------------------------------------------------------------------
     // src — synchronous IDL attribute (WHATWG §4.8.11.2)
-    // Each assignment destroys the current engine and starts a fresh one, exactly
-    // as the browser's load algorithm resets all media element state on src change.
+    // Each assignment overwrites the engine's presentation state in place. The
+    // resolver FSM routes back through teardown → rebuild on the same engine,
+    // mirroring how the browser's load algorithm resets media state on src change
+    // — without recreating the engine or re-capturing its signals. Setting an
+    // empty src un-resolves the presentation, tearing the current source down to
+    // the engine's fresh-but-attached "no source" state.
     // -------------------------------------------------------------------------
 
     get src(): string {
@@ -122,27 +124,8 @@ export function SimpleHlsMediaMixin<Base extends Constructor<any>>(BaseClass: Ba
     }
 
     set src(value: string) {
-      const prevMediaElement = this.#signals.context.mediaElement.get();
-
       this.#cancelPendingPlay();
-      this.#engine.destroy();
-      this.#engine = this.#createEngine();
-
-      // Apply explicit preload before setting context so it's already in
-      // state.preload when syncPreload's read effect runs on the attach —
-      // the read effect only overwrites when the element's `preload` is a
-      // W3C value (which a freshly-created <video> with no attribute is not).
-      if (this.#preload) {
-        this.#signals.state.preload.set(this.#preload);
-      }
-
-      if (prevMediaElement) {
-        this.#signals.context.mediaElement.set(prevMediaElement);
-      }
-
-      if (value) {
-        this.#signals.state.presentation.set({ url: value });
-      }
+      this.#signals.state.presentation.set(value ? { url: value } : undefined);
     }
 
     // -------------------------------------------------------------------------

packages/spf/src/playback/engines/hls/tests/adapter.test.ts

@@ -112,26 +112,35 @@ describe('SimpleHlsMediaElement', () => {
       expect(media.engine).toBe(engine);
     });
 
-    it('creates a new engine when src is set', () => {
+    it('reuses the same engine instance when src is set', () => {
       const media = new SimpleHlsMediaElement();
       const initial = media.engine;
       media.src = 'https://example.com/v1.m3u8';
-      expect(media.engine).not.toBe(initial);
+      expect(media.engine).toBe(initial);
     });
 
-    it('destroys the old engine when src changes', () => {
+    it('reuses the same engine instance when src changes', () => {
+      const media = new SimpleHlsMediaElement();
+      media.src = 'https://example.com/v1.m3u8';
+      const engine = media.engine;
+      media.src = 'https://example.com/v2.m3u8';
+      expect(media.engine).toBe(engine);
+    });
+
+    it('does not destroy the engine when src changes', () => {
       const media = new SimpleHlsMediaElement();
       media.src = 'https://example.com/v1.m3u8';
       const spy = vi.spyOn(media.engine, 'destroy');
       media.src = 'https://example.com/v2.m3u8';
-      expect(spy).toHaveBeenCalledOnce();
+      expect(spy).not.toHaveBeenCalled();
     });
 
-    it('re-attaches the media element to the new engine when src changes', () => {
+    it('keeps the attached media element across src changes', () => {
       const media = new SimpleHlsMediaElement();
       const el = document.createElement('video');
       media.attach(el);
       media.src = 'https://example.com/v1.m3u8';
+      media.src = 'https://example.com/v2.m3u8';
       expect(media.engine.context.mediaElement.get()).toBe(el);
     });
 
@@ -328,7 +337,7 @@ describe('SimpleHlsMediaElement', () => {
       expect(media.engine.state.preload.get()).toBe('auto');
     });
 
-    it('survives src reassignment — explicit preload is preserved across engine recreation', () => {
+    it('survives src reassignment — explicit preload persists on the recycled engine', () => {
       const media = new SimpleHlsMediaElement();
       media.preload = 'none';
       media.src = 'https://example.com/v.m3u8';
@@ -336,16 +345,21 @@ describe('SimpleHlsMediaElement', () => {
       expect(media.engine.state.preload.get()).toBe('none');
     });
 
-    it('explicit preload is re-applied before owners.patch on src change so syncPreload preserves it', () => {
+    it('keeps explicit preload in engine state across src changes', () => {
       const media = new SimpleHlsMediaElement();
       const el = document.createElement('video');
       media.attach(el);
       media.preload = 'none';
       media.src = 'https://example.com/v.m3u8';
-      // syncPreload fires when context.mediaElement is set on the new engine.
-      // The freshly-created <video> has no preload attribute (mediaElement.preload === '')
-      // so the read effect's "only overwrite for W3C values" rule leaves state alone.
+      // The engine is recycled, so state.preload is engine-wide preference that
+      // simply persists across the src change — no re-application needed.
       expect(media.engine.state.preload.get()).toBe('none');
+
+      // Changing preload, then changing src again, keeps the latest value on the
+      // same engine — not reset to a default by the source change.
+      media.preload = 'auto';
+      media.src = 'https://example.com/v2.m3u8';
+      expect(media.engine.state.preload.get()).toBe('auto');
     });
   });