Repository navigation
Merge livekit-ffi / livekit-uniffi together into a new livekit-ffi with room-apis andcore-modules modes - #1438
Merged
Merged
Conversation
1egoman
added this pull request to stack #1440
September 18, 2026 17:14
1egoman
commented
Sep 18, 2026
Comment on lines
+175
to
+193
|
|
||
| # `--workspace` above builds livekit-ffi with its default features, which | ||
| # select livekit-ffi/room-apis. core-modules is mutually exclusive with | ||
| # it, so it needs its own invocation or nothing would compile the | ||
| # core-modules code per-commit across this matrix. (Before the | ||
| # livekit-uniffi merge that coverage came free from the separate crate.) | ||
| - name: Build (Cargo, livekit-ffi core-modules) | ||
| if: ${{ !contains(matrix.target, 'android') }} | ||
| run: | | ||
| set -euo pipefail | ||
| FLAGS=$(python3 .github/scripts/platform_features.py --surface core-modules) | ||
| cargo build --release -p livekit-ffi --target ${{ matrix.target }} $FLAGS | ||
|
|
||
| - name: Build (Android, livekit-ffi core-modules) | ||
| if: ${{ contains(matrix.target, 'android') }} | ||
| run: | | ||
| set -euo pipefail | ||
| FLAGS=$(python3 .github/scripts/platform_features.py --surface core-modules) | ||
| cargo ndk --target ${{ matrix.target }} build --release -p livekit-ffi $FLAGS |
Contributor
Author
There was a problem hiding this comment.
These builds ensure that core-modules gets built in ci to ensure it compiles on every commit.
I think it might make sense to build all configurations here though? Not just android, but also swift and flutter too?
Comment on lines
+376
to
+427
| # This is here because pub.dev's automated publishing *requires* a | ||
| # tag-push-triggered workflow as part of its trusted publishing | ||
| # configuration. If the default github actions worker were to publish the | ||
| # tag, then github prevents that from triggering other workflows to | ||
| # prevent workflow recursion. So this "breaks the cycle" and allows the | ||
| # downstream workflow to be triggered to satisfy pub.dev. | ||
| - name: Generate Repo Writer GitHub App token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 | ||
| with: | ||
| app-id: ${{ vars.REPO_WRITER_GH_APP_ID }} | ||
| private-key: ${{ secrets.REPO_WRITER_GH_APP_PRIVATE_KEY }} | ||
| owner: livekit | ||
| repositories: | | ||
| rust-sdks | ||
|
|
||
| - name: Create release tag | ||
| env: | ||
| GH_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| TAG: ${{ needs.get-tag.outputs.tag_name }} | ||
| SHA: ${{ github.sha }} | ||
| run: | | ||
| gh release upload ${{ needs.get-tag.outputs.tag_name }} ${{ github.workspace }}/ffi-builds/* | ||
| gh release edit ${{ needs.get-tag.outputs.tag_name }} --draft=false | ||
| set -euo pipefail | ||
|
|
||
| # This workflow runs on every push to main and adopts whatever draft | ||
| # release it finds, which may be several commits old. Tagging a commit | ||
| # whose Cargo.toml says a different version would publish mismatched | ||
| # packages, and uniffi-dart-publish.yml would reject it later anyway. | ||
| crate_version=$(grep -m1 '^version = ' livekit-ffi/Cargo.toml | cut -d'"' -f2) | ||
| if [ "${TAG#livekit-ffi/v}" != "$crate_version" ]; then | ||
| echo "::error::tag ${TAG} does not match livekit-ffi version ${crate_version} at ${SHA}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| existing=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/${TAG}" --jq '.object.sha' 2>/dev/null || true) | ||
| if [ -n "$existing" ]; then | ||
| # Do not fail: the assets are already uploaded, and failing here | ||
| # would strand the release as a draft. No push event was emitted, | ||
| # so the downstream publishes need a manual dispatch. | ||
| echo "::warning::tag ${TAG} already exists at ${existing}; not re-creating it." | ||
| echo "::warning::uniffi-packages.yml and uniffi-dart-publish.yml did NOT run; dispatch them with this tag if the packages still need publishing." | ||
| exit 0 | ||
| fi | ||
|
|
||
| gh api --method POST "repos/${GITHUB_REPOSITORY}/git/refs" \ | ||
| -f ref="refs/tags/${TAG}" \ | ||
| -f sha="${SHA}" | ||
|
|
||
| - name: Publish release | ||
| env: | ||
| TAG: ${{ needs.get-tag.outputs.tag_name }} | ||
| run: gh release edit "$TAG" --draft=false |
Contributor
Author
There was a problem hiding this comment.
This was a weird one - a LLM mentioned to me midway through making this update that the trusted publishing setup for pub.dev (flutter package registry) requires that a new tag be pushed to trigger the github actions workflow that publishes the package.
Previously, this worked fine (sort of "by accident") because when a new livekit-uniffi tag was made by the knope github app upon release, that triggered the workflow.
The problem is livekit-ffi works differently, and as part of merging these two packages we end up inheriting this process. livekit-ffi instead publishes a draft release first, then uploads all artifacts to that draft release, and then once the release is all ready to go, at the very end marks it as no longer draft. The problem is that "mark as no longer draft" step was being run as the github actions default user account, which per github's documentation will not trigger other workflows. So if we were to upload the flutter release with this process the separate workflow that pub.dev trusted publishing requires would never get kicked off and the package publish would always fail.
To work around this, I've reached for the livekit-repo-writer bot which credentials are passed in as org-level secrets. Now what happens:
livekit-ffigithub release is created as draft, and all artifacts uploaded as it was previously- A new tag is manually created matching the old
livekit-uniffiformat (for backwards compatibility) usinglivekit-repo-writer, which kicks off the flutter job required for pub.dev trusted publishing - The
livekit-ffirelease is marked as no longer in draft.
cc @hiroshihorie - What do you think about these updates? Can you go through the flutter publish workflow in this branch and make sure that it all still looks good to you?
Contributor
There was a problem hiding this comment.
Odd restriction, but if that's what necessary I guess we don't have choice without disabling trusted publishing?
Contributor
Author
There was a problem hiding this comment.
Yep that is my understanding, and we 100% want to keep trusted publishing.
Contributor
There was a problem hiding this comment.
Curiously, what are the special part for the livekit-ffi tag that made by the livekit-repo-writer ?
can livekit-repo-writer create a canonical livekit-ffi release tag, and after Flutter succeeds, publish the GitHub release, if flutter failed, delete the tag ?
It is not blocking this PR
Contributor
Author
There was a problem hiding this comment.
@xianshijing-lk I will tag @hiroshihorie as I think he might be able to provide a better answer, I didn't set up this flutter publishing workflow so I'm unfamiliar with the exact nuances.
Contributor
Author
There was a problem hiding this comment.
As it has been a few weeks with no movement, I'm going to assume this will work for now in its current state.
Member
There was a problem hiding this comment.
Went through the Dart publish path on this branch and it LGTM
Comment on lines
+26
to
+40
| // livekit-ffi/core-modules is pure Rust: no libwebrtc download, no linker | ||
| // configuration (`configure_linker` panics on visionOS/tvOS/Mac Catalyst, | ||
| // which the Swift package builds) and no protobuf generation. | ||
| // | ||
| // This has to be a `cfg` rather than a `CARGO_FEATURE_ROOM_APIS` lookup: | ||
| // `prost-build` and `webrtc-sys-build` are optional build-dependencies | ||
| // gated on the same feature, so under core-modules those paths do not | ||
| // resolve at all. | ||
| #[cfg(feature = "room-apis")] | ||
| { | ||
| download_webrtc(); | ||
| copy_webrtc_license(); | ||
| configure_linker(); | ||
| generate_protobuf(); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Note to reviewers: build.rs becomes a no-op if room-apis isn't passed (ie, livekit-ffi is being built in core-modules mode).
Comment on lines
+137
to
+168
| # Which FFI mode each downstream package build is configured to use: room-apis or core-modules. | ||
| # | ||
| # This is the single source of truth: CI and Makefile.toml resolve their build | ||
| # flags from here via .github/scripts/platform_features.py, so a platform | ||
| # cannot silently drift onto the wrong surface. `room-apis` and `core-modules` | ||
| # are mutually exclusive, so every core-modules entry must turn the default | ||
| # feature set off. | ||
| # | ||
| # $ .github/scripts/platform_features.py --list | ||
| # | ||
| # A build site that produces an artifact for a platform names that platform. | ||
| # The script also answers `--surface <feature>` for the few sites that are not | ||
| # platform builds at all -- the per-commit gates in builds.yml and tests.yml, | ||
| # and the shared host build in Makefile.toml that the bindgens read metadata | ||
| # from. | ||
| # | ||
| [package.metadata.platform-features] | ||
| # room-apis -- SDKs with no WebRTC stack of their own. These consume the C ABI | ||
| # (and, increasingly, UniFFI) out of the livekit-ffi release archives. | ||
| python = { features = ["room-apis", "rustls-tls-native-roots"] } | ||
| node = { features = ["room-apis", "rustls-tls-native-roots"] } | ||
| unity = { features = ["room-apis", "rustls-tls-native-roots"] } | ||
| ffi-ios = { no-default-features = true, features = ["room-apis", "rustls-tls-webpki-roots"] } | ||
| ffi-android = { no-default-features = true, features = ["room-apis", "rustls-tls-webpki-roots"] } | ||
| ffi-windows-arm64 = { no-default-features = true, features = ["room-apis", "native-tls"] } | ||
|
|
||
| # core-modules -- SDKs that already include their own WebRTC stack and consume | ||
| # this crate purely over UniFFI. | ||
| swift = { no-default-features = true, features = ["core-modules"] } | ||
| android = { no-default-features = true, features = ["core-modules"] } | ||
| dart = { no-default-features = true, features = ["core-modules"] } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
@ladvoc I took your suggestion from the compile all features in ci pull request #1393 and opted to manage which sdk build maps to which "distribution" of livekit-ffi in livekit-ffi/Cargo.toml.
I'm curious your thoughts here. In particular, I think the android vs ffi-android difference is a bit sloppy. I couldn't really figure out a more elegant way to do it though because making a platform map to an arbitrary array of builds resulted in a much larger set of github actions changes which I didn't want to mix with this.
Comment on lines
-210
to
-229
| # MARK: - Node | ||
|
|
||
| [tasks.node-bindgen] | ||
| private = true | ||
| dependencies = ["build", "locate-lib"] | ||
| env = { CARGO_NET_GIT_FETCH_WITH_CLI = true } | ||
| install_crate = { crate_name = "uniffi-bindgen-node", binary = "uniffi-bindgen-node", test_arg = "--help" } | ||
| install_crate_args = ["--git", "https://github.com/livekit/uniffi-bindgen-node.git"] | ||
| command = "uniffi-bindgen-node" | ||
| args = [ | ||
| "--out-dir", "${PACKAGES_DIR}/${LANG}", | ||
| "--out-dirname-api", "import-meta-url", | ||
| "--out-node-version", "^18", | ||
| "--out-verbose-logs", | ||
| "--crate-name", "${CARGO_MAKE_CRATE_FS_NAME}", | ||
| "${LIB_PATH}" | ||
| ] | ||
|
|
||
| [tasks.node-format] | ||
| private = true |
Contributor
Author
There was a problem hiding this comment.
Note to reviewers - as part of this I outright dropped the old livekit-uniffi node build. It was super out of date (still using uniffi-bindgen-node) and we don't need a node build of livekit-ffi in core-modules mode anyway.
My plan is after this is merged to rebase james' pull request #1374 on top of this, merging it with the existing @livekit/rtc-ffi-bindings builds which are being made today for node. These builds would look identical to how they work today except for they would also now also contain the node bindings as well.
Comment on lines
+52
to
+73
| ## Things that bite | ||
|
|
||
| - **Never enable both features.** Each pins its own UniFFI namespace via | ||
| `setup_scaffolding!`, and a crate may declare only one. A `compile_error!` in | ||
| `src/lib.rs` enforces this; another rejects selecting neither. | ||
| - **The cargo-make tasks below take no `-p`** and rely on the working | ||
| directory, so they need explicit feature flags or they build the default | ||
| livekit-ffi/room-apis and all of libwebrtc. `Makefile.toml` resolves those from | ||
| the table above — do not write them out by hand. | ||
| - **`build.rs` is gated on `room-apis`.** It downloads libwebrtc, configures the | ||
| linker, and panics on any target it does not recognise — visionOS, tvOS and | ||
| Mac Catalyst among them. That gate is what makes the core-modules Apple | ||
| builds possible. | ||
| - **`prost` is split.** `prost` is 0.14, for this crate's own generated protos; | ||
| `prost_012` is 0.12, for the `livekit-protocol` types used in | ||
| `src/core_modules/`. The surfaces are mutually exclusive, so the two versions | ||
| never compile together. | ||
| - **Verify UniFFI API changes by compiling the Kotlin bindings** | ||
| (`cargo make android-package`). A green `cargo build` proves nothing there; | ||
| see the UniFFI section of the root [AGENTS.md](../AGENTS.md) for the specific | ||
| naming traps. | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Note to self: I could probably make this sound a little less LLM generated haha...
Comment on lines
+16
to
+27
| # Dart binding generator. Not published to crates.io, so pinned by git rev. The | ||
| # rev must target the same uniffi-rs release (0.31) as the `uniffi` dependency | ||
| # above, or it cannot read the compiled metadata. | ||
| # | ||
| # Pinned to upstream main rather than a release: the newest release | ||
| # (v0.2.1+v0.31.2) predates the three fixes this needs -- custom types leaking | ||
| # into FFI signatures (#154), object converter statics colliding with Rust | ||
| # methods named `write` (#156), and each crate re-declaring the runtime | ||
| # scaffolding, which kept livekit-datatrack's types from crossing over (#150). | ||
| # Move to a version requirement once a release carries them. | ||
| uniffi-dart = { git = "https://github.com/Uniffi-Dart/uniffi-dart", rev = "67547701d849c9ef6302f79bb2189bb6075f69fa", optional = true } | ||
| camino = { version = "1", optional = true } |
Contributor
Author
There was a problem hiding this comment.
(Note to reviewers - all this is copied from the old livekit-uniffi Cargo.toml)
1egoman
force-pushed
the
livekit-ffi-livekit-uniffi-merge
branch
from
September 18, 2026 19:59
a102481 to
93ab911
Compare
Contributor
Changeset ✓This PR includes a changeset covering all affected packages:
|
1egoman
force-pushed
the
livekit-ffi-livekit-uniffi-merge
branch
from
September 18, 2026 20:14
93ab911 to
9fe164f
Compare
1egoman
marked this pull request as ready for review
September 18, 2026 20:15
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Note
Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.
🔍 Devin Review: 3 flags
Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
ladvoc
approved these changes
Sep 21, 2026
Comment on lines
+376
to
+427
| # This is here because pub.dev's automated publishing *requires* a | ||
| # tag-push-triggered workflow as part of its trusted publishing | ||
| # configuration. If the default github actions worker were to publish the | ||
| # tag, then github prevents that from triggering other workflows to | ||
| # prevent workflow recursion. So this "breaks the cycle" and allows the | ||
| # downstream workflow to be triggered to satisfy pub.dev. | ||
| - name: Generate Repo Writer GitHub App token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 | ||
| with: | ||
| app-id: ${{ vars.REPO_WRITER_GH_APP_ID }} | ||
| private-key: ${{ secrets.REPO_WRITER_GH_APP_PRIVATE_KEY }} | ||
| owner: livekit | ||
| repositories: | | ||
| rust-sdks | ||
|
|
||
| - name: Create release tag | ||
| env: | ||
| GH_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| TAG: ${{ needs.get-tag.outputs.tag_name }} | ||
| SHA: ${{ github.sha }} | ||
| run: | | ||
| gh release upload ${{ needs.get-tag.outputs.tag_name }} ${{ github.workspace }}/ffi-builds/* | ||
| gh release edit ${{ needs.get-tag.outputs.tag_name }} --draft=false | ||
| set -euo pipefail | ||
|
|
||
| # This workflow runs on every push to main and adopts whatever draft | ||
| # release it finds, which may be several commits old. Tagging a commit | ||
| # whose Cargo.toml says a different version would publish mismatched | ||
| # packages, and uniffi-dart-publish.yml would reject it later anyway. | ||
| crate_version=$(grep -m1 '^version = ' livekit-ffi/Cargo.toml | cut -d'"' -f2) | ||
| if [ "${TAG#livekit-ffi/v}" != "$crate_version" ]; then | ||
| echo "::error::tag ${TAG} does not match livekit-ffi version ${crate_version} at ${SHA}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| existing=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/${TAG}" --jq '.object.sha' 2>/dev/null || true) | ||
| if [ -n "$existing" ]; then | ||
| # Do not fail: the assets are already uploaded, and failing here | ||
| # would strand the release as a draft. No push event was emitted, | ||
| # so the downstream publishes need a manual dispatch. | ||
| echo "::warning::tag ${TAG} already exists at ${existing}; not re-creating it." | ||
| echo "::warning::uniffi-packages.yml and uniffi-dart-publish.yml did NOT run; dispatch them with this tag if the packages still need publishing." | ||
| exit 0 | ||
| fi | ||
|
|
||
| gh api --method POST "repos/${GITHUB_REPOSITORY}/git/refs" \ | ||
| -f ref="refs/tags/${TAG}" \ | ||
| -f sha="${SHA}" | ||
|
|
||
| - name: Publish release | ||
| env: | ||
| TAG: ${{ needs.get-tag.outputs.tag_name }} | ||
| run: gh release edit "$TAG" --draft=false |
Contributor
There was a problem hiding this comment.
Odd restriction, but if that's what necessary I guess we don't have choice without disabling trusted publishing?
1egoman
requested review from
MaxHeimbrock,
alan-george-lk,
davidliu,
hiroshihorie,
pblazej and
xianshijing-lk
September 22, 2026 15:20
xianshijing-lk
approved these changes
Sep 22, 2026
xianshijing-lk
left a comment
xianshijing-lk
left a comment
Contributor
There was a problem hiding this comment.
lgtm with some questions / nits
Comment on lines
+376
to
+427
| # This is here because pub.dev's automated publishing *requires* a | ||
| # tag-push-triggered workflow as part of its trusted publishing | ||
| # configuration. If the default github actions worker were to publish the | ||
| # tag, then github prevents that from triggering other workflows to | ||
| # prevent workflow recursion. So this "breaks the cycle" and allows the | ||
| # downstream workflow to be triggered to satisfy pub.dev. | ||
| - name: Generate Repo Writer GitHub App token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 | ||
| with: | ||
| app-id: ${{ vars.REPO_WRITER_GH_APP_ID }} | ||
| private-key: ${{ secrets.REPO_WRITER_GH_APP_PRIVATE_KEY }} | ||
| owner: livekit | ||
| repositories: | | ||
| rust-sdks | ||
|
|
||
| - name: Create release tag | ||
| env: | ||
| GH_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| TAG: ${{ needs.get-tag.outputs.tag_name }} | ||
| SHA: ${{ github.sha }} | ||
| run: | | ||
| gh release upload ${{ needs.get-tag.outputs.tag_name }} ${{ github.workspace }}/ffi-builds/* | ||
| gh release edit ${{ needs.get-tag.outputs.tag_name }} --draft=false | ||
| set -euo pipefail | ||
|
|
||
| # This workflow runs on every push to main and adopts whatever draft | ||
| # release it finds, which may be several commits old. Tagging a commit | ||
| # whose Cargo.toml says a different version would publish mismatched | ||
| # packages, and uniffi-dart-publish.yml would reject it later anyway. | ||
| crate_version=$(grep -m1 '^version = ' livekit-ffi/Cargo.toml | cut -d'"' -f2) | ||
| if [ "${TAG#livekit-ffi/v}" != "$crate_version" ]; then | ||
| echo "::error::tag ${TAG} does not match livekit-ffi version ${crate_version} at ${SHA}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| existing=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/${TAG}" --jq '.object.sha' 2>/dev/null || true) | ||
| if [ -n "$existing" ]; then | ||
| # Do not fail: the assets are already uploaded, and failing here | ||
| # would strand the release as a draft. No push event was emitted, | ||
| # so the downstream publishes need a manual dispatch. | ||
| echo "::warning::tag ${TAG} already exists at ${existing}; not re-creating it." | ||
| echo "::warning::uniffi-packages.yml and uniffi-dart-publish.yml did NOT run; dispatch them with this tag if the packages still need publishing." | ||
| exit 0 | ||
| fi | ||
|
|
||
| gh api --method POST "repos/${GITHUB_REPOSITORY}/git/refs" \ | ||
| -f ref="refs/tags/${TAG}" \ | ||
| -f sha="${SHA}" | ||
|
|
||
| - name: Publish release | ||
| env: | ||
| TAG: ${{ needs.get-tag.outputs.tag_name }} | ||
| run: gh release edit "$TAG" --draft=false |
Contributor
There was a problem hiding this comment.
Curiously, what are the special part for the livekit-ffi tag that made by the livekit-repo-writer ?
can livekit-repo-writer create a canonical livekit-ffi release tag, and after Flutter succeeds, publish the GitHub release, if flutter failed, delete the tag ?
It is not blocking this PR
ladvoc
pushed a commit
that referenced
this pull request
Sep 22, 2026
Previously, `prost` 0.12 was being used in some places (ie, `livekit-protocol`), and 0.14 in others (ie, `livekit`, `livekit-signaling`, etc). This made it annoying in the `livekit-ffi` / `livekit-uniffi` merge (#1438), because a single crate was now re-exporting potentially many different subcrates with differering prost versions. So this meant that the new merged `livekit-ffi` now had to track two different `prost` versions which was pretty gross. I can't figure out a good reason _why_ 0.12 is used in the places it is, so this pull request upgrades everything to 0.14 and pins all `prost` versions to the workspace version via `workspace = true`. I'm opting to break this out from the `livekit-ffi` / `livekit-uniffi` merge because it is sufficiently decoupled and seems like a good idea to do regardless of how that merge effort goes. Also, upgrades `pbjson-types` to `0.9` since that specifies a prost `0.14` peer dependency. Closes CLT-2441 --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
1egoman
force-pushed
the
livekit-ffi-livekit-uniffi-merge
branch
from
September 22, 2026 20:58
ca86380 to
fe9ce4f
Compare
1egoman
force-pushed
the
livekit-ffi-livekit-uniffi-merge
branch
from
October 9, 2026 00:32
fe9ce4f to
d40cc30
Compare
Contributor
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
🐛 2 issues in files not directly in the diff
🐛 High-quality resampling selects lower quality
High and VeryHigh requests pass SoxQualityRecipe discriminants instead of their SoX recipe values. They select recipes 3 and 4 instead of 4 and 6, degrading resampled audio.
⚠️ Variable-rate video drops valid frames
When GStreamer emits frames slower than its advertised framerate, observe_access_unit treats a timestamp gap as queue loss. It discards valid delta frames until the next keyframe, interrupting video.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
Contributor
|
As far as I am concerned, this will not affect Unity for now. |
jhugman
added a commit
that referenced
this pull request
Oct 12, 2026
…indings DataTrackFrame, RemoteDataTrackPipelineOptions and DataTrackSubscribeOptions are field-identical to core_modules' records of the same name, down to the uniffi defaults. #1438 brings both surfaces into one crate, after which they collapse into a single ungated module: uniffi namespaces are per-crate, so an ungated record lands in livekit_ffi or livekit_uniffi according to which surface is built, and every downstream SDK keeps the module path it has today. Each carries a marker saying so, since nothing else in the file would lead a reader there. Doc comments reach the foreign bindings, which the generated Python makes plain: DataTrackStream shipped its own TODO and a `crate::migration::data_track` path into the Python docstring an SDK user reads. Internal notes are `//` from here — what stays `///` is the sentence describing the type.
jhugman
added a commit
that referenced
this pull request
Oct 12, 2026
DataTrackInfo's sid is an Arc<RwLock<DataTrackSid>> the managers write through: local/manager.rs reassigns it on republish, remote/manager.rs when a publisher reconnects, both in place on the DataTrackInfo the live track already holds. The track survives the reconnect; its SID does not. Everything the FFI surface hands over is a snapshot of that moment — OwnedLocalDataTrack and OwnedRemoteDataTrack flatten the info into protobuf at from_track, and nothing re-announces it afterwards. So the earlier reasoning for leaving info() out, that a caller holding a migrated handle already received the info with it, holds for name, uses_e2ee, schema and frame_encoding, and fails for the one field that moves. info() reads through to the live value instead, which closes the gap for uniffi callers. It is the first method here with no protobuf request behind it, so it widens the surface rather than migrating it — deliberate, because the alternative is a method whose absence has a wrong answer. The protobuf half of the same gap is untouched and wants its own issue: there is no info request and no event when the SID is reassigned, so python, node, unity and c++ hold a stale SID with no way to refresh it. That is a feature, not a migration, and it predates this PR. DataTrackInfo, DataTrackSchemaId and the DataTrackSid converter mirror core_modules' field for field, and carry the same markers as the other records for the dedupe #1438 enables. Registering the converter here is safe meanwhile: the two surfaces are mutually exclusive, so only one is ever built.
lukasIO
approved these changes
Oct 14, 2026
Follow the tag prefix of the merged crate. The workflow now starts on a `livekit-ffi/v*` tag, takes the version from that prefix, and compares it with the version in livekit-ffi/Cargo.toml. The staging directory and the guard against a native library in the package keep their paths correct for the new crate. The name of the package on pub.dev does not change in this commit. Users of the package therefore need no change.
Before the merge, livekit-uniffi was a separate crate. A build of the workspace thus compiled it, and a test run of the workspace ran its tests. That is no longer true. The `core-modules` feature is not a default feature, so a build of the workspace does not reach this code, and its thirteen tests would stop running without any message. Add one build step and one test step that select the feature. The two features are mutually exclusive, so these cannot join the workspace commands and need their own invocation. Both steps take their flags from the platform table.
This workflow generates the Node protobuf code from the files in livekit-ffi/protocol. Its path filter matched all of livekit-ffi, the two root manifests and all of .github. After the merge that filter also matches every change to the core modules, which have no protobuf files. The workflow writes a commit back to the branch, so it should run only when it has work to do. Match the protobuf directory, the Node bindings crate and this workflow file. This is less than the filter matched before the merge.
The readme described one FFI interface for Python, Node and Unity. The crate now has two surfaces, and the file must say which SDK uses which. Give each surface its section: what it contains, which SDKs consume it, where its code is and the command that builds it. Point at the platform table for the full list. Add the notes that save time later: the two features cannot be selected together, the cargo-make tasks need explicit feature flags, the build script is gated, and prost has two versions. The file AGENTS.md is a symbolic link to this file, so both are correct.
Correct the places that describe two FFI crates: the agent notes, the root readme, the notes of three other crates, the comments that point at files in the deleted crate, and the note about which crates the feature job checks. Add the changeset for the merge. It records that the two surfaces are now one crate with two features, that everything which built livekit-ffi builds the same way, and that what built livekit-uniffi now selects the core-modules feature. The changeset also records the one visible difference for a downstream SDK: the native library is now liblivekit_ffi. No package name and no import changes.
Review question on the table: should every entry set `no-default-features = true`? Yes. The entries for python, node and unity did not set it, so they took the `default` feature list and added their own on top. They give the correct flags only while `default` stays as it is now. A later change to `default` would move these three platforms without any sign. Set the flag on all three. The resolved dependency tree does not change, because `default` lists the same two features that these entries list. Make the script ask for the flag on every entry and not only on a core-modules entry. An entry that omits it now stops with a message that says why.
The capture source work on main adds livekit-ffi/src/server/capture.rs, which imports from `crate::conversion`. The rebase puts that file in the room_apis module, where the path no longer resolves, and a build with the `capture` feature stops with an error. The cause is visibility. The glob re-export at the crate root republishes only the public items of the surface, and `conversion` is private. The paths `crate::proto` and `crate::server` continue to work, because those modules are public, but `crate::conversion` does not. Make `conversion` visible to the crate and re-export it at the crate root. The module stays out of the public API, and the path is the same as before the move. Code that comes from main thus needs no change now or later.
Every task in Makefile.toml built livekit-ffi/core-modules, because [tasks.build] asked the resolver for a surface and not for a platform. Nothing that a task said could change it. A bindgen only chose an output language, and the library it read was already built. Each task now names its platform in LK_PLATFORM, and the build resolves its flags from the platform table in Cargo.toml. To add a bindgen, add a row to that table. The kotlin, dart and swift tasks keep the features they had, because all three platforms are on core-modules. The python task changes: the Python SDK consumes the room-apis release archive, so its bindings now come from that surface. This makes the task the local equivalent of the Python bindings step in .github/workflows/ffi-builds.yml. LK_PLATFORM must be set on a task that drives the work through `run_task`. A task that uses `extend` has its own env out of scope while its dependencies run, and `build` is a dependency, so bindgen-kotlin and bindgen-python change to the wrapper shape that bindgen-dart already uses. The android and swift tasks name the platform in their scripts, for the same reason the android profile switch is in its script. The flags are resolved with `set -e` in front. If the resolver fails, the assignment fails and the shell stops. Without it the flags would be empty and cargo would quietly build the default features, which is the wrong surface for most of these tasks.
These three steps build no artifact for any platform. They run over eleven target triples and ship nothing. Their only purpose is to show that the core-modules surface still compiles and that its tests still pass. Naming a platform here would mean choosing one that does not describe the step, which is the problem that the platform table exists to remove. State the two flags instead, with a comment that says where platform builds get theirs. This leaves the resolver with one job: answer for a platform.
Nothing calls it now. The packaging asks for a platform, and the per-commit gates state their flags directly. The option existed because the packaging built one surface for several platforms at once and had no platform to name. That is no longer true: each task names the platform it builds for. Removing it also removes the question of what a caller should pass when no platform describes it. The answer now is that a build site which produces an artifact always has a platform, and one which does not is not in the table.
The script imported the core-modules API from `../generated/python`. No task writes to that directory: bindgen-python writes to `packages/python`. The script was therefore already broken, and no CI job or cargo-make task ran it. It is now also unreachable in principle. The python bindings come from the room-apis surface, which has none of the functions that the script calls. The file is in the git history if the core-modules python bindings are wanted again.
Dropping the commit that edited two older changesets took their bumps out of this pull request. The check counts a bump only from a changeset that the pull request changes, so livekit, livekit-api, livekit-capture and livekit-signaling lost their coverage, although this branch still changes crates that they depend on. Declare those four here, with the dependents that this changeset already names.
The two Windows build jobs failed. A Windows runner uses PowerShell, and both steps were written for bash: a backslash at the end of a line continues it in bash, but not in PowerShell. PowerShell read the second line as a new statement and stopped with "Missing expression after unary operator". The step failed before cargo started. Ask for bash, as the equivalent steps in tests.yml and uniffi-cdylib.yml do. The android step has the same shape and runs only on Linux, but say it there too: one of the two without it is how this comes back. Remove `set -euo pipefail`. GitHub already starts a `shell: bash` step with `-eo pipefail`.
Previously, prost 0.12 was being used in some places (livekit-protocol), and 0.14 in others (livekit, livekit-signaling, etc). This made it annoying in the livekit-ffi / livekit-uniffi merge, because a single crate was now re-exporting potentially many different subcrates, potentially with different prost versions. I can't figure out a good reason _why_ 0.12 is used in the places it is, so this pull request upgrades everything to 0.14 and pins all prost versions to the workspace version via `workspace = true`. Also, upgrades `pbjson-types` to `0.9` since that specifies a prost 0.14 peer dependency.
The crate no longer holds two versions of prost. The upgrade to prost 0.14 across the workspace removed the renamed `prost_012` dependency, and this note was the only place left that still described it. Verified: `prost_012` appears nowhere else in the tree.
The text explained the excluded dependencies by the Apple targets they make possible. That is a consequence, not the reason. An SDK that brings its own WebRTC stack must not get a second one from this crate. The Apple targets are still mentioned, under the build script note that actually depends on them.
The file is new in this branch, written in 2026. Its header said 2025.
The script answers for one crate, livekit-ffi, and the name now says so. The word "platform" described only the key it takes. The table it reads keeps its name, because that name describes the keys and not the crate.
The build script carried nine `cfg` attributes, one on each item that only the room-apis surface needs. Put those items in a `room_apis` module with one entry point, and gate the module. Two attributes remain: one on the module and one on the call in `main`. The behaviour does not change. Verified: the core-modules build still runs no step, and the room-apis build still downloads libwebrtc, configures the linker and generates the protobuf code.
The crate is gone, and knope ignores a changeset entry naming a package that is not in knope.toml -- without an error, and without the `changeset-check.yml` gate catching it (changeset_detect.py filters unknown packages out before the `invalid` check). `protocol_thread_schema.md` named it alongside live packages, so the stray line was merely inert. `uniffi_swift_dsyms.md` named nothing else, so it contributed no changelog entry *and was never deleted* -- it would have sat in .changeset/ being silently re-ignored on every future release, losing the dSYM entry permanently. Verified with knope 0.23.1: `knope --dry-run release` now consumes and deletes both files and emits the dSYM entry under livekit-ffi 0.13.0. Also corrects the framework name in that entry, since the changelog it lands in ships after the rename: RustLiveKitUniFFI -> RustLiveKitFFI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The framework the cargo-swift fork produces is RustLiveKitFFI now, so the comment explaining why this crate sets ffi_module_name pointed at a name that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The module builds livekit-ffi/core-modules and publishes io.livekit:livekit-ffi-android; the heading still said UniFFI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1egoman
force-pushed
the
livekit-ffi-livekit-uniffi-merge
branch
from
October 19, 2026 22:40
d40cc30 to
bb49119
Compare
Contributor
Author
|
Ok we need to get this merged, I had a LLM do another pass and it didn't find any other serious issues! So I think we are good to go finally! |
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
7 participants
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.
Background
Ever since we introduced livekit-uniffi at the start of 2026, it has been getting increasingly confusing what the difference between
livekit-ffi(existing C ABI ffi solution used by python, node, unity, and c++ sdks that exposes a full rust owned webrtc stack) andlivekit-uniffi(new uniffi powered ffi solution used by swift, android, and flutter that exposes utility modules) actually are to those that haven't been intimately involved with the rust adoption initiative.This has become even more confusing now that we have recently added uniffi to the livekit-ffi crate... 😬
The original intention was that
livekit-uniffiwould be a somewhat transitional "playground" we could use to experiment with uniffi and make sure we were ready to commit to it before moving everything wholesale over to it. We're now at the point where we're ready to commit to it, so this change aims to consolidate all FFI usage (both C ABI FFI and uniffi flavors) into one monolithiclivekit-fficrate again.Overview
Moves all code from
livekit-uniffiintolivekit-ffiand outright removes thelivekit-unifficrate. This implements "phase 3" of the C ABI -> uniffi migration plan document discussed last month.livekit-ffinow has two top level "modes" controlled by features:room-apismode is exactly whatlivekit-ffiused to be - the C ABI ffi stack using thelivekitcrate that manages a full webrtc stack. This build is still built in ci exactly as it was before, and consumed by python/node/unity/c++.core-modulesmode ie exactly whatlivekit-uniffiused to be - the newer uniffi exposed set of utilities which are consumed by the swift, android, and flutter sdks.Common functionality
As we migrate the
room-apismode to fully use uniffi, there will inevitably be bits and pieces we want to expose to both "modes". This pull request surfaced one initial candidate - thebuild_versionfunction. This was exposed identically in both old uniffi implementations so now this is defined once and not gated by either "mode".Backwards compatibility
This pull request should be fully backwards compatible. That means in practice that "core modules" builds will still include identifiers within that mention
uniffi- ie,LiveKitUniFFIin swift, etc. I've broken out a second step to this process in #1439 which actually cuts over the names to the newffinaming, but as this is a breaking change we'll need to ensure that all downstream sdks do this update in lock step.We can also choose to not merge this second step if it is going to be too disruptive and the resulting naming debt is deemed acceptable.
TODO
prostvsprost_012) which seem to have made their way in becauselivekit-ffiandlivekit-uniffihad different versions in use. Standardize on whatever the repo uses generally.mainonce Upgrade to prost 0.14 across the whole project #1447 merged.