# SPEC — Video Pipeline Optimization: Error-Proofing and Latency Reduction for `sink`'s Compositor

## Status

**Specify phase, post Spec Revision.** Per `plevion-docs/WORKFLOW.md`, this spec has been through one independent-model (Opus 5) Spec Revision pass — see "Revision Changelog" at the end for the full findings list. Several of the original recommendations were factually wrong or mechanically unworkable; this version corrects them, not just appends a changelog on top of stale content. Still not approved — this needs a human decision before Plan/Build.

## Objective

The compositor pipeline (`sink`'s `apps.compositor`, primarily `services/dispatcher.py` + `tasks.py`, with `apps.capturer`/`apps.sources` as direct dependencies for connection/auth) needs to be **error-proof and lightning fast** — the user's own framing, not softened here. This spec investigates seven areas end-to-end (DB queries, network/connection architecture, retry/error-handling, the bus-polling loop, the Celery process model, whether any hot path warrants a Rust/C rewrite, and encoder/resource tuning) and makes a grounded recommendation for each — including "no change needed" where the evidence doesn't support intervention. **Don't overengineer** is a hard constraint here, not a platitude.

## Why this spec exists — the incident that triggered it

This session already found and fixed several real pipeline bugs, all merged (sink PR #125, #126): a bus error on one branch cascading into the pipeline's permanent placeholder branch; no operator notification when a source got auto-removed; `x264enc`'s `key-int-max` left stale after MediaMTX moved to LL-HLS; `force_encoder_ keyframe` wrongly scoped to CUT-only transitions; `stop_composition` leaking an INTERACTIVE source's browser/`CaptureSession` on composition stop.

**The incident that motivates this spec specifically**: live logs from composition `c154ade3-...` (captured after PR #126 was already deployed) showed all 4 of its real branches (2 `CONTROLLER_STREAM` sources, 2 overlays) failing within the same second, ~7 seconds after a fresh pipeline start:

```
14:44:38  Compositor pipeline STARTS (sources=2 overlays=2)
14:44:40  Output sink connects
14:44:47  ALL FOUR branches fail within the same second — "Internal data stream error"
          (plus the placeholder branch cascades too, recreated fine — isolation worked)
14:44:47  retry_add_source's re-add for one source immediately fails twice more —
          "Could not write to resource"
14:44:47  The shared x264 ENCODER itself then fails: "Encode x264 frame failed"
14:44:48  Pipeline stopped entirely
```

The per-branch isolation worked correctly — each of the 4 failures was individually, correctly attributed and soft-removed via `_notify_source_auto_removed`. The bug is what happened next: the shared x264 encoder failing is **unattributable** by design (`_source_id_from_error_src` has no branch name to walk up to — the encoder aggregates every branch's output), so `_handle_bus_error` falls through to its pre-existing "tear down the entire pipeline" fallback — the original, correct behavior for a genuinely pipeline-level failure, not a regression. **Nothing ever restarts it, and (corrected by Spec Revision — see below) in the general case nothing *can*, even manually.**

Confirmed by reading the actual code:
- `run_compositor`'s `finally` block (`tasks.py:421-425`) tears down GStreamer state and calls `_clear_compositor_task_id`, which sets `compositor_task_id=None` (`tasks.py:639-641`).
- The **only** place in the entire `sink` codebase that checks `if not composition.compositor_task_id` and relaunches the pipeline is `publish_composition` (`services/lifecycle.py:148-149`) — an explicit, operator-triggered action, and (see Area 3a) it only works if that `finally` block actually ran.
- `_clear_compositor_task_id`'s own docstring claims "the only reconnect trigger in events.py" — this is stale/misleading: `services/events.py`'s `on_stream_status_change`/`_restore_sources` only ever flags `need_update=True` on stream reconnect (`services/events.py:67-95`); grepped the whole `src/` tree and the only real `if not composition.compositor_task_id` occurrence is `lifecycle.py:148`. Worth a one-line docstring fix during Build.

**The initial trigger (all 4 branches failing within the same second) is not conclusively root-caused.** The original version of this spec proposed CPU contention under concurrent-composition load as the leading hypothesis. **Spec Revision found a materially stronger, previously-missed candidate** — see Area 2/"Open Questions" — and also a plausible alternative that the isolation mechanism's own pad churn triggered the encoder failure rather than merely failing to recover from it. All three are listed as open, competing hypotheses now, not a single-hypothesis frame.

## Investigation Methodology

Grounded in direct code reading and the actual k8s manifests governing production — every finding below cites file:line, verified against current code (this version's citations were independently re-checked during Spec Revision; one wrong citation from the original draft is corrected below, noted where it occurred).

**Explicit methodology gap, unchanged from the original draft**: "check Django DB queries" was investigated via static code reading, not a live query-count profile. Recommended as real, cheap Plan-phase work before Build on Area 1's fix.

## Area 1 — Django DB Queries in the Pipeline Hot Path

Two concrete N+1-shaped findings — both still hold, one fix is corrected below to avoid a pessimization Spec Revision caught.

**Finding 1a — `_attach_initial_sources` (`tasks.py:452-476`), missing `select_related` on the FK chain a subset of rows actually walks.** `parent_source` and `capture_session` are both real `ForeignKey`s (`models.py:201-220`). For every attached INTERACTIVE source that is a copy, `get_governing_source` triggers one query to fetch `parent_source`, then `governing.capture_session` triggers a second — up to 2N extra queries for N attached interactive copies, on every pipeline start. Fix: `.select_related('parent_source__capture_session', 'capture_session')` on the base queryset. Note (Spec Revision, minor): this adds two LEFT JOINs to every row including non-INTERACTIVE ones — still a net win, just not entirely free.

**Finding 1b — `active_scene_id` is independently re-fetched at three call sites within `_dispatch_command`'s handlers** (`_handle_add_overlay`: `tasks.py:835`, `_handle_add_source`: `tasks.py:861`, `_handle_set_source_visibility`: `tasks.py:913` — `_attach_initial_sources`'s own fetch at `tasks.py:452` is a fourth call to the same query, but it runs once at pipeline boot, not per tick, so it isn't part of this redundancy). **Corrected recommendation** (the original draft's fix was a pessimization, caught in Spec Revision): an idle `run_compositor` tick currently issues **zero** DB queries — resolving `active_scene_id` unconditionally once per loop iteration would add 2 queries/second/composition where there are none today. The actual fix: resolve it once per `get_compositor_commands` drain, lazily, only when the drain is non-empty, and thread it through to whichever handlers in that batch need it. Safe on staleness: `switch_scene` writes `Composition.active_scene` before sending its own dispatch command (`lifecycle.py:904` before `914`), so a drain always sees a consistent value.

**What's already fine, don't touch**: every `.save(update_fields=[...])` call in the error/removal paths is already scoped to exactly the fields it changes. `_notify_source_auto_removed`'s copy-group handling (`tasks.py:538-547`) is one SELECT plus up to N `save(update_fields=...)` UPDATEs in a loop, not a hidden N+1 SELECT pattern (corrected characterization, Spec Revision M3) — the "leave it alone" conclusion still holds, since a copy group is normally small and this only runs on an actual error.

## Area 2 — Network/Connection Architecture

**Reversed from the original draft's "no change recommended" — Spec Revision found the actual hot-path network cost, and it's real.**

GStreamer's own RTSP element behavior is fine as originally assessed: `add_source`'s `rtspsrc` is forced to TCP (`protocols=4`, `dispatcher.py:1224`) with a 200ms `latency` (`dispatcher.py:1223`); no pad-probe gating is a deliberate, already-tested decision (`dispatcher.py:1202-1207` documents a prior attempt deadlocking — no `GLib.MainLoop` exists to service it, see Area 4); connection pooling doesn't apply to long-lived GStreamer elements the way it would an HTTP client. **All of that is still correct — but it isn't the actual network cost on this path.**

**The real finding**: `add_source` (`dispatcher.py:1174-1265`) calls `get_object_jwt(...)` at line 1218, **synchronously, per source**, every time a source is attached. `get_object_jwt` (`apps/common/services/auth.py:34-38`) makes **two blocking HTTP round-trips to the `auth` service** — `get_code_token_for_object` (`requests.post`, `timeout=10`) then `_exchange_code_for_jwt` (`requests.post`, `timeout=10`) — with no `requests.Session` (no connection reuse), no caching, no retry. `start_composition_task` makes a third such pair for the output JWT (`tasks.py:58-92`). This runs **inside** `_attach_initial_sources`'s loop and inside `_dispatch_command`'s `add_source` handler — i.e. inside the bus-polling loop itself (see Area 4's corrected latency model, which this directly invalidates). Worst case: N sources × up to 20s of blocking HTTP, during which the bus is not polled and the output sink is not serviced.

**A second, sharper consequence**: `get_object_jwt` swallows its own exceptions and returns `None` on any auth hiccup (`auth.py:63-65`). `add_source`'s `if jwt: ... set_property('user-id'/'user-pw', ...)` guard (`dispatcher.py:1225`) means a `None` JWT silently attaches an **unauthenticated** `rtspsrc` — which MediaMTX then rejects. This matches the incident's own log line, "Could not write to resource," and is a materially stronger candidate for the incident's root trigger than the CPU-contention hypothesis the original draft led with (see "Open Questions").

**Recommendation**: add a pooled `requests.Session` (or an equivalent keep-alive HTTP client) shared across `get_object_jwt` calls within one pipeline's lifetime, and a short-lived in-process JWT cache keyed by object id (these JWTs are short-lived by design, so this is a TTL cache, not indefinite caching) — cuts N-sources-worth of blocking round-trips down to whatever's actually needed, and removes the "hung on `auth`" single point of stall from the loop entirely if combined with Area 4's fix below. This is real, scoped, non-overengineered work — not a rewrite of the auth flow itself.

## Area 3 — Retry/Error-Handling Design

Substantially revised from the original draft. Spec Revision found the original recommendation (build A first, C as a safety net, defer B) doesn't hold up: Option A as originally sketched doesn't mechanically work, and there's a more severe, previously-unstated problem (a hard-killed worker leaves a composition permanently unrecoverable, not just "dead until manual republish") that any design here has to solve first.

**3a-i. The unrecoverable state — this needs to be fixed regardless of which recovery mechanism gets built, and it's worse than the original draft described.**

`_clear_compositor_task_id` is the *only* writer that ever clears `compositor_task_id` (confirmed by grepping `src/`). If `run_compositor`'s process is killed before that `finally` block runs — a SIGKILL from an OOM event, a node eviction, or (see 3a-iv) a routine rolling deploy — `compositor_task_id` stays set to a now-dead task's id. Consequences, chained:
- `publish_composition`'s own guard, `if not composition.compositor_task_id` (`lifecycle.py:148`), is `False` — an operator's manual re-publish (the original draft's assumed recovery path) **silently does nothing**.
- Stopping first doesn't help either: `stop_composition` sets `phase = ENDED` (`lifecycle.py:414`), and `_validate_publish_transition` then raises `COMP_SOURCE_009` on any subsequent re-publish attempt from `ENDED`.
- Net: the only way out today is direct DB surgery. This is a strictly worse failure mode than "dead until someone notices" — it's "dead, and the obvious fix doesn't work either."

**3a-ii. Corrected recovery design: a liveness-keyed watchdog first, event-driven restart deferred.**

The original draft recommended Option A (event-driven restart mirroring `retry_add_source`) as primary and Option C (periodic watchdog) as a secondary safety net. Spec Revision reversed this, for concrete reasons:

- **Option A doesn't mechanically transfer from `retry_add_source`.** `retry_add_source` is safe to fire up to 30 times because the receiver is idempotent (`_handle_add_source` no-ops if already attached, `tasks.py:856-858`) and because it targets one branch, not the whole pipeline. `composition.start` (`start_composition_task`, `tasks.py:58-92`) has **no equivalent no-op guard** — it only checks row existence. Mirroring the retry pattern at the whole-pipeline level as originally proposed would, in the worst case, spawn up to 30 concurrent `run_compositor` tasks for the same composition, each building a full GStreamer pipeline publishing to the same `live/{id}` MediaMTX path — against a 4-slot worker (see Area 5), this is a crash amplifier, not a recovery mechanism. It also has no guard against racing a concurrent, legitimate `stop_composition` call (would resurrect a deliberately-stopped composition), and no queue-flush semantics (a `STOP` or a stale `retry_add_source` command queued before the death survives in Redis — `_CMD_TTL` refreshes on every push, `dispatcher.py:110-123` — and a restarted pipeline would drain and immediately re-stop itself on its first tick).
- **The `match is None` branch (where a restart would be triggered from) is also not the only way `run_compositor` dies.** There are at least five exit paths: (1) `set_state(PLAYING) == FAILURE` (`tasks.py:374`, a bare `return`, no `finally` even runs since it's before the `try:` at line 370 for the pipeline/bus construction itself); (2) EOS; (3) the unattributable-ERROR case this section is about; (4) output-connect timeout; (5) an explicit STOP command. An in-process, event-driven restart only ever covers (3) — none of the others, and definitely not a SIGKILL that skips the `finally` entirely.
- **Option C, corrected, covers all of them, including the worker-process-death case Option A structurally can't reach.** The original draft's proposed watchdog query (`Composition.objects.filter(phase__in=LIVE_PHASES, compositor_task_id='')`) was itself broken — `compositor_task_id` is cleared to `None`, never `''` (`CharField(null=True)`, `models.py:64`), so that filter matches nothing, ever; and calling `publish_composition` directly on an already-LIVE composition raises `COMP_SOURCE_008` (`lifecycle.py:160-171`), so "re-publish" isn't the right action even once the query is fixed. **Corrected design**: key the watchdog on a liveness signal instead of `compositor_task_id`'s presence/absence — e.g. `output_live` (`models.py:70`, already means "actually receiving frames") combined with a heartbeat timestamp `run_compositor`'s own loop already ticks every ~500ms (a new, small addition: touch `Composition.updated_at`, or a dedicated `pipeline_heartbeat_at` field, once per N loop iterations — cheap, avoids a write every single tick). The watchdog then finds compositions `phase in LIVE_PHASES` with a stale heartbeat and dispatches `composition.start` **directly** (bypassing `publish_composition`'s phase-transition validation entirely, since this is recovery, not a phase change) after confirming no other `run_compositor` is already alive for that id.

**3a-iii. Option B (isolate the x264 encoder) — still deferred, but the reasoning is corrected.** The original draft's justification for deferring B ("if CPU pressure turns out to be the cause, B would be solving a symptom") applies with equal force to A/C, which the draft built anyway — an internal inconsistency Spec Revision flagged. The actually-defensible distinction: **A/C are root-cause-agnostic recovery** (worth building regardless of what caused any given pipeline death), while **B is cause-specific** (only worth building once encoder failures are confirmed to keep recurring after A/C ship, and confirmed not to be fully explained by Area 2's auth- latency fix or 3b's retry-storm fix). Defer B on that basis, not on the "solving a symptom" framing.

**3a-iv. New finding — routine rolling deploys hit the exact same unrecoverable state as a crash, and this is the *common* case, not an edge case.** `run_compositor` has no SIGTERM handling of its own, no `terminationGracePeriodSeconds` override, and no `preStop` hook in `sink-worker.yml`. Celery's warm shutdown waits for running tasks to finish on their own — but `run_compositor` never returns voluntarily while a composition is live. Every rolling deploy of `sink-worker` therefore ends in a SIGKILL against any live composition's task, hitting 3a-i's unrecoverable state. **This means the watchdog in 3a-ii isn't just an OOM/eviction safety net — it's required for this codebase's own normal deploy process to not silently kill live broadcasts on every release**, which raises this fix's priority relative to the original draft's framing of it as a rare-edge-case safety net.

**3b. Per-source retry (`retry_add_source`) exhaustion is silent — unchanged finding from the original draft, still valid.** `_SOURCE_RETRY_MAX_ATTEMPTS = 30` at `_SOURCE_RETRY_DELAY_S = 2`s (`tasks.py:43-44`), flat interval. Confirmed: on the final failed attempt, `retry_add_source` just stops rescheduling itself — no second notification. Recommend a distinct `source_retry_exhausted` WS event on the final attempt, reusing `broadcast_group_event`. Flat 60s without backoff is fine at this per-source scale (unlike Option A's now-abandoned whole-pipeline version of the same pattern, where a persistent cause would crash-loop — this per-source version doesn't have that failure mode, since a single failed re-add doesn't spawn additional pipelines).

## Area 4 — The Bus-Polling Loop's Architecture

**Verdict unchanged ("no change to the polling architecture itself") but the reasoning is corrected — the original draft's latency model was wrong.**

The original draft argued the 500ms `bus.timed_pop_filtered` poll (`tasks.py:388-420`) "is not the dominant term" in perceived latency. That's still directionally right, but for a different reason than originally stated: the real risk isn't the 500ms poll interval itself, it's that **`_dispatch_command`'s handlers run inline in the loop**, and (per Area 2) `add_source`'s handler can block for up to 20s on `auth` HTTP calls — during which the bus isn't polled and the output sink isn't serviced at all. Fixing Area 2's JWT-fetch cost (pooled session + short-lived cache) addresses this loop directly, without touching the polling architecture. **Recommend**: fix Area 2 first, then re-measure actual command-dispatch latency before considering any change to the poll interval or moving to an event-driven `bus.add_signal_watch()` — the latter would resurrect the exact `GLib.MainLoop` failure mode `dispatcher.py:1202-1207`'s own comment documents as already having been tried and having deadlocked in this codebase's specific architecture. Shortening the poll interval alone (the original draft's fallback suggestion) is now understood to not address the actual dominant term — dropped as a recommendation.

## Area 5 — Whether Any Part of This Should Move Outside Celery

**The core verdict is unchanged (no process-model rewrite, Celery-as-long-running- task-host is a legitimate pattern) — but the capacity analysis was wrong in the original draft, in a way that directly contradicts Area 3, and the justification for "no rewrite" contained factual errors about Celery's own guarantees.**

`run_compositor` occupies one worker slot for a composition's *entire* live lifetime (`sink-worker.yml:75`, `--concurrency=4`, prefork; `sink-worker.yml:8`, `replicas: 1`). **Corrected finding**: this is not a "4 simultaneous compositions" ceiling as originally stated. `CELERY_TASK_ROUTES` (`settings/common.py:312-314`) routes only `capturer.*` to its own queue — every other task, including `recording.record` (a *second* long-running task per **recorded** composition, `Composition.recording_task_id`, `models.py:62`), `retry_add_source`, and `apply_scene_auto_return`, shares the same 4-slot default queue as `composition.start`/`run_compositor` itself. Real ceiling for recorded compositions is closer to 2, not 4. **More seriously**: at saturation, `retry_add_source` and `apply_scene_auto_return` cannot execute at all — meaning Area 3b's entire per-source retry mechanism, and scene-preset auto-return (an on-air-visible feature), silently stop working platform-wide the moment the queue saturates, not just for a hypothetical 5th composition. Area 3a-ii's watchdog-dispatched `composition.start` would land on this same starved queue too — the recovery mechanism this spec proposes would be unable to run under precisely the load conditions that make it most needed. This is a direct contradiction between the original Area 3 and Area 5 that the first draft never reconciled.

**Corrected Celery justification** (the original draft's "no rewrite needed" reasoning cited two things that turned out to be wrong, though the conclusion itself still holds): Celery's prefork pool does **not** provide re-execution of a killed task by default — `-Ofair` (cited in the original draft) is a prefetch-fairness profile, not crash recycling, and isn't even configured here; without `task_acks_late` (absent from settings), an acked-then-killed task is simply lost, not retried. And "k8s pod lifecycle integration" is, per 3a-iv, actively a gap right now (no graceful-shutdown handling for a task that never returns on its own), not a strength. **The conclusion survives anyway**: none of this argues for a bespoke process manager instead of Celery — it argues for fixing the specific gaps (task-loss-on-kill, no graceful shutdown) within the existing Celery/k8s setup, which Area 3a-ii/3a-iv already cover. A rewrite of the process model itself would still have to solve the exact same problems from scratch.

**Recommendation, corrected**: (1) resolve the Area-3/Area-5 contradiction before anything else — either give `run_compositor`/`composition.start` and the recovery watchdog their own dedicated queue/worker (mirroring how `capturer.*` already gets one), so a saturated default queue can't starve the very mechanisms meant to recover from failure, or raise capacity with real load data once it exists; (2) drop the original draft's proposal to add a new `Composition.status` distinction for "queued vs. running" — Spec Revision correctly noted this is already fully derivable from existing fields (`compositor_task_id is None` + `phase`/`target_phase` set + `output_live is False` already means "queued, not yet started") — the gap is only that nothing surfaces it in the UI, which is a much smaller fix than a new field. Real target concurrency numbers still need production load data — flagged as an open question, not resolved here.

## Area 6 — Whether Hot Paths Warrant a Rust/C Rewrite

**No rewrite justified anywhere in this pipeline's own code — conclusion unchanged, confirmed correct in Spec Revision, one completeness gap fixed.**

`PyGObject` is bindings; the actual encode/decode/composite loop runs entirely inside native GStreamer with zero Python per-frame. Python runs at exactly four points in this codebase, not three as the original draft stated — Spec Revision caught the omission: pipeline construction (one-time per branch attach), bus message handling (event-driven, not per-frame), command dispatch (human-interaction-rate, not per-frame), and `_make_raw_queue_overrun_logger`'s `_on_overrun` callback (`tasks.py:299-334`), which its own docstring acknowledges "could fire at up to the encoder's own frame rate" since it runs on a GStreamer streaming thread — still not a rewrite justification (rate-limited, trivial per-call work), but a claim of exhaustiveness should actually be exhaustive. Confirmed independently that FADE transitions run through native `GstController.InterpolationControlSource` (`dispatcher.py:941-947`), not a per-frame Python ramp — reinforces the conclusion rather than weakening it. `sink-worker-local`'s own observed CPU usage (~2 cores per composition, almost entirely `x264enc` under `ultrafast`/`zerolatency`) is additional evidence Python isn't the cost center here.

## Area 7 — Encoder/Pipeline Construction Settings and Resource Limits

**7a. `tune=zerolatency`/`speed-preset=ultrafast` (`dispatcher.py:710-711`) — no change recommended, conclusion unchanged.** These are the correct choices for a live, low-latency composited broadcast, and the `key-int-max` fix already shipped in PR #126 addresses the main latency lever available here. One caveat added in Spec Revision: the "`key-int-max=5` = 200ms at 25fps" equivalence assumes the compositor actually negotiates 25fps output — the only literal `framerate=25/1` caps in the file are on the *placeholder* branch specifically (`dispatcher.py:825`), not a pipeline-wide pin confirmed elsewhere. Moved to Assumptions below rather than stated as settled.

**7b. Resource limits are already tight under today's single-composition load.** `sink-worker`: requests 500m/limits 6 CPU, `replicas: 1` (`sink-worker.yml:118-124`). `sink-capturer-worker`: requests 500m/limits 4 CPU, `replicas: 1` (`sink-capturer-worker.yml:137-143`). This session's own `kubectl top` snapshot showed `sink-capturer-worker-local` at ~3.9 cores (pinned against its 4-core limit from a single interactive overlay) and `sink-worker-local` at ~2 cores for one composition. At `--concurrency=4`, four concurrent compositions at ~2 cores each would want ~8 cores against a 6-core pod limit — real cgroup throttling risk under concurrent load, independent of whether it explains this specific incident (see "Open Questions" — Spec Revision surfaced two competing, better-evidenced hypotheses for the incident itself, so this is now presented as a real, separately-worth-fixing resource concern, not the leading incident theory).

## Assumptions

- The live incident logs are representative of a real, recurring failure mode, not a one-off fluke — not yet reproduced under controlled conditions.
- Area 1's DB-query findings are from static code reading, not a live query-count profile (see "Investigation Methodology").
- No production load data exists for how many compositions are actually concurrently live in practice — Area 5's queue-contention finding is directionally real (routing math is exact) but its downstream sizing recommendation is not.
- **New, made explicit per Spec Revision** (previously used implicitly, never stated): (a) the `auth` service is assumed fast/available on the pipeline's hot path — Area 2 shows this assumption is exactly what's untested and possibly false; (b) no other default-queue Celery task competes for a worker slot at the moment a composition needs one — shown false in Area 5 (`recording.record` alone violates it); (c) `compositor_task_id` reliably reflects pipeline liveness — shown false in Area 3a-i (it only reflects "did the `finally` block run," not "is anything actually alive").
- `x264enc`'s output framerate is assumed to be pinned at 25fps pipeline-wide (Area 7a) — only directly confirmed for the placeholder branch's own caps.

## Open Questions

1. **What actually caused the 4-branch simultaneous failure? Three competing hypotheses now, not one** (corrected from the original single-hypothesis framing): (a) CPU contention under concurrent-composition load (Area 7b — real resource tightness, but unconfirmed whether multiple compositions were actually live at the incident timestamp); (b) `auth`-service latency/failure causing a silently unauthenticated `rtspsrc` that MediaMTX then rejects (Area 2 — matches the exact logged error text, "Could not write to resource," more precisely than (a) does); (c) the isolation mechanism's own rapid pad churn (4 `remove_source`/`remove_ overlay` calls releasing compositor/audiomixer request pads within the same second, immediately followed by `retry_add_source` re-adding one) destabilizing the shared aggregator/encoder — a known GStreamer failure class for rapid pad add/remove on a live aggregator. Needs either a controlled reproduction or historical correlation (was `auth` slow/erroring at `14:44:47`? were multiple compositions live?) before Plan commits resources to any one of these specifically — Area 2's fix is worth doing regardless of which hypothesis turns out to be dominant, since it's a real gap either way.
2. **Real target concurrency for `sink-worker`**, and how to resolve the Area 3/Area 5 queue-contention conflict — dedicated queue vs. raised capacity vs. both. No production load data yet to size this.
3. **Does Option B (isolate the x264 encoder) still need to get built?** Deferred pending real-world recurrence data after Area 2's fix (auth latency) and Area 3's fix (unrecoverable-state watchdog + retry-storm removal) ship — if encoder failures stop recurring once those land, B was solving a now-eliminated symptom.
4. Does raising `sink-worker`'s CPU limit (Area 7b), or splitting it into a dedicated queue (Area 5), have downstream node-capacity implications on `dev`/eventual `prod` — this spec only checked local-dev's own manifests.
5. **New**: what's the right heartbeat cadence/field for Area 3a-ii's watchdog (touch `updated_at` every tick vs. every Nth tick vs. a dedicated field) — a Plan- phase sizing question, not resolved here.

## Boundaries (carried forward into Plan/Build)

- **Ask first**: any change to `apps.compositor.services.dispatcher` — per `sink`'s own `CLAUDE.md` Boundaries, this is the live GStreamer pipeline; a regression here is directly, immediately user-visible on a running broadcast. Applies to Areas 1, 2, 3, and 7 above (2, 4, 6 conclude "no dispatcher.py change," but 2's JWT-caching fix and 1's select_related fix still touch this module's call sites).
- **Never**: change `CompositionSource.need_update`'s auto-apply semantics as a side effect of any fix here — unrelated to this spec's scope but adjacent enough to restate; Area 3's watchdog is about the pipeline *process*, not about auto-applying pending source changes.

## Revision Changelog (Spec Revision pass, independent Opus 5 model)

Full independent review completed. Summary of what changed and why — see the body above for the corrected content itself, this section records what was wrong and how severely.

**Blocking findings, all resolved in this revision:**
- Option C's originally-proposed watchdog query (`compositor_task_id=''`) could never match anything (`None`, not `''`) and its "re-publish" action would raise on an already-LIVE composition — redesigned around a liveness-heartbeat signal and a direct `composition.start` dispatch instead (Area 3a-ii).
- A hard-killed worker process leaves a composition **permanently** unrecoverable (manual re-publish silently no-ops; stop-then-republish also blocked) — not previously stated at all; now Area 3a-i, and identified as the actual priority problem this section needs to solve.
- Option A's "mirror `retry_add_source`" design doesn't mechanically work — `composition.start` has no idempotency guard, so the pattern as originally proposed could spawn up to 30 concurrent pipelines for one composition. Option A dropped as primary; corrected watchdog design (3a-ii) adopted instead, reasoning in 3a-ii/J1.
- Area 5's concurrency ceiling was mis-stated (didn't account for `recording.record` sharing the same 4 worker slots) and, more seriously, didn't surface that queue saturation silently disables `retry_add_source`/`apply_scene_auto_return` platform- wide — a direct, previously-unacknowledged contradiction with Area 3's own recovery mechanisms needing that same queue. Corrected in Area 5, flagged as Open Question 2.
- A materially stronger incident hypothesis was completely missing from the original draft: `add_source`'s synchronous, unpooled, uncached `get_object_jwt` HTTP calls to `auth` (up to 20s per source, inside the hot loop), and the silent unauthenticated- `rtspsrc`-on-`None`-JWT behavior that matches the incident's exact logged error text more precisely than the original CPU-contention lead. Area 2's verdict reversed from "no change" to a concrete recommendation; Area 4's latency model corrected to match; Open Question 1 rewritten to present three competing hypotheses instead of one.

**Should-fix findings, resolved:**
- Area 1b's original fix (resolve `active_scene_id` once per loop iteration) would have added 2 queries/second/composition to a currently-zero-query idle loop — corrected to resolve lazily only on a non-empty command drain.
- Area 5's Celery justification cited `-Ofair` and prefork "crash recycling" as existing guarantees; neither is configured/accurate. Conclusion (no process-model rewrite) still holds for a different, corrected reason — the real gaps are task-loss-on-kill and no graceful shutdown, both addressed by Area 3a's fixes rather than requiring a bespoke process manager.
- New finding: routine rolling deploys hit the same unrecoverable state a crash does, since `run_compositor` never returns on its own and there's no SIGTERM/preStop handling — this is the *common* case for Area 3a-i's problem, not an edge case (Area 3a-iv).
- Area 5's proposed new `Composition.status` field was redundant — the queued-vs- running distinction is already fully derivable from existing fields. Dropped in favor of just surfacing what already exists.
- Area 4's "shorten the poll interval" fallback recommendation is no longer recommended — it wouldn't have addressed the actual dominant term (Area 2's blocking HTTP calls running inline in the loop).

**Judgment calls, adopted:**
- Build the corrected Option C (liveness watchdog) as the primary/first mechanism, not Option A — A's mechanical problems (above) make it unsuitable as designed, and C, corrected, covers every termination path A structurally can't reach (a killed worker process, a bare `return` before `try:`, etc.).
- The "defer Option B" reasoning was inconsistently applied in the original draft (the same "might be solving a symptom" argument applies equally to A/C, which were built anyway) — reframed on the actually-defensible distinction: A/C are root-cause- agnostic recovery (justified regardless of what caused any given failure), B is cause-specific (justified only once the cause is confirmed and other fixes don't already eliminate it).

**Minor findings, resolved**: one wrong citation (`force_encoder_keyframe` is `dispatcher.py:958-990`, not `947-962` — corrected everywhere it's cited); Area 1's "already fine" list mischaracterized an `.exists()` check as a save, and `_notify_source_auto_removed`'s copy-group handling as a single query when it's one SELECT plus a per-copy UPDATE loop (both corrected, conclusions unchanged); Area 6's "three Python entry points" corrected to four (the raw-queue overrun logger); Area 7a's 25fps-pipeline-wide assumption moved to Assumptions rather than stated as confirmed; Area 2's "the two 200ms latencies are one shared constant" corrected — they're two independently-set literals that happen to currently match, not one source of truth.

**What Spec Revision confirmed as already correct, unchanged**: the core diagnosis (the encoder is genuinely unattributable by `_source_id_from_error_src`, genuinely falls through to full teardown, genuinely never gets restarted); the stale-docstring catch in `_clear_compositor_task_id`; Area 2's pad-probe-gating reasoning (a prior `GLib.MainLoop` attempt really did deadlock, confirmed against the code's own comment); Area 3c (`_SOURCE_RETRY_LOCK_TTL_S`'s derivation); Area 6's rewrite verdict and its FADE-is-native-GstController confirmation; roughly 20 of 21 file:line citations checked were accurate on the first pass.