Adds lightweight in-memory tracking for the direct streaming paths that do not create IFFmpegSegmenterService workers:
MPEG-TS direct / legacy (StreamingMode.TransportStream, ts-legacy)
HLS Direct (StreamingMode.HttpLiveStreamingDirect)
The new IDirectStreamSessionTracker exposes the seam future channel-state wiring can use to count active direct viewers by channel. TrackedFileStreamResult registers when MVC actually executes the streaming response and deregisters when response streaming completes, fails, or is aborted.
Root cause
onAir work in #97 can only observe IFFmpegSegmenterService.Workers. That registry is populated by segmenter/hybrid sessions, but the direct response paths stream without registering any worker/session, so direct viewers are invisible even while bytes are flowing.
Deferred
This intentionally does not wire GET /api/channels/state; that handler lives on the #97 SPA stack branch. Follow-up wiring should combine IFFmpegSegmenterService.IsActive(channelNumber) with IDirectStreamSessionTracker.IsActive(channelNumber) / viewer counts.
Verification
dotnet build ErsatzTV.sln
TZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj -> 326 passed
TZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj -> 492 passed, 1 skipped
## Summary
Adds lightweight in-memory tracking for the direct streaming paths that do not create `IFFmpegSegmenterService` workers:
- MPEG-TS direct / legacy (`StreamingMode.TransportStream`, `ts-legacy`)
- HLS Direct (`StreamingMode.HttpLiveStreamingDirect`)
The new `IDirectStreamSessionTracker` exposes the seam future channel-state wiring can use to count active direct viewers by channel. `TrackedFileStreamResult` registers when MVC actually executes the streaming response and deregisters when response streaming completes, fails, or is aborted.
## Root cause
`onAir` work in #97 can only observe `IFFmpegSegmenterService.Workers`. That registry is populated by segmenter/hybrid sessions, but the direct response paths stream without registering any worker/session, so direct viewers are invisible even while bytes are flowing.
## Deferred
This intentionally does not wire `GET /api/channels/state`; that handler lives on the #97 SPA stack branch. Follow-up wiring should combine `IFFmpegSegmenterService.IsActive(channelNumber)` with `IDirectStreamSessionTracker.IsActive(channelNumber)` / viewer counts.
## Verification
- `dotnet build ErsatzTV.sln`
- `TZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj` -> 326 passed
- `TZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj` -> 492 passed, 1 skipped
Fable review — verdict: FIX-FIRST (do not merge yet)
Design and scope are right: the seam mirrors the IFFmpegSegmenterService idiom, singleton DI is appropriate, TrackedFileStreamResult correctly spans the real response lifetime (dispose verified for normal completion, IOException, and client abort — MVC's FileResultExecutor swallows the cancellation), no double-counting (internal ts-legacy concat fetch untracked; hybrid/segmenter modes untouched), no public API surface change, and no conflict with PR #120 (Startup.cs hunks are disjoint). Tests genuinely exercise the risky paths.
Two SUBSTANTIAL findings go back to Codex per the handoff protocol (no fixes applied here):
1. Register/Remove race orphans sessions.DirectStreamSessionTracker.Register does _sessions.GetOrAdd(...) then channelSessions.TryAdd(...) (two steps). Concurrent Remove of the channel's last session sees IsEmpty and prunes the inner dict from _sessions between those steps — the new session lands in an orphaned dict and is invisible to IsActive/GetViewerCount/GetActiveSessions for its whole stream (wrong onAir=false, possibly for hours; likely under channel-zapping churn). Simplest fix: drop the empty-dict pruning (bounded by channel count); alternative: verify-and-retry in Register. Needs a test.
2. HEAD probes create phantom viewers on ts-legacy.[HttpHead] on iptv/channel/{n}.ts (+hdhr) executes the action and now registers a session until the discarded copy aborts — transiently flipping onAir/viewer count for a non-viewer (Jellyfin probes with HEAD). Fix: skip tracking when HttpMethods.IsHead(Request.Method) (also consider HLS-Direct). One test.
NITS: none — naming, formatting, and assertions are fine as-is.
Neither finding invalidates the interface, so the deferred #97onAir wiring is unaffected. A follow-up Codex prompt with these findings verbatim has been handed to the user.
## Fable review — verdict: FIX-FIRST (do not merge yet)
Design and scope are right: the seam mirrors the `IFFmpegSegmenterService` idiom, singleton DI is appropriate, `TrackedFileStreamResult` correctly spans the real response lifetime (dispose verified for normal completion, IOException, and client abort — MVC's FileResultExecutor swallows the cancellation), no double-counting (internal ts-legacy concat fetch untracked; hybrid/segmenter modes untouched), no public API surface change, and **no conflict with PR #120** (Startup.cs hunks are disjoint). Tests genuinely exercise the risky paths.
Two SUBSTANTIAL findings go back to Codex per the handoff protocol (no fixes applied here):
**1. Register/Remove race orphans sessions.** `DirectStreamSessionTracker.Register` does `_sessions.GetOrAdd(...)` then `channelSessions.TryAdd(...)` (two steps). Concurrent `Remove` of the channel's last session sees `IsEmpty` and prunes the inner dict from `_sessions` between those steps — the new session lands in an orphaned dict and is invisible to `IsActive`/`GetViewerCount`/`GetActiveSessions` for its whole stream (wrong `onAir=false`, possibly for hours; likely under channel-zapping churn). Simplest fix: drop the empty-dict pruning (bounded by channel count); alternative: verify-and-retry in Register. Needs a test.
**2. HEAD probes create phantom viewers on ts-legacy.** `[HttpHead]` on `iptv/channel/{n}.ts` (+hdhr) executes the action and now registers a session until the discarded copy aborts — transiently flipping `onAir`/viewer count for a non-viewer (Jellyfin probes with HEAD). Fix: skip tracking when `HttpMethods.IsHead(Request.Method)` (also consider HLS-Direct). One test.
NITS: none — naming, formatting, and assertions are fine as-is.
Neither finding invalidates the interface, so the deferred #97 `onAir` wiring is unaffected. A follow-up Codex prompt with these findings verbatim has been handed to the user.
Register/remove race: I will use the simple bounded-memory fix from the review: stop pruning empty per-channel dictionaries in DirectStreamSessionTracker.Remove. Channel keys are bounded by channel count, and leaving the dictionary installed makes the existing GetOrAdd + TryAdd registration path safe from the orphaned-dictionary interleaving. I will add a deterministic regression test with a tracker hook/seam that simulates a concurrent last-session removal between GetOrAdd and TryAdd.
HEAD phantom viewers: I will suppress tracking inside TrackedFileStreamResult.ExecuteResultAsync when HttpMethods.IsHead(context.HttpContext.Request.Method) is true. That centralizes the behavior for both direct MPEG-TS and HLS-direct uses of the result wrapper, without changing the existing action/process behavior or any public API. I will add a result test asserting HEAD execution does not register an active direct session.
Round 2 fix approach before coding:
1. Register/remove race: I will use the simple bounded-memory fix from the review: stop pruning empty per-channel dictionaries in `DirectStreamSessionTracker.Remove`. Channel keys are bounded by channel count, and leaving the dictionary installed makes the existing `GetOrAdd` + `TryAdd` registration path safe from the orphaned-dictionary interleaving. I will add a deterministic regression test with a tracker hook/seam that simulates a concurrent last-session removal between `GetOrAdd` and `TryAdd`.
2. HEAD phantom viewers: I will suppress tracking inside `TrackedFileStreamResult.ExecuteResultAsync` when `HttpMethods.IsHead(context.HttpContext.Request.Method)` is true. That centralizes the behavior for both direct MPEG-TS and HLS-direct uses of the result wrapper, without changing the existing action/process behavior or any public API. I will add a result test asserting HEAD execution does not register an active direct session.
Fixed the register/remove race by removing empty per-channel dictionary pruning from DirectStreamSessionTracker.Remove; this prevents Register from adding to an orphaned dictionary after concurrent last-session disposal. Added deterministic regression coverage via a protected registration hook in a test subclass.
Fixed HEAD phantom viewers by bypassing direct-session tracking inside TrackedFileStreamResult.ExecuteResultAsync when HttpMethods.IsHead(...) is true. Added a test asserting HEAD execution makes no tracker Register call.
Rebased onto current origin/main before the fixes. Local verification is green:
dotnet build ErsatzTV.sln -> passed, 0 errors
TZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj -> 368 passed, 0 failed
TZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj -> 493 passed, 1 skipped
Round 2 fixes pushed in `63508451`:
- Fixed the register/remove race by removing empty per-channel dictionary pruning from `DirectStreamSessionTracker.Remove`; this prevents `Register` from adding to an orphaned dictionary after concurrent last-session disposal. Added deterministic regression coverage via a protected registration hook in a test subclass.
- Fixed HEAD phantom viewers by bypassing direct-session tracking inside `TrackedFileStreamResult.ExecuteResultAsync` when `HttpMethods.IsHead(...)` is true. Added a test asserting HEAD execution makes no tracker `Register` call.
Rebased onto current `origin/main` before the fixes. Local verification is green:
- `dotnet build ErsatzTV.sln` -> passed, 0 errors
- `TZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj` -> 368 passed, 0 failed
- `TZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj` -> 493 passed, 1 skipped
Round 2 is ready for re-review. Final head is f9bb2306730845c712be2d160f0ec151c4345c08.
Fixes since a964286adcfca6820a7c51c3355193df6d7f0fbf:
Removed empty per-channel dictionary pruning from DirectStreamSessionTracker.Remove so concurrent register/remove cannot orphan a newly registered direct session. Added deterministic regression coverage in DirectStreamSessionTrackerTests.
Added HEAD bypass in TrackedFileStreamResult.ExecuteResultAsync so HEAD probes do not call IDirectStreamSessionTracker.Register. Added regression coverage in TrackedFileStreamResultTests.
Rebased onto current origin/main; added an empty chore: retrigger PR checks commit after run 222 hit a runner checkout-action cache failure before repo checkout. Run 223 is green.
Verification:
Local dotnet build ErsatzTV.sln: passed, 0 errors
Local TZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj: 368 passed, 0 failed
Local TZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj: 493 passed, 1 skipped
CI run 223 on f9bb2306730845c712be2d160f0ec151c4345c08: Build & test (.NET) green; EF migration integrity (SQLite + MySql) green
Round 2 is ready for re-review. Final head is `f9bb2306730845c712be2d160f0ec151c4345c08`.
Fixes since `a964286adcfca6820a7c51c3355193df6d7f0fbf`:
- Removed empty per-channel dictionary pruning from `DirectStreamSessionTracker.Remove` so concurrent register/remove cannot orphan a newly registered direct session. Added deterministic regression coverage in `DirectStreamSessionTrackerTests`.
- Added HEAD bypass in `TrackedFileStreamResult.ExecuteResultAsync` so HEAD probes do not call `IDirectStreamSessionTracker.Register`. Added regression coverage in `TrackedFileStreamResultTests`.
- Rebased onto current `origin/main`; added an empty `chore: retrigger PR checks` commit after run 222 hit a runner checkout-action cache failure before repo checkout. Run 223 is green.
Verification:
- Local `dotnet build ErsatzTV.sln`: passed, 0 errors
- Local `TZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj`: 368 passed, 0 failed
- Local `TZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj`: 493 passed, 1 skipped
- CI run 223 on `f9bb2306730845c712be2d160f0ec151c4345c08`: `Build & test (.NET)` green; `EF migration integrity (SQLite + MySql)` green
Fable re-review (delta since a964286a) — verdict: APPROVE
Both round-1 substantial findings are correctly fixed in 63508451:
Race fixed by dropping the empty-dict pruning in Remove (the recommended option — inner dicts are bounded by channel count, and GetOrAdd makes the two-step register race-free without it). The regression test is deterministic, not a stress test: the OnRegisteringSession() seam disposes the channel's last session exactly between GetOrAdd and TryAdd — the precise interleaving that orphaned sessions before — and asserts the new session stays visible.
HEAD fixed in TrackedFileStreamResult.ExecuteResultAsync (covers ts-legacy and HLS-Direct in one place), with a test asserting Register is never called for HEAD.
Also verified: the rebase onto main is byte-identical to the original commit (same patch-id as a964286a — nothing smuggled in), chore: retrigger PR checks is genuinely empty, and CI is green on f9bb2306 (Build & test + EF migration integrity).
Ready to merge pending user consent. After merge, #99 stays open for the final step: combine IDirectStreamSessionTracker.IsActive with IFFmpegSegmenterService.IsActive in GET /api/channels/state (now on main via PR #120).
## Fable re-review (delta since a964286a) — verdict: APPROVE
Both round-1 substantial findings are correctly fixed in 63508451:
1. **Race fixed** by dropping the empty-dict pruning in `Remove` (the recommended option — inner dicts are bounded by channel count, and `GetOrAdd` makes the two-step register race-free without it). The regression test is deterministic, not a stress test: the `OnRegisteringSession()` seam disposes the channel's last session exactly between `GetOrAdd` and `TryAdd` — the precise interleaving that orphaned sessions before — and asserts the new session stays visible.
2. **HEAD fixed** in `TrackedFileStreamResult.ExecuteResultAsync` (covers ts-legacy and HLS-Direct in one place), with a test asserting `Register` is never called for HEAD.
Also verified: the rebase onto main is byte-identical to the original commit (same patch-id as a964286a — nothing smuggled in), `chore: retrigger PR checks` is genuinely empty, and CI is green on f9bb2306 (Build & test + EF migration integrity).
Ready to merge pending user consent. After merge, #99 stays open for the final step: combine `IDirectStreamSessionTracker.IsActive` with `IFFmpegSegmenterService.IsActive` in `GET /api/channels/state` (now on main via PR #120).
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Adds lightweight in-memory tracking for the direct streaming paths that do not create
IFFmpegSegmenterServiceworkers:StreamingMode.TransportStream,ts-legacy)StreamingMode.HttpLiveStreamingDirect)The new
IDirectStreamSessionTrackerexposes the seam future channel-state wiring can use to count active direct viewers by channel.TrackedFileStreamResultregisters when MVC actually executes the streaming response and deregisters when response streaming completes, fails, or is aborted.Root cause
onAirwork in #97 can only observeIFFmpegSegmenterService.Workers. That registry is populated by segmenter/hybrid sessions, but the direct response paths stream without registering any worker/session, so direct viewers are invisible even while bytes are flowing.Deferred
This intentionally does not wire
GET /api/channels/state; that handler lives on the #97 SPA stack branch. Follow-up wiring should combineIFFmpegSegmenterService.IsActive(channelNumber)withIDirectStreamSessionTracker.IsActive(channelNumber)/ viewer counts.Verification
dotnet build ErsatzTV.slnTZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj-> 326 passedTZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj-> 492 passed, 1 skippedFable review — verdict: FIX-FIRST (do not merge yet)
Design and scope are right: the seam mirrors the
IFFmpegSegmenterServiceidiom, singleton DI is appropriate,TrackedFileStreamResultcorrectly spans the real response lifetime (dispose verified for normal completion, IOException, and client abort — MVC's FileResultExecutor swallows the cancellation), no double-counting (internal ts-legacy concat fetch untracked; hybrid/segmenter modes untouched), no public API surface change, and no conflict with PR #120 (Startup.cs hunks are disjoint). Tests genuinely exercise the risky paths.Two SUBSTANTIAL findings go back to Codex per the handoff protocol (no fixes applied here):
1. Register/Remove race orphans sessions.
DirectStreamSessionTracker.Registerdoes_sessions.GetOrAdd(...)thenchannelSessions.TryAdd(...)(two steps). ConcurrentRemoveof the channel's last session seesIsEmptyand prunes the inner dict from_sessionsbetween those steps — the new session lands in an orphaned dict and is invisible toIsActive/GetViewerCount/GetActiveSessionsfor its whole stream (wrongonAir=false, possibly for hours; likely under channel-zapping churn). Simplest fix: drop the empty-dict pruning (bounded by channel count); alternative: verify-and-retry in Register. Needs a test.2. HEAD probes create phantom viewers on ts-legacy.
[HttpHead]oniptv/channel/{n}.ts(+hdhr) executes the action and now registers a session until the discarded copy aborts — transiently flippingonAir/viewer count for a non-viewer (Jellyfin probes with HEAD). Fix: skip tracking whenHttpMethods.IsHead(Request.Method)(also consider HLS-Direct). One test.NITS: none — naming, formatting, and assertions are fine as-is.
Neither finding invalidates the interface, so the deferred #97
onAirwiring is unaffected. A follow-up Codex prompt with these findings verbatim has been handed to the user.Round 2 fix approach before coding:
Register/remove race: I will use the simple bounded-memory fix from the review: stop pruning empty per-channel dictionaries in
DirectStreamSessionTracker.Remove. Channel keys are bounded by channel count, and leaving the dictionary installed makes the existingGetOrAdd+TryAddregistration path safe from the orphaned-dictionary interleaving. I will add a deterministic regression test with a tracker hook/seam that simulates a concurrent last-session removal betweenGetOrAddandTryAdd.HEAD phantom viewers: I will suppress tracking inside
TrackedFileStreamResult.ExecuteResultAsyncwhenHttpMethods.IsHead(context.HttpContext.Request.Method)is true. That centralizes the behavior for both direct MPEG-TS and HLS-direct uses of the result wrapper, without changing the existing action/process behavior or any public API. I will add a result test asserting HEAD execution does not register an active direct session.a964286adcto6350845101Round 2 fixes pushed in
63508451:DirectStreamSessionTracker.Remove; this preventsRegisterfrom adding to an orphaned dictionary after concurrent last-session disposal. Added deterministic regression coverage via a protected registration hook in a test subclass.TrackedFileStreamResult.ExecuteResultAsyncwhenHttpMethods.IsHead(...)is true. Added a test asserting HEAD execution makes no trackerRegistercall.Rebased onto current
origin/mainbefore the fixes. Local verification is green:dotnet build ErsatzTV.sln-> passed, 0 errorsTZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj-> 368 passed, 0 failedTZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj-> 493 passed, 1 skippedRound 2 is ready for re-review. Final head is
f9bb2306730845c712be2d160f0ec151c4345c08.Fixes since
a964286adcfca6820a7c51c3355193df6d7f0fbf:DirectStreamSessionTracker.Removeso concurrent register/remove cannot orphan a newly registered direct session. Added deterministic regression coverage inDirectStreamSessionTrackerTests.TrackedFileStreamResult.ExecuteResultAsyncso HEAD probes do not callIDirectStreamSessionTracker.Register. Added regression coverage inTrackedFileStreamResultTests.origin/main; added an emptychore: retrigger PR checkscommit after run 222 hit a runner checkout-action cache failure before repo checkout. Run 223 is green.Verification:
dotnet build ErsatzTV.sln: passed, 0 errorsTZ=UTC dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj: 368 passed, 0 failedTZ=UTC dotnet test ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj: 493 passed, 1 skippedf9bb2306730845c712be2d160f0ec151c4345c08:Build & test (.NET)green;EF migration integrity (SQLite + MySql)greenFable re-review (delta since
a964286a) — verdict: APPROVEBoth round-1 substantial findings are correctly fixed in
63508451:Remove(the recommended option — inner dicts are bounded by channel count, andGetOrAddmakes the two-step register race-free without it). The regression test is deterministic, not a stress test: theOnRegisteringSession()seam disposes the channel's last session exactly betweenGetOrAddandTryAdd— the precise interleaving that orphaned sessions before — and asserts the new session stays visible.TrackedFileStreamResult.ExecuteResultAsync(covers ts-legacy and HLS-Direct in one place), with a test assertingRegisteris never called for HEAD.Also verified: the rebase onto main is byte-identical to the original commit (same patch-id as
a964286a— nothing smuggled in),chore: retrigger PR checksis genuinely empty, and CI is green onf9bb2306(Build & test + EF migration integrity).Ready to merge pending user consent. After merge, #99 stays open for the final step: combine
IDirectStreamSessionTracker.IsActivewithIFFmpegSegmenterService.IsActiveinGET /api/channels/state(now on main via PR #120).