libmoq: add moq_origin_consume_announced to wait for a broadcast#1552
Conversation
moq_origin_consume does a synchronous lookup that returns BroadcastNotFound (-24) when the broadcast hasn't been announced yet, forcing C callers to poll in a retry loop right after connecting. Add a callback-based variant backed by moq_net::OriginConsumer::announced_broadcast that waits for the announcement and then delivers the broadcast handle. A true thread-blocking call isn't viable here: ffi::enter holds the runtime mutex for the duration of every FFI call, so blocking would stall all other calls (including the close needed to cancel the wait). The callback model matches the rest of the libmoq surface and follows the existing terminal-callback lifetime contract (>0 live handle, 0 clean close, <0 error; terminal fires even after an explicit close, and that's where user_data is freed). moq_origin_consume_announced_close cancels a pending wait. Scoped to libmoq (leaf crate, no in-workspace Rust dependents); the cbindgen header regenerates with both functions. uniffi wrappers are unaffected. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds an async "consume until announced" feature: Origin can register a wait task for a broadcast path, spawn a Tokio task that waits for the path announcement or cancellation, buffer the discovered broadcast, and invoke the caller-provided on_broadcast callback with a positive handle then a terminal code. It adds slab-backed task tracking, FFI wrappers moq_origin_consume_announced / moq_origin_consume_announced_close, integration tests for normal and cancelled flows, and README updates (including a change from moq_consume_frame_chunk to moq_consume_frame). 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rs/libmoq/README.md`:
- Around line 36-37: Update the C API list in the README so the documented
function names match the exported symbols: replace the stale
moq_consume_frame_chunk entry with moq_consume_frame (the symbol exported from
src/api.rs) and verify the nearby framed-function entry (e.g.,
moq_origin_consume_announced and moq_origin_consume_announced_close) is
spelled/named exactly as in the code; ensure the README uses the exact function
names moq_consume_frame, moq_origin_consume_announced, and
moq_origin_consume_announced_close so they match the Rust exports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db4bf4ed-ed28-47b9-8c19-e6cc0938c2ff
📒 Files selected for processing (4)
rs/libmoq/README.mdrs/libmoq/src/api.rsrs/libmoq/src/origin.rsrs/libmoq/src/test.rs
The C API list documented moq_consume_frame_chunk(frame, index, dst), but api.rs exports moq_consume_frame(frame, dst). Correct the name and signature. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reconcile main into dev. Key conflict resolutions: - conducer crate renamed to kio (main #1547): applied across all of dev's newer code; dropped the stale conducer path-dep, kept dev's new flate2 dep. - moq-mux: kept dev's thiserror Result (#1495); dropped main's CatalogSource as dead code since dev's catalog::Consumer already unifies Hang/MSF. - moq-net: kept dev's OriginConsumer/AnnounceConsumer split (#1434) and the TrackConsumer end_at cap; kept dev's non-optional auto-created origins on the lite session/publisher (#e770). - stats: combined main's StatsConfig + liveness retention (#1537, #1548) with dev's AnnounceConsumer usage. - libmoq + moq-native: kept main's auto-reconnect (#1544), terminal-callback contract (#1546), and consume_announced (#1552), adapted to dev's AnnounceConsumer and OriginProducer connect API. Restored the InitFailed error variant and made moq-rtc handle the now-fallible Log::init. cargo check/clippy/test all pass on the merged workspace.
Summary
moq_origin_consumedoes a synchronous lookup that returnsBroadcastNotFound(-24) when the broadcast hasn't been announced yet. Right aftermoq_session_connect, the announcement often arrives after the call, so C consumers have to pollmoq_origin_consumein a retry-until-announced loop.This adds a callback-based variant that waits for the announcement instead of racing it:
moq_origin_consume_announced(origin, path, path_len, on_broadcast, user_data)— registers a wait on a specific path, backed bymoq_net::OriginConsumer::announced_broadcast(exactly what the staleTODOinorigin.rspointed at). When the broadcast is announced,on_broadcastfires with the broadcast handle (> 0), then a terminal0.moq_origin_consume_announced_close(task)— cancels a pending wait.Why callback-based rather than thread-blocking
ffi::enterholds the runtime mutex for the duration of every FFI call, so a call that blocked the caller's thread would stall all other FFI calls (including thecloseneeded to cancel it). The whole libmoq surface is non-blocking/callback-driven, and this follows the existing terminal-callback lifetime contract introduced in #1546:> 0= live broadcast handle,0= clean close (terminal),< 0= error (terminal).*_close, and that's where the caller freesuser_data.moq_consume_catalog/moq_consume_track, freed withmoq_consume_close.Scope / cross-package sync
Scoped to
rs/libmoq(a leaf crate with no in-workspace Rust dependents). The cbindgen header (target/include/moq.h) regenerates with both functions.doc/lib/cis high-level and needs no change;README.mdgained the two signatures. The uniffi wrappers (py/swift/kt/go) are unaffected, since Rust ownership handles their lifetimes.Test plan
cargo test -p libmoq(21 pass, incl. newconsume_announced_localwait-then-publish andconsume_announced_close_cancels)cargo clippy -p libmoq --all-targetscleancargo fmt -p libmoq --checkclean(Written by Claude)