Bound time-to-first-byte so a first play actually plays
Playback failed in Jellyfin the day after deployment, for uncached videos only. The handler blocked on the full download-and-mux before sending anything at all -- not even response headers -- so a 46-minute upload sat silent for 79 seconds and the client gave up. The proxy counted it a success, which is why /healthz and doctor both looked fine. Yesterday's "DirectPlay verified" only ever ran against videos already pulled during testing, so the first-play path was never exercised. The output is already a fragmented MP4, so it is readable while being written; a finished file only buys a correct duration and working seeks. Cap the wait at --first-byte-grace (12s, explicit in the unit) and stream whatever has not muxed by then. Measured: 156s -> 12.0s TTFB on a 66-minute upload, with ranges on a complete file unchanged. The cost is a first play with no seek bar when the grace is missed. Every later play of that video is perfect. Also fixes a hang found while testing this: the streaming loop waited on "finished and good" rather than "finished", so a producer that died after writing some bytes held the connection for the full 45s stall timeout and then dropped it. The stale yt-dlp warning pointed at youtube-automate's venv, the tree we are decommissioning; it now names ytstream's. /healthz reports the serving mode and grace. Eight new proxy tests, 353 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+142
-2
@@ -105,8 +105,9 @@ def growing_server(tmp_path):
|
||||
yield from _serve(manager)
|
||||
|
||||
|
||||
def _serve(manager):
|
||||
server = ThreadingHTTPServer(("127.0.0.1", 0), proxy.make_handler(manager, 30))
|
||||
def _serve(manager, wait_timeout=30, grace=proxy.FIRST_BYTE_GRACE):
|
||||
server = ThreadingHTTPServer(
|
||||
("127.0.0.1", 0), proxy.make_handler(manager, wait_timeout, grace))
|
||||
threading.Thread(target=server.serve_forever, daemon=True).start()
|
||||
try:
|
||||
yield server.server_address[1]
|
||||
@@ -214,6 +215,145 @@ def test_growing_file_is_chunked_and_tracks_to_eof(growing_server):
|
||||
assert body == BODY
|
||||
|
||||
|
||||
# ------------------------------------------------- time-to-first-byte grace
|
||||
#
|
||||
# Waiting for a complete mux is the only way to get a correct duration and
|
||||
# working seeks, but the wait grows with the video's length and sends nothing at
|
||||
# all -- not even headers -- while it lasts. A 46-minute upload blocked for 79s
|
||||
# and Jellyfin reported a playback error on every first play of a long video.
|
||||
# These tests pin the compromise: wait, but only for a bounded grace.
|
||||
|
||||
|
||||
def _partial(tmp_path, *, finishes_after):
|
||||
"""A half-written file that completes after `finishes_after` seconds."""
|
||||
path = tmp_path / "out.mp4"
|
||||
path.write_bytes(BODY[:1000])
|
||||
session = StubSession(path, finished=False)
|
||||
|
||||
def finish():
|
||||
time.sleep(finishes_after)
|
||||
with open(path, "ab") as handle:
|
||||
handle.write(BODY[1000:])
|
||||
session.final.set()
|
||||
|
||||
threading.Thread(target=finish, daemon=True).start()
|
||||
return session
|
||||
|
||||
|
||||
def test_a_slow_mux_streams_instead_of_blocking(tmp_path):
|
||||
"""The bug: this used to block for --wait-timeout with the socket silent."""
|
||||
session = _partial(tmp_path, finishes_after=1.0)
|
||||
server = _serve(StubManager(session), wait_timeout=30, grace=0.2)
|
||||
port = next(server)
|
||||
try:
|
||||
started = time.monotonic()
|
||||
status, head, body = request(port, {"Range": "bytes=0-"})
|
||||
elapsed = time.monotonic() - started
|
||||
|
||||
assert status == 200
|
||||
assert head.get("Transfer-Encoding") == "chunked"
|
||||
assert body == BODY
|
||||
# The whole point: it returned on the mux's schedule, not the timeout's.
|
||||
assert elapsed < 10, f"blocked {elapsed:.1f}s -- grace not applied"
|
||||
finally:
|
||||
server.close()
|
||||
|
||||
|
||||
def test_a_mux_that_lands_inside_the_grace_keeps_ranges_and_seeking(tmp_path):
|
||||
"""The fast path must survive: a short video still gets a real 206."""
|
||||
session = _partial(tmp_path, finishes_after=0.2)
|
||||
server = _serve(StubManager(session), wait_timeout=30, grace=10)
|
||||
port = next(server)
|
||||
try:
|
||||
status, head, body = request(port, {"Range": "bytes=100-199"})
|
||||
|
||||
assert status == 206
|
||||
assert head["Content-Range"] == f"bytes 100-199/{TOTAL}"
|
||||
assert head.get("Transfer-Encoding") is None
|
||||
assert body == BODY[100:200]
|
||||
finally:
|
||||
server.close()
|
||||
|
||||
|
||||
def test_grace_at_the_wait_timeout_restores_strict_finished_file_only(tmp_path):
|
||||
"""The old behaviour stays reachable, for a caller that really wants it."""
|
||||
session = _partial(tmp_path, finishes_after=60)
|
||||
server = _serve(StubManager(session), wait_timeout=0.3, grace=0.3)
|
||||
port = next(server)
|
||||
try:
|
||||
status, _, _ = request(port)
|
||||
assert status == 504
|
||||
finally:
|
||||
server.close()
|
||||
|
||||
|
||||
def test_a_failed_producer_with_bytes_is_served_rather_than_erroring(tmp_path):
|
||||
"""Most of a video beats none of it -- except in strict mode, where the
|
||||
caller asked for a good file and must be told it cannot have one."""
|
||||
path = tmp_path / "out.mp4"
|
||||
path.write_bytes(BODY)
|
||||
session = StubSession(path, finished=True)
|
||||
session.failed = "ffmpeg exited 1"
|
||||
|
||||
lenient = _serve(StubManager(session), wait_timeout=30, grace=1)
|
||||
port = next(lenient)
|
||||
try:
|
||||
assert request(port)[0] == 200
|
||||
finally:
|
||||
lenient.close()
|
||||
|
||||
strict = _serve(StubManager(session), wait_timeout=1, grace=1)
|
||||
port = next(strict)
|
||||
try:
|
||||
assert request(port)[0] == 502
|
||||
finally:
|
||||
strict.close()
|
||||
|
||||
|
||||
def test_a_failed_producer_with_no_bytes_is_always_502(tmp_path):
|
||||
path = tmp_path / "out.mp4"
|
||||
path.write_bytes(b"")
|
||||
session = StubSession(path, finished=True)
|
||||
session.failed = "yt-dlp[video] exited 1"
|
||||
|
||||
server = _serve(StubManager(session), wait_timeout=30, grace=1)
|
||||
port = next(server)
|
||||
try:
|
||||
assert request(port)[0] == 502
|
||||
finally:
|
||||
server.close()
|
||||
|
||||
|
||||
def test_growing_flag_means_no_wait_at_all(tmp_path):
|
||||
"""--growing is a grace of zero, and must not be overridden by the default."""
|
||||
session = _partial(tmp_path, finishes_after=0.5)
|
||||
manager = StubManager(session, growing=True)
|
||||
server = _serve(manager, wait_timeout=30, grace=proxy.FIRST_BYTE_GRACE)
|
||||
port = next(server)
|
||||
try:
|
||||
started = time.monotonic()
|
||||
status, head, _ = request(port)
|
||||
assert status == 200
|
||||
assert head.get("Transfer-Encoding") == "chunked"
|
||||
# Would have waited the full 12s default grace if growing were ignored.
|
||||
assert time.monotonic() - started < 5
|
||||
finally:
|
||||
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."""
|
||||
manager = manager_factory()
|
||||
status = manager.status()
|
||||
assert status["mode"] == "wait-then-stream"
|
||||
assert status["first_byte_grace_s"] == proxy.FIRST_BYTE_GRACE
|
||||
|
||||
growing = manager_factory(growing=True)
|
||||
assert growing.status()["mode"] == "growing"
|
||||
assert growing.status()["first_byte_grace_s"] == 0.0
|
||||
|
||||
|
||||
# ------------------------------------------------------------------- routing
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user