Skip to content

fix(mediaplayer): detect VOD seekability under Proton/Wine#975

Merged
dooly123 merged 3 commits into
BasisVR:developerfrom
towneh:fix/mediaplayer-proton-vod-pacing
Jul 21, 2026
Merged

fix(mediaplayer): detect VOD seekability under Proton/Wine#975
dooly123 merged 3 commits into
BasisVR:developerfrom
towneh:fix/mediaplayer-proton-vod-pacing

Conversation

@towneh

@towneh towneh commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

VOD played at delivery speed for Linux users running the Windows build under Proton, reported
after a test session as "playing at about 4x speed" through the content. The multiplier isn't
fixed: it's download bandwidth over stream bitrate, so it varies by connection.

The live-vs-VOD probe decides whether to pace delivery, and it establishes seekability from
Content-Length:

BOOL haveLen = WinHttpQueryHeaders(h->request,
    WINHTTP_QUERY_CONTENT_LENGTH | WINHTTP_QUERY_FLAG_NUMBER64, ...);
h->seekable = (haveLen && clen > 0 && rangeable) ? 1 : 0;

Wine's winhttp doesn't implement WINHTTP_QUERY_FLAG_NUMBER64. Its QUERY_MODIFIER_MASK covers
only REQUEST_HEADERS | SYSTEMTIME | NUMBER, so the flag isn't stripped from the attribute index,
the header lookup misses, and the query returns FALSE. Under Proton haveLen was therefore always
false, seekable stayed 0, paced and pace_delivery stayed 0, and pace_gate() returned
immediately, leaving the demuxer to consume the file as fast as the CDN would serve it. The comment
above the detection in basis_media_core.c describes that outcome exactly.

The same failed query left content_length at -1, so seeking was unavailable for those users too.
That has a knock-on in multiplayer: late joiners are synced by seeking to the owner's playhead, so
a Proton client joining an in-progress VOD couldn't take the seek and started from zero, while also
racing through the content.

Content-Length is now read as a string and parsed. That's the default query path on native Windows,
and it avoids the >4GB truncation that the 32-bit WINHTTP_QUERY_FLAG_NUMBER (which Wine does
implement) would bring. The two STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER queries are untouched.

Parsing headers means validating them, since they're remote input:

  • The whole field must be a plain in-range integer. _wcstoi64 accepts a numeric prefix
    ("123junk" as 123) and saturates on overflow, either of which hands a bogus finite length to
    the seek and pacing logic.
  • On a 206 the Content-Length covers the part that came back, so a range-capping proxy could
    present a truncated length as the whole file. Only Content-Range can establish a total there.
  • Content-Range must match the exact bytes <first>-<last>/<complete> grammar with coherent
    bounds, and its first byte must be 0. That's what the probe asks for, so a response describing
    any other start isn't the body the caller thinks it's reading.

Body length and representation total are different quantities answering different questions, so
they're tracked separately. Finite and rangeable is enough to call a source on-demand and pace it,
while the complete length stays unknown unless it can be proven.

Includes the rebuilt Windows x64 plugin (RIST-enabled, Unity 6000.5.4f1 PluginAPI headers). Android
is unaffected, since basis_win_http.cpp is Windows-only.

Required checks

All boxes below must be ticked before this PR can merge. If a check is genuinely N/A, tick it anyway and explain under Notes.

  • Tested — I built and ran this locally. The change works in the editor and (where relevant) in a built player.
  • Transform access is combined and limited — In hot paths, transform reads/writes go through TransformAccessArray or are otherwise batched. I have not added per-frame transform.position / transform.rotation / transform.localPosition calls inside loops. Whenever I need both position and rotation, I use the combined APIs — SetPositionAndRotation / SetLocalPositionAndRotation for writes, GetPositionAndRotation / GetLocalPositionAndRotation for reads — instead of two separate property accesses; the combined call does one local-to-world matrix traversal instead of two.
  • Addressables used for asset/memory loading — Any new asset loads go through Addressables. No new Resources.Load, no direct asset references that pull large content into memory on scene load.
  • No new GetComponent / AddComponent where avoidable — Where unavoidable, the result is cached on a field, and any GetComponent<T> is replaced with TryGetComponent<T>(out var x) — bare GetComponent will be denied. TryGetComponent is the modern API (Unity 2019.2+) and skips the Editor-only GC allocation GetComponent causes when a component is missing: Unity wraps the null return in a managed "fake null" object so its overloaded == operator can still detect destroyed C++ objects, and constructing that wrapper allocates; TryGetComponent returns a bool plus out parameter and never builds the wrapper. None of these calls run inside Update, LateUpdate, FixedUpdate, jobs, or other per-frame code paths.
  • Per-frame work is scheduled through BasisEventDriver — Any new per-frame work hooks into BasisEventDriver rather than adding standalone Update / LateUpdate / FixedUpdate callbacks on a MonoBehaviour.
  • Anything added to BasisEventDriver is bulletproof, or guarded by try/catchBasisEventDriver runs the single per-frame tick that drives the whole framework (network apply, local player sim, blendshapes, JigglePhysics, nameplates, and more) as one sequential chain. An unhandled exception anywhere in that chain aborts the rest of the tick, so every step after the throwing one is silently skipped for that frame. New work added to the driver must either be guaranteed not to throw, or be wrapped in a try/catch that contains the failure and surfaces it through BasisDebug — logged once / rate-limited, never every frame (see the existing HVRBasisBuiltInAddresses.Simulate() guard for the pattern). Expect this to be scrutinized closely in review.
  • Considered jobification — I asked whether this work can be moved to a Unity Job (Burst-compiled where possible). If it can, it is. If it cannot, the reason is in Notes.
  • No needless { get; set; } properties or access lockdowns — Public fields are fine; Basis is meant to be read and modified freely, so don't wall things off private/internal without a real reason. Don't wrap a field in { get; set; } when the accessors do nothing — property accessors have a real performance cost vs direct field access, and the lead maintainer prefers plain fields (or a method / setter-only property when only the setter needs logic) over a noop-getter pair. For .Instance singletons, callers reassigning Type.Instance is allowed; if that would break your code, log a warning or throw — don't block the assignment. Locking down access is not your call.
  • Camera access goes through BasisLocalCameraDriver — Code that needs the local camera (transform, projection, rig data, etc.) pulls it from BasisLocalCameraDriver rather than looking one up itself. Don't roll a separate camera discovery path.
  • Logging uses BasisDebug — All new logging calls go through BasisDebug.Log / BasisDebug.LogWarning / BasisDebug.LogError (with an appropriate LogTag) instead of UnityEngine.Debug.Log / Debug.LogWarning / Debug.LogError. BasisDebug routes through Basis's tagged, color-coded logger and respects the project-wide LoggingDisabled toggle so logging can be killed at runtime; bare Debug.Log calls bypass that and will be denied.
  • No scene-wide discovery for dependencies — New code is architected so it does not need FindObjectOfType / FindObjectsOfType / GameObject.Find / FindGameObjectsWithTag to locate what it depends on. References are wired in — registered through an existing manager/driver, injected at init, or passed in by the caller — rather than discovered by scanning the scene at runtime. If a scene scan is genuinely unavoidable, justify it under Notes.
  • No allocations in hot paths — Per-frame code (Update / LateUpdate / FixedUpdate, simulation loops, jobs, anything called once per frame or more) does not allocate. No new on reference types, no LINQ, no string concatenation/interpolation, no boxing, no foreach over interface-typed collections. Allocate once at init and reuse the buffer.
  • No debugging in hot paths — No log calls of any kind on per-frame paths, including BasisDebug. Hot-path logging floods the console and incurs cost on every frame regardless of whether the message is filtered out downstream. If a hot-path log is needed while iterating, gate it behind #if UNITY_EDITOR and remove (or leave gated) before merge.
  • Hot-path collection access is optimized — Cache .Count (lists) / .Length (arrays) into a local int before the loop instead of re-reading the property each iteration. Prefer T[] (with a separate length int when the array is over-sized) over List<T> where the data is hot — Unity's mono BCL doesn't expose CollectionsMarshal.AsSpan(List<T>), so a list can't be fed into Span<T> / unsafe paths cleanly. Where the perf justifies it, drop into Span<T> / ref locals / Unsafe.As / unsafe pointer code to skip bounds checks and copies, and call out the invariants you're relying on under Notes so reviewers can sanity-check them.

Testing details

Tick the platforms you actually tested on. Leave the rest unticked — these are informational and do not block merge.

  • Windows
  • Linux
  • Android
  • iOS
  • macOS

Input / control mode coverage:

  • Tested in VR (note headset under Notes)
  • Tested in desktop / non-VR mode
  • Tested with phone controls (mobile touch input)
  • N/A — change does not touch player/XR/input code

Where applicable, confirm these flows still work after your changes:

  • Hot-switching (desktop ↔ VR mode swap at runtime)
  • Avatar swapping
  • Server swapping (joining / leaving / changing servers)
  • N/A — change does not touch any of the above

Notes

Verified on Windows; the Proton path itself is not verified. Being straight about the split,
because it matters for how much weight to give this.

What was covered: a Windows x64 standalone build, playing HTTPS VOD across MP4 and audio-only
lanes. Every HTTPS VOD open goes through basis_win_http_open, so the new parsing ran on every
one of them. The test endpoints answer 206 with Content-Range, which means the Content-Range
path was exercised against real servers and produced correct results: duration reported correctly,
seek works in both directions, and a late joiner syncs to the owner's playhead, which depends on
the same probe. So this is positive confirmation the new parsing computes the right total, not
just an absence of crashes.

What was not covered: Proton itself. I don't have a Linux machine running it, and there's no Linux
lane in the media player's TESTING.md to run against. The Wine behaviour behind the diagnosis is
from reading Wine's dlls/winhttp/request.c, where QUERY_MODIFIER_MASK omits
WINHTTP_QUERY_FLAG_NUMBER64, rather than from observing it on a device. If anyone here runs
Basis under Proton, a single VOD playing at 1x rather than racing would confirm it, and the seek
bar working would confirm the knock-on fix.

Android is unaffected and was not tested for this: basis_win_http.cpp is Windows-only, and the
Android path uses basis_jni_https.c, which this PR doesn't touch.

The header parsers were exercised directly rather than by inspection, using a harness that
#includes the translation unit under test so it runs the shipped statics rather than a copy that
could drift. 30 cases pass, covering the ordinary nginx 206, capped ranges, both * forms,
incoherent bounds, wrong units, whitespace inside the grammar, non-zero start offsets, numeric
prefixes and int64 overflow. It isn't committed here: it's Windows-only, and the existing native
gates (fuzz-demux, conformance-demux) run on Linux, so wiring it in belongs with the CI work
rather than bolted onto this fix. Happy to add it if you'd prefer it landed together.

The gameplay and perf checks above are N/A: this is a native HTTP header query plus the rebuilt
binary. No C# is touched, so nothing here goes near transforms, Addressables, components,
BasisEventDriver, the camera driver, logging, scene discovery, or any per-frame path.

One deliberate non-change, since it looks like an inconsistency on a close read.
Accept-Ranges: bytes without a 206 still marks a source seekable for pacing purposes, and that
is intentional. Seeking itself is gated separately and more strictly: basis_win_http_can_reseek
and basis_win_http_reseek both additionally require range_ok, which is only set on a 206. So a
server that merely advertises ranges is treated as on-demand and paced correctly, but never has a
ranged re-request issued against it.

towneh added 3 commits July 20, 2026 09:35
VOD played at delivery speed for Linux users running the Windows build under
Proton, reported as roughly 4x through the content. The multiplier is not
fixed; it is download bandwidth over stream bitrate.

The live-vs-VOD probe established seekability from Content-Length queried with
WINHTTP_QUERY_FLAG_NUMBER64. Wine's winhttp does not implement that flag - its
QUERY_MODIFIER_MASK covers only REQUEST_HEADERS, SYSTEMTIME and NUMBER - so it
is left in the attribute index, the header lookup misses, and the query fails.
Under Proton the source therefore always looked non-seekable, delivery pacing
never engaged, and pace_gate() became a no-op, letting the demuxer consume the
file as fast as the CDN would serve it. The same failed query left
content_length at -1, so seeking was unavailable there too, which also cost
those clients the seek that syncs a late joiner to the owner's playhead.

Read the length headers as strings and parse them instead. That is the default
query path on native Windows and avoids the 4GB truncation the 32-bit
WINHTTP_QUERY_FLAG_NUMBER would bring.

Parsing them means validating them, since these are remote input:

- The whole field must be a plain in-range integer. _wcstoi64 would accept a
  numeric prefix ("123junk" as 123) and saturate to the signed maximum on
  overflow, either of which hands a bogus finite length to the seek and pacing
  logic.
- On a 206 the Content-Length covers the part that came back, so a
  range-capping proxy could present a truncated length as the whole file. Only
  Content-Range can establish a total there.
- Content-Range must match the exact "bytes <first>-<last>/<complete>" grammar
  with coherent bounds. The grammar carries no whitespace of its own, and a
  value must not pass on the strength of its tail. Either "*" form is unknown.
- Its first byte must be 0, because that is what the probe requests. A response
  describing any other start is not the body the caller believes it is reading.

Body length and representation total are distinct quantities, and they answer
different questions, so they are now tracked separately. Finite and rangeable
is enough to call a source on-demand and pace it; the complete length stays
unknown unless it can be proven. Conflating them would have meant a server that
states no total looking non-seekable, which would replay the same
fast-forward bug by another route.
The Windows build under a compatibility layer runs against reimplemented
WinHTTP, Media Foundation and D3D11, so no native Windows lane covers it.
Record what to check there and that it needs a real Proton user.
Built RIST-enabled against the Unity 6000.5.4f1 PluginAPI headers.
@towneh
towneh marked this pull request as ready for review July 20, 2026 11:12
@towneh
towneh requested a review from dooly123 July 20, 2026 11:14
@towneh towneh added the bug Something isn't working label Jul 20, 2026
@dooly123
dooly123 merged commit 9a0e18a into BasisVR:developer Jul 21, 2026
14 of 16 checks passed
@towneh
towneh deleted the fix/mediaplayer-proton-vod-pacing branch July 21, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants