[kixelated]

Explains and fixes the two Nightly benchmark regressions from quest/m0/nightly/bench-regressions.md, and finishes the quest.

Attribution

Paired measurement: each revision built in its own worktree and target directory, three alternating rounds, pinned to one P-core (Intel Core Ultra 7 270K), with Criterion at the BASE comparison's settings (1 s warmup, 2 s measurement, 30 samples). The numbers are the median of the paired deltas.

Bus: moq-dev/moq#8010 against its parent. All 22 bus_* cases are slower, and every round agrees in sign: +4% to +12% here (Nightly's aarch64 runner saw +16% to +29%). moq-dev/moq#8121 touches only encode/producer.rs, which the bus never runs (it borrows the Input type), so it is not a candidate. The cause: every ring read now runs Comfort::played over each sample, a serial f64 sum of squares. That measurement exists to set comfort noise for a silence the writer declares, but only the playback sink's rings ever receive end(). The bus and AEC reference rings never declare a silence, so they paid for it on every block. A build where played returns immediately recovers the whole regression, so the rest of #8010's ring changes cost nothing measurable.

Parameter sets: moq-dev/moq#8199 against its parent. h264-split-pps/distinct is +11% / +16% / +18% / +40% at n = 1 / 16 / 256 / 4096. moq-dev/moq#8140 changes only H.265 SPS parsing and is outside the H.264 split path. The cause: adopt_parameter_sets became retain plus extend, which inserts each set one at a time, where the old code made a single structural clone_from. That is where the cost that grows with n comes from. Its layer test (sets.keys().any(...) for each retained set, and the same nesting in reconcile_keyframe_params's missing check) is also O(retained × sets) once layers differ, which a hostile H.265 stream could drive.

Changes

• ring::Config gains comfort: bool. Only the playback sink (and the test helpers that model it) sets it. A ring without it never measures what plays, and a silence declared on it fills with plain silence. The bus and AEC reference set false.
• adopt_parameter_sets uses BTreeMap::append, which moves the map in when the retained sets are emptied and merges in linear time otherwise. Both layer checks use a 64-bit mask (nuh_layer_id is 6 bits) instead of nested scans.

Results (paired, this branch against main at 6724807)

Case	Change
bus_sources/48000hz/1, 4, 16, 64	-6.4%, -9.3%, -10.7%, -7.2%
bus_sources/44100hz/1, 4, 16, 64	-4.4%, -5.5%, -6.2%, -5.0%
bus_mixes/full/1, 4, 16, 64	-10.8%, -9.3%, -5.7%, -1.4%
bus_mixes/sparse/1, 4, 16, 64	-11.2%, -9.7%, -6.5%, -2.5%
bus_minus/*	-5.3% to -6.5%
h264-split-pps/distinct/1, 16, 256, 4096	-5.0%, -6.1%, -9.1%, -21.7%
h264-split-pps/repeated/*	-0.5% to -4.4%

The bus returns to its pre-#8010 cost. That matches the bound from the build where played does nothing.

distinct no longer grows with n. Compared across runs with #8199's parent, about +5% to +8% remains at every n. That remainder is the (layer, id) key #8199 introduced so an enhancement layer's sets never replace the base layer's. It is a correctness trade, accepted here and below the paired 10% threshold.

Choices made without asking

• Rejected: keeping the measurement everywhere and summing squares in independent f64 lanes so it vectorizes. It recovered only 4% to 10%, against the 5% to 16% no-op bound, and no benchmark shows the playback sink needs the speedup.
• Kept the residual parameter-set cost described above rather than packing the key into a u64, which would trade readability for a few percent.

Caused by

• moq-dev/moq#8010 - measured comfort-noise levels on rings that can never declare a silence
• moq-dev/moq#8199 - adopted parameter sets by inserting each one, plus a nested layer scan

Impact

Public API: none (ring::Config is crate-private). Wire: none.

Checks

just check passed (6312 tests), including the new an_unmeasured_declared_silence_is_silent test. quest check --base origin/main passed.

Follow-ups

None.

Models

Size: M
Implementer: Claude Code, Opus 5.5, medium effort

(Written by Opus 5.5 with medium)

🤖 Generated with Claude Code

Review

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1bf79-d64c-7482-8df8-9d0170620337, round 1
• Head: 450d86f
• Base: origin/main at 7cce8cf
• Score: 9/10, The changes appear sound: the layer bitmask preserves parameter-set handling and comfort measurement remains enabled for playback, though builds and tests were not run as instructed.
• Findings: none

[coderabbitai]

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 11 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: 87f820ff-8970-48c1-9098-a4b48a4a9711

Commits

Reviewing files that changed from the base of the PR and between 6724807 and 450d86f.

Files selected for processing: 8

• quest/m0/nightly/README.md
• quest/m0/nightly/bench-regressions.md
• rs/moq-audio/src/aec.rs
• rs/moq-audio/src/bus.rs
• rs/moq-audio/src/playback/mixer.rs
• rs/moq-audio/src/playback/sink.rs
• rs/moq-audio/src/ring.rs
• rs/moq-mux/src/codec/annexb.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

• X
• Mastodon
• Reddit
• LinkedIn

[pullfrog]

ℹ️ No critical issues — one stale doc line worth a look.

Reviewed changes

• Bus and AEC stop measuring comfort noise — ring::Config gains a crate-private comfort flag; only the playback sink (and the test helpers that model it) enables it, so the bus and AEC reference rings skip the per-sample sum of squares.
• Parameter sets adopt in linear time — adopt_parameter_sets replaces the per-set layer scan with a 64-bit mask plus BTreeMap::append, and reconcile_keyframe_params reuses the same mask.
• Quest finished — deletes quest/m0/nightly/bench-regressions.md and its row in the parent README.

ℹ️ Read::Comfort's doc no longer matches the unmeasured case

rs/moq-audio/src/ring.rs:73-75 still says the rest of the buffer is "filled with comfort noise", and the module doc at rs/moq-audio/src/ring.rs:9-10 says a declared silence "fills it with comfort noise". With comfort: false the reader still returns Read::Comfort but fills plain silence, which the new an_unmeasured_declared_silence_is_silent test asserts. A short caveat on both ("when the queue measures") keeps the docs truthful for the configuration this PR adds.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed 450d86f. No actionable findings. Comfort measurement remains enabled for playback and is disabled for the bus/AEC rings; an unmeasured ring retains a zero comfort level. The parameter-set layer mask matches the six-bit H.265 layer domain (H.264 uses layer zero), preserving replacement within defined layers and retention of other layers.

Verification limits: GitHub-only static review. No builds, audio tests, codec tests or benchmarks executed; performance claims were not independently measured.