[bjowe]

Adds per-line video to the intercom: WHIP publish and WHEP consume of H264/VP8 over SMB, video-source pinning with SSRC whitelisting, and the supporting session/line model fields (videoEnabled, hasVideo, isWhepReceiver, whepSourceSessionId) with pin reconciliation when a publisher leaves.

Closes #341

Companion PR (frontend): Eyevinn/intercom-frontend#684

[birme]

Automated code-reviewer verdict (daily-backlog-pr Phase 3): NEEDS CHANGES

Reviewed the full +2611/−191 diff across 24 files. Substantial, mostly careful WebRTC/SDP work with strong test coverage in several areas — but two real defects should block:

Blocking

• src/api_productions.ts (~:1126) — reply.code(204) lost its .send(). The diff changes reply.code(204).send() → reply.code(204) on the PATCH /session/:sessionId handler (the browser's WebRTC handshake finalize call). The handler is async and now neither calls .send() nor returns a payload, so Fastify never flushes the response and the request hangs until socket timeout. Critical connect path, no test covers it. Restore .send() (and note 204 isn't in the declared response schema — 200/400/500 only).

High

• WHIP publisher DELETE does not reconcile pinned receivers (src/api_whip.ts ~:328). The /session/:sessionId DELETE correctly does getReceiversPinnedToSession → reconfigureEndpoint (strip stale ssrc-whitelist) → updateSessionVideoPin(..., null), but the WHIP DELETE path (how a publisher actually leaves) only calls clearWhepSourceIfPinned and skips the receiver-whitelist reconciliation. Receivers pinned to the departed publisher keep a stale whitelist → frozen video until renegotiation. The headline "pin reconciliation when a publisher leaves" feature is only wired into one of the two publisher-removal paths.

Medium

• Unbounded pinnedSessionId strings on new SetLineWhepSourceRequest/SetSessionVideoSourceRequest schemas (no maxLength) — consistency/known-concern class here (low exploitability: used only as CouchDB _id lookup).
• VideoSmbPayloadParameters widened to Type.Record(Type.String(), Type.String()) — removes key validation; add a comment / value-length bound.
• GET /session/:sessionId/name has no schema block at all (params or response) — inconsistent with every other route; new unauthenticated read surface.

Nits

• No tests for the 204 answer path (would have caught the blocker), PATCH .../whep-source, or GET .../name.
• any typing where SfuVideoStream exists; positional-boolean growth in allocateEndpoint/createEndpoint.

Good: correct ISmbProtocol usage, the transport-media find-by-fingerprint fix, keyframe cycle, NaN feedback guard, and codec-overlap rejection are solid and tested. Fix the blocker + WHIP-delete gap and this is close.

[birme]

This PR looks like part of a multi-repo/multi-PR dependency stack together with Eyevinn/intercom-frontend#684 (frontend video support). Holding off on automated review/merge here — this needs a human (or the relevant implementation agent) to assess the whole stack together, not a per-PR pass.

[bjowe]

Thanks — #341 covered the two items you flagged as blocking. Pushed a small follow-up for the
rest of the review:

• PATCH /session/:sessionId returns 410 when the session is gone, but the schema declared
  204/400/500. Added 410.
• GET /session/:sessionId/name had no schema block. Added params + 200/404.
• pinnedSessionId was unbounded on SetLineWhepSourceRequest and
  SetSessionVideoSourceRequest. Bounded to 200, matching the WHIP param schemas.
• VideoSmbPayloadParameters was an unbounded record. The keys are codec-specific
  (x-google-* for VP8, profile-level-id / packetization-mode for H264) so they can't be
  enumerated — it stays a record, with bounded keys and values and a comment saying why.
• Tests for the 410 path, GET .../name, and the WHIP delete reconciliation from feat: add video support for intercom lines (WHIP/WHEP, H264) #341, which
  shipped without any.

npm run typecheck clean, npm run lint 0 errors, npm test 290/290.

[birme]

Code Review — video support (WHIP/WHEP, H264) — Needs Changes

Reviewed by daily-backlog-pr (Phase 3). Substantial, well-documented SDP/WebRTC work with strong test coverage; the video path is cleanly gated behind videoEnabled/offerHasVideo so audio-only lines are largely unaffected. One blocking regression plus a few warnings before merge. CI (lint/pretty/ts) is green — these are correctness findings, not formatting.

Blocking

• src/api_productions.ts:521 and :1297 — reintroduces the documented Sprint-5 isActive regression. Both participant maps now emit isActive: s.isWhip ? true : Boolean(s.isActive). backend-rules records that this override was removed and the correct pattern is !!s.isActive / Boolean(s.isActive) ("With MongoDB, WHIP/WHEP sessions never showed as inactive because of this override — this is now corrected."). main currently has the fixed form. Worse, this PR sets isWhip=true for WHEP receivers too (api_whep.ts createUserSession(..., true /*isWhip*/, true /*isWhepReceiver*/)), so both WHIP publishers and WHEP receivers would be reported permanently active and never age out. toUserResponse (:45-48) and production_manager.ts:589 correctly use Boolean(...), so these two sites look like an accidental revert. Use isActive: Boolean(s.isActive); if always-active is genuinely intended for video sources, reconcile it with backend-rules and apply it consistently.

Warnings

• src/api_productions.ts:1232 and src/api_whip.ts:380 — pin-reconciliation wraps getReceiversPinnedToSession + reconfigureEndpoint in an empty catch {}. An SMB reconfigure failure leaves a stale ssrc-whitelist (video freezes) with no log. Not blocking the delete is fine, but at least Log().warn(err).
• src/smb.ts (sendEndpointAction → reconfigureEndpoint/requestKeyframe) — new SMB REST calls have no request timeout (only getConferencesWithUsers is timed). requestKeyframe issues two sequential reconfigure PUTs; if SMB is unreachable these can block indefinitely. Add an AbortSignal.timeout as elsewhere.
• Unjustified any on SMB payload fields — sourceVideo: any, streams: any[], subscribeToVideo.streams: any[] (api_whep.ts:147,161,163, api_productions.ts:1083, api_productions_core_functions.ts:163). Shapes are known (SfuVideoStream[]); prefer typing against SmbEndpointDescription['video'].

Suggestions

• parseInt(ssrcsSplit[0/1]) at api_productions_core_functions.ts:251,256,260 omit radix 10 (inconsistent with the rest of the file).
• production_manager.ts:335-340,352 — (s as any)._id/pinnedVideoSessionId casts can drop now that pinnedVideoSessionId is on the model.
• connection.ts:245 — RTX PT advertised without a per-SSRC a=ssrc-group:FID; confirm with domain knowledge that SMB-originated offers don't need the FID group.
• Good hardening noted: the media.rtp?.[0] guard (:229-234) and the transportMedia find-by-fingerprint||iceUfrag fix are genuine correctness improvements.

Cross-repo coupling (with intercom-frontend #684)

This changes the wire contract: PATCH /session/:sessionId now 204 (was 200: string); new required hasVideo field + optional isWhepReceiver/videoEnabled/whepSourceSessionId; new routes (whep-source, video-source with 409/425 semantics, GET /session/:id/name); PT 96/97 normalization and H264 profile-level-id forcing. Merge as a coordinated pair with #684 and smoke-test that an existing audio-only production still negotiates.

Moving the board item back to Ready for these changes.

[birme]

Code Review

Verdict: LGTM

Summary: A large but well-structured backend video-support changeset. SDP work correctly uses sdp-transform with .find()-by-type media access and null guards, DTLS setup is negotiated correctly (and now hard-fails on a missing fingerprint), the new SMB reconfigure/requestKeyframe calls carry an AbortSignal.timeout, DB work stays behind the withRetry-wrapped driver, ISmbProtocol is used everywhere in production code, and the bug fixes ship with regression tests (204 flush, 410, hasVideo deferral, codec negotiation, pin reconciliation). Two non-blocking gaps remain (one untested new route, one silent catch). No Blocking items.

Blocking

None.

Warnings

• src/api_productions.ts:677 — The new PATCH /production/:productionId/line/:lineId/whep-source route has no functional server.inject() test. Only api_auth_guard.test.ts exercises it (and only to confirm the API-key guard). Its non-trivial branches — 404 (production not found), 404 (line not found), 500 (setLineWhepSource returns falsy), and the no-op fast path in production_manager.ts:352 — are all unverified. Every sibling route in this PR (/video-source, /session/:id/name) got dedicated tests; this one should too.
• src/api_productions.ts:1173 — The auto-pin resolution block in PATCH /session/:sessionId swallows every error with a bare catch {} and only a comment, with no log. The equivalent WHEP pin-resolution catch (src/api_whep.ts:222) logs Log().warn(...). Since this PR exists largely to fix a "pinning sometimes doesn't work" class of bug, silently discarding the failure removes the one diagnostic that would explain a receiver that comes up on default rotation. Add a Log().warn like the WHEP path.

Suggestions

• src/production_manager.ts:553 and :571 — { hasVideo } as any / { pinnedVideoSessionId } as any; both fields are now declared on the UserSession model, so these casts are likely removable (only smbPresenceKey needs the escape hatch).
• src/api_productions.ts:1295 and src/api_whip.ts:393 — (receiver as any)._id?.toString?.(); consistent with existing debt but new occurrences — a typed _id accessor would be cleaner.
• src/api_productions_core_functions.ts:259, :802, :836 — offer.media[0] / parsedAnswer.media[0] index access persists (259 is a documented last-resort fallback; 802/836 read index 0 while labelled "audio media description"). While this function is being heavily edited, consider aligning with the .find(m => m.type === 'audio') invariant.

Domain Note

This change is squarely in WHIP/WHEP session lifecycle, video source pinning (SSRC whitelisting), dominant/pinned-source selection, and keyframe (PLI) solicitation. Consider consulting the intercom-expert agent to validate: the empty-vs-absent ssrc-whitelist semantics, the whitelist remove→re-add keyframe trick in smb.ts:requestKeyframe, and the ssrc-rewrite relay choice for WHIP publishers.

Cross-repo frontend contract assumptions (companion PR intercom-frontend#684) this backend relies on — verify they hold on the frontend side: the 425 "retry shortly" handshake for per-session video-source pin and WHEP creation; the new response fields isWhepReceiver / hasVideo / videoEnabled / whepSourceSessionId (where null means "pin cleared", distinct from absent); and the "exactly one active WHIP publisher with video" auto-pin stopgap that depends on the frontend's per-session pin eventually landing.

---

Automated review by the daily-backlog-pr pipeline (code-reviewer).

Merge held (not a review blocker): This PR is the backend half of a cross-repo video-support stack together with intercom-frontend#684, which currently has a Needs-Changes verdict (a cluster of test/a11y warnings). Both PRs are being actively iterated by the author today. The pipeline is not auto-merging this half independently — a multi-PR dependency stack needs the author/a human to coordinate the joint landing, not a mechanical per-PR merge. This approval records that the backend changeset itself passed review and is ready whenever the stack is landed together.