Repository navigation
feat(moq-audio)!: positioned bus channels, and Bus::channel is Bus::live - #8273
Conversation
Quest-Claim: 379e7870721270a7
…sition `Bus::positioned` opens a channel on the bus's own clock: each write names the bus position its first frame plays at, and the bus places it there to the sample, with silence before it. A write that would land where the bus already mixed, overlap the channel's own audio, or end past `bus::REACH` fails whole rather than moving. Live sources keep `Bus::channel`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 2 minutes. View limit details
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. |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — reviewed the full diff at 0ff57b0, read rs/moq-audio/src/bus.rs end to end, and ran the bus tests.
- New positioned channel:
Bus::positionedopens abus::Positionedwhosewrite(position, data)queues PCM at an exact bus frame, withbus::REACHpublic at 200 ms. - Shared port:
ChannelandPositionednow share the privatePort(id, remix, input, strip, centered, bus), andFeed::readreads either a live ring or a positioned queue. - Placement rules: late, overlapping, or past-reach writes fail whole with
Error::Unsupported; the input rate must match the bus; layout is remixed and panned as usual. - Tests and docs: three new bus tests cover sample-exact placement, the failure cases, and rate/layout/NaN refusal; the quest file is removed and the CHANGELOG updated.
The implementation matches the approach the PR describes: the queue front tracks Mixer::position, the reach check is inclusive, and Port::drop releases the feed and the center hold. The one open item is the API-shape decision the PR already flags for the maintainer, Bus::positioned (additive) versus a position on Channel::write (breaking). The additive route is consistent with bus being unpublished and with keeping live and positioned buffers separate.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
Pairs bus::Live with bus::Positioned. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed 135e7f4 against the current base. No actionable findings.
Checked positioned queue placement and consumption under the mixer lock, whole-write rejection for late/overlapping/out-of-reach input, layout conversion, strip delay/pan, driver shutdown, and shared Port cleanup. The new tests cover irregular block boundaries, first writes after silence, exact reach, rejected writes leaving the queue unchanged, and rate/layout validation. The Live rename is reflected in capture, benchmarks, examples, and migration/changelog notes.
Verification limits: static GitHub source and test review only; no builds, tests, benchmarks, or real-time callback measurements run.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — reviewed the full diff at 135e7f4, read rs/moq-audio/src/bus.rs end to end, and traced the rename and positioned-queue paths; the prior review at 0ff57b0 covered the same behavior.
- Positioned channel:
Bus::positioned/bus::Positioned::write(position, data)queues PCM so its first frame mixes at the named bus frame; the queue front tracksMixer::positionand the reach/overlap/ordered checks run under the mixer lock. - Whole-write rejection: late, overlapping, or past-
bus::REACHwrites returnError::Unsupportedbefore touching the queue, so a failed write queues nothing and the(position - next) + frames <= reachbound keeps theVecDequeat or below the preallocated reach. - Shared internals:
LiveandPositionedshare the privatePortfor id, remix, strip hold, center hold, and mixer membership;Feed::readserves the live ring or the positioned deque. - Rename:
Bus::channel/bus::ChanneltoBus::live/bus::Livewith no behavior change, reflected in the bench, doctest, capture path,MIGRATE.md, andCHANGELOG.md. - Quest:
quest/m2/bus-positioned-write.mdand its milestone## Requiredline are deleted; no remaining references.
Verification limits: no build or test run in this session (the checkout has no debug target). I checked the placement arithmetic by construction for partial drains, gaps, exact reach, u64::MAX, mono panning, and driver-drop paths; the three new tests cover the same cases on a stepped driver.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Finishes quest/m2/bus-positioned-write.md.
Decisions
Bus::positioned/bus::Positionedand renameBus::channel/bus::ChanneltoBus::live/bus::Live(rejected: rework to the planned breakingChannel::write(position, ...); rejected: thewrite(Write { position: Option<u64>, data })fallback). Live and positioned audio never share a buffer, so live priming, pad and trim can't move positioned audio, and the pair now reads symmetrically. Breaking changes are fine. Decided 2026-11-08, maintainer.Problem
A live bus channel (
bus::Channel, nowbus::Live) write has no position. It lands behind whatever the channel's jitter ring holds, which is 50 ms of priming on a fresh channel, nothing on a drained one, and the ring's pad and trim move it further. A caller that schedules sound against video (moq-dev/studio#38, moq-dev/studio#93) has to copy the privateLATENCYand the ring's pad rule, and still lands a first write after silence up to one block early, or trails video by up to 37.5 ms after a drain.Approach
Bus::positioned(input, strip)opens abus::Positionedchannel whosewrite(position, data)names the bus position (the framesDriver::positioncounts) its first frame plays at. The bus puts it there to the sample, then the strip's sync delay applies as for any source.positionwith silence, then appends. No priming, pad, trim, or resampling sits between a write and its position.bus::REACH(200 ms, the same memory a live channel's ring holds) past the next frame mixed. The bus never shifts or trims a write to fit.LiveandPositionedshare the conversion, strip hold, and mixer membership through a privatePort, and the mixer reads either throughFeed::read.Impact
Bus::positioned,bus::Positioned(write,strip),bus::REACH; renamesBus::channeltoBus::liveandbus::Channeltobus::Live, behavior unchanged. Migrate by renaming the calls and the type (MIGRATE.md). No other crate or binding reaches the bus. CHANGELOG updated.playout::WritertoPositioned, broadcast and media sources line up with video to the sample, and the copies ofLATENCYand the ring's pad rule go.Choices made without asking
Error::Unsupported, rather than dropping the late part (what the studio'sWriterdoes today) or summing an overlap. The caller owns the clock it schedules on, so it decides whether to trim, skip, or re-buffer. A one-frame overlap from rounding fails too; a caller that rounds positions keeps its writes contiguous itself.REACHis public so a caller knows how early it may hand audio over without copying a private constant. It isLATENCY + HEADROOM, the memory a live channel already holds.Writer) already write at the bus rate.Tests
New, in
bus::tests, on a driver-stepped virtual clock with no sleeps:a_positioned_write_plays_at_its_position: writes before the first advance, contiguous, and after an exact 7-frame gap play sample-exact over irregular block sizes. A first write after silence plays at the sample it names (onset atnext + 333, mid-block). With a 10 ms sync delay it moves by exactly 480 frames.a_positioned_write_fails_rather_than_moves: already mixed, straddling a previous write by one frame, before the previous write's end, one frame past the reach, and a position nearu64::MAXall fail and leave nothing queued; a write ending exactly at the reach plays where it said; a write after the driver drops is dropped.a_positioned_channel_takes_the_bus_rate: another rate and a layout without speakers are refused; a mono input is panned at -3 dB per side; a NaN is refused.Validation
just checkandjust cipass after the rename.cargo nextest run -p moq-audio --lib bus::passes (27 tests).Suggested follow-ups
playout::Writer(broadcast and media sources) and replays toBus::positioned, and delete its copies ofBUS_LATENCYand the ring's pad rule. Recommend: plan it in the studio repo, it is the consumer this API exists for.Other open PRs
moq-dev/moq#8220 also touches
rs/moq-audio/src/bus.rs, but only one line of an existing test (start.saturating_add); it merges cleanly either way. quest/m2/bus-minus-several.md changesMix::minus, which this PR does not touch.Models
Size: M
Implementer: Claude Code, Claude Opus 5.5 (claude-opus-5-5), high effort
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, XS)
Reviewer: Codex, gpt-6.1-sol, xhigh effort
Review
Rewritten by
quest reviewafter each round.01a1bbdf-5377-7290-bcba-f88065f8876f, round 1The positioned queue preserves sample placement and bounded buffering with no actionable behavioral defects found; builds and tests were not run, as instructed.The reviewer ran no build;
just checkandjust cipassed locally at 135e7f4.(Written by Claude Opus 5.5 with medium)
🤖 Generated with Claude Code