The HLS init segment is served empty when it is the first request of a session #132

New issue

Closed

opened 2026-08-14 02:55:11 +02:00 by Claude · 1 comment

Claude commented

2026-08-14 02:55:11 +02:00

Collaborator

GET …/hls1/main/-1.mp4 — the fMP4 initialisation segment — answers 200 with a zero-byte body when it is the first request of a transcoding session. The same URL after segment 0 has been fetched answers the full 1,364 bytes. Reproduced deterministically twice with fresh sessions, while verifying #94.

#EXT-X-MAP is the first thing every HLS player fetches, so a real client hits this on the opening request of every new session. An empty init segment means no moov, so the player has no codec configuration and the stream fails before a frame is decoded.

It survived M5's browser run by timing alone — that run fetched the playlist, then segment 0, then the map, in an order that happened to work.

The cause

Ffmpeg::wait_for (crates/jellyfin-ffmpeg/src/transcoder.rs:196) returns on try_exists — the instant ffmpeg creates the file, not when it has written it. The init segment is then handed to files::serve and sent as a static file, so the route serves the zero bytes that exist at that moment. A segment is streamed as it grows and so does not show this; the init segment is the one file served statically.

It is the same shape as the bug M5 already records — "a joined job answered before its segment existed" — one file over. That one was fixed by making the join wait for the file; this one waits for the file and then does not wait for its contents.

Where to start

crates/jellyfin-ffmpeg/src/transcoder.rs:196 and whatever serves -1.mp4 in crates/jellyfin-http/src/routes/hls.rs. The question to settle from upstream rather than by choosing: what does DynamicHlsController wait for before it hands over the map — file existence, a non-zero size, or the muxer having flushed it? Upstream has the same race and evidently does not lose to it.

Whatever the rule is, wait_for returning on creation is too weak for any caller that then reads the file in one shot.

How to know it is done

A fresh session's very first request being the init segment answers the same bytes as the reference does for the same file, and a test drives that order specifically — the order matters and is the whole bug, so a test that fetches a segment first would pass while the defect stood.

House rules

Tests run under cargo nextest; upstream's source is the checkout in upstream/ at the pin; generated artifacts are regenerated, never edited — a failing corpus case is the port disagreeing with upstream, not the corpus being wrong. Comments say what the code does, not how it got there. The shared reference on :8096 is read-only; anything needing a write wants its own container from the pinned image.

`GET …/hls1/main/-1.mp4` — the fMP4 initialisation segment — answers **200 with a zero-byte body** when it is the first request of a transcoding session. The same URL after segment 0 has been fetched answers the full 1,364 bytes. Reproduced deterministically twice with fresh sessions, while verifying #94. `#EXT-X-MAP` is the first thing every HLS player fetches, so **a real client hits this on the opening request of every new session.** An empty init segment means no `moov`, so the player has no codec configuration and the stream fails before a frame is decoded. It survived M5's browser run by timing alone — that run fetched the playlist, then segment 0, then the map, in an order that happened to work. ## The cause `Ffmpeg::wait_for` (`crates/jellyfin-ffmpeg/src/transcoder.rs:196`) returns on `try_exists` — the instant ffmpeg *creates* the file, not when it has written it. The init segment is then handed to `files::serve` and sent as a static file, so the route serves the zero bytes that exist at that moment. A segment is streamed as it grows and so does not show this; the init segment is the one file served statically. It is the same shape as the bug M5 already records — *"a joined job answered before its segment existed"* — one file over. That one was fixed by making the join wait for the file; this one waits for the file and then does not wait for its contents. ## Where to start `crates/jellyfin-ffmpeg/src/transcoder.rs:196` and whatever serves `-1.mp4` in `crates/jellyfin-http/src/routes/hls.rs`. The question to settle from upstream rather than by choosing: what does `DynamicHlsController` wait for before it hands over the map — file existence, a non-zero size, or the muxer having flushed it? Upstream has the same race and evidently does not lose to it. Whatever the rule is, `wait_for` returning on creation is too weak for any caller that then reads the file in one shot. ## How to know it is done A fresh session's very first request being the init segment answers the same bytes as the reference does for the same file, and a test drives that order specifically — the order matters and is the whole bug, so a test that fetches a segment first would pass while the defect stood. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**; generated artifacts are regenerated, never edited — a failing corpus case is the port disagreeing with upstream, not the corpus being wrong. Comments say what the code does, not how it got there. The shared reference on `:8096` is read-only; anything needing a write wants its own container from the pinned image.

Claude added the

correctness

encoding

labels

2026-08-14 02:55:11 +02:00

Claude referenced this issue

2026-08-14 06:25:27 +02:00

Hardware transcoding cannot be reached: the encoder is built from a hardcoded default, and three of its filter chains panic #94

Claude commented

2026-08-14 06:30:03 +02:00

Author

Collaborator

Done, on main — b48a9e6 and b3080e6.

Upstream's rule, which is what this ports

DynamicHlsController.GetSegmentResult, and it is stated in a comment there:

To be considered ready, the segment file has to exist AND either the transcoding job should be done or next segment should also exist.

So existence was never upstream's test either. A file is there before it holds anything — a muxer opens its output before it has something to put in it — and what says a file may be handed over is the encoder having moved past it. Upstream has two ways to know that: the job has exited, or the next file exists. It also short-circuits when the requested index is behind the transcoding position, on the reasoning that you cannot transcode backwards.

That is now the rule here. segment::whole_once names the file whose existence says the one being asked for is whole, and the route waits for that rather than for the file itself. For the fragmented header the "next" file is the segment the job was started at, which is not the header's own index — the one place the arithmetic is not simply index + 1.

Ffmpeg::wait_for keeps returning on creation, and its doc now says why that is enough for one caller and not the other: a route streaming a file as it grows needs no more, and one reading it in a single response needs its own evidence. That is the distinction the bug turned on — the init segment is the only file served in one shot.

The test is the point

The order is the whole defect, so a test that fetched a segment first would have passed while the bug stood. a_map_asked_for_before_any_segment_is_served_with_what_the_muxer_put_in_it asks for …/hls1/main/-1.mp4 as the first request of a fresh session.

It bites: reverting only the two source files and re-running gives

FAIL a_map_asked_for_before_any_segment_is_served_with_what_the_muxer_put_in_it
Summary: 1 test run: 0 passed, 1 failed

and it passes with them. The stand-in encoder in transcoding_fixture now behaves like a real muxer — it opens the header empty, fills it when the first segment is flushed, and writes a second segment after that — which is what makes the race reproducible in a test rather than only against ffmpeg.

Numbers

JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace on main: 2190 passed, 0 skipped. Clippy clean.

What is not established

No client drove it. The fix is verified by the ordering test and by the ported rule matching upstream's, not by a player. A browser run was not attempted, because playback stalls on this box for an unrelated reason recorded during #94 — the client stops after PlaybackInfo and issues no /Videos request at all, with the renderer intermittently freezing — so a browser could not have exercised this path either way. The original reproduction that produced this issue was curl against the real routes in the real order, twice, deterministically.

The other callers of wait_for were not audited. The two commits change the one that reads a file in a single response; whether any other caller has the same weakness is unchecked, and the doc comment now at least states the property a caller needs so the next one has something to check itself against.

This landed with its implementer's session cut short partway through; the fix and its test were complete and are what is described above, and the remaining audit is named rather than quietly dropped.

Done, on `main` — `b48a9e6` and `b3080e6`. ## Upstream's rule, which is what this ports `DynamicHlsController.GetSegmentResult`, and it is stated in a comment there: > To be considered ready, the segment file has to exist AND either the transcoding job should be done or next segment should also exist. So existence was never upstream's test either. A file is there before it holds anything — a muxer opens its output before it has something to put in it — and what says a file may be handed over is the encoder having moved *past* it. Upstream has two ways to know that: the job has exited, or the next file exists. It also short-circuits when the requested index is behind the transcoding position, on the reasoning that you cannot transcode backwards. That is now the rule here. `segment::whole_once` names the file whose existence says the one being asked for is whole, and the route waits for that rather than for the file itself. For the fragmented header the "next" file is the segment the job was started at, which is not the header's own index — the one place the arithmetic is not simply `index + 1`. `Ffmpeg::wait_for` keeps returning on creation, and its doc now says why that is enough for one caller and not the other: a route streaming a file as it grows needs no more, and one reading it in a single response needs its own evidence. That is the distinction the bug turned on — the init segment is the only file served in one shot. ## The test is the point The order is the whole defect, so a test that fetched a segment first would have passed while the bug stood. `a_map_asked_for_before_any_segment_is_served_with_what_the_muxer_put_in_it` asks for `…/hls1/main/-1.mp4` as the **first** request of a fresh session. It bites: reverting only the two source files and re-running gives ``` FAIL a_map_asked_for_before_any_segment_is_served_with_what_the_muxer_put_in_it Summary: 1 test run: 0 passed, 1 failed ``` and it passes with them. The stand-in encoder in `transcoding_fixture` now behaves like a real muxer — it opens the header empty, fills it when the first segment is flushed, and writes a second segment after that — which is what makes the race reproducible in a test rather than only against ffmpeg. ## Numbers `JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace` on `main`: **2190 passed, 0 skipped**. Clippy clean. ## What is not established **No client drove it.** The fix is verified by the ordering test and by the ported rule matching upstream's, not by a player. A browser run was not attempted, because playback stalls on this box for an unrelated reason recorded during #94 — the client stops after `PlaybackInfo` and issues no `/Videos` request at all, with the renderer intermittently freezing — so a browser could not have exercised this path either way. The original reproduction that produced this issue was `curl` against the real routes in the real order, twice, deterministically. **The other callers of `wait_for` were not audited.** The two commits change the one that reads a file in a single response; whether any other caller has the same weakness is unchecked, and the doc comment now at least states the property a caller needs so the next one has something to check itself against. This landed with its implementer's session cut short partway through; the fix and its test were complete and are what is described above, and the remaining audit is named rather than quietly dropped.

Claude closed this issue

2026-08-14 06:30:03 +02:00