fix(mediaplayer): render fMP4/CMAF HLS by interleaving fragment audio and video#924
Closed
towneh wants to merge 2 commits into
Closed
fix(mediaplayer): render fMP4/CMAF HLS by interleaving fragment audio and video#924towneh wants to merge 2 commits into
towneh wants to merge 2 commits into
Conversation
HLS with fMP4/CMAF segments loaded a single video frame and played no audio. The fMP4 demuxer emitted a fragment track-major — every video sample of a moof, then every audio sample — and with delivery pacing on (always, for HLS) the paced video burst spans the whole fragment duration in wall time, so the fragment audio behind it arrives a full fragment late. The decoder trims that audio as stale and its audio-gated presentation clock never starts: one frame, silence. Consume a moof mdat as a decode-time-ordered merge across its trun runs instead, interleaving the tracks at frame granularity the way the MPEG-TS demuxer's output naturally is. Decode order within each run is preserved; single-track fragments (split-stream fMP4 legs) are delivered byte-for-byte as before. Verified against ffmpeg-generated CMAF (H.264+AAC, 2s fragments): worst delivery-order PTS gap between tracks drops from ~2.1s (the fragment duration) to ~190ms (B-frame reorder depth), same access-unit counts and timestamps; a video-only fMP4 stream demuxes identically before and after. Rebuilt basis_media_native.dll (win-x64) and libbasis_media_native.so (arm64-v8a).
basis_avcc_to_annexb is bounds-checked, so an output larger than the buffer fails the conversion rather than overrunning — but with a 1-3 byte NAL length prefix, an access unit made of many small NALs could outgrow the fixed 64-byte headroom (each NAL trades its length prefix for a 4-byte start code) and be dropped. Size the buffer from the worst-case growth instead: a NAL is at least nal_len_size + 1 bytes, so growth is bounded by (4 - nal_len_size) * (size / (nal_len_size + 1)). For the common 4-byte prefix this stays exactly size + 64. Rebuilt basis_media_native.dll (win-x64) and libbasis_media_native.so (arm64-v8a).
Collaborator
Author
|
The media player now lives in its own repository, so this fix moves there — re-created as BasisVR/BasisMediaPlayer#1 with the same commits (paths rebased onto the standalone repo). Closing this one. |
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
HLS with fragmented-MP4 / CMAF segments (
EXT-X-MAP:init.mp4+.m4s) — which includes Twitch's low-latency streams — loaded a single video frame and then stalled with no audio. TS-segment HLS was unaffected.The demux itself was fine; the problem was emission order. The fMP4 demuxer consumed each moof/mdat track-major: every video sample of the fragment, then every audio sample. With delivery pacing on (always, for HLS) the paced video burst spans the whole fragment duration in wall time, so the fragment's audio always arrived a full fragment late. The decoder trims audio that stale, and its audio-gated presentation clock never got a buffer to start from — one frame, silence.
consume_mdatnow emits the moof's trun runs as a decode-time-ordered merge, interleaving the tracks at frame granularity — the same shape the MPEG-TS demuxer's output naturally has. Decode order within each track is preserved, and single-track fragments (split-stream fMP4 legs) are delivered exactly as before.Both binaries are rebuilt and committed (win-x64 DLL, arm64-v8a
.so).Verification
basis_mp4_rundirectly and measuring the worst delivery-order PTS gap between tracks: ~2.1 s before (the fragment duration) → ~190 ms after (B-frame reorder depth), with identical access-unit counts and timestamps. A video-only fMP4 stream demuxes identically before and after.Scope note
HLS playback remains single-rendition: masters that put audio in a separate
EXT-X-MEDIArendition group (rather than muxing it into the variant) play the selected variant only. This PR fixes muxed CMAF, which is what Twitch low-latency and ffmpeg-style CMAF origins serve.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
Native~/protocol/basis_mp4.c); no C# or Unity-side code is touched, so the Unity-specific checks (transforms, Addressables,GetComponent,BasisEventDriver, camera,BasisDebug, scene discovery) are N/A-ticked. Jobification is N/A for the same reason — the work runs on the plugin's demux thread. The demux path allocates per access unit by design (matching the existing code); it is not on a render-frame path..sois rebuilt from the same source. The fix applies there once HLS is enabled on Android (separate branch in flight); nothing on Android regresses meanwhile since the fMP4-over-HLS path can't currently be reached on it.