audio: send each frame as its own group#1414
Conversation
Audio frames were being packed into ~100ms groups for relay efficiency. For real-time use cases, that bounds end-to-end latency at the group boundary since the relay cannot forward a group until it is closed. Go back to one-frame-per-group: each frame is flushed to the relay immediately, and the codec's packet loss concealment (Opus PLC, AAC PLC) handles individual frame drops. Applies to the browser publish encoder and the Rust opus/aac mux importers, which all had the same 100ms grouping pattern.
|
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 (2)
WalkthroughThis PR refactors audio codec implementations across JavaScript and Rust to use per-frame keyframe grouping instead of time-based or frame-count-based batching. The JavaScript 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@js/publish/src/audio/encoder.ts`:
- Around line 169-171: The current call to producer.encode(frame,
frame.timestamp as Time.Micro, true) causes each frame to remain buffered until
the next keyframe because Container.Legacy.Producer.encode() finalizes the
previous group on a new keyframe; to fix the one-frame latency, after writing
the frame (where producer.encode or producer.writeFrame is invoked) explicitly
close the current group by calling the producer's group close method (e.g., call
producer.#group?.close() or producer.group.close() immediately after
writeFrame/encode) so each frame's group is finalized right away; alternatively,
if you prefer to keep buffering, update the comment near the producer.encode
call to state that frames are buffered until the next keyframe rather than being
forwarded immediately.
🪄 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: 2acbe249-3f3e-4281-91a1-e07df3e32167
📒 Files selected for processing (3)
js/publish/src/audio/encoder.tsrs/moq-mux/src/import/aac.rsrs/moq-mux/src/import/opus.rs
The previous commit marked every audio frame as a keyframe but the Legacy producer only closes the prior group when it sees the next keyframe. That left each frame's group open until the following frame arrived, adding one frame interval of latency. Add a finishGroup() method on Legacy.Producer (mirroring the Rust container Producer's finish_group) and call it from the audio encoder after each frame so the relay can forward it without delay.
Producer's keyframe/group bookkeeping is only useful for video GoPs. For audio, each frame is its own group, so use Moq.Track.writeFrame directly (which appends a group, writes the frame, and closes it). Expose Legacy.encodeFrame as a standalone function for callers that only need the timestamp+payload encoding without the Producer state machine. Producer keeps using it internally.
2 participants
Audio frames were being packed into ~100ms groups for relay efficiency.
For real-time use cases, that bounds end-to-end latency at the group
boundary since the relay cannot forward a group until it is closed.
Go back to one-frame-per-group: each frame is flushed to the relay
immediately, and the codec's packet loss concealment (Opus PLC, AAC PLC)
handles individual frame drops.
Applies to the browser publish encoder and the Rust opus/aac mux
importers, which all had the same 100ms grouping pattern.