Skip to content

feat(moq-audio)!: positioned bus channels, and Bus::channel is Bus::live - #8273

Merged
kixelated merged 6 commits into
mainfrom
quest/m2/bus-positioned-write
Nov 8, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m2/bus-positioned-write

Conversation

@kixelated

@kixelated kixelated commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m2/bus-positioned-write.md.

Decisions

  • Positioned writes get their own channel type, and the live one is renamed to pair with it: ✅ keep Bus::positioned / bus::Positioned and rename Bus::channel / bus::Channel to Bus::live / bus::Live (rejected: rework to the planned breaking Channel::write(position, ...); rejected: the write(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, now bus::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 private LATENCY and 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 a bus::Positioned channel whose write(position, data) names the bus position (the frames Driver::position counts) its first frame plays at. The bus puts it there to the sample, then the strip's sync delay applies as for any source.

  • Its audio waits in a plain queue inside the mixer whose front is always the bus's next position. A write takes the mixer lock, so it sees exactly where the mix is, even mid-advance. It fills the gap from the end of its queued audio to position with silence, then appends. No priming, pad, trim, or resampling sits between a write and its position.
  • A write the bus cannot place fails whole, and queues nothing: any part already mixed, any part overlapping audio this channel already holds (writes only move forward), or an end more than 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.
  • It takes only the bus's sample rate, since positions count bus frames. Another layout is remixed and panned as on any channel.
  • After the driver drops, writes are dropped, as a live channel's are.
  • Live and Positioned share the conversion, strip hold, and mixer membership through a private Port, and the mixer reads either through Feed::read.

Impact

  • Public API (moq-audio, breaking): adds Bus::positioned, bus::Positioned (write, strip), bus::REACH; renames Bus::channel to Bus::live and bus::Channel to bus::Live, behavior unchanged. Migrate by renaming the calls and the type (MIGRATE.md). No other crate or binding reaches the bus. CHANGELOG updated.
  • Wire: none.
  • Users: once the studio moves playout::Writer to Positioned, broadcast and media sources line up with video to the sample, and the copies of LATENCY and the ring's pad rule go.

Choices made without asking

  • Late, overlapping, or out-of-reach writes fail whole with Error::Unsupported, rather than dropping the late part (what the studio's Writer does 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.
  • REACH is public so a caller knows how early it may hand audio over without copying a private constant. It is LATENCY + HEADROOM, the memory a live channel already holds.
  • Bus rate only. Positioning a resampled input would need the converter's chunking and filter delay accounted to the sample, and its tail would stick in the converter until the next write. Callers that schedule on the bus clock (the studio's Writer) already write at the bus rate.
  • Positioned writes lock the mixer for a bounds check and a copy of at most 200 ms; conversion and remixing happen before the lock. A driver on a device callback can wait for that copy.

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 at next + 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 near u64::MAX all 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 check and just ci pass after the rename.
  • cargo nextest run -p moq-audio --lib bus:: passes (27 tests).

Suggested follow-ups

  • moq-dev/studio: move playout::Writer (broadcast and media sources) and replays to Bus::positioned, and delete its copies of BUS_LATENCY and 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 changes Mix::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 review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1bbdf-5377-7290-bcba-f88065f8876f, round 1
  • Head: 135e7f4
  • Base: origin/main at f6a0e54
  • Score: 9/10, The positioned queue preserves sample placement and bounded buffering with no actionable behavioral defects found; builds and tests were not run, as instructed.
  • Findings: none

The reviewer ran no build; just check and just ci passed locally at 135e7f4.

(Written by Claude Opus 5.5 with medium)

🤖 Generated with Claude Code

kixelated and others added 2 commits November 7, 2026 23:31
…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>
@kixelated kixelated changed the title Bus writes land at a bus position feat(moq-audio): positioned bus channels place each write at a bus position Nov 8, 2026
@kixelated
kixelated marked this pull request as ready for review November 8, 2026 07:46
@coderabbitai

coderabbitai Bot commented Nov 8, 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 2 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: 5941f4d1-d6fe-40ea-acc6-63eadd5dcd92
Commits

Reviewing files that changed from the base of the PR and between f6a0e54 and 135e7f4.

Files selected for processing: 6

  • quest/m2/README.md
  • quest/m2/bus-positioned-write.md
  • rs/moq-audio/CHANGELOG.md
  • rs/moq-audio/MIGRATE.md
  • rs/moq-audio/benches/bus.rs
  • rs/moq-audio/src/bus.rs
  • 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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ 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::positioned opens a bus::Positioned whose write(position, data) queues PCM at an exact bus frame, with bus::REACH public at 200 ms.
  • Shared port: Channel and Positioned now share the private Port (id, remix, input, strip, centered, bus), and Feed::read reads 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.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated kixelated changed the title feat(moq-audio): positioned bus channels place each write at a bus position feat(moq-audio)!: positioned bus channels, and Bus::channel is Bus::live Nov 8, 2026
@kixelated
kixelated marked this pull request as ready for review November 8, 2026 15:56

@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 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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ 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 tracks Mixer::position and the reach/overlap/ordered checks run under the mixer lock.
  • Whole-write rejection: late, overlapping, or past-bus::REACH writes return Error::Unsupported before touching the queue, so a failed write queues nothing and the (position - next) + frames <= reach bound keeps the VecDeque at or below the preallocated reach.
  • Shared internals: Live and Positioned share the private Port for id, remix, strip hold, center hold, and mixer membership; Feed::read serves the live ring or the positioned deque.
  • Rename: Bus::channel / bus::Channel to Bus::live / bus::Live with no behavior change, reflected in the bench, doctest, capture path, MIGRATE.md, and CHANGELOG.md.
  • Quest: quest/m2/bus-positioned-write.md and its milestone ## Required line 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.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 8, 2026
Merged via the queue into main with commit 79960a9 Nov 8, 2026
17 checks passed
@kixelated
kixelated deleted the quest/m2/bus-positioned-write branch November 8, 2026 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant