fix(recorder): retry an EOS the stalled track refused

[wagenet]

Important

This PR can probably be closed. Note added by Claude (Claude Code), on behalf of @wagenet, 2026-10-08.

Live Recorder (builtin.liverecorder, #1034, merged 2026-10-08) is set to replace this block, and it does not have the problem this PR fixes: it has no watchdog that ends a stalled track, so there is no EOS to be refused and nothing waits on one.

This PR stays open, in draft, only as a fallback until Live Recorder has run in a real production. It needs no review in the meantime.

Depends on #927, and carries its commits until it merges. Review only the last commit, fix(recorder): retry an EOS the stalled track refused.

Refs #757

Problem

When the recorder's watchdog ends a stalled track, #927 parks the input: it unlinks it from the parser, installs the park probe, and sends the EOS to the old parser, ignoring the result. If the parser refuses the EOS (flushing, inactive, out of its parent), or the input has no parser, no EOS reaches the track's splitmuxsink pad. The muxer waits on that pad, so the recording stays frozen. The track is already marked ended, and the watchdog only looks at tracks that are still recording, so it never tries again.

Change

All in backend/src/blocks/builtin/recorder.rs.

• A second route for the EOS. If the parser does not take the watchdog's EOS, it goes to the track's splitmuxsink sink pad, the one the muxer waits on. Nothing pushes into that pad, because the watchdog picks only a track whose queue has drained. A pad that already holds an EOS counts as delivered. The track stays ended either way, so fix(recorder): record a stalled track again once it resumes #927's new-file resume still applies.
• Retry until taken. An EOS no route took is kept as eos_pending on the track. StallCheck::poll re-sends it to the muxer pad on every poll; the refusal is logged once.
• File switches keep the parser-only route. The three parks in the file-switch path may have data still queued, and an EOS sent past the queue would overtake it. A file switch also clears any pending EOS when it hands the track's pad back.

Rejected: pushing the EOS out of the input pad, and reopening the track while its input carries no EOS. #927 parks the input instead, so the input never carries an EOS and there is nothing to reopen.

Evidence

• A refused EOS no longer freezes the recording: a_stalled_track_whose_chain_refuses_the_eos_still_ends (recorder_stall_test.rs). Against fix(recorder): record a stalled track again once it resumes #927's recorder.rs alone it fails with the recording stayed frozen on a track whose chain refused the EOS: 0 video buffers reached the muxer within 20 s. It asserts that the watchdog did not end the track before the refusal was set up, so it cannot pass by accident.
• The muxer-pad route: an_eos_the_chain_refuses_goes_to_the_muxer_pad and an_input_with_no_chain_still_ends_the_muxer_pad fail when the route is removed. The integration test still passes in that case, because the retry recovers one poll later.
• The retry: an_eos_nothing_took_is_retried_until_the_muxer_pad_takes_it fails when the retry is disabled.
• A rejoining guest whose audio and video return apart records the same as on fix(recorder): record a stalled track again once it resumes #927 alone, across five timings (scratch test, not committed).

Tests

Ran on macOS, GStreamer 1.28.6:

• cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings: clean.
• cargo test --workspace --no-fail-fast --test recorder_test --test recorder_stall_test: 30 and 4 passed, 0 failed.
• cargo test --workspace --lib: 908 passed, 0 failed, 5 ignored (all pre-existing), plus 28 and 101 in the other crates.

Not run: the full cargo test --workspace (the other integration tests do not touch the recorder), the WHIP rig, and GStreamer 1.24.2 as on CI.

Out of scope

Recovery still takes as long as the watchdog's stall rule from #757: a track is ended only once backpressure reaches every input.

🤖 Generated with Claude Code

[srperens]

Noted as a draft, so I am holding off on a full review until it is marked ready for
review. If you want one before then, say so here or request me as a reviewer.

[srperens]

Verdict: Request changes — the refusal-and-retry mechanism is right and its tests really executed in CI, but one behaviour added on top of it (retiring a track whose muxer pad already carries EOS) rests on a claim the code contradicts, and the test named as its guard would pass without it.

Requested changes

1. backend/src/blocks/builtin/recorder.rs:414-426: either name the case where a finished track really is retried every poll — and make a_track_that_finished_does_not_hide_a_later_stall fail when only that pre-check is reverted — or drop the pre-check and say in the body that it saves one poll and one warn line rather than fixing a hang.
2. If it stays, say what it does on a healthy recording: it sets retired outside a freeze, arming add_retired_input_probe on the block's input sink pad with nothing to clear it. main armed that probe only from end_stalled_track, and no test covers this path.

Claims

Claim	Verdict	Evidence
A finished track "stays the furthest behind for ever ... every poll retries it"	CONTRADICTED	backend/src/blocks/builtin/recorder.rs:418 — ` // refuse a pad that already carries EOS, so every poll retries it and the`. Its EOS reached the muxer pad through the block input, so that identity's src pad holds the sticky EOS; input_clean is false at recorder.rs:293 — ` if !delivered && input_clean {` — so retired stays set and recorder.rs:406 — ` if track.activity.retired.load(Ordering::Relaxed) {` skips it next poll. One poll, not for ever
Reverting the pre-check makes a_track_that_finished_does_not_hide_a_later_stall record no video	CONTRADICTED	backend/tests/recorder_stalled_track_test.rs:530 — `/// seconds after that. Reverting the fix pins the counted video at zero: the`. By the row above, audio 0 self-retires on its first pick and audio 1 is ended on the next poll, 500 ms later; the test measures a 5 s window after 20 s
The new tests ran rather than skipped	CONFIRMED	Check (Linux) job 107216232548: recorder_stalled_track_test "5 passed", including a_stalled_track_whose_branch_is_coming_apart_still_ends, plus the four new unit tests. .github/workflows/ci.yml:146 — ` STROM_REQUIRE_GST_PLUGINS: 1` turns a missing-element skip into a failure

Diagnosis — Root cause matches: the old retired.swap(true) retired before knowing the push landed, so one refusal stranded the track. Reporting delivery, reopening only while the input is clean, and the splitmuxsink-pad fallback all address it at the right layer. Coverage is BOUNDED: a refusal leaving the sticky EOS on the input whose muxer pad also refuses still hangs — the body says so, and #804 owns it. send_event on the muxer pad is EXTERNAL: it cannot block the watchdog while only a Some(0) queue is ever ended.

Radius — LOCAL: end_stalled_track has no caller outside watch_tracks and the tests, and no contract surface moves. Behaviour changes beyond the reporter's case for any recorder with 2+ tracks where one ends with a real EOS: a played-out file, or a demuxer dropping a stream. Media Player cannot reach it: backend/src/blocks/builtin/mediaplayer/bridge.rs:506 — `/// EOS triggers advancing to the next file instead of propagating downstream.`

Overlaps — none of the three kinds. #804 is adjacent only: it owns bounding the join in pipeline/lifecycle.rs, which this PR does not touch. No open PR changes recorder.rs.

Tests & CI — Nine checks green at 4a87cbc; macOS/Windows skipped and nothing here is platform-specific. recorder.rs is now 2091 lines; CLAUDE.md asks for a split suggestion in this range, and the watchdog is a clean seam.

Confidence: HIGH

[wagenet]

Claude here, on behalf of @wagenet.

Both points are right. Since the input-clean reopen rule went in, a finished track's input carries its own EOS, so the first refusal leaves it closed and the next poll ends the stalled track. The pre-check saved one poll, and it armed the drop probe on healthy recordings.

92d0181 removes it. The one poll that picks a finished track now logs at debug instead of warning that the recording stays stalled.

Checked on 92d0181:

• a_track_that_finished_does_not_hide_a_later_stall passes without the pre-check.
• It fails ("0 video buffers reached the muxer in 5 s") when a refused track is also reopened while its input carries the EOS. So it guards the reopen rule, not the pre-check; its doc comment and the body's revert table now say so.
• fmt, clippy, full suite: 845 passed, 0 failed, 2 ignored.

With the pre-check gone, retired is set only by end_stalled_track, as on main. The watchdog split out of recorder.rs is left for a follow-up.

[srperens]

Verdict: Approve — supersedes my review of 2026-09-24 at 4a87cbc3; the verdict does not stand, because the code both CONTRADICTED rows were about is gone. Requested change 1 was taken by its second branch (drop the pre-check), and that removal also settles requested change 2: the watchdog once again sets retired only from inside end_stalled_track, i.e. only during a freeze.

Claims

Claim	Verdict	Evidence
A finished track is picked once, then leaves the watchdog on its own	CONFIRMED	backend/src/blocks/builtin/recorder.rs:293 — if !delivered && input_clean { — its input carries its own EOS, so retired is not cleared and backend/src/blocks/builtin/recorder.rs:406 — if track.activity.retired.load(Ordering::Relaxed) { — skips it from the next poll
Reverting the input_clean guard pins a_track_that_finished_does_not_hide_a_later_stall at zero video	CONFIRMED	backend/src/blocks/builtin/recorder.rs:371 — ` .min_by_key(
The new behaviour executed in CI at this head	CONFIRMED	Check (Linux) job 107878176449: recorder_stalled_track_test "5 passed", a_track_that_finished_does_not_hide_a_later_stall ... ok, plus the four unit tests named in the revert table, each ... ok

Diagnosis — Unchanged and still right; the delta only moves the finished-track case from a pre-check to the refusal path, which is where the evidence for it already was. Dropping the pre-check also restores main's behaviour for a recording whose tracks all finish: backend/src/blocks/builtin/recorder.rs:413 — still_watching += 1; — counts them again, so the thread polls until teardown rather than returning early. No spurious ending follows, because track_holding_the_recording first requires some track to be backed up, and finished tracks are all drained. Coverage stays BOUNDED exactly as before, with #804 owning the remainder.

SPECULATIVE (not verified): if a refusal ever left the input pad clean and the muxer pad already carrying EOS, the track would be reopened and the new debug! would repeat each poll instead of once. I could not construct that state from the code — every refusal I can trace that leaves the input clean is a flushing or deactivated pad, which does not co-occur with a delivered EOS downstream.

Radius — LOCAL, as before: end_stalled_track still has no caller outside watch_tracks and the tests, and the delta touches one branch of one loop.

Overlaps — none of the three kinds. #789 edits recorder.rs textually only; #804 and #835 are adjacent and own different mechanisms.

Tests & CI — Nine checks green at 92d0181; macOS/Windows skipped and nothing here is platform-specific. The three gst_mini_object_unref GStreamer-CRITICALs in that test binary appear identically at 4a87cbc3, so this delta did not introduce them — worth its own look, not this PR's. The file-size point from my previous review stands unchanged.

Confidence: HIGH

Superseded by my review at 92d0181: the code both CONTRADICTED rows were about was removed at this head.

[wagenet]

Claude (AI), on behalf of @wagenet.

Rebased onto upstream/main (now 9b60255). #912 folded recorder_stalled_track_test.rs into the stalled_track module of recorder_test.rs, so both commits' test changes conflicted as modify/delete. I moved the two new tests into that module on its shared helpers: start_recorder now returns the Recorder so the torn-branch test can reach audio_input_0, and the finished-track test builds its two-audio recorder from add_mp4_recorder, audio_source and link_without_eos. Test bodies, timings and assertions are unchanged. recorder.rs merged without conflict.

Ran (macOS): cargo fmt --all -- --check, cargo clippy --workspace --all-targets -- -D warnings (clean); cargo test --test recorder_test 14 passed; cargo test --lib recorder 10 passed. With upstream's recorder.rs swapped in, a_stalled_track_whose_branch_is_coming_apart_still_ends fails and the other four stalled_track tests pass, as expected: the finished-track test guards against one variant of the fix, not against upstream.

[srperens]

Comment by Claude (AI), on behalf of @srperens.

Thanks for this. The refused-EOS case is a real hole in the #757 watchdog, and the fix is at the right layer for the Recorder as it is.

The Live Recorder below has no watchdog and never ends a track, so there is no EOS to be refused: a stalled track simply comes back in the same file.

Where we're heading. We're aiming to deprecate the Recorder in favour of a new block, Live Recorder (builtin.liverecorder), in #1034. It replaces splitmuxsink with a live aggregator muxer (isofmp4mux, matroskamux or mpegtsmux) and a small sink of our own that does the splitting. A track that stops then holds nothing up, and when it comes back it continues in the same file. There is no watchdog, no ended track, and no new file per stall. Both blocks will exist side by side until the new one has run in real productions. The Recorder stays as it is in the meantime.

We'd really like your help with this:

• Test it on the rigs where you found these bugs (WHIP guests, SRT inputs, per-seat and program recordings). Swap builtin.recorder for builtin.liverecorder; the pads and properties are the same. It builds and runs on GStreamer 1.24 and later, like the rest of Strom.
• Tell us what you think of the approach, and what it still lacks for your use.
• Review feat(live recorder): a recorder where a quiet track holds nothing up #1034. You know the failure cases better than anyone, and its "Out of scope" section lists the gaps we know about.

[wagenet]

Comment by Claude (AI), on behalf of @wagenet.

Closing. Live Recorder (#1034) has no watchdog that ends a track, so there is no EOS to refuse. This PR also depended on #927, which is closed. The branch wagenet/recorder-eos-refused stays as it is.