Fetch the next track while the current one plays - #29
Merged
Conversation
Every transition stalled for as long as yt-dlp took to extract and deliver first bytes - 3 to 8 seconds per track, because the player only started resolving the next one after the current ended. That work cannot be made cheaper. The command resolves metadata for the embed and the player loop then launches a second yt-dlp that redoes the whole extraction, and skipping format processing does not help: measured 2.83s with process=False against 2.70s for the full extraction. The cost is the round trip to YouTube. So it moves off the critical path instead. Timing is deliberate. A prefetched yt-dlp sits blocked on a full pipe until we consume it, and YouTube drops connections that idle too long, so fetching starts PREFETCH_LEAD_SECONDS before the current track ends rather than at its start. Short tracks and live streams, which have no useful end to count back from, fetch immediately. The dangerous part is not the fetch but the discard. Skip, remove, shuffle and previous all change what plays next, so a prefetched stream is matched by identity against the track actually being advanced to - not by URL, since two queue entries for the same song are different tracks. A mismatch is closed, not reused and not leaked. destroy() closes a ready prefetch and cancels one in flight; leaking there would put back exactly the zombies that #6 removed. Track-loop mode skips prefetching entirely: the next track is the current one. Measured on the host: a transition that costs 7.13s today costs 0.000s when the stream was already fetched. Closes #27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #27
The problem
The player only started resolving the next track after the current one ended, so every transition stalled on yt-dlp — 3 to 8 seconds, every time.
Measured on the production host, playing a YouTube URL:
Two separate extractions of the same video: the command resolves metadata for the "Now Playing" embed, then the player loop launches a new yt-dlp that redoes the whole thing to download.
That work cannot be made cheaper. I tested the obvious idea — skipping format processing, since
spawn_streamresolves formats again anyway:process=FalseNo difference. The cost is the round trip to YouTube, not local work. So this PR moves it off the critical path instead of trying to shrink it.
The result
Timing is deliberate
A prefetched yt-dlp sits blocked on a full pipe until we consume it, and YouTube drops connections that idle too long — fetching at the top of a six-minute track would leave a dead stream by the time it was wanted.
So fetching starts
PREFETCH_LEAD_SECONDS(30s) before the current track ends. Short tracks and live streams, which have no useful end to count back from, fetch immediately.The risky part is the discard, not the fetch
A prefetched stream is a live yt-dlp process. One that is fetched and never claimed is precisely the leak that produced a 19-day zombie before #6. Most of the new tests are about that.
destroy()with a ready prefetchdestroy()with a fetch in flightMatching is by identity, not URL. Two queue entries for the same song are different tracks — one has been claimed, the other has not. Matching on URL would hand the same stream to both.
test_matching_is_by_identity_not_by_urlpins it.Verification
275 passed, three consecutive runs, 17 new. The new tests use an async fixture soget_running_loopsees the loop pytest-asyncio actually runs the test on, and they wait onrun_in_executorcompletions rather than assuming a few event-loop yields are enough — the first draft asserted too early and failed for that reason.Covered: when the delay fires for long, short and live tracks; that the front of the queue is what gets fetched; that a second call does not fetch twice; every discard path above; and that a track ending early (skip, stop, effect change) returns from the wait promptly instead of sitting on its timeout.