fix(voip): increase live audio buffer headroom - #280
Conversation
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/voip/src/media/__tests__/audio-engine.test.ts">
<violation number="1" location="packages/voip/src/media/__tests__/audio-engine.test.ts:75">
P2: The 'keeps only the tail of an oversized chunk' test no longer exercises the truncation path. With the new 750 ms cap (12000 samples at 16 kHz), a 10000-sample chunk no longer exceeds extMaxBuffer, so the `incoming.length > this.extMaxBuffer` branch in feedExternalAudio is never entered. Feed a chunk larger than 12000 samples (e.g. 20000) and assert the resulting level (750 ms) so the oversized-chunk regression test still covers its intended code path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const level = engine.feedExternalAudio(new Float32Array(10_000)) | ||
| assert.equal(level, 500) | ||
| assert.equal(engine.getLiveBufferMs(), 500) | ||
| assert.equal(level, 625) |
There was a problem hiding this comment.
P2: The 'keeps only the tail of an oversized chunk' test no longer exercises the truncation path. With the new 750 ms cap (12000 samples at 16 kHz), a 10000-sample chunk no longer exceeds extMaxBuffer, so the incoming.length > this.extMaxBuffer branch in feedExternalAudio is never entered. Feed a chunk larger than 12000 samples (e.g. 20000) and assert the resulting level (750 ms) so the oversized-chunk regression test still covers its intended code path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/voip/src/media/__tests__/audio-engine.test.ts, line 75:
<comment>The 'keeps only the tail of an oversized chunk' test no longer exercises the truncation path. With the new 750 ms cap (12000 samples at 16 kHz), a 10000-sample chunk no longer exceeds extMaxBuffer, so the `incoming.length > this.extMaxBuffer` branch in feedExternalAudio is never entered. Feed a chunk larger than 12000 samples (e.g. 20000) and assert the resulting level (750 ms) so the oversized-chunk regression test still covers its intended code path.</comment>
<file context>
@@ -63,17 +63,17 @@ test('feedExternalAudio caps the live buffer and drops oldest on overflow', () =
const level = engine.feedExternalAudio(new Float32Array(10_000))
- assert.equal(level, 500)
- assert.equal(engine.getLiveBufferMs(), 500)
+ assert.equal(level, 625)
+ assert.equal(engine.getLiveBufferMs(), 625)
})
</file context>
1 participant
Summary
Why
Short upstream stalls and scheduling jitter could exhaust the previous 500 ms ceiling and drop recent speech before it reached the call. The larger ceiling remains bounded while providing more headroom.
Validation
@zapo-js/voiptests: 72/72 passed@zapo-js/voiptypecheck: passed@zapo-js/voipbuild (CJS and ESM): passed