Trickplay sheets already on disk are neither catalogued nor moved #147

New issue

Closed

opened 2026-08-16 15:13:37 +02:00 by Claude · 1 comment

Claude commented

2026-08-16 15:13:37 +02:00

Collaborator

Split (3) of #99's comment, untouched by the batch that closed it. #99 landed the
generation, the row and RefreshTrickplayImages; two upstream pieces remain,
both in MediaBrowser.Providers/Trickplay/ and TrickplayManager.cs at the pin.

TrickplayMoveImagesTask (key MoveTrickplayImages) moves sheets between
the media directory and the data directory as the library option changes. This
server writes only to the data directory, so a library configured for
media-adjacent storage keeps its sheets where nothing looks for them.
DiscoverExistingTrickplayAsync catalogues sheets a user placed
themselves, with interval-snapping arithmetic that has to be ported rather
than invented.

Two upstream behaviours to port faithfully rather than correct: DiscoverExisting
records the configured width rather than the even one the scaler produced,
and it records the tile count as ThumbnailCount. Both look like defects,
and both are numbers a client reads back out of TrickplayInfos, so ours has to
agree with them.

Where to start

crates/jellyfin-server/src/artefacts.rs carries the sweep #99 landed — the
paging, the CanGenerateTrickplay guards, the tiling and the row write are all
there and this is a third entry point into the same machine. The on-disk
convention is <data>/data/trickplay/<xx>/<hyphenated guid>/<width> - <tw>x<th>/<n>.jpg
and is verified byte-identical against upstream.

How to know it is done

Neither piece is reachable from a harness run today — see the issue filed beside
this one about the fixture carrying no sheets. That is the precondition for
measuring either of these against a pair rather than against a unit test.

House rules

Tests run under cargo nextest; upstream's source is the checkout in
upstream/ at the pin (v12.0-rc5, 6d501ba4), not the clone beside the
repository. Generated artifacts are regenerated, never edited. A comment says
what the code does, not how it got there. The reference on :8096 is read-only;
anything needing a write gets its own container from the pinned image over a
cp -a copy of a snapshot, on its own ports. Kill a server by the PID
ss -lntpH 'sport = :NNNN' reports.

Split (3) of #99's comment, untouched by the batch that closed it. #99 landed the generation, the row and `RefreshTrickplayImages`; two upstream pieces remain, both in `MediaBrowser.Providers/Trickplay/` and `TrickplayManager.cs` at the pin. - **`TrickplayMoveImagesTask`** (key `MoveTrickplayImages`) moves sheets between the media directory and the data directory as the library option changes. This server writes only to the data directory, so a library configured for media-adjacent storage keeps its sheets where nothing looks for them. - **`DiscoverExistingTrickplayAsync`** catalogues sheets a user placed themselves, with interval-snapping arithmetic that has to be ported rather than invented. Two upstream behaviours to port faithfully rather than correct: `DiscoverExisting` records the **configured** width rather than the even one the scaler produced, and it records the **tile** count as `ThumbnailCount`. Both look like defects, and both are numbers a client reads back out of `TrickplayInfos`, so ours has to agree with them. ## Where to start `crates/jellyfin-server/src/artefacts.rs` carries the sweep #99 landed — the paging, the `CanGenerateTrickplay` guards, the tiling and the row write are all there and this is a third entry point into the same machine. The on-disk convention is `<data>/data/trickplay/<xx>/<hyphenated guid>/<width> - <tw>x<th>/<n>.jpg` and is verified byte-identical against upstream. ## How to know it is done Neither piece is reachable from a harness run today — see the issue filed beside this one about the fixture carrying no sheets. That is the precondition for measuring either of these against a pair rather than against a unit test. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin** (`v12.0-rc5`, `6d501ba4`), not the clone beside the repository. Generated artifacts are regenerated, never edited. A comment says what the code does, not how it got there. The reference on `:8096` is read-only; anything needing a write gets its own container from the pinned image over a `cp -a` copy of a snapshot, on its own ports. Kill a server by the PID `ss -lntpH 'sport = :NNNN'` reports.

Claude added the

jobs

labels

2026-08-16 15:13:37 +02:00

Claude referenced this issue

2026-08-16 15:15:37 +02:00

Trickplay sheets are never generated, so scrubbing a video shows nothing #99

Claude referenced this issue

2026-08-16 15:48:12 +02:00

The dashboard lists 2 of the 15 jobs upstream runs #97

Claude commented

2026-08-16 17:02:37 +02:00

Author

Collaborator

Landed in 8a7f4d8.

What went in

SaveTrickplayWithMedia is now a library setting (save_trickplay_with_media),
and the four places that had to agree about it do:
Paths::trickplay_of is the one rule for where a video's sheets are — the data
directory, or <file>.trickplay beside the file — and the cutting, the
discovery, the serving route and the new task all go through it.

DiscoverExistingTrickplayAsync runs before anything is cut, and does both
halves: rows whose sheets are gone from both locations are dropped (a new
forget_trickplay_widths), and a <width> - <tw>x<th> directory nothing
describes is catalogued with upstream's own arithmetic — the tile count bounds
the thumbnail count, and the interval is the configured one where the runtime
allows it and the middle of what the runtime allows where it does not, rounded
half to even because that is what Math.Round does.

MoveTrickplayImages is declared rather than argued absent. ABSENT is down to
five, D34 is rewritten, scenarios/tasks.json asks for the new id, and the
harness README's count is updated.

Also ported, because it is the same branch: a library that has stopped
extracting no longer takes the sheets away when they live beside the media —
upstream treats those as the viewer's own and leaves them where they are.

One correction to this issue's premise

DiscoverExisting records the configured width rather than the even one
the scaler produced, and it records the tile count as ThumbnailCount.

Those two belong to a different branch. DiscoverExistingTrickplayAsync records
the width it parsed out of the directory name — which is the even one, because
that is what wrote the directory — and computes ThumbnailCount from the
runtime rather than from the tile count.

The branch that does what the issue describes is the "Import existing tiles"
block inside RefreshTrickplayDataInternal
(TrickplayManager.cs:437), which sets Width = width (configured) and
ThumbnailCount = existingFiles.Length (tiles). It is unreachable at the pin.
Discovery runs first and files a row keyed on the directory's width; the import
branch is guarded by HasTrickplayResolutionAsync(video.Id, actualWidth), and
the directory is named with actualWidth, so that guard is always true by the
time it is reached. The only way past it is a folder holding files none of which
are .jpg — discovery skips those and GetImageSize would throw on them.

So the port implements DiscoverExisting as upstream writes it, and does not
reproduce the two numbers, because nothing upstream produces them either.

What is not measured

Neither piece is reachable from a harness run: the snapshot still carries no
sheets. That is #148, and it is the precondition for measuring either of these
against a pair rather than against the unit tests beside them.

Landed in `8a7f4d8`. ## What went in `SaveTrickplayWithMedia` is now a library setting (`save_trickplay_with_media`), and the four places that had to agree about it do: `Paths::trickplay_of` is the one rule for where a video's sheets are — the data directory, or `<file>.trickplay` beside the file — and the cutting, the discovery, the serving route and the new task all go through it. `DiscoverExistingTrickplayAsync` runs before anything is cut, and does both halves: rows whose sheets are gone from **both** locations are dropped (a new `forget_trickplay_widths`), and a `<width> - <tw>x<th>` directory nothing describes is catalogued with upstream's own arithmetic — the tile count bounds the thumbnail count, and the interval is the configured one where the runtime allows it and the middle of what the runtime allows where it does not, rounded half to even because that is what `Math.Round` does. `MoveTrickplayImages` is declared rather than argued absent. `ABSENT` is down to five, `D34` is rewritten, `scenarios/tasks.json` asks for the new id, and the harness README's count is updated. Also ported, because it is the same branch: a library that has stopped extracting no longer takes the sheets away when they live beside the media — upstream treats those as the viewer's own and leaves them where they are. ## One correction to this issue's premise > `DiscoverExisting` records the **configured** width rather than the even one > the scaler produced, and it records the **tile** count as `ThumbnailCount`. Those two belong to a different branch. `DiscoverExistingTrickplayAsync` records the width it parsed out of the *directory name* — which is the even one, because that is what wrote the directory — and computes `ThumbnailCount` from the runtime rather than from the tile count. The branch that does what the issue describes is the "Import existing tiles" block inside `RefreshTrickplayDataInternal` (`TrickplayManager.cs:437`), which sets `Width = width` (configured) and `ThumbnailCount = existingFiles.Length` (tiles). **It is unreachable at the pin.** Discovery runs first and files a row keyed on the directory's width; the import branch is guarded by `HasTrickplayResolutionAsync(video.Id, actualWidth)`, and the directory is named with `actualWidth`, so that guard is always true by the time it is reached. The only way past it is a folder holding files none of which are `.jpg` — discovery skips those and `GetImageSize` would throw on them. So the port implements `DiscoverExisting` as upstream writes it, and does not reproduce the two numbers, because nothing upstream produces them either. ## What is not measured Neither piece is reachable from a harness run: the snapshot still carries no sheets. That is #148, and it is the precondition for measuring either of these against a pair rather than against the unit tests beside them.

Claude closed this issue

2026-08-16 17:04:01 +02:00