Skip to content

feat: Data Tracks - #1004

Merged
davidliu merged 51 commits into
mainfrom
dl/rust-testing
Sep 15, 2026
Merged

davidliu merged 51 commits into
mainfrom
dl/rust-testing

Conversation

@davidliu

@davidliu davidliu commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Adds data track support, sharing the Rust UniFFI core (livekit-uniffi-android) with the other SDKs.
Most of room/datatrack/ is a thin wrapper around that core. The Android-specific work is the WebRTC, signaling, and reconnect glue.
Actual changes

  • RTCEngine / DataTrackFrameSender_data_track channel, send backpressure, wait-for-open, reconnect
  • SignalClient — publish / unpublish / subscriber-handle / request-response forwarding
  • Room, LocalParticipant, RemoteParticipant — public API, attach/park remote tracks, events
  • IncomingDataTrackManager / OutgoingDataTrackManager — bridge UniFFI managers to the engine
  • DataTrackCryptor / E2EEManager — E2EE via the existing data-packet cryptor
  • DataChannelManager, RTCModule, test DI — channel metering and injectable factories

Simple wrappers (FFI type/API mapping, little Android-specific logic)

  • LocalDataTrack, RemoteDataTrack, DataTrackStream
  • DataTrackFrame, DataTrackInfo, DataTrackSid, DataTrackSchema, DataTrackPublishOptions, DataTrackException
  • DataTrackManagerFactory, IncomingDataTrackEvent

E2E tests: https://github.com/livekit/e2e-android/pull/13

@changeset-bot

changeset-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d4c5e7a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
client-sdk-android Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@davidliu
davidliu marked this pull request as draft August 20, 2026 16:39

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@davidliu
davidliu marked this pull request as ready for review August 26, 2026 18:40
@davidliu
davidliu requested review from ladvoc and pblazej August 26, 2026 18:40
@pblazej

pblazej commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Compared this against the Swift (#975), JS (#1994) and Rust implementations, looking for behavior diffs.

Before merge

  1. DataTrackPublished is dropped, not deferredRoom.kt:905 gates it on emitWhenConnected, but joinImpl attaches tracks while state is CONNECTING/RECONNECTING, so it never fires for tracks present at join or after a full reconnect. DataTrackUnpublished posts directly (:821), so apps can see an Unpublished with no Published. JS deliberately uses plain emit for both; Swift notifies unconditionally.
  2. Data-track init can fail Room.connectRTCEngine.kt:303 calls handleSfuJoinResponse unguarded and builds the UniFFI manager there on every connect, catching only HandleSignalResponseException. So an UnsatisfiedLinkError fails the connect for apps that never touch data tracks — reachable on API 21–23, where tools:overrideLibrary (AndroidManifest.xml:17) hides that the AAR won't load and nothing gates on SDK_INT. Swift's call is try?. handlePacketReceived has no try/catch at all and runs on a WebRTC thread.
  3. waitUntilInboundDataTrackChannelReady() precedes resendSubscriptionUpdates()RTCEngine.kt:781. If the SFU opens the subscriber channel in response to a subscription, the wait can't clear until the call after it runs; at best it adds MAX_ICE_CONNECT_TIMEOUT_MS to every full reconnect with remote tracks. Swift, JS and Rust all resubscribe straight after transport connect — suggest deleting the gate. The unit test hand-feeds the channel, so it passes either way.
  4. A dead transport can report CONNECTEDRTCEngine.kt:362 suppresses DISCONNECTED while reconnecting, then the success branch sets CONNECTED unconditionally after up to ~40 s of waiting. An ICE failure in between is swallowed and overwritten, and connectionChangeListener can't cover it because reconnect() early-returns while reconnectingJob is live. Not data-track scoped.

Public surface — cheaper to settle now

  • DataTrackPublishOptions (:22) allows either field null and never checks compatibility (JsonSchema + Cdr passes locally, fails remotely). Swift uses two constructors, JS validates against COMPATIBLE_SCHEMA_ENCODINGS. A validating factory is additive later, but the permissive constructor is forever. No test covers an invalid pair.
  • The LocalDataTrack KDoc's "releasing the last reference unpublishes it" is Swift's ARC text — on the JVM the DropGuard waits on the UniFFI Cleaner, so a dropped reference can leave the publication live on the SFU. Nothing is AutoCloseable and close() only clears the map. I'd fix the doc now and let AutoCloseable follow.

Small

  • RTCEngine.kt:1079 — the observer wrapper captures the old manager while the gate reads the current field, so a stale CLOSED flips an already-open replacement back to WAITING and the next publish burns the full timeout. Swift guards with guard dataChannel === publisherChannel.
  • IncomingDataTrackManager.kt:62 — the delegate captures its manager and Rust holds it as a JNI global ref, so the cycle spans the FFI boundary and retains the engine and both PCs unless close() runs. Swift split out a weak-holding shim. Same shape in OutgoingDataTrackManager.
  • SignalClient.kt:745 — unguarded parseFrom inside the UniFFI forward task throws across the boundary; Swift logs and skips.
  • OutgoingDataTrackManager.kt:78startsWith("Timed out") to classify the error; reword RTCEngine.kt:1024/1038 and every timeout silently becomes a disconnect. Two exception types instead.
  • RTCEngine.kt:1580 — subscribe-only clients negotiate a publisher PC via the shared sendDataTrackSignalRequest; both references keep subscribe signal-only, and the hasPublished guard is redundant (:814).
  • DataChannelManager.waitUntilOpen() has three tests and no production caller; sendOrQueue's per-frame copyOf() copies already-fresh packets on the RTC thread.

private var reliableDataChannelSub: DataChannel? = null
private var lossyDataChannel: DataChannel? = null
private var lossyDataChannelSub: DataChannel? = null
private var dataTrackDataChannelSub: DataChannel? = null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this dataTrackDataChannelSub needed ? I don't see how it is used rather than assign and clear, is it intentional ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, looks like all of the sub channels may need DataChannelManager handling, will handle in a follow up PR.

@pblazej pblazej left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, added 2 non-blockers above

Comment thread sample-app-common/src/main/java/io/livekit/android/sample/CallViewModel.kt Outdated
Comment thread sample-app-common/src/main/java/io/livekit/android/sample/CallViewModel.kt Outdated
@MaxHeimbrock

MaxHeimbrock commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

A second draft PR for these changes according to @1egoman's "Commit Reshuffle" strategy to build a story line through commits and some ideas from me for structured PR description for API changes: #1019

Please have a look and give feedback to @1egoman and me if this strategy could help review PRs.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

@davidliu
davidliu merged commit 12433f2 into main Sep 15, 2026
6 checks passed
@davidliu
davidliu deleted the dl/rust-testing branch September 15, 2026 10:50
@davidliu davidliu mentioned this pull request Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants