hang: cross-broadcast track references in renditions#1371
Open
kixelated wants to merge 4 commits into
Open
Conversation
Add an optional `broadcast` field on video/audio renditions that points
at the broadcast publishing the track, expressed as a relative path
("../source", etc.). Lets a worker author a downstream catalog that
sidecars unchanged renditions against an upstream broadcast without
republishing the bytes. Watchers resolve the field against the catalog
broadcast's path and subscribe on the same connection.
- moq-lite: new `PathRelative` type and `Path::resolve` for `..`-aware
resolution, with full unit coverage.
- @moq/hang: mirror `resolveBroadcast` helper for the browser side.
- @moq/watch: `Broadcast.trackBroadcast` looks up the override broadcast
with lifetime tied to the calling Effect; audio/video decoder + MSE
backends use it.
- rs/hang/examples/subscribe.rs: demonstrate consumer-side resolution
via `OriginConsumer::announced_broadcast`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
Your plan currently allows 4 reviews/hour. Refill in 6 minutes and 18 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis change adds an optional per-rendition broadcast reference to audio and video configs and implements path utilities to normalize and resolve relative broadcast strings. Runtime code (Broadcast.trackBroadcast and updated decoder/MSE codepaths) now resolves and caches override broadcasts and falls back to the catalog broadcast when absent or self-referential. Importers populate the new field as None by default. Rust and JS tests exercise normalization, resolution, serialization, and subscription behavior. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ 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 |
Addresses correctness, lifecycle, and parity issues in the catalog broadcast-override path: - watch: rewrite Broadcast.trackBroadcast so the override path doesn't depend on this.active, caches remote broadcasts per resolved path on this.signals (one shared subscription instead of one per backend), and treats self-references (rel resolving back to the catalog's own path) as a fall-through to active. - moq-lite: PathRelative::new and From<String> now strip `.` segments alongside empty ones, matching POSIX intuition so `./foo` resolves to the expected path instead of a literal `.` segment. Path::resolve early-returns when rel is empty. - hang/examples/subscribe.rs: treat Some(empty), self-referential, and empty-resolved overrides as no-op so announced_broadcast can't be called on an empty path (which would hang forever). - hang catalog tests: strengthen the round-trip to actually re-decode the encoded output, and add tests for the None-omission and empty-string normalization paths. - hang catalog schemas: normalize the JS-side `broadcast` field via a shared zod transform that mirrors the Rust PathRelative normalization, so both sides agree byte-for-byte after deserialization. - moq-lite: fix em-dash in deserialize comment and tighten the PathRelative doc to clarify it has no Encode/Decode but does ride inside catalog JSON payloads. - Add bun tests for resolveBroadcast covering the same cases as the Rust resolve tests (dot, dotdot clamping, empty rel, empty base, self-reference). Follow-up: VideoConfig/AudioConfig should be #[non_exhaustive] with builders to avoid future SemVer breaks; deferred since it requires migrating all moq-mux import sites to a builder pattern. Written by Claude Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Notable conflicts resolved: - moq-mux import/* files were moved under codec/<name>/import.rs and container/<name>/import.rs; re-applied `broadcast: None` to all the new VideoConfig/AudioConfig struct literal sites (including new sites in mkv/import.rs, msf/consumer.rs, hang/producer.rs, and export tests that didn't exist on this branch). - moq_lite -> moq_net rename: updated the catalog `broadcast` field type and JS @moq/lite -> @moq/net imports in path.ts / path.test.ts. - watch/broadcast.ts: kept origin/main's #runAnnouncedNow derived signal and the override path's #isPathAnnounced helper by factoring the latter out as a shared helper that #runAnnouncedNow now calls. - watch/video/decoder.ts: kept the trackBroadcast override call and layered on origin/main's clearCurrentFrame/buffered cleanup when the broadcast goes undefined. - hang/examples/subscribe.rs: kept the empty/self-ref guards and switched to the new moq_mux::container::Consumer API for the subscribe call. - fmp4/import.rs: kept origin/main's av1_from_av1c refactor with `broadcast: None` added. Written by Claude Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Origin/main (#1485) marked VideoConfig and AudioConfig #[non_exhaustive] and added `::new()` constructors that the existing in-tree call sites now use. Resolved by taking main's version of every moq-mux import site and the hang/examples/video.rs example (they use the new constructor pattern) and re-adding the optional `broadcast` field to the structs + constructors. The catalog tests in root.rs likewise switch to the builder style. Written by Claude Co-Authored-By: Claude Opus 4.7 (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.
Summary
broadcastfield on video/audio rendition configs (e.g."../source") so a downstream catalog can reference tracks published in another broadcast without republishing bytes.PathRelativetype +Path::resolveinmoq-lite(Rust) with full unit coverage; mirrorresolveBroadcasthelper for@moq/hang.@moq/watchBroadcast.trackBroadcast(effect, configBroadcast)looks up the override broadcast on the same connection; audio/video decoder + MSE backends honor it. The Rustsubscribeexample demonstrates the consumer flow viaOriginConsumer::announced_broadcast.Test plan
cargo test --all-targets(incl. newPathRelative/resolvetests + serde round-trip incatalog::root)bun run --filter='*' testjust checkbroadcast: "../source", watch via@moq/watchand confirm the watcher subscribes against the source broadcast🤖 Generated with Claude Code