From 22d88280800b096c38fc8efa0d3f959b84cabb23 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 11:32:12 +0100 Subject: [PATCH] Never serve a partial file: it makes Jellyfin transcode The 12s first-byte grace was the wrong trade and a real play found it within the hour. A 2-hour upload took 3 minutes to start, played 6 seconds, and stalled. Jellyfin had run ffmpeg with -probesize 1G against the growing stream and then transcoded to HLS with libx264. The cause is the container. A fragmented MP4 with empty_moov has no duration in its header, so the only way to get one is to sum every fragment -- probing a growing file reads all of it. Jellyfin cannot establish duration, codec or bitrate, so it abandons direct play and transcodes a stream it also cannot seek. It was targeting 4.83 Mbps against a source measured at 3.29: re-encoding a stream that already fit, because it could not measure it. The same video once complete reports SupportsDirectPlay with the exact runtime and bitrate. So FIRST_BYTE_GRACE defaults to infinite again, with --wait-timeout raised to 600s for a 2-hour upload. A cold long video is slow to start, which is accepted: the fetch outlives the request so a retry is instant, and a retryable stall beats a transcode that wastes a gigabyte and cannot work. Both failure modes are recorded at the constant in the order measured so the 12s cap is not reintroduced. Co-Authored-By: Claude Opus 5 --- deploy/ytstream-proxy.service | 16 +++-- plan.md | 74 +++++++++++++++++++ proxy/ytstream_proxy.py | 99 +++++++++++++++++--------- tests/test_discovery.py | 129 ++++++++++++++++++++++++++++++++++ tests/test_proxy.py | 31 ++++++-- tests/test_runner.py | 34 +++++++++ tests/test_strm.py | 52 ++++++++++++++ ytstream/discovery.py | 70 +++++++++++++++++- ytstream/runner.py | 22 ++++++ ytstream/strm.py | 33 +++++++++ 10 files changed, 514 insertions(+), 46 deletions(-) diff --git a/deploy/ytstream-proxy.service b/deploy/ytstream-proxy.service index 70272f0..59cda86 100644 --- a/deploy/ytstream-proxy.service +++ b/deploy/ytstream-proxy.service @@ -21,11 +21,15 @@ UMask=0002 Environment=PATH=/var/lib/ytstream/venv/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin Environment=PYTHONUNBUFFERED=1 -# --first-byte-grace is the difference between "plays" and "playback error". -# Waiting for a complete mux costs ~3s to extract plus duration/50..110 to pull, -# so a 46-minute upload blocked this socket for 79 silent seconds and Jellyfin -# gave up long before. Do not raise this to --wait-timeout without re-reading -# the module docstring: that restores the behaviour that broke first plays. +# No --first-byte-grace: requests wait for the COMPLETE mux, bounded by +# --wait-timeout. Do not add a grace here without reading FIRST_BYTE_GRACE in the +# proxy. Short version, both measured 2026-08-13: waiting sends nothing for ~80s +# on a 46-minute upload and the client gives up, but serving the partial file +# instead makes Jellyfin drag a gigabyte through this proxy to probe a fragmented +# MP4 that has no duration in its header, and then TRANSCODE a stream it cannot +# seek -- 3 minutes to start, 6 seconds of video, then a permanent stall. +# A slow cold start is the better failure: the fetch outlives the request, so the +# retry is instant. --wait-timeout is 600 to cover a 2-hour upload (2.4 GB). WorkingDirectory=/opt/ytstream ExecStart=/var/lib/ytstream/venv/bin/python3 /opt/ytstream/proxy/ytstream_proxy.py \ @@ -35,7 +39,7 @@ ExecStart=/var/lib/ytstream/venv/bin/python3 /opt/ytstream/proxy/ytstream_proxy. --max-pipelines 2 \ --max-retries 2 \ --max-starts 20 --starts-window 3600 \ - --first-byte-grace 12 + --wait-timeout 600 Restart=always RestartSec=5 diff --git a/plan.md b/plan.md index 7f16dba..55643a5 100644 --- a/plan.md +++ b/plan.md @@ -1272,3 +1272,77 @@ he subscribes to something and nothing happens, which is the feature not working Reversible per channel with `ytstream unsubscribe`. `approve --all`: **108 added, 0 failed, 70 s.** + +## 22. Serving a partial file to Jellyfin is worse than making it wait — 2026-08-13 + +§19 capped the wait for a complete mux at 12s and streamed the partial file after +that, to stop long videos failing to start. That was the wrong trade, and a real +play found it within the hour: a 2-hour upload took **3 minutes to start, played 6 +seconds, and stalled permanently**. + +### What Jellyfin actually did + +``` +ffmpeg -analyzeduration 200M -probesize 1G -i http://127.0.0.1:8099/watch/JYZYnkXMxdU + -codec:v:0 libx264 -preset veryfast -maxrate 4830438 ... -f hls +``` + +It **transcoded**, after dragging up to a gigabyte through the proxy to probe. The +Jellyfin log closes it out: `Playback stopped ... Stopped at "6016" ms`. + +### Why — and it is the container, not the client + +A fragmented MP4 written with `empty_moov` **has no duration in its header**. The +only way to obtain one is to sum every fragment, so probing a *growing* file means +reading all of it — hence `-probesize 1G`. Jellyfin therefore cannot establish +duration, codec or bitrate, stops trusting direct play, and transcodes a stream it +also cannot seek. + +The same video once complete, via `PlaybackInfo`: + +| | | +|---|---| +| `SupportsDirectPlay` | **True** | +| `RunTimeTicks` | 1278 s (database says 1279) | +| `Bitrate` | 3,290,533 | +| Streams | h264 720p 3.16 Mbps + aac 128 kbps | + +The transcode had been targeting `-maxrate 4830438` — **4.83 Mbps, above the +source's 3.29**. Jellyfin was re-encoding a stream that already fitted, purely +because it could not measure it. ffmpeg patches the real duration into the moov on +close, which is what makes the complete file cheap to probe and safe to direct-play. + +### The decision + +`FIRST_BYTE_GRACE` now defaults to **infinite** — wait for the complete mux, +bounded by `--wait-timeout` (raised to 600s to cover a 2-hour, 2.4 GB upload). A +cold long video is slow to start, and that is accepted deliberately: + +* the fetch outlives the request, so a **retry is instant**; +* the failure is a stall the user can retry, not a runaway transcode that wastes a + gigabyte of transfer and cannot succeed. + +Both failure modes are now recorded at the constant, in the order they were +measured, so nobody re-tries the 12s cap and rediscovers this. `/healthz` reports +`mode: wait-for-complete` and `first_byte_grace_s: null` — `null` rather than a +number because `json.dumps` renders an infinite float as `Infinity`, which is not +valid JSON. + +### Still unresolved + +**A cold long video does not start on the first press.** The grace was the wrong +fix for it, and the right one is not in yet. The options, none free: + +1. **Pre-warm** the newest episode per channel after each run — turns the common + case ("watch the latest") instant, at the cost of fetching videos nobody asked + for, which is the premise the whole design rejects. Bounded, though: 119 videos. +2. **Faster pull.** `yt-dlp -o -` uses a single connection; measured 4.3 MB/s on a + 525 MB upload. If concurrent ranged fetches work on these formats, a 2-minute + wait could become 20 seconds and the problem mostly disappears. +3. **Give Jellyfin the metadata up front** so it never probes: duration, codec and + bitrate are all known before a byte is fetched. `` in the NFO was + measured not to affect scan probing, but its effect on `PlaybackInfo` + specifically has not been tested — and that is the one that decides transcoding. + +Option 2 is the one to measure first: it is the only one that costs nothing and +helps every case. diff --git a/proxy/ytstream_proxy.py b/proxy/ytstream_proxy.py index 7a6ed16..ffa6b85 100644 --- a/proxy/ytstream_proxy.py +++ b/proxy/ytstream_proxy.py @@ -26,25 +26,20 @@ Safety rails, because a Jellyfin library scan can ask for every episode at once: hour; a refresh storm hits the cap in seconds. --cache-gb tmpfs budget; least-recently-used complete files are evicted. -Serving modes. The output is a fragmented MP4, so it is readable while it is -still being written; the only thing a finished file buys is a correct duration -and working seeks, because ffmpeg patches the real duration into the moov on -close and ignores both mvhd.duration and an injected mehd box before then. +Serving modes. The output is a fragmented MP4, so it is *readable* while being +written -- but do not serve it that way to Jellyfin. See FIRST_BYTE_GRACE below: +a growing fragmented MP4 has no duration in its header, so Jellyfin drags up to a +gigabyte through this proxy trying to probe one and then transcodes a stream it +cannot seek. A slow start beats that. - default wait for the mux, but only for --first-byte-grace seconds, - then serve what exists. Short videos finish inside the - grace and keep the exact duration and ranges; long ones - start immediately and regain both when the mux lands. - --first-byte-grace raise it above --wait-timeout for the old strict behaviour - (finished file or 504), which is only safe for a client - that will wait minutes for its first byte. + default wait for the complete mux, bounded by --wait-timeout, then + serve it with real ranges and the exact duration. A cold + long video is slow to start; the fetch continues after the + client gives up, so the next attempt is instant. + --first-byte-grace finite value: stop waiting after N seconds and stream the + partial file. Fast start, but only for a client that + tolerates an unseekable, duration-less stream. --growing grace of zero: serve as soon as any byte exists. - -Why the grace exists at all, measured 2026-08-13 on this machine: the wait is -~3s to extract plus roughly duration/50..110 to pull and mux, so a 46-minute -upload took 79 seconds during which the handler sent nothing -- not even -response headers. Jellyfin reported "playback error" on the first play of every -long video while already-cached ones played perfectly. """ import argparse @@ -73,15 +68,35 @@ POT_ARGS = "youtubepot-bgutilhttp:base_url=http://127.0.0.1:4416" CLIENT_ARGS = "youtube:player_client=default" MAX_HEIGHT = 720 STALL_TIMEOUT = 45.0 -# How long a request may block waiting for a *complete* mux before it gives up and -# streams the partial file instead. This is a cap on time-to-first-byte, not a -# prediction: measured cold on 2026-08-13, completion ran from 10s (8-minute -# upload) to 156s (66-minute upload), and it does not follow the duration closely -# because YouTube's per-format throttling varies and one transient failure plus a -# retry costs ~5s of extraction before any byte is pulled. So do not tune this -# expecting to cover "videos up to N minutes" -- it catches the quick cases, -# absorbs a single retry, and bounds the bad case at something a player tolerates. -FIRST_BYTE_GRACE = 12.0 +# How long a request may block waiting for a *complete* mux before giving up and +# streaming the partial file instead. Infinite by default, i.e. wait for the whole +# thing (bounded by --wait-timeout), because serving a partial file to Jellyfin is +# WORSE than making it wait. Measured 2026-08-13, in this order: +# +# 1. Waiting for the complete file with no cap meant a 46-minute upload sent +# nothing for 79s and the client gave up -> "playback error". +# 2. So this was capped at 12s and the partial file streamed instead. A +# 2-hour upload then took 3 minutes to start, played 6 seconds, and stalled. +# Jellyfin had run: +# ffmpeg -analyzeduration 200M -probesize 1G -i .../watch/ ... libx264 +# It TRANSCODED, after dragging up to a gigabyte through the proxy to probe. +# +# The reason is the container. A fragmented MP4 with empty_moov carries no +# duration in its header -- the only way to get one is to sum every fragment, so +# probing a growing file reads all of it. Jellyfin cannot establish duration, +# codec or bitrate, so it stops trusting direct play and transcodes a stream it +# also cannot seek. On the *complete* file ffmpeg patches the real duration into +# the moov on close, the probe is cheap, and Jellyfin reported SupportsDirectPlay +# with the exact runtime and a 3.29 Mbps bitrate -- comfortably under the +# 4.83 Mbps cap it had been transcoding down to. +# +# So a slow start is the correct failure: the fetch continues after the client +# gives up, and the next attempt is instant. A partial file is a fast start +# followed by a transcode that cannot work. +# +# Set a finite value only for a client that tolerates an unseekable, duration-less +# stream. Jellyfin does not. +FIRST_BYTE_GRACE = float("inf") VIDEO_ID_RE = re.compile(r"^[A-Za-z0-9_-]{11}$") _log_lock = threading.Lock() @@ -330,6 +345,10 @@ class Manager: # Reported by /healthz so the serving mode is visible without reading the # unit file. Set by main(); the handler holds the value it actually uses. self.first_byte_grace = 0.0 if growing else FIRST_BYTE_GRACE + # True when a request waits for the whole mux rather than ever serving a + # partial file. Overwritten by main(); the default matches the default + # grace so a Manager built directly (tests) reports the same thing. + self.strict = not growing self.lock = threading.Lock() self.sessions = {} self.counters = {"requests": 0, "started": 0, "reused": 0, @@ -460,8 +479,16 @@ class Manager: def status(self): with self.lock: return { - "mode": "growing" if self.growing else "wait-then-stream", - "first_byte_grace_s": self.first_byte_grace, + # `null` rather than Infinity: json.dumps would happily emit the + # latter, and it is not valid JSON for whoever reads this. + "mode": ("growing" if self.growing + else "wait-for-complete" if self.strict + else "wait-then-stream"), + # `null` rather than a number when strict: there is no cap, and + # json.dumps would otherwise emit Infinity, which is not JSON. + "first_byte_grace_s": ( + None if self.strict or self.first_byte_grace == float("inf") + else self.first_byte_grace), "no_fetch": self.no_fetch, "max_pipelines": self.max_pipelines, "max_starts": self.max_starts, @@ -768,9 +795,11 @@ def main(): help="hard cap on blocking for the mux; only reachable when " "--first-byte-grace is raised to meet it") ap.add_argument("--first-byte-grace", type=float, default=FIRST_BYTE_GRACE, - help=f"seconds to wait for a complete mux before streaming " - f"the partial file instead (default {FIRST_BYTE_GRACE:g}). " - f"Raise to --wait-timeout for finished-file-or-504") + help="seconds to wait for a complete mux before streaming the " + "partial file instead. Default is to wait for the whole " + "thing: a partial fragmented MP4 makes Jellyfin transcode " + "after a 1 GB probe. Only set this for a client that " + "tolerates an unseekable, duration-less stream") ap.add_argument("--max-starts", type=int, default=20, help="max cold starts per window; bounds a runaway library " "refresh (default 20)") @@ -814,6 +843,7 @@ def main(): args.max_retries, args.max_starts, args.starts_window) grace = 0.0 if args.growing else min(args.first_byte_grace, args.wait_timeout) + mgr.strict = not args.growing and grace >= args.wait_timeout mgr.first_byte_grace = grace if args.no_fetch: @@ -822,9 +852,10 @@ def main(): if grace <= 0: log("growing mode: a first play seeks badly and shows no duration until " "the mux lands") - elif grace >= args.wait_timeout: - log(f"strict mode: a request blocks up to {grace:g}s for a complete mux " - f"and 504s otherwise -- long videos will fail to start") + elif mgr.strict: + log(f"waiting for a complete mux, up to {grace:g}s. A cold long video is " + f"slow to start and the fetch outlives the request, so a retry is " + f"instant -- see FIRST_BYTE_GRACE for why partial is worse") else: log(f"waiting up to {grace:g}s for a complete mux, then streaming the " f"partial file") diff --git a/tests/test_discovery.py b/tests/test_discovery.py index a58ff0c..1b65942 100644 --- a/tests/test_discovery.py +++ b/tests/test_discovery.py @@ -413,3 +413,132 @@ def test_reclassifying_a_materialised_video_removes_its_files( assert stats["shorts"] == 1 assert videos.get(conn, "vid00000001")["state"] == videos.SKIPPED_SHORT assert not path.exists() + + +# ------------------------------------------------- the min_keep_videos floor +# +# §5 defines retention as max(retention_days, min_keep_videos newest) and gives +# the reason: a channel that uploads every six weeks has nothing inside a 30-day +# window and would appear in Jellyfin as an empty series. reap.py implemented the +# second half; discovery did not, so it only protected videos that had already +# been materialised. Measured on the real 119 subscriptions: 441 episodes but 57 +# channels with none, 55 of them holding skipped_old rows. + + +def _old(conn, channel, count, *, start=1): + """`count` skipped_old videos, newest first at 2026-01-{start}...""" + made = [] + for index in range(count): + made.append(add_video( + conn, channel["id"], f"old{index + start:08d}", + upload_date=f"2026-01-{index + start:02d}", + state=videos.SKIPPED_OLD, + )) + return made + + +def test_a_channel_with_nothing_in_the_window_keeps_its_newest(conn, settings, channel): + _old(conn, channel, 8) + + promoted = discovery.top_up_to_min_keep(conn, settings, channel) + + assert promoted == 5 + listed = conn.execute( + "SELECT video_id FROM video WHERE state = ? ORDER BY upload_date DESC", + (videos.LISTED,), + ).fetchall() + # The five NEWEST, not the first five found. + assert [row["video_id"] for row in listed] == [ + "old00000008", "old00000007", "old00000006", "old00000005", "old00000004", + ] + + +def test_the_floor_counts_what_is_already_there(conn, settings, channel): + """A channel with 3 in-window videos needs only 2 older ones.""" + for index in range(3): + add_video(conn, channel["id"], f"new{index:08d}", upload_date="2026-08-10") + _old(conn, channel, 6) + + assert discovery.top_up_to_min_keep(conn, settings, channel) == 2 + + +def test_a_busy_channel_is_untouched(conn, settings, channel): + for index in range(9): + add_video(conn, channel["id"], f"new{index:08d}", upload_date="2026-08-10") + _old(conn, channel, 4) + + assert discovery.top_up_to_min_keep(conn, settings, channel) == 0 + assert conn.execute( + "SELECT count(*) FROM video WHERE state = ?", (videos.SKIPPED_OLD,) + ).fetchone()[0] == 4 + + +def test_topping_up_is_idempotent(conn, settings, channel): + """It runs every poll, every hour. Twice must not mean ten episodes.""" + _old(conn, channel, 8) + + first = discovery.top_up_to_min_keep(conn, settings, channel) + second = discovery.top_up_to_min_keep(conn, settings, channel) + + assert (first, second) == (5, 0) + assert conn.execute( + "SELECT count(*) FROM video WHERE state = ?", (videos.LISTED,) + ).fetchone()[0] == 5 + + +def test_aged_out_videos_are_never_revived(conn, settings, channel): + """They were on disk and were deleted. Reviving them presents months of old + episodes to Jellyfin as new, which is what the tombstone exists to stop.""" + for index in range(6): + add_video(conn, channel["id"], f"gone{index:08d}", + upload_date=f"2026-02-{index + 1:02d}", state=videos.AGED_OUT) + + assert discovery.top_up_to_min_keep(conn, settings, channel) == 0 + assert conn.execute( + "SELECT count(*) FROM video WHERE state = ?", (videos.AGED_OUT,) + ).fetchone()[0] == 6 + + +def test_shorts_and_livestreams_do_not_count_towards_the_floor(conn, settings, channel): + """They are excluded by policy and can never be episodes, so a channel whose + newest uploads are all Shorts must reach further back for long-form ones.""" + for index in range(4): + add_video(conn, channel["id"], f"shrt{index:08d}", + upload_date="2026-08-11", state=videos.SKIPPED_SHORT) + add_video(conn, channel["id"], "live0000001", + upload_date="2026-08-11", state=videos.SKIPPED_LIVE) + _old(conn, channel, 7) + + assert discovery.top_up_to_min_keep(conn, settings, channel) == 5 + + +def test_a_floor_of_zero_disables_it(conn, settings, channel): + settings.set("min_keep_videos", "0") + _old(conn, channel, 6) + + assert discovery.top_up_to_min_keep(conn, settings, channel) == 0 + + +def test_a_channel_with_no_videos_at_all_stays_empty(conn, settings, channel): + """Two of the real 119 have no long-form uploads whatsoever (their UULF + playlist 404s). There is nothing to promote and no directory should appear.""" + assert discovery.top_up_to_min_keep(conn, settings, channel) == 0 + + +def test_promoted_videos_survive_the_next_reap(conn, settings, channel): + """The whole point: the two halves of max(window, N newest) must agree. If + reap deleted what discovery just promoted, channels would flicker hourly.""" + from ytstream import reap + + _old(conn, channel, 5) + discovery.top_up_to_min_keep(conn, settings, channel) + for row in conn.execute( + "SELECT video_id FROM video WHERE state = ?", (videos.LISTED,) + ).fetchall(): + videos.mark_materialised( + conn, row["video_id"], rel_path=f"c/{row['video_id']}.strm", + season=2026, episode=1, upload_date="2026-01-01", duration=900, + title="t", + ) + + assert reap.candidates(conn, settings) == [] diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 9d3fee9..9a8c2d7 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -341,14 +341,37 @@ def test_growing_flag_means_no_wait_at_all(tmp_path): server.close() -def test_status_reports_the_serving_mode_and_grace(manager_factory): - """/healthz has to show this: it is the difference between 'plays' and - 'playback error', and it is otherwise invisible without the unit file.""" +def test_the_default_is_to_wait_for_a_complete_file(manager_factory): + """The default must not serve a partial file. Jellyfin responds to one by + dragging a gigabyte through the proxy to probe a fragmented MP4 that has no + duration in its header, then transcoding a stream it cannot seek: measured + 2026-08-13, 3 minutes to start, 6 seconds of video, permanent stall.""" + assert proxy.FIRST_BYTE_GRACE == float("inf") + + status = manager_factory().status() + assert status["mode"] == "wait-for-complete" + assert status["first_byte_grace_s"] is None + + +def test_status_is_valid_json_with_no_grace_cap(manager_factory): + """json.dumps emits `Infinity` for an infinite float, which is not JSON and + breaks any client that parses /healthz strictly.""" + payload = json.dumps(manager_factory().status()) + assert "Infinity" not in payload + assert json.loads(payload)["first_byte_grace_s"] is None + + +def test_status_reports_a_finite_grace_when_one_is_set(manager_factory): manager = manager_factory() + manager.strict = False + manager.first_byte_grace = 12.0 + status = manager.status() assert status["mode"] == "wait-then-stream" - assert status["first_byte_grace_s"] == proxy.FIRST_BYTE_GRACE + assert status["first_byte_grace_s"] == 12.0 + +def test_status_reports_growing_mode(manager_factory): growing = manager_factory(growing=True) assert growing.status()["mode"] == "growing" assert growing.status()["first_byte_grace_s"] == 0.0 diff --git a/tests/test_runner.py b/tests/test_runner.py index 5a7336e..4b49410 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -230,3 +230,37 @@ def test_summarise_is_a_single_line(): assert "channels=119" in text assert "materialised=7" in text assert "aged_out=8" in text + + +def test_run_prunes_channels_that_have_nothing_to_show(conn, settings, media_root, + monkeypatch): + """End to end: a subscribed channel whose directory exists only because + subscribe() wrote tvshow.nfo must not survive a run as an empty series.""" + from conftest import add_channel + from ytstream import runner + + empty = add_channel(conn, "UC" + "s" * 22, "Dead Channel", "dead-channel") + tree = media_root / "dead-channel" + tree.mkdir() + (tree / "tvshow.nfo").write_text("") + + assert runner.prune_empty_channels(conn) == 1 + assert not tree.exists() + + +def test_run_keeps_a_channel_with_a_materialised_video(conn, settings, media_root): + from conftest import add_channel, add_video + from ytstream import runner, videos + + row = add_channel(conn, "UC" + "t" * 22, "Alive", "alive") + tree = media_root / "alive" + (tree / "Season 2026").mkdir(parents=True) + (tree / "Season 2026" / "ep.strm").write_text("url") + video = add_video(conn, row["id"], "vid00000009") + videos.mark_materialised(conn, "vid00000009", + rel_path="alive/Season 2026/ep.strm", season=2026, + episode=1, upload_date="2026-08-01", duration=900, + title="t") + + assert runner.prune_empty_channels(conn) == 0 + assert tree.exists() diff --git a/tests/test_strm.py b/tests/test_strm.py index 14d30eb..649a2d1 100644 --- a/tests/test_strm.py +++ b/tests/test_strm.py @@ -310,3 +310,55 @@ def test_a_renamed_episode_removes_its_old_files(conn, settings, media_root, assert not old.exists() assert (media_root / second["rel_path"]).exists() assert len(list(media_root.rglob("*.strm"))) == 1 + + +# ------------------------------------- pruning channels with nothing to show +# +# §5: a channel with nothing inside the window must not leave an empty series in +# Jellyfin. runner creates directories lazily to honour that, but +# channels.subscribe() mkdirs to write tvshow.nfo and a poster, so every +# subscription got one regardless. Measured after approving all 119 real +# subscriptions: 57 empty series. + + +def test_prune_removes_a_channel_with_no_episodes(conn, media_root, channel): + tree = media_root / "clabretro" + tree.mkdir() + (tree / "tvshow.nfo").write_text("") + (tree / "poster.jpg").write_bytes(b"x" * 100) + + assert strm.prune_if_no_episodes(channel) is True + assert not tree.exists() + + +def test_prune_leaves_a_channel_that_has_episodes(conn, settings, media_root, + channel, video, no_thumbs): + strm.materialise(conn, settings, channel, video) + + assert strm.prune_if_no_episodes(channel) is False + assert (media_root / "clabretro").exists() + + +def test_prune_finds_episodes_in_any_season(conn, media_root, channel): + """The .strm lives a directory down, so a shallow check would delete it.""" + deep = media_root / "clabretro" / "Season 2019" + deep.mkdir(parents=True) + (deep / "clabretro - S2019E1010 - Old [dQw4w9WgXcQ].strm").write_text("url") + + assert strm.prune_if_no_episodes(channel) is False + assert deep.exists() + + +def test_prune_is_a_no_op_when_there_is_no_directory(conn, channel): + assert strm.prune_if_no_episodes(channel) is False + + +def test_prune_refuses_a_blank_channel_directory(conn, media_root): + from conftest import add_channel + + row = add_channel(conn, "UC" + "r" * 22, "Blank", "") + keep = media_root / "keep" + keep.mkdir() + + assert strm.prune_if_no_episodes(row) is False + assert keep.exists() diff --git a/ytstream/discovery.py b/ytstream/discovery.py index a412509..e514d0f 100644 --- a/ytstream/discovery.py +++ b/ytstream/discovery.py @@ -424,6 +424,62 @@ def rescan_channel( return cursor.rowcount +def top_up_to_min_keep( + conn: sqlite3.Connection, settings: Settings, channel: sqlite3.Row +) -> int: + """Promote the newest `skipped_old` rows until the channel has `min_keep_videos` + videos that will end up on disk. Returns how many were promoted. + + §5 defines retention as **`max(retention_days, min_keep_videos newest)`**, and + `reap.candidates()` implements the second half — but only for videos that were + materialised in the first place. Discovery judges every video against the + window alone and tombstones the rest as `skipped_old`, so a channel that has + not uploaded inside the window materialises *nothing* and appears in Jellyfin + as an empty series. That is the precise outcome §5 introduced this setting to + prevent: "keeps its last 5 videos permanently visible instead of showing an + empty shelf". + + Measured 2026-08-13 on the real 119 subscriptions: 441 videos materialised — + exactly the in-window count Phase 0 predicted — but **57 channels had zero + episodes**, and 55 of those were sitting on `skipped_old` rows. §5's own cost + estimate was `441 + (52 x 5) ~= 700`, so the shortfall was in the code, not in + the measurement. + + Only `materialised` and `listed` count towards the floor: `skipped_short` and + `skipped_live` are excluded by policy and can never be episodes, so a channel + whose five newest uploads are all Shorts correctly reaches further back for + long-form ones. + + `aged_out` rows are deliberately not revived, for the reason `rescan_channel` + gives. reap() protects the newest `min_keep_videos` on disk, so anything + promoted here sits inside its protected set and will not be deleted straight + back out — the two halves of the rule now agree. + """ + keep = max(0, settings.get_int("min_keep_videos")) + if not keep: + return 0 + + have = conn.execute( + "SELECT count(*) FROM video WHERE channel_pk = ? AND state IN (?, ?)", + (channel["id"], videos.MATERIALISED, videos.LISTED), + ).fetchone()[0] + shortfall = keep - have + if shortfall <= 0: + return 0 + + with conn: + cursor = conn.execute( + "UPDATE video SET state = ? WHERE id IN (" + " SELECT id FROM video WHERE channel_pk = ? AND state = ?" + " ORDER BY upload_date DESC, id DESC LIMIT ?)", + (videos.LISTED, channel["id"], videos.SKIPPED_OLD, shortfall), + ) + if cursor.rowcount: + log.info("%s: kept %d older video(s) to reach the %d-video floor", + channel["title"], cursor.rowcount, keep) + return cursor.rowcount + + def poll_all( conn: sqlite3.Connection, settings: Settings, channel_pk: int | None = None ) -> dict: @@ -434,7 +490,7 @@ def poll_all( rows = channels.all_channels(conn) totals = {"channels": 0, "queued": 0, "old": 0, "known": 0, "repaired": 0, - "titled": 0, "shorts": 0, "live": 0, "failed": 0} + "titled": 0, "shorts": 0, "live": 0, "kept": 0, "failed": 0} for channel in rows: totals["channels"] += 1 if not channel["backfilled"]: @@ -450,7 +506,17 @@ def poll_all( if "error" in stats: totals["failed"] += 1 - for key in ("queued", "old", "known", "repaired", "titled", "shorts", "live"): + else: + # After the poll, so it sees this run's discoveries and only reaches + # back for older videos when the window genuinely left the channel + # short. Skipped on a failed poll: a channel we could not read looks + # empty, and topping it up from tombstones would be guessing. + kept = top_up_to_min_keep(conn, settings, channel) + if kept: + stats["kept"] = kept + + for key in ("queued", "old", "known", "repaired", "titled", "shorts", + "live", "kept"): totals[key] += stats.get(key, 0) log.info("%s: %s", channel["title"], stats) return totals diff --git a/ytstream/runner.py b/ytstream/runner.py index 5dbf9ed..83b2ee1 100644 --- a/ytstream/runner.py +++ b/ytstream/runner.py @@ -80,6 +80,23 @@ def materialise_all( return stats +def prune_empty_channels(conn: sqlite3.Connection) -> int: + """Remove directories for subscribed channels that have no episodes to show. + + Runs after materialising and reaping, so it only sees the settled state. See + `strm.prune_if_no_episodes` for why these directories exist at all. + """ + pruned = 0 + for channel in conn.execute( + "SELECT c.* FROM channel c WHERE NOT EXISTS (" + " SELECT 1 FROM video v WHERE v.channel_pk = c.id AND v.state = ?)", + (videos.MATERIALISED,), + ).fetchall(): + if strm.prune_if_no_episodes(channel): + pruned += 1 + return pruned + + def run( conn: sqlite3.Connection, settings: Settings, channel_pk: int | None = None ) -> dict: @@ -94,6 +111,7 @@ def run( result["poll"] = discovery.poll_all(conn, settings, channel_pk) result["materialise"] = materialise_all(conn, settings) result["reap"] = reap.run(conn, settings) + result["pruned"] = prune_empty_channels(conn) if result["materialise"]["materialised"] or result["reap"]["aged_out"]: jellyfin.from_settings(settings).refresh() @@ -124,6 +142,10 @@ def summarise(result: dict) -> str: ] if sync.get("refused"): parts.append(f"SYNC_REFUSED={sync['refused']}") + if poll.get("kept"): + parts.append(f"kept_for_floor={poll['kept']}") + if result.get("pruned"): + parts.append(f"pruned_empty={result['pruned']}") if made.get("errors"): parts.append(f"errors={made['errors']}") return " ".join(parts) diff --git a/ytstream/strm.py b/ytstream/strm.py index 9ba31db..b64f6d7 100644 --- a/ytstream/strm.py +++ b/ytstream/strm.py @@ -207,6 +207,39 @@ def remove(video: sqlite3.Row) -> int: return removed +def prune_if_no_episodes(channel: sqlite3.Row) -> bool: + """Remove a subscribed channel's directory if it holds no episodes. + + §5 says a channel with nothing inside the window must not leave an empty + series in Jellyfin, and `runner.materialise_pending` honours that by creating + the directory lazily, from the first episode. But `channels.subscribe()` calls + `_write_show_metadata()`, which mkdirs to write tvshow.nfo and the poster — + so every subscription got a directory whether or not it had anything to show. + Measured 2026-08-13 after approving all 119 subscriptions: **57 empty series**. + + `top_up_to_min_keep()` fixes 55 of those by keeping older videos. This handles + the remainder — the channels with no long-form uploads at all, whose UULF + playlist 404s (2 of the real 119). It runs after materialising, so "no + episodes" means none were written this run either. + + Only ever removes a directory with no `.strm` in it, so an active channel is + never touched. `subscribe()` runs once per channel, so a pruned directory does + not come back every hour — it reappears only when the channel finally uploads + something and `write_show()` recreates it. + """ + directory = channel_dir(channel) + if directory == config.MEDIA_ROOT or not str(channel["dir_name"]).strip(): + log.error("refusing to prune %s: unsafe channel directory", directory) + return False + if not directory.is_dir(): + return False + if any(directory.rglob("*.strm")): + return False + shutil.rmtree(directory) + log.info("pruned %s: subscribed but no episodes to show", directory) + return True + + def remove_channel_tree(channel: sqlite3.Row) -> bool: """Delete a whole channel directory, on unsubscribe.