feat: migrate video playback from AVPro to UUAV#9463
Conversation
|
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. |
NickKhalow
added
force-build
|
Warnings not reduced: 14591 => 14683 — remove at least one warning to merge. Warnings/errors in files changed by this PR (115) |
|
Tests: 24314 passed, 2 failed Failed tests (2)
|
…uuav Resolved asmdef conflicts by unioning references: kept UUAV.AVProCompat (branch) and added DCL.McpServer (dev) in DCL.Plugins and DCL.EditMode.Tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review: feat: migrate video playback from AVPro to UUAV
STEP 2 — Root-cause check
This PR replaces a commercial video playback dependency (AVPro Video Ultra) with an embedded FFmpeg-based Rust/C# plugin (UUAV). This is a deliberate component swap, not a bug fix. The migration strategy — providing an AVProCompat facade in the same RenderHeads.Media.AVProVideo namespace so existing consumers compile unchanged — is a sound approach that minimizes integration risk.
Verdict: PASS — the change addresses a dependency swap at its root.
STEP 3 — Design & integration
Architecture: The UUAV plugin is structured in three layers:
- Native (Rust) — FFmpeg 8 demux/decode, D3D11VA/Metal HW accel, audio ring buffer. Ships as prebuilt
uuav.dll/libuuav.dylibvia LFS. - Runtime (C#) —
UUAVPlayer(MonoBehaviour),UUAVRuntime(static lifecycle),NativeMethods(P/Invoke). Standalone, no DCL dependency. - AVProCompat (C#) — Thin facade (
MediaPlayer,IMediaControl,ITextureProducer, etc.) inRenderHeads.Media.AVProVideonamespace so DCL'sMediaStreamsystems compile without changes.
Owner search: The MediaStream lifecycle is owned by MediaPlayerPluginWrapper (injection), MediaPlayerCustomPool (creation/pooling), and per-scene ECS systems (CreateMediaPlayerSystem, UpdateMediaPlayerSystem, CleanUpMediaPlayerSystem). The UUAV plugin does NOT duplicate these lifecycle owners — it replaces the underlying engine behind the same facade. The existing create/pool/release/cleanup paths remain unchanged.
Extension method compatibility verified: MediaPlayerExtensions.cs defines CloseCurrentStream, CrossfadeVolume, UpdatePlayback, UpdatePlaybackProperties as extension methods on MediaPlayer. All call only properties/methods that exist on the compat MediaPlayer (Stop(), CloseMedia(), Events, AudioVolume, MediaOpened, Control). Confirmed compatible.
Security review: Protocol whitelist (UNTRUSTED_STREAMING_PROTOCOLS) correctly restricts untrusted scene URLs to streaming protocols (https,http,tls,tcp,...). file: protocol is only added in UNITY_EDITOR builds. No credentials, secrets, or hardcoded tokens found. FFI boundary uses proper marshaling and string cleanup via uuav_string_free. No security issues identified.
STEP 5 — Line-level findings
See inline comments below for all findings with suggestion blocks.
Summary of findings:
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | P1 | DCL.Plugins.asmdef |
AV_PRO_PRESENT define likely not activated — package not registered with UPM |
| 2 | P2 | UUAVBackend.cs |
Stop() only pauses, does not seek to 0 (diverges from AVPro semantics) |
| 3 | P2 | MediaPlayer.cs |
Same Stop() issue on the compat MonoBehaviour |
| 4 | P2 | UUAVPlayer.cs |
#region AI generated with TODO "recheck carefully" should not ship |
| 5 | P2 | UUAVPlayer.cs |
Redundant P/Invoke uuav_player_state() calls in Update() — cache once |
| 6 | P2 | package.json |
TODO placeholder values in description and keywords |
| 7 | P2 | Enums.cs |
Typos in comment: "Effectivly" → "Effectively", "compatabilitty" → "compatibility" |
STEP 6 — Complexity assessment
COMPLEX — Introduces an embedded native Rust/FFmpeg plugin with C# FFI bindings (P/Invoke, render-thread callbacks via GL.IssuePluginEvent, audio-thread callbacks via OnAudioFilterRead), an AVPro API compatibility layer, and modifies assembly definitions, package references, and the media playback pipeline.
STEP 7 — QA assessment
QA_REQUIRED: YES — This changes the entire video/audio playback engine. User-visible media playback (video screens, audio streams in scenes) must be tested on Windows. Audio sync, seeking, looping, pooling (scene transitions), and error handling all need manual verification.
STEP 8 — Non-blocking warnings
No warnings (Main scene not modified).
STEP 9 — Verdict
REVIEW_RESULT: FAIL ❌
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Introduces an embedded native Rust/FFmpeg plugin with C# P/Invoke bindings, AVPro API compat layer, audio/render thread callbacks, and modifies assembly definitions and package references across the media playback pipeline.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub
| }, | ||
| { | ||
| "name": "com.renderheads.avpro.video-ultra", | ||
| "name": "com.nickkhalow.uuav", |
There was a problem hiding this comment.
[P1] AV_PRO_PRESENT scripting define likely not activated. The versionDefines entry maps com.nickkhalow.uuav → AV_PRO_PRESENT, but this package is not registered with Unity Package Manager — it is not listed in the project's root Explorer/Packages/manifest.json, nor is it embedded in the Packages/ directory. It lives in Assets/Plugins/UUAV/Packages/UUAV/ which Unity treats as regular project assets, NOT as an installed UPM package.
Without AV_PRO_PRESENT, all MediaStream systems are compiled out (#if AV_PRO_PRESENT in MediaPlayerPluginWrapper.InjectToWorld), silently disabling video playback in builds. The CI build/tests would pass since the gated code simply doesn't exist.
The PR description mentions adding AV_PRO_PRESENT to ProjectSettings scripting defines, but that change is not present in this diff.
Fix (choose one):
- Add
"com.nickkhalow.uuav": "file:../Assets/Plugins/UUAV/Packages/UUAV"toExplorer/Packages/manifest.jsonso UPM discovers the package and theversionDefinestriggers, OR - Add
AV_PRO_PRESENTto the Standalone scripting defines inProjectSettings/ProjectSettings.asset, OR - Move the UUAV package into
Explorer/Packages/com.nickkhalow.uuav/(standard embedded package location)
Please verify by checking the Console for the [MediaPlayerPluginWrapper] Inject log message after applying the fix — if you see Ignore Inject instead, the define is not active.
… feat/migrate-avpro-to-uuav
Security-Focused Dependency Review — PR #9463 (AVPro → UUAV) · re-run on synchronize
|
Co-authored-by: Muna <44584806+decentraland-bot@users.noreply.github.com> Signed-off-by: Nick Khalow <71646502+NickKhalow@users.noreply.github.com>
Co-authored-by: Muna <44584806+decentraland-bot@users.noreply.github.com> Signed-off-by: Nick Khalow <71646502+NickKhalow@users.noreply.github.com>
… feat/migrate-avpro-to-uuav
|
Claude Code is working… I'll analyze this and get back to you. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Claude finished @NickKhalow's task in 3m 2s —— View job Security-Focused Dependency Review — PR #9463 (AVPro → UUAV) · re-run on synchronize
Re-reviewed against the current tip ( ✅ Fixed this round (verified on branch)
🟡 Non-blocking correctness —
|
| Dependency / file | Version | Source | Type | Scope | Risk | Confidence |
|---|---|---|---|---|---|---|
com.renderheads.avpro.video-ultra (removed) |
— | git (internal) | managed+native | runtime | — | HIGH |
uuav.dll / libuuav.dylib |
crate 0.x | vendored Rust src (native/src) |
native binary | runtime | MEDIUM | HIGH |
| 12× FFmpeg Windows DLLs | FFmpeg 8 (BtbN LGPL) | prebuilt drop | native binary | runtime | MEDIUM | HIGH |
| 8× FFmpeg macOS dylibs | FFmpeg 8 (src build) | prebuilt drop | native binary | runtime | MEDIUM | HIGH |
Rust crate deps (ffmpeg-sys-next, windows, objc2*, dashmap, parking_lot, ringbuf, …) |
pinned via Cargo.lock |
crates.io | source (transitive) | build | LOW | MEDIUM |
Positive controls — all still hold (re-verified):
- ✅ Full native source vendored (
native/src/*.rs) +Cargo.lock+Cargo.tomlcommitted — reviewable/rebuildable. - ✅ No custom network/process primitives in Rust source — re-grepped
native/srcforreqwest/hyper/TcpStream/std::net/std::process/Command::new/libc::system/tokio::net→ 0 hits. Network I/O goes only through FFmpeg's whitelisted protocol layer. - ✅ SSRF / local-file hardening intact —
UUAVRuntime.cs:15UNTRUSTED_STREAMING_PROTOCOLS = "https,http,tls,tcp,crypto,data,udp,rtp,rtcp,rtsp";file:appended only under#if UNITY_EDITOR(:73-77);concat/subfiledeliberately omitted (comment:14). - ✅ macOS refuses to init on any non-Metal graphics API (
UUAVRuntime.cs:58-64). - ✅ No auto-run install/build hooks; only editor auto-run is benign
[RuntimeInitializeOnLoadMethod]init +Application.quittingteardown.
MEDIUM-risk items (human sign-off, unchanged)
M1 — FFmpeg binaries have no committed integrity attestation. Re-grepped both READMEs and native/scripts for sha256/checksum/gpg/signature → none. FFmpeg decodes untrusted scene URLs in the shipped client. → commit upstream SHA-256s or verify in CI; name an owner for FFmpeg security bumps.
M2 — uuav.dll/libuuav.dylib not verifiably built from the vendored source. Nothing ties the committed binaries to the committed native/src. → build in CI from source, or document toolchain + producer.
M3 — LFS inconsistency persists (confirmed on disk). .gitattributes:16 has *.dll filter=lfs but no *.dylib rule. Verified via git cat-file -s: uuav.dll blob = 132 B (LFS pointer), but libuuav.dylib = 803 KB and libavcodec.62.dylib = ~13 MB stored raw in history. All 8 macOS dylibs bloat the repo permanently. → add *.dylib filter=lfs diff=lfs merge=lfs -text and migrate.
M4 — http/udp/rtp/rtsp enabled for untrusted URLs. Same SSRF reachability AVPro had (not a regression). → drop http/rtsp/udp if scenes only use HLS/DASH-over-HTTPS, or record as accepted risk.
Other non-security notes (from CI, unchanged)
- Convention tests still failing:
UUAVRuntime.cs:67usesApplication.quitting(VerifyShouldNotUseApplicationQuitting) and UUAV usesDebug.Log*vsReportHub(CheckForDebugUsage). → allowlist the vendoredUUAVassemblies in the convention tests, or route throughReportHub. InconsistentNamingwarning set on the vendored UUAV enums remains (cosmetic; vendored code).
Verdict
The frameTimeBudget NRE blocker from my last pass is fixed; the residual AV_PRO_PRESENT orphan-define issue is a non-blocking correctness/coverage concern (null-safe, no crash). Supply-chain custom-code provenance is good. Residual supply-chain gaps — M1 FFmpeg integrity, M2 unverifiable native artifacts, M3 LFS .dylib inconsistency — still need human sign-off; none is a clear high-severity exploit.
DEPENDENCY_REVIEW: NEEDS_ATTENTION
· feat/migrate-avpro-to-uuav
2 participants
Pull Request Description
What does this PR change?
Migrates video/audio media streaming from the AVPro Video Ultra package to UUAV - an FFmpeg-based player with a native Rust core, embedded directly in the repo under
Assets/Plugins/UUAV. Supported platforms: Windows x86_64 (D3D11) and macOS arm64 (Metal); Linux stays compiled out, as before.com.renderheads.avpro.video-ultrafromPackages/manifest.json- UUAV ships in-tree, no external package reference.Plugin layout (
Assets/Plugins/UUAV)native/- the Rust crate that builds the native player (uuav.dll/libuuav.dylib): FFmpeg 8 demux/decode, hardware-accelerated video decoding, audio pulled throughOnAudioFilterRead. Platform-specific code lives in_windows/_macossibling modules (hw_device_*,video_decoder_*,video_output_*) selected with#[cfg(target_os)]at the dispatch site - no cross-platform stub contracts.CVPixelBuffer/IOSurface → two Metal plane textures (YR8Unorm, UVRG8Unorm) blitted on the plugin's ownMTLCommandQueue. FFmpeg (n8.1) is built from source vianative/scripts/build-ffmpeg-macos.shinto a git-ignored.third_party/(source-build sonames differ from the Windows DLLs: avcodec 62 etc. -build.shresolves majors dynamically).build.shalso handles@rpathdylib naming and the mandatory ad-hoc code signing on arm64.build.shis cross-platform: it detects the host OS, builds the right cargo target, and deploys binaries intoPackages/UUAV/Runtime/Plugins/{x86_64,macOS}.Packages/UUAV/Runtime- the C# binding layer (NativeMethods,UUAVRuntime,UUAVPlayer,UUAVClient.asmdef) plus the prebuilt binaries. On both platforms the native side presents NV12 planes; Unity wraps them withTexture2D.CreateExternalTextureand converts to RGB with the bundledHidden/UUAV/NV12ToRGBshader (Graphics.Blit), with frame presentation driven byGL.IssuePluginEvent. On macOS,UUAVRuntimerefuses to initialize on any graphics API other than Metal. Local-file playback is editor-only; player builds accept streaming URLs only.Packages/UUAV/AVProCompat- aRenderHeads.Media.AVProVideo.MediaPlayerfacade overUUAVPlayer(MediaPlayer,MediaPlayerEvent,Enums,Interfaces,UUAVBackend), so the existingMediaStreamcode (systems, pool, tests) keeps compiling against the AVPro API surface largely unchanged.Packages/UUAV/Example- a minimal standalone example scene/assembly for the player, independent of Explorer.Explorer integration
MediaStreamto the compat surface: drops theAutoOpenflag inMediaPlayerCustomPooland the manualSetAudioSourcewiring inMultiMediaPlayer(the compatMediaPlayerhandles audio itself); updatesMediaPlayer.prefabaccordingly.UUAV.AVProCompatinto theECS.Unity,DCL.Plugins, andDCL.EditMode.Testsasmdefs; adds injection/skip logs toMediaPlayerPluginWrapper.AV_PRO_PRESENTversion-define inDCL.Pluginsfromcom.renderheads.avpro.video-ultratocom.nickkhalow.uuav(ProjectSettingsscripting defines are unchanged vsdev).Test Instructions
Steps (standard run):
Expected result:
Video streams in scenes play back correctly (e.g. video screens in Genesis Plaza or any scene using
VideoPlayer/media streams), with audio in sync.Steps (fresh account):
Expected result:
Same as above - media streams play with no errors in logs.
Prerequisites
Test Steps
PBVideoPlayer).Additional Testing Notes
AV_PRO_PRESENTgate).AV_PRO_PRESENTversion-define is keyed on thecom.nickkhalow.uuavpackage name; verify it still activates now that UUAV is embedded underAssets/rather than installed via UPM (watch for the[MediaPlayerPluginWrapper] InjectvsIgnore Injectlog on startup).uuav.dll/libuuav.dylib, FFmpeg libraries).MediaStreamconsumes - it is poll-based and will need extending as new AVPro API surface is used.Quality Checklist
Code Review Reference
Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does - especially useful for first-time contributors.