Skip to content

feat(moq-ffi): browser parity inventory, insert_datagram, largest, broadcast closed - #7304

Merged
moq-bot[bot] merged 6 commits into
mainfrom
quest/m2/rs2ts/wasm-bindings
Oct 31, 2026
Merged

moq-bot[bot] merged 6 commits into
mainfrom
quest/m2/rs2ts/wasm-bindings

Conversation

@kixelated

@kixelated kixelated commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

First slice of Complete the browser WASM bindings. The quest is [L] and does not finish here, so it is rewritten to what remains.

Summary

  • Inventory. @moq/net's in-tree consumers are now compared against moq-ffi on wasm32. The new "Parity with @moq/net" table is in test/wasm/README.md. The model layer is mostly covered. The large gaps are at the connection layer: reconnect with backoff, the WebSocket fallback, and probe. Smaller gaps are group closed/arrived/expired, frame arrival time, tryReadFrame, abort(Error), timescale Timestamp, and the group cache constants.
  • New moq-ffi model bindings, which mirror existing moq-net APIs. Consumers use them in js/publish/src/video/encoder.ts, js/e2ee/src/track.ts, and js/watch/src/broadcast.ts:
    • MoqTrackProducer::insert_datagram(sequence, frame) sends a datagram at the caller's own sequence. append_datagram continues past it.
    • MoqTrackProducer::largest() -> Option<MoqPosition> adds a new MoqPosition { group, frame } record.
    • MoqBroadcastConsumer::closed() resolves once the broadcast ends.
  • Wrappers (per AGENTS.md "Cross-Package Sync"):
    • Python, Go, and Swift get methods.
    • Python, Go, Swift, Kotlin, and Dart export a Position alias.
    • The wrapper READMEs are updated.
    • The committed Dart bindings are regenerated.
  • Browser harness:
    • Fix: test/wasm/uniffi no longer type-checked on main. follow gained an epoch argument in moq-dev/moq#7193, which broke the nightly wasm-baseline --uniffi. This PR fixes it.
    • New cases: largest, insertDatagram above 2^53, and local broadcast closed. Also a remote case where the publisher closes the broadcast and the subscriber's closed resolves. The remote case runs on the lite-06, transport-22, and lite-02/SETUP relays.
  • Docs:
    • README: moq-ffi's README gains a Browser section covering scope, config limits, and how to generate and test.
    • Changelog: the moq-ffi CHANGELOG is updated. There is no version bump.
  • .taplo.toml: now excludes c/moq/build/**. just c check leaves a c/moq/build/odd/<triple> symlink to target/. Through it, taplo found the ubrn install's .crates.toml and failed just check once the UniFFI harness had run.

Public API: additive in moq-ffi and every wrapper. Wire: none.

Choices made without asking

  • Which gaps this slice closes: only model gaps that map 1:1 onto existing moq-net APIs. Connection-layer parity is left to the rewritten quest: reconnect, the WebSocket fallback (qmux on wasm32 plus a browser WebSocket transport), and probe. Frame arrival time is left there too, because it needs the Rust and JS clocks to agree first.
  • MoqGroupConsumer::closed is not added. The group's state lives in the Task that read_frame holds. A concurrent closed would need its own handle to the group, and that would weaken cancel()'s guarantee that it releases the group.
  • Swift labels: insertDatagram(_ payload:, sequence:, timestampUs:) follows appendDatagram's payload-first shape.

Decisions

Maintainer decisions, 2026-10-30:

  1. Land this slice and continue the quest in a follow-up PR, or keep iterating here? The quest file is already rewritten to what remains.
    • ✅ (a) Land now. Each slice is reviewable on its own, and the harness fix unbreaks nightly.
    • (b) Keep this draft open for the connection-layer work.
  2. C ABI parity for these three methods. rs/moq-c is a separate C ABI written directly over moq-net, not over moq-ffi (rs/moq-c/src/publish.rs:1141), and cpp/moq wraps no datagram API today.
    • ✅ (a) Leave C/C++ as is; skip the moq-c methods. No C consumer has asked for them.
    • (b) Plan a quest to add insert_datagram, largest, and broadcast closed to the C ABI.

Checks

  • just check origin/main passes: rs (281 passed), py (107 passed), go, kt, dart, c, cpp, js, toml, quest. Swift was skipped: this host has no Swift toolchain, so CI is the first to compile the Swift wrapper.
  • just test wasm --uniffi passes every case on all three relays, in both the normal and the strict-CSP (interpreted) modes. It ran on an earlier head with the same Rust and cases.
  • Not run locally: the full just test wasm-baseline --uniffi sweep. Its consumer change is the follow(path, undefined) argument, which is type-checked.

Suggested follow-ups

  • Close the remaining rows of the parity table, as the rewritten quest describes. The WebSocket fallback needs qmux to build on wasm32: its Transport traits require Send, and it uses tokio::spawn.
  • Nightly wasm-baseline --uniffi broke silently when moq-dev/moq#7193 changed follow. Consider type-checking test/wasm/uniffi in the PR gate whenever rs/moq-ffi changes. That needs the ubrn build, so it may stay nightly-only.

Review

  • Reviewer: Codex (quest review), gpt-6.1-sol, medium effort; quest built from origin/main's flake.nix pin (01d051cf).
  • Round 1 at 68d74d41: one finding. The new moq-ffi methods lacked hand-written wrappers.
    • Fixed for Python, Go, Swift, Kotlin, and Dart.
    • Refuted for C/C++: rs/moq-c is its own C ABI over moq-net (rs/moq-c/src/publish.rs:1141, rs/moq-c/src/api.rs:3034), and cpp/moq/include has no datagram API.
  • Round 2 at efd01fb76d9ce0029994a94869f9a2b742c92677 (a new session, since the AGENTS.md set changed, which weighed the responses): no findings.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

@kixelated kixelated changed the title Complete the browser WASM bindings feat(moq-ffi): browser parity inventory, insert_datagram, largest, broadcast closed Oct 30, 2026
@kixelated
kixelated marked this pull request as ready for review October 30, 2026 20:53
@coderabbitai

coderabbitai Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 35 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6c6d496b-416f-4137-b703-95a19ec8d83b
Commits

Reviewing files that changed from the base of the PR and between 9edd0e4 and efd01fb.

Files selected for processing: 30
  • .taplo.toml
  • dart/moq/lib/src/aliases.dart
  • dart/moq_ffi/lib/src/moq.dart
  • go/wrapper/README.md
  • go/wrapper/moq_test.go
  • go/wrapper/publish.go
  • go/wrapper/subscribe.go
  • go/wrapper/types.go
  • kt/README.md
  • kt/moq/src/jvmAndAndroidMain/kotlin/dev/moq/Aliases.kt
  • py/moq-rs/README.md
  • py/moq-rs/moq/__init__.py
  • py/moq-rs/moq/publish.py
  • py/moq-rs/moq/subscribe.py
  • py/moq-rs/moq/types.py
  • py/moq-rs/tests/test_local.py
  • quest/m2/rs2ts/wasm-bindings.md
  • rs/moq-ffi/CHANGELOG.md
  • rs/moq-ffi/README.md
  • rs/moq-ffi/src/consumer.rs
  • rs/moq-ffi/src/producer.rs
  • rs/moq-ffi/src/test.rs
  • swift/README.md
  • swift/Sources/Moq/Aliases.swift
  • swift/Sources/Moq/Broadcast.swift
  • swift/Sources/Moq/Track.swift
  • test/wasm/README.md
  • test/wasm/baseline/consumer.ts
  • test/wasm/baseline/uniffi.ts
  • test/wasm/uniffi/cases.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-30T20:54:56.972049Z efd01fb Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Landing. Maintainer decisions (2026-10-30): land the browser-parity slice and skip the moq-c methods. Codex reviewed efd01fb76d with no findings, and just check origin/main passed on it. Swift was not compiled locally.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 30, 2026 20:54

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit: efd01fb

No actionable bugs found in the changed bindings, wrappers, and harness cases. The direction is sound: these thin, additive APIs reuse moq-net's existing sequence, timestamp validation, position, and broadcast-lifetime semantics rather than duplicating them. Keeping connection-layer parity separate makes this slice easier to verify; the explicit-sequence and >2^53 browser cases are useful coverage.

Verification limits: static review of the diff and relevant Rust implementation only; I did not run builds, tests, browser relays, or Swift compilation. This is not a claim that those checks passed.

@github-actions
github-actions Bot disabled auto-merge October 31, 2026 00:49
@github-actions

Copy link
Copy Markdown
Contributor

quest land removed this pull request from the queue: it conflicts with the base or the PRs queued before it in dart/moq_ffi/lib/src/moq.dart, kt/README.md, swift/README.md; merge the base, push, and land it again (run).

@kixelated

Copy link
Copy Markdown
Collaborator Author

Re-landing after the queue stopped with merge_conflict (2026-10-31T00:49:34Z). The maintainer says the land queue has been fixed since then.

  • Head efd01fb76d9ce0029994a94869f9a2b742c92677 is unchanged. It merges cleanly into main at 1d8bf24998d485ec2c343b4546fb890ab0624723 (git merge-tree reports no conflict), so nothing was pushed.
  • The PR body's ## Review record names this head (round 2, no findings). The online review pullrequestreview-5718558559 on the same head reports no actionable bugs, and no review threads are unresolved.
  • CI passes (13/13). quest check --base origin/main --branch quest/m2/rs2ts/wasm-bindings passes (670 documents ok), and the quest file is rewritten to the remaining work.
  • Decisions recorded on 2026-10-30 still stand: land this slice now, and leave the C/C++ ABI as is.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 31, 2026 03:52
@moq-bot
moq-bot Bot merged commit 2bb0581 into main Oct 31, 2026
6 checks passed
@moq-bot
moq-bot Bot deleted the quest/m2/rs2ts/wasm-bindings branch October 31, 2026 07:48
kixelated added a commit that referenced this pull request Oct 31, 2026
Port main's new insert_datagram (#7304) onto exact MoqTimestamp records:
the moq-ffi producer, the Swift wrapper, the wasm uniffi cases, the
moq-ffi tests, and the kt/swift READMEs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant