Repository navigation
fix(preview): the proxy copy for the video at the playhead is made ahead of the rest - #6586
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · b9bfce6f
The priority plumbing works. Changed suites pass locally (core mediaProxy.test.ts 63/63, studio-server preview.test.ts + proxyTranscoder 218/218). The only blocker is the red Comments run. ↓ inline
Scope
Read all 8 files in full, plus proxyTranscoder.ts resolveProxy/prioritize/enqueue and the lastRelativeTime writers in media.ts. Adding a custom request header could force a CORS preflight, but that isn't a new risk: the existing X-HF-Proxy-Progress response header already assumes same-origin. Several CI workflows were still running when I posted this. No prior reviews.
— Genesis
|
|
||
| export const PREVIEW_PROXY_RETRY_PARAM = "hf-proxy-retry"; | ||
| export const PREVIEW_PROXY_PROGRESS_HEADER = "X-HF-Proxy-Progress"; | ||
| /** Sent with "1" while the video is at the playhead: its copy goes ahead of the rest in the queue. */ |
There was a problem hiding this comment.
blocker: the Comments workflow is red on this line: RAISED comment share: packages/core/src/previewProxyBox.ts is 6.8% (5/73), at the base 5.6% (4/71). Drop the line; the constant's name and its use in proxyWait.ts already say what it does.
| /** Sent with "1" while the video is at the playhead: its copy goes ahead of the rest in the queue. */ |
| retryProxyCopy(project.dir, file, proxyVariant, proxyBox); | ||
| } | ||
| servedPath = await waitForProxy(resolveProxy(project.dir, file, proxyVariant, proxyBox), 0); | ||
| const priority = c.req.header(PREVIEW_PROXY_PRIORITY_HEADER) === "1"; |
There was a problem hiding this comment.
important: priority is sticky on the server. prioritize() never demotes a job, and enqueue() puts a new priority job behind the priority jobs already queued. So the body's claim that "a clip the playhead leaves stops asking for priority" only holds on the client. Failure case: paused, you rest on clips C, D and E for one poll each (≥2 s), then land on F while both slots are busy. F now queues behind C, D and E, so the copy under the playhead is no longer made first. A single jump is fine. If you want the claim to hold after several jumps, two fixes would work: make the newest priority ask go to the front (waitQueue.unshift for priority), or demote a job when a later ask for it arrives without the header. Otherwise, narrow the PR body to match what the code does.
…ask moves it to the front
…handling into helpers
Edit accuracy: accurate 271 (base branch 271), smooth 135 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
What
When the preview waits for a video's proxy copy (a codec the browser cannot decode, or a lighter copy), the request for the video that is at the playhead now carries
X-HF-Proxy-Priority: 1. The preview route passes that to the transcoder's existingpriorityoption, which queues that copy ahead of the rest and never refuses it for a full queue.Why
A project of many HEVC clips on a machine with no HEVC decoder shows only the captions over black until the proxies are made. The server pre-warms every copy in project order and transcodes two at a time with a bounded queue, so the clip the person is looking at waited behind the others, and once the queue was full its request got a 503 and retried every 5 s with the rest. The
priorityoption was written for exactly this ("the copy a viewer is waiting on") but no route ever set it.Measured on a Linux box with 24 portrait HEVC clips (8 s each): the first picture came after about 20 to 30 s and all 24 copies after about 53 to 65 s, with nothing on screen saying why. This change does not shorten the total; it makes the video under the playhead the first one made, including after a jump to another clip.
How
isActiveAtPlayheadinmedia.tsreads the sync state the tick already keeps for a clip inside its window.waitForServedProxysends the header on each ask while that holds, so a clip the playhead leaves stops asking for priority and one it enters starts. On the server the copy asked for last goes first, and a repeat ask moves a queued copy to the front, so after several jumps the clip under the playhead is still the one made next. The header is on the poll, not the URL, so the element'ssrcand the cached copy's etag are unchanged.Tests
mediaProxy.test.ts: the ask carries the header while the clip is at the playhead and not after the playhead leaves.preview.test.ts: the header becomespriority: true; the existing exact-argument assertions now include{ priority: false }.proxyTranscoder.test.ts: the copy asked for last runs first, and a repeat ask moves it to the front.