Skip to content

feat: livekit cast improvements - #7347

Merged
cyaiox merged 57 commits into
devfrom
feat/enhance-livekit-track-selector
Apr 27, 2026
Merged

feat: livekit cast improvements#7347
cyaiox merged 57 commits into
devfrom
feat/enhance-livekit-track-selector

Conversation

@cyaiox

@cyaiox cyaiox commented Mar 5, 2026

Copy link
Copy Markdown
Member

Pull Request Description

What does this PR change?

Enhances the LiveKit media player to support full Cast scenarios with multiple participants, automatic speaker tracking, presentation bot priority, data messaging between scenes and participants, and self-healing stream recovery.

Video routing

  • Active speaker tracking: In CurrentStream mode, video automatically follows the dominant speaker using room.ActiveSpeakers with a 1.5s debounce to prevent flickering
  • Presentation bot priority: Participants with identity prefix presentation-bot: are always prioritized — once locked on, speaker tracking is suppressed until the bot leaves
  • Video muted state: Tracks TrackPublication.Muted per frame and renders a black texture when muted
  • Resolution capping: LiveKit video textures are capped at 1920x1080 to prevent GPU stalls
  • Auto-recovery: When a specific user stream dies, falls back to CurrentStream (first available track)

Audio routing

  • Multi-audio playback: Plays all audio tracks from the room simultaneously instead of just the paired one — all participants are heard regardless of which video is displayed
  • Additive discovery: New participants joining mid-session are picked up without disrupting existing audio sources
  • Rescan throttling: Audio rescans every 2s (AUDIO_RESCAN_INTERVAL_SECONDS), or immediately when a dead track is detected

Data messaging API

  • PublishData(topic, data) — Send messages to a topic (rate-limited to 10 msg/s, 16KB cap)
  • SubscribeToTopic(topic) — Subscribe to receive messages on a topic
  • ConsumeMessages(topic) — Consume buffered messages for a topic
  • UpdateMetadata(metadata) — Update local participant metadata (broadcast by LiveKit)
  • Handles non-protobuf data on the shared DataPipe gracefully (InvalidProtocolBufferException)

Stream metadata

  • Enriched getActiveVideoStreams response with displayName, speaking, trackName, width, height
  • Display name resolved from participant metadata JSON (displayName field), falling back to Name then Identity

Documentation

  • Updated docs/cast.md covering all new features

Test Instructions

Prerequisites

  • Deploy the template: Streaming Theatre to .zone/.org

Test Steps — Video routing

  1. Open the Admin tools smart item and start a DCL Cast
  2. Join the cast as a speaker (cast app) with camera on
  3. Verify the video screen streams your camera
  4. Have a second speaker join — verify video switches to whoever is talking (with ~1.5s hold)
  5. Stop talking — verify video stays on the last speaker (no flickering)

Test Steps — Presentation bot

  1. Start a cast session with a presentation bot connected
  2. Verify video locks onto the bot's track immediately
  3. Have other participants speak — verify video stays on the bot
  4. Disconnect the bot — verify video falls back to active speaker tracking

Test Steps — Audio

  1. Have multiple speakers join the cast
  2. Verify audio plays from all participants simultaneously
  3. Have a new participant join mid-session — verify their audio is picked up within ~2s
  4. Have a participant leave and rejoin — verify audio recovers

Test Steps — Data messaging

  1. From a scene, call SubscribeToTopic("test")
  2. From another participant, call PublishData("test", "hello")
  3. Call ConsumeMessages("test") — verify the message is returned with sender identity
  4. Verify rate limiting: publish >10 messages in 1s, confirm excess are dropped

Test Steps — Screen share recovery

  1. Join the cast and share your screen
  2. Select the screen option from the Showcase dropdown
  3. Stop sharing your screen
  4. Verify the video screen falls back to the first available stream (your camera)

Additional Testing Notes

  • Verify participant names display correctly for unauthenticated Cast users (castV2)
  • Verify no audio leaks (sources properly released) after stopping/restarting a stream
  • Verify video renders black when a track is muted

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated
  • Performance impact has been considered
  • For SDK features: Test scene is included

Code Review Reference

Please review our Code Review Standards before submitting.

Screenshots

image

@github-actions

github-actions Bot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

@cyaiox cyaiox added the force-build Used to trigger a build on draft PR label Mar 5, 2026
@cyaiox
cyaiox force-pushed the feat/enhance-livekit-track-selector branch from 4609f6b to 46fbe57 Compare March 6, 2026 19:40
@cyaiox cyaiox added force-build Used to trigger a build on draft PR and removed force-build Used to trigger a build on draft PR labels Mar 6, 2026
@cyaiox
cyaiox force-pushed the feat/enhance-livekit-track-selector branch from 46fbe57 to dd905a9 Compare March 10, 2026 18:23
@cyaiox cyaiox added force-build Used to trigger a build on draft PR and removed force-build Used to trigger a build on draft PR labels Mar 10, 2026
@cyaiox

cyaiox commented Mar 10, 2026

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@Ludmilafantaniella

Copy link
Copy Markdown
Contributor

Tested! Functionality works as expected ✅

TC5-6 TC3-4-evi TC1-2-evi
7347-evi.mp4

⚠️ Side note — cosmetic issues found:

  • "Showcase List" button label wraps to two lines instead of rendering on a single line.
  • Showcase camera dropdown: neither option shows a checkmark by default, even though the camera is already active when Cast is activated. The checkmark only appears after manually selecting an option. Feels a bit counterintuitive visually.
  • Horizontal scrollbar visible in the camera dropdown (already flagged — dev will fix by adjusting dropdown width).

None of these block the functionality, just leaving them on record!

11.03.2026_11.08.39_REC.mp4
image

@cyaiox
cyaiox force-pushed the feat/enhance-livekit-track-selector branch from 390bb2e to 66e96cc Compare March 11, 2026 23:24
@cyaiox cyaiox added force-build Used to trigger a build on draft PR and removed force-build Used to trigger a build on draft PR labels Mar 11, 2026
@cyaiox
cyaiox force-pushed the feat/enhance-livekit-track-selector branch from 43e595a to ccdc25a Compare March 12, 2026 21:04
@cyaiox cyaiox added force-build Used to trigger a build on draft PR and removed force-build Used to trigger a build on draft PR labels Mar 12, 2026
@m3taphysics

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @m3taphysics's task in 2m 53s —— View job


Code Review

  • Gather context
  • Read changed files
  • Analyze for bugs, security, and performance issues
  • Post review findings

Issues Found

🔴 Performance — EnsureAudioIsPlaying polls participants lock every frame when room has no audio (line 100)

When audioSources.Count == 0 (room has no audio tracks, or after initial open), anyDied is false and the guard condition !anyDied && audioSources.Count > 0 evaluates to false. The method falls through every frame to OpenAllAudioStreams(), which acquires lock(room.Participants) and iterates all participants. For any scene where audio is temporarily unavailable, this is a per-frame lock + allocation in a hot path. Fix this →

🔴 Audio disruption on every video recovery (line 71)

When a user stream's video dies, EnsureVideoIsPlaying() calls OpenMedia(LivekitAddress.CurrentStream()). OpenMedia always calls CloseCurrentStream() first, which calls ReleaseAllAudioSources() — pooling all audio MonoBehaviours — then immediately re-acquires them. This causes a brief audio dropout on every video fallback, even when audio streams are healthy. The video and audio reset paths need to be separated. Fix this →

🟡 Dead code — FindPairedAudio never called (line 255)

Private method with no callers. Violates the project's code standards: "Don't design for hypothetical future requirements." Remove until actually needed. Fix this →

🟡 Silent JsonException swallowing (line 39)

catch (JsonException) { } in ResolveDisplayName silently discards all metadata parse failures. Malformed participant metadata would silently fall back to Name/Identity with no trace in logs, making it impossible to diagnose in production. Log a warning via ReportHub. Fix this →

🟡 asCurrent values written but never read in CommsApiWrap.cs

asCurrent is used only as a null-check sentinel — the stored (identity, track) tuple is never read back. This is misleading; it should be a plain bool hasWrittenCurrent = false. Fix this →


REVIEW_RESULT: FAIL ❌

Comment thread Explorer/Assets/DCL/SDKComponents/MediaStream/LivekitPlayer.cs Outdated
Comment thread Explorer/Assets/DCL/SDKComponents/MediaStream/LivekitPlayer.cs Outdated
Comment thread Explorer/Assets/DCL/SDKComponents/MediaStream/LivekitPlayer.cs Outdated
@cyaiox
cyaiox marked this pull request as ready for review March 17, 2026 12:49
@cyaiox
cyaiox requested review from a team as code owners March 17, 2026 12:49
…rack-selector

# Conflicts:
#	Explorer/Assets/DCL/Infrastructure/Utility/DecentralandUrls/DecentralandUrl.cs
#	Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsSource.cs
Comment thread Explorer/Assets/DCL/SDKComponents/MediaStream/Systems/UpdateMediaPlayerSystem.cs Outdated
Comment thread Explorer/Packages/manifest.json Outdated
Comment thread Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsSource.cs Outdated
NickKhalow and others added 4 commits April 27, 2026 16:09
…rack-selector

# Conflicts:
#	Explorer/Assets/DCL/Infrastructure/Utility/DecentralandUrls/DecentralandUrl.cs
#	Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsSource.cs
Co-authored-by: Copilot <copilot@github.com>
@claude

claude Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cyaiox's task in 1m 58s —— View job


Security Dependency Review

  • Identify all dependency changes in the PR
  • Look up registry metadata for changed packages
  • Assess provenance, runtime capability, and shipping impact
  • Classify risk and produce summary

Step 1 — Dependency Changes Identified

One dependency changed (packages-lock.json). No binary files, no .asmdef changes, no new plugin files.

Field Value
Package com.decentraland.livekit-sdk
Manifest declaration https://github.com/decentraland/client-sdk-unity.git (no tag/commit pinned)
Old resolved hash ecf91956ccf788abc2dd748f105cd336d771b8d8
New resolved hash c1d582d62644c33e53e3c521c53663f37ad24fbe
Source GitHub — Decentraland-owned fork of LiveKit client-sdk-unity
Type Source code (C# managed)
Scope Runtime (ships in desktop client)

Step 2 — Registry Metadata

[Registry metadata] The package at https://github.com/decentraland/client-sdk-unity is maintained by Decentraland (dalkia, NickKhalow, fcolarich). It is an internal fork of the upstream LiveKit Unity SDK. No separate NuGet or Unity Registry listing exists; it is sourced directly from GitHub.

[Registry metadata] The upstream LiveKit SDK README notes: "This SDK is currently in Developer Preview mode and not ready for production use" — but this refers to the upstream; the Decentraland fork has been in active use and production for this project.

[PR] The single commit between the two locked hashes is "fix: collections thread safety (#65)" authored by NickKhalow (April 24, 2026). Three files changed (+39/−10 lines):

  • Runtime/Scripts/Rooms/ActiveSpeakers/DefaultActiveSpeakers.cs — replaced List<string> with ConcurrentDictionary<string,byte> (non-WebGL) / HashSet<string> (WebGL) to fix the exact thread-safety issue raised across five prior review passes on this PR
  • Runtime/Scripts/Rooms/Participants/Participant.cs — replaced Dictionary with ConcurrentDictionary for tracks; TryRemove instead of direct removal
  • Runtime/Scripts/Rooms/Participants/ParticipantsHub.cs — thread-safe comment annotation

This is the minimal targeted fix for the ActiveSpeakers race condition flagged in every prior review cycle.


Step 3 — Assessment

A. Provenance / source trust

[PR] Publisher is Decentraland's own organization on GitHub — this is a first-party internal fork, not a third-party package. The author of this specific commit (NickKhalow) is an active reviewer on this very PR.

[PR] ⚠️ The manifest declaration https://github.com/decentraland/client-sdk-unity.git carries no tag, no commit hash, and no branch specifier — it resolves to the default branch HEAD at package-resolve time. The packages-lock.json pins the resolved commit hash (c1d582d...), which mitigates the risk in practice, but the lock file can be bypassed or refreshed, leaving the manifest floating.

[Inference] Because this is a Decentraland-owned repo, supply-chain risk from a malicious maintainer hijack is lower than for a third-party package, but the unpinned manifest is still a hygiene concern.

B. Runtime capability

[PR] The changed code is pure C# data-structure replacements (List → ConcurrentDictionary). No network capability, no native code, no reflection, no dynamic loading introduced by this diff. No new permissions or capabilities.

C. Shipping impact

[PR] com.decentraland.livekit-sdk is a runtime dependency used by LivekitPlayer.cs — it ships in the end-user desktop client on all platforms.

D. Maintenance / known risk

[Registry metadata] The repo is actively maintained by the Decentraland team. This specific commit was explicitly requested and reviewed by the core team on this PR thread. No CVEs or advisories found.

E. Transitive risk

[PR] The lock entry's transitive dependencies (com.cysharp.unitask, com.nickkhalow.richtypes, io.livekit.unity) are unchanged in this PR. No new transitive dependencies introduced.


Step 4 — Risk Summary

Dependency Version Source Type Scope Risk Confidence
com.decentraland.livekit-sdk Hash c1d582d (floating branch in manifest) github.com/decentraland/client-sdk-unity Source code (C#) Runtime MEDIUM HIGH

Medium Risk Detail

Concern: The manifest.json entry has no version pin — https://github.com/decentraland/client-sdk-unity.git resolves to whatever the default branch HEAD is at package-resolve time. The packages-lock.json provides the actual pinned hash, but if the lock file is deleted, regenerated, or bypassed (e.g. Clean Packages Cache in Unity), the project will pull the latest unreviewed commit.

Evidence: [PR] manifest.json line: "com.decentraland.livekit-sdk": "https://github.com/decentraland/client-sdk-unity.git" — no #<commit> or #<tag> suffix.

What remains unknown: Whether the team's Unity project setup enforces the lock file in CI/build pipelines, or whether the lock file is always committed and treated as authoritative.

Recommendation: Pin to a tag or commit in manifest.json:

"com.decentraland.livekit-sdk": "https://github.com/decentraland/client-sdk-unity.git#c1d582d62644c33e53e3c521c53663f37ad24fbe"

Or create a tagged release in the client-sdk-unity repo and reference it with #v<version>. This is consistent with how other packages in this manifest are handled (e.g. com.decentraland.rpc-csharp is pinned to commit f3dd251c78...).

Note: The content of this specific update (thread-safety fix using ConcurrentDictionary) is well-scoped, well-reasoned, and directly addresses a confirmed bug reported through five review cycles. The change itself carries no meaningful security risk.


DEPENDENCY_REVIEW: NEEDS_ATTENTION

The specific change is safe and appropriate. The NEEDS_ATTENTION flag is solely for the unpinned manifest reference on com.decentraland.livekit-sdk. Pin to the resolved commit (or a tag) in manifest.json to eliminate supply-chain drift risk before merge.

@DafGreco DafGreco left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️ PR reviewed and approved by QA on both platforms following instructions playing both happy and un-happy path

Regressions for this ticket had been performed in order to verify that the normal flow is working as expected:

  • [ ✔️] Backpack and wearables in world
  • [ ✔️] Emotes in world and in backpack
  • [✔️ ] Teleport with map/coordinates/Jump In
  • [✔️ ] Chat and multiplayer
  • [ ✔️] Profile card
  • [ ✔️] Camera
  • [ ✔️] Skybox
  • [ ✔️] Settings

Evidence:

20260427-1659-53.3530319.mp4
20260427-1648-06.8028566.mp4
20260427-1650-31.4924589.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

force-build Used to trigger a build on draft PR new-dependency

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants