Data streams v2 - #997
Draft
1egoman wants to merge 14 commits into
Draft
Conversation
Data streams v2 needs three things that landed upstream in protocol #1621
(first tagged v1.46.8); this SDK was pinned at v1.45.8-29:
- ClientInfo.Capability.CAP_COMPRESSION_DEFLATE_RAW = 2 (advertise)
- ParticipantInfo.capabilities = 21 (read remote caps)
- DataStream.Header.inline_content / compression (v2 wire fields)
Pinned to 28e604c, the same commit client-sdk-swift@data-streams-v2 and
rust-sdks use, so all three SDKs agree on the wire contract.
Two unrelated breaks from crossing five minor protocol versions, both fixed
here so the bump lands green:
- SignalClient's messageCase `when` gained STORE_DATA_BLOB_RESPONSE and
GET_DATA_BLOB_RESPONSE; stubbed as TODO like their neighbours.
- RoomAgentDispatch gained an `attributes` map, which ProtoConverterTest
requires the Kotlin DTO to mirror. Added as a nullable field with a
default, so it is source- and serialization-compatible.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the livekit-uniffi dependency to the SDK and everything needed to exercise
it from a host JVM test run. No SDK behavior changes yet.
The artifact is unreleased, so it resolves from mavenLocal() (already enabled on
this branch). Build it with `cargo make android-package-local` in rust-sdks, or
for a JVM-test-only workflow just generate the Kotlin bindings plus a host
cdylib -- no NDK required, and it avoids three Android target dirs' worth of
disk.
Three obstacles found and handled along the way:
- Nobody had ever generated Kotlin bindings for the data stream FFI
(`packages/kotlin/` did not exist in rust-sdks), and doing so surfaces two
uniffi codegen bugs that stop the generated file compiling at all:
a Rust method named `close` collides with the AutoCloseable `close()` uniffi
synthesizes, and an error variant field named `message` collides with
Throwable.message. scripts/patch-uniffi-kotlin.py patches the generated
output -- a build artifact, so rust-sdks source is untouched -- following the
precedent of the existing swift-workarounds task. Both still need a real
upstream fix.
- livekit-uniffi's AAR declares minSdk 24 against this SDK's 21. The native
library is built for platform 21, so the declaration is the only conflict;
overridden via tools:overrideLibrary in both manifests.
- The AAR depends on jna's *aar*, which carries only Android dispatch
libraries, so host JVM tests died in JNA before ever reaching our code. The
plain jna jar is added as a test dependency for its desktop libjnidispatch.
gradle/uniffi-native-lib.gradle points JNA at a host build of the library,
auto-discovering a sibling rust-sdks checkout and overridable via
LIVEKIT_UNIFFI_LIB_DIR. UniffiNativeLibraryTest asserts the library loads and
its checksums match the bindings, so a broken setup fails once with a clear
message instead of once per data stream test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the types and read path for per-peer feature negotiation, which the data
streams v2 send path needs in order to decide whether a given recipient can
accept an inline or compressed stream.
- ClientCapability: public enum mirroring ClientInfo.Capability. fromProto
returns null for unrecognized values instead of throwing, unlike most
fromProto helpers here -- capabilities are an open set, so a peer on a newer
SDK must be tolerated, not fatal.
- ClientProtocolVersion.DATA_STREAM_V2 (2), documented as a baseline
commitment rather than an optional feature.
- Participant.capabilities, populated from ParticipantInfo.capabilities
alongside the existing clientProtocol.
Purely additive, and deliberately read-only for now: this SDK does not yet
advertise v2 or any capability. Advertising has to wait until the receive path
can actually handle inline and compressed streams, otherwise peers would
start sending framings the current Kotlin implementation cannot parse. That
flip lands with the cutover.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Introduces the single place that touches livekit-uniffi. Not wired into anything
yet -- nothing injects it, so this commit changes no behavior.
DataStreams owns both FFI managers, the topic to handler registry, the FFI
delegates and the error mapping, so that everything above it keeps dealing only
in this SDK's own types.
Notable choices:
- The outgoing manager is built eagerly; the incoming one lazily on the first
inbound packet. Its payload cap comes from RoomOptions, which is not final
until connect() -- after this class is constructed -- so reading it eagerly
would silently ignore a maxPayloadSize passed to connect().
- Outbound packets go through an unbounded channel drained by one coroutine.
The FFI delegate is a synchronous callback on a Rust runtime thread and can
neither block nor suspend, but sending has to await publisher connection and
data channel backpressure. This keeps emission order and restores the
backpressure the previous implementation had.
- Stream handlers are dispatched onto our own scope rather than run inline on
the FFI thread, so an app handler that blocks cannot stall the core's runtime
and with it every other incoming stream.
- Delegates hold their owner strongly, unlike Swift. The JVM collects cycles,
so Swift's weak back-reference buys nothing here. What does matter is close()
running: the FFI's handle map holds the delegates from a static root, so
DataStreams registers with CloseableManager to release the native handles.
- Room state the send path needs (remote identities, protocols, capabilities,
the payload cap) arrives as assignable lambdas rather than by injecting Room,
which would be a Dagger cycle. Same pattern Room already uses for the RPC
managers.
Also adds the additive options this needs: `compress` on both stream option
classes and RoomOptions.dataStreamOptions.maxPayloadSize, all defaulted to
previous behavior.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cuts the data stream implementation over to livekit-uniffi. The hand-written
packet building and stream reassembly are gone; the managers are now thin
adapters over DataStreams.
The public API is unchanged. IncomingDataStreamManager and
OutgoingDataStreamManager keep their interfaces, so Room's and LocalParticipant's
delegation is untouched, and TextStreamSender / ByteStreamSender /
TextStreamReceiver / ByteStreamReceiver keep working by reusing their existing
seams: an FFI-backed StreamDestination, and a Channel pumped from an FFI reader.
The interface's handleStreamHeader/handleDataChunk/handleStreamTrailer remain as
shims that rebuild a packet, though Room now forwards whole packets instead --
v2 headers carry inline content and compression that only the core reads.
Behavior changes fall out of the implementation moving into a Rust actor, and
existing tests were updated to match rather than papered over:
- Sending and receiving are now asynchronous. A completed write means the core
accepted the payload, not that it reached the wire, and an incoming stream is
delivered after a round trip through the core. Tests that asserted
synchronously now await the outcome.
- Send failures no longer reach the caller: the core acknowledges a send when
it hands the packets over.
- StreamException.EncryptionTypeMismatch is unreachable; the core normalizes
encryption type at the boundary.
Three problems this shook out, all now handled:
- Everything on the FFI boundary runs on a real dispatcher, never the caller's.
The core resumes calls from its own runtime threads, which a virtual-time
test dispatcher can never deliver; worse, an unconfined dispatcher resumed
our coroutines *inline on a core runtime thread*, where waiting on data
channel backpressure deadlocked the runtime and stopped every stream.
- MockDataChannel's buffers became copy-on-write; sends now genuinely arrive
from several threads and assertions iterate the list concurrently.
- Test classes each get their own JVM. The core is process-global state loaded
via JNA, and Robolectric's per-class classloaders re-initializing the
bindings left callbacks pointing nowhere -- whole classes would time out
depending on execution order. Costs ~90s on this module; see the comment in
livekit-android-test/build.gradle.
All 326 tests pass, verified stable over repeated cold runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ith tests
Completes the cutover by telling peers what we can now do, and adds the tests
for the parts of it that only fail in interop.
Advertisement, deferred until the receive path could handle what it invites:
- ConnectOptions.clientProtocol now defaults to DATA_STREAM_V2.
- The connect URL carries a `capabilities` param, and ClientInfo the matching
repeated field, both sourced from one ADVERTISED_CLIENT_CAPABILITIES list.
Compression is advertised unconditionally, since it is done by the core
rather than a platform codec.
Tests (54 new, 380 total, all passing):
- DataStreamsV2SendTest walks the framing matrix from the spec's "Minimum
required test cases" -- pre-v2 room, all-v2 room, v2-without-the-capability,
mixed room, targeted subsets, compress opt-out, incremental writers -- and
asserts on the packets that reach the engine. These are really tests of our
registry wiring: the core picks the framing from what we report about each
recipient, so getting that wrong produces packets a peer cannot read while
everything still looks fine locally.
- DataStreamsV2ReceiveTest covers the framings only v2 produces (inline,
inline compressed, a deflate stream spread across chunks), plus topic
routing, sender identity, abort-on-disconnect, the payload cap, and a
multi-byte text round trip through the byte channel the public reader uses.
- DataStreamsConversionTest pins the translation layer, in particular the
error mapping, which is lossy by design -- several core failures fold onto
one pre-existing public exception, and a wrong fold is invisible.
- ConnectionParamsTest asserts the advertisement on the actual connect URL.
The Swift SDK shipped this wiring broken on one of its two connect paths, so
it is asserted on the wire rather than on the values feeding it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
spotlessCheck is a CI gate; this is its output plus one flake fix. awaitJob waited only for the RPC itself, but a completed call can still have siblings finishing behind it -- closing the request stream, emitting a disconnect event -- whose continuations are posted back to the test dispatcher from the core's threads. Occasionally one landed after the test body returned and surfaced as "unfinished coroutines found during the tear-down". Pump briefly after the job completes so they run inside the test. Verified over four consecutive cold runs of the full suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Every failure the core can report is now distinguishable, rather than several
folding onto the nearest pre-existing case.
New exceptions:
- HeaderTooLargeException, PayloadTooLargeException. Both are subclasses of
LengthExceededException rather than siblings, so existing code catching that
keeps catching every size-limit failure while new code can tell them apart.
Neither failure mode existed before v2 (there was no header budget and no
payload cap), so nothing was relying on the old folding.
- InternalException, which previously arrived as a TerminatedException.
TerminatedException gains a `reason`, defaulted and @jvmoverloads'd so
single-argument construction is unchanged. It separates the five remaining cases
that share the type -- already closed, invalid header, missed chunk, send
failed, invalid file name -- plus IO.
Io no longer maps to AbnormalEndException. A local file read failing is not the
remote closing the stream on us, which is what that exception documents; it is
now TerminatedException with reason IO.
The one caveat: StreamException is sealed, so an exhaustive `when` over it in
consumer code will need a branch for InternalException. Catch-based handling,
which is how exceptions are used here, is unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Surfacing that field was fallout from the protocol bump, not part of data streams, and it does not belong in this change. ProtoConverterTest requires every proto field to be mirrored on the Kotlin DTO, which is why it was added. Whitelisted instead, alongside the fields already listed there, so the test states plainly that it is not surfaced yet. Worth knowing: the SDK therefore cannot set agent dispatch attributes, which the server now accepts. That is a real gap, just an unrelated one -- if it is wanted, it should be its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…blocker
Adds instrumented tests and, in running them, found that the SDK could not have
worked on a 16 KB page-size device at all.
livekit-uniffi's AAR pins jna 5.16.0, whose libjnidispatch.so fails to load
there:
E linker: ".../libjnidispatch.so" program alignment (8192) cannot be smaller
than system page size (16384)
which surfaces as an inscrutable `NoClassDefFoundError: com.sun.jna.Native` --
JNA's classpath fallback masking the real dlopen failure. Our own
liblivekit_uniffi.so is fine (NDK r27 aligns to 16 KB by default), and so is
WebRTC's; JNA's prebuilt library is the only one that fails. Verified by loading
all three directly: only jnidispatch failed, and jna 5.19.1 loads.
The SDK now overrides the transitive pin. This matters beyond the emulator:
Android requires apps targeting API 35+ to support 16 KB page sizes, and such
devices ship today, so every data stream would have died on the first FFI call.
The real fix belongs in livekit-uniffi's own build.
DataStreamsOnDeviceTest (10 tests, passing on an API 37 arm64 emulator) covers
what the host JVM tests cannot:
- the AAR's .so loading on Android, through JNA, from packaged jniLibs;
- the bindings' Android cleaner path, chosen at API 34+, which uses
android.system.SystemCleaner. Under Robolectric that throws
IllegalAccessError and needs a JVM flag, so this is the only place it runs as
written;
- the v2 framings produced on-device matching the host build's, including a
send-to-receive loopback through the core that reconstructs a compressed
inline payload;
- this SDK's conversions and error mapping in an Android runtime.
No mocking framework: androidTest has no Mockito here, so these drive the FFI
directly with a capturing delegate, the same seam client-sdk-swift's tests use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces scripts/patch-uniffi-kotlin.py, which rewrote the generated bindings
after the fact, with fixes in livekit-uniffi itself. The bindings now compile as
generated, so there is no post-processing step to remember or keep working.
The two collisions needed different mechanisms:
- `close` is fixed by [bindings.kotlin.rename] in livekit-uniffi's uniffi.toml,
which is per-language exactly as wanted: Kotlin sees `closeStream()` while
Swift, Python and Node keep `close()`. Only this SDK's two call sites change.
- `message` could not be. UniFFI keys its rename table by crate name but looks
up enum and record *members* by the item's full module path, so a rename for
anything declared in a submodule is accepted and silently ignored -- which is
also why the method rename works, since methods key off the crate name. That
looks like an upstream bug and is worth reporting. There is no field-level
`#[uniffi(name)]` attribute in 0.31 either (uniffi_macros takes field names
straight from the Rust identifier), so the field is renamed to `reason` in
Rust. That is global rather than Kotlin-only, but it is a better name for an
error detail anyway, and Swift binds these positionally so its mapping is
unaffected.
Renaming the field changes the FFI metadata, so the AAR and all three Android
libraries were rebuilt; the checksum check between bindings and .so would fail
otherwise.
Verified: 380 unit tests and 10 instrumented tests on an API 37 emulator, both
against bindings generated with no manual edits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every debug launch of sample-app dies in Activity.onCreate:
java.lang.AbstractMethodError: abstract method "androidx.lifecycle.ViewModel
androidx.lifecycle.ViewModelProvider$Factory.create(kotlin.reflect.KClass,
androidx.lifecycle.viewmodel.CreationExtras)"
on receiver leakcanary.internal.ViewModelClearedWatcher$...
LeakCanary watches ViewModels by registering a ViewModelProvider.Factory, and it
is binary-incompatible with the lifecycle 2.8.0 this project resolves: 2.8.0's
KMP refactor made `create(KClass, CreationExtras)` part of the interface, and
LeakCanary implements only the older overload. Checked 2.14 as well as the
pinned 2.8.1 -- both crash -- so this is not a stale-version problem and there
is no version to bump to.
Removing the auto-install provider is LeakCanary's own documented way to turn it
off, and it is the smallest change that gets the app running. It is scoped to
sample-app's debug manifest, so nothing else is affected.
Pre-existing and unrelated to data streams -- the same dependency is on main --
but it blocks running the sample app at all, which is where the data streams
panel in the next commit lives.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a developer panel for exercising data streams by hand, reached from a new icon beside the RPC tester in the in-call controls. Conceptually the Android counterpart of livekit-examples/rust-dev-client#18, and structured the same way that PR describes: a send section and a subscriptions section. Until now the sample could only send a fixed `lk.chat` text stream from an AlertDialog and surfaced receipts as a Toast -- no topic, no destination, no byte streams, and nowhere to watch what arrived. Send: text or bytes, a topic, a destination (a remote participant or everyone), and a content box with `hello world` and `20k random` presets. Bytes are the UTF-8 of the same box. The result line reports the new stream's id, or the error. The 20k preset is deliberately random rather than a repeated character: random data does not compress, so it exercises the compressed multi-packet path instead of collapsing into a single inline packet. Subscribe: register a topic as text or bytes and watch it fill up. Each subscription is a card with its own scrolling list of arrivals, newest first, showing sender, size, time and a preview -- truncated for text, hex plus utf8 for bytes, and capped at 100 per topic. State lives on CallViewModel next to the RPC tester's, so subscriptions keep collecting while the panel is closed and are unregistered with the room in onCleared. Subscriptions are keyed on (topic, kind) because text and byte handlers are separate registries and the same topic can carry one of each. A topic something else already owns -- `lk.chat`, or the RPC topics the Room registers -- comes back as a failed Result and is shown, not thrown. Read failures are recorded as the preview, so a PayloadTooLarge or an aborted sender is visible rather than silent. Verified against a local livekit-server with two emulators in one room: text round-tripped, 20k random arrived as 19.5KB with the preview truncated, bytes and text coexisted on one topic with the hex/utf8 preview correct, the destination dropdown listed the peer, and subscribing to `lk.chat` was refused without a crash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Initial port of data streams v2 to the android sdk. The corresponding swift change (which this is fairly heavily patterned off of ) can be found here: livekit/client-sdk-swift#1075
Previously, the android sdk had its own data streams implementation. With data streams v2, the rust implementation will both be quite a bit more stable and gain some new features that make it significantly more performant (single packet data streams and DEFLATE compression when it makes the payload smaller). This roughly doubles data stream throughput in local testing.
So, port the android sdk to use the rust sdk data streams v2 implementation, and completely remove the pre-existing kotlin implementation. This is a substantial change which needs thorough testing.
New behaviors worth being aware of
All of these are either data streams v2 related changes, or bug fixes.
1.
compressoptionWhen sending a data stream, there is a new
compressoption. Just like how this works on web / rust, this defaults totrue. If set tofalse, then compression will be disabled (useful if you know the data you are sending isn't compressible / you are doing your own compression, which is not uncommon in robotics use cases). The vast majority of users should leave this set totrue.2. Max data stream size
In a rust data streams v2 pull request review comment, we decided that for security reasons it made sense to introduce a maximum data stream size as a DOS protection. This limit is by default
5gb- any data stream that is larger will now read up until that point, and if the stream keeps going, a "payload too large" error will be raised on the stream and exposed to a user on the subsequent.read()call.If a user is sending a large file, they can override this by setting a new
maxPayloadByteLengthoption on theroom.connectcall:room.connect( url = wsUrl, token = token, options = ConnectOptions( autoSubscribe = true ), roomOptions = RoomOptions( dynacast = false, dataStream = DataStreamOptions(maxPayloadByteLength = 1000) ) )3. Throwing new data stream errors types
The old kotlin specific data streams implementation wasn't quite as strict and didn't surface as many error cases to the caller which were encountered while reading the stream as the rust implementation now does. This needs some testing in some of these edge cases to make sure I didn't inadvertently make this backwards incompatible in a non-aceptable way.
Uniffi integration
In addition to the data streams v2 features, I've started integrating the uniffi kotlin bindings into this sdk based off of DL's in progress branch. For the most part, this has gone fairly smoothly. I've kept everything wired up with maven local for the time being and will leave it as a cleanup item for an android expert (likely DL) to this working properly prior to merging.
As part of this, I have also fixed two issues in the
livekit-uniffikotlin bindgen:Any uniffi objects which have a method with the name
closedoes not build on uniffi-rs0.31. I made a ticket here: Kotlin bindings cannot have a method namedclose, if it does the generated code isn't valid mozilla/uniffi-rs#2955. I have worked around this by renamingclose->close_streamas a kotlin specific override here: livekit/rust-sdks@1fe0ebaAny uniffi enums which have tagged cases that contain fields names
messagefail to build:Unfortunately, this one can't be fixed in the same way, so I've opted to rename all error cases that contain
messageto instead bereason, also in here: livekit/rust-sdks@1fe0eba. The uniffi-rs maintainers seem to be unable to figure out a good way to fix this - the latest related issue was closed: mozilla/uniffi-rs#2938Warning
This pull request was LLM generated and has only been reviewed by a human who isn't a domain expert in android development. I have tested this and confirms it works in the happy path, but no other validation has been done.
A more thorough review of this needs to occur before it could be merged.
Todo