fix(mediaplayer): sync audio-only sources to remote players#976
Merged
dooly123 merged 2 commits intoJul 21, 2026
Merged
Conversation
Loading an audio-only URL (.wav, and equally .mp3 / .m4a / .opus) played for whoever loaded it but never reached anyone else. Peers did nothing while a video was playing, and once their current VOD ended they replayed that video rather than the new source. Late joiners received the audio correctly, which isolates it to the live broadcast rather than the URL itself. Readiness was video-only. In BasisNativeVideoSource.Pump the whole readiness block sits behind the video frame counter and a non-null OutputTexture, neither of which an audio-only source ever produces, so OnReady never fired on any client. The engine handles audio-only correctly - it reaches Playing once audio frames are flowing on a source that announced no video track - but that state was never plumbed through to the C# event, which also left IsPrepared and Status wrong for audio-only. The URL broadcast then hung off that event. SetUrl skipped its up-front BroadcastFullState for directly-playable URLs and deferred to OnReady, which never ran, and FullState is the only message carrying a URL. currentSyncedUrl was set locally and never transmitted, so peers kept the source they already held and played that on the next bare Play command. Fire OnReady when the engine reaches Playing with an audio format and no video size - a pairing the engine only reports once the source has announced no video track, with split-stream excluded since its video leg announces later - and broadcast unconditionally from SetUrl. Broadcasting there happens before LoadUrl, while the player still holds the outgoing media, so the serialized state and playhead would describe the source being replaced and land on peers as the incoming source's start position. Mark that broadcast as a fresh load so it describes the load being started instead. The intent has to survive the pre-network-ready deferral queue as well, and be retired once the load reaches OnReady, after which the player's own state is the truth and later local commands must not be re-serialised as a pending load at position zero. Applying a resolved page URL also ignored the owner's advertised state: ApplyPendingRemoteState only ever stopped or paused, leaving playback to the peer's own AutoPlayOnSourceAssigned, so a peer with that unticked sat stopped while the owner played. The direct-URL path already forces this around LoadSource, so both paths now agree. IsPlaying and IsPaused are independent here, so a source that arrived paused is resumed rather than started.
Audio-only sources carry no video track, so a readiness regression on that path is invisible from the owner's client and only shows up on peers. Name the two-client scenario, including the load-while-peers-are-playing case and the peer with autoplay disabled.
towneh
marked this pull request as ready for review
July 20, 2026 10:50
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.
Summary
Loading an audio-only URL (
.wav, and equally.mp3/.m4a/.opus) played for whoever loadedit but never reached anyone else. Remote clients did nothing while a video was playing, and once
their current VOD ended they replayed that video rather than the new source. Late joiners got the
audio fine, which is what isolates it to the live broadcast rather than the URL itself.
Two things combined.
Readiness was video-only. In
BasisNativeVideoSource.Pump, the whole readiness block sitsinside
if (fc != lastFrameCounter), the video frame counter, and is further gated onOutputTexture != null. An audio-only source never ticks that counter and never produces atexture, so
OnReadynever fired, on any client. The native engine handles audio-only correctly,reaching
PLAYINGonce audio frames are flowing on a source that announced no video track, butthat state was never plumbed through to the C# event.
IsPreparedandStatuswere left wrong foraudio-only too, so the damage went wider than the sync.
The URL broadcast hung off that event.
SetUrlskipped its up-frontBroadcastFullState()fordirectly-playable URLs and deferred to the
OnReadypath, which never ran. FullState is the onlymessage carrying a URL, so
currentSyncedUrlwas set locally and never transmitted. Peers receivedonly the bare
Playcommand and played whatever source they still held.The changes:
OnReadywhen the engine reachesPlayingwith an audio format and no video size. Theengine only reports that pairing once the source has announced no video track at all, and
split-stream (whose video leg announces later) is excluded, so it's a sound audio-only signal
rather than a guess about timing.
SetUrl.LoadUrl, while the player still holdsthe outgoing media, so
SerializeFullStatedescribes the source being replaced rather than theone being loaded. The direct-URL apply path feeds that state and playhead straight into
StartPosition, so peers need the broadcast to describe the load being started. The page-URLpath stashes state instead of applying it that way, so it never took that route.
reaches
OnReady, after which the player's own state and position are the truth and later localcommands must not be re-serialised as a pending load at position zero.
ApplyPendingRemoteStateonly ever stopped or paused; starting playback was left to the peer's own
AutoPlayOnSourceAssigned, so a peer with that unticked sat stopped while the owner played. Thedirect-URL path already forces this around
LoadSource, so the two now agree.IsPlayingandIsPausedare independent here, so a source that arrived paused is resumed rather than started.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.
TransformAccessArrayor are otherwise batched. I have not added per-frametransform.position/transform.rotation/transform.localPositioncalls inside loops. Whenever I need both position and rotation, I use the combined APIs —SetPositionAndRotation/SetLocalPositionAndRotationfor writes,GetPositionAndRotation/GetLocalPositionAndRotationfor reads — instead of two separate property accesses; the combined call does one local-to-world matrix traversal instead of two.Resources.Load, no direct asset references that pull large content into memory on scene load.GetComponent/AddComponentwhere avoidable — Where unavoidable, the result is cached on a field, and anyGetComponent<T>is replaced withTryGetComponent<T>(out var x)— bareGetComponentwill be denied.TryGetComponentis the modern API (Unity 2019.2+) and skips the Editor-only GC allocationGetComponentcauses when a component is missing: Unity wraps thenullreturn in a managed "fake null" object so its overloaded==operator can still detect destroyed C++ objects, and constructing that wrapper allocates;TryGetComponentreturns aboolplusoutparameter and never builds the wrapper. None of these calls run insideUpdate,LateUpdate,FixedUpdate, jobs, or other per-frame code paths.BasisEventDriver— Any new per-frame work hooks intoBasisEventDriverrather than adding standaloneUpdate/LateUpdate/FixedUpdatecallbacks on a MonoBehaviour.BasisEventDriveris bulletproof, or guarded bytry/catch—BasisEventDriverruns 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 atry/catchthat contains the failure and surfaces it throughBasisDebug— logged once / rate-limited, never every frame (see the existingHVRBasisBuiltInAddresses.Simulate()guard for the pattern). Expect this to be scrutinized closely in review.{ get; set; }properties or access lockdowns — Public fields are fine; Basis is meant to be read and modified freely, so don't wall things offprivate/internalwithout 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.Instancesingletons, callers reassigningType.Instanceis allowed; if that would break your code, log a warning or throw — don't block the assignment. Locking down access is not your call.BasisLocalCameraDriver— Code that needs the local camera (transform, projection, rig data, etc.) pulls it fromBasisLocalCameraDriverrather than looking one up itself. Don't roll a separate camera discovery path.BasisDebug— All new logging calls go throughBasisDebug.Log/BasisDebug.LogWarning/BasisDebug.LogError(with an appropriateLogTag) instead ofUnityEngine.Debug.Log/Debug.LogWarning/Debug.LogError.BasisDebugroutes through Basis's tagged, color-coded logger and respects the project-wideLoggingDisabledtoggle so logging can be killed at runtime; bareDebug.Logcalls bypass that and will be denied.FindObjectOfType/FindObjectsOfType/GameObject.Find/FindGameObjectsWithTagto 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.newon reference types, no LINQ, nostringconcatenation/interpolation, no boxing, noforeachover interface-typed collections. Allocate once at init and reuse the buffer.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_EDITORand remove (or leave gated) before merge..Count(lists) /.Length(arrays) into a localintbefore the loop instead of re-reading the property each iteration. PreferT[](with a separate length int when the array is over-sized) overList<T>where the data is hot — Unity's mono BCL doesn't exposeCollectionsMarshal.AsSpan(List<T>), so a list can't be fed intoSpan<T>/ unsafe paths cleanly. Where the perf justifies it, drop intoSpan<T>/reflocals /Unsafe.As/unsafepointer 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.
Input / control mode coverage:
Where applicable, confirm these flows still work after your changes:
Notes
Verified on Windows x64 standalone and Quest Pro. Both platforms were exercised against a
two-client setup with the Quest as the peer rather than the owner, since the owner's own
playback worked throughout and is not what this fixes.
Covered on both: an audio-only source loaded by the owner now plays on remote clients; loading it
while a peer is mid-playback of a video switches correctly and starts near the beginning rather
than at the outgoing video's playhead; late join still works and lands on the owner's current
position; and video playback is unaffected, which is the regression that mattered most here since
the readiness path is shared. On Quest the video lane also covers the Vulkan
AccessTexturepath,which is where readiness differs from desktop. Quest logcat was clean of media player errors.
Fixtures were
.wav(LPCM) plus the other audio-only extensions on the directly-playable list.Verification did turn up a separate defect, which this PR does not cause and does not fix. When the
owner reaches the end of a source it broadcasts an unconditional stop, and remote clients apply it
wherever their own playhead happens to be. A late joiner is inherently a little behind the owner,
so it loses that much off the end. Measured at 96% on a 1:40 clip, roughly 4 seconds short.
It is not specific to this change or to audio:
HandleLocalEndedis pre-existing and untouchedhere, and a video late joiner is affected the same way. This change only makes it visible for
audio-only sources, which never reached remote clients at all before. Loading a URL with the remote
client already present is unaffected, since both clients stay in sync and finish together. Tracked
separately.
A behaviour change worth a second opinion: a world author who deliberately unticks
AutoPlayOnSourceAssignedon a networked player will now see peers start playing when the ownerdoes, on the resolver path as well as the direct one. The reasoning is that in a networked instance
the owner's state should win, and that's what the direct-URL path has always done. It is still a
change for that configuration rather than a pure fix, so happy to gate it if you'd rather it stayed
opt-out.
On the perf checks: the added readiness test rides the existing
Pumptick rather than introducingany per-frame work of its own, and is latched behind
readyFiredso it stops evaluating once aload is ready. It calls two native getters with
out intparameters, so no allocation, no logging,no collection access. The rest of the diff is event and message plumbing outside any hot path.