feat(#626): downmix stereo to mono in offscreen audio graph#822
feat(#626): downmix stereo to mono in offscreen audio graph#822Shan7Usmani wants to merge 2 commits into
Conversation
🚀 Thank You for Contributing to Late-MeetPlease ensure that:
Thank you for contributing 💙 |
|
Warning Review limit reached
More reviews will be available in 59 minutes and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTwo independent changes: ChangesHTML Escaping Hardening
Offscreen Audio Downmix Routing
Sequence Diagram(s)sequenceDiagram
participant TabSource as Tab MediaStream Source
participant MicSource as Mic MediaStream Source
participant downmixNode as downmixNode (GainNode, mono)
participant analyser as AnalyserNode
participant recorderDestination as MediaStreamAudioDestinationNode
participant playbackDestination as AudioDestination
TabSource->>downmixNode: connect (recorder path)
TabSource->>analyser: connect (raw signal)
TabSource->>playbackDestination: connect (playback)
downmixNode->>recorderDestination: connect (channelCount=1, explicit)
MicSource->>downmixNode: connect (recorder path)
MicSource->>analyser: connect (raw signal)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 unit tests (beta)
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 |
github-actions
Bot
added
type:code
|
👋 Thank you @Shan7Usmani for your contribution to Late-Meet!
Please review any automated suggestions or code review comments that may appear below! We will review your PR as soon as possible! Please consider starring the repository ⭐ to show your support! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| assert.equal(escapeHtml(imgXss), "<img src=x onerror="alert(1)">"); | ||
|
|
||
| const safe = escapeHtml(xss); | ||
| assert.ok(!safe.includes("<script>")); |
|
|
||
| for (const payload of payloads) { | ||
| const escaped = escapeHtml(payload); | ||
| assert.ok(!escaped.includes("<script>"), `payload still contains <script>: ${payload}`); |
The Semgrep rule javascript.lang.security.audit.unknown-value-with-script-tag was flagging test assertions that check escaped output no longer contains <script> tags. Since these are intentional safety checks on already-escaped data, add nosemgrep comments to suppress the warnings.
![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
🧹 Nitpick comments (1)
src/utils/sanitize.test.ts (1)
3-4: ⚡ Quick winAdd a compatibility assertion for the deprecated
sanitize.tsexport path.Line 3-4 now only validates
escapeHtmlvia./domHelpers; this won’t catch accidental breakage of the./sanitizere-export that this migration is preserving.Suggested test addition
import { escapeHtml } from "./domHelpers"; -import { sanitizeClassName, sanitizeDataAttr } from "./sanitize"; +import { + escapeHtml as deprecatedEscapeHtml, + sanitizeClassName, + sanitizeDataAttr, +} from "./sanitize"; test("escapeHtml prevents XSS in text content", () => { + assert.equal( + deprecatedEscapeHtml("<b>x</b>"), + "<b>x</b>", + ); assert.equal(escapeHtml(null), "");🤖 Prompt for 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. In `@src/utils/sanitize.test.ts` around lines 3 - 4, The test file currently imports escapeHtml only from the new location ./domHelpers, which means breakage of the deprecated re-export path from ./sanitize would go undetected. Add an import statement that also imports escapeHtml from ./sanitize to verify the deprecated export path continues to work and won't break during the migration.
🤖 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 `@src/dashboard.ts`:
- Around line 1100-1102: The variables speaker, timeStr, and text are being
double-escaped in the data attributes at lines 1100-1102. Since these values are
already HTML-escaped, remove the sanitizeDataAttr() function wrapper from all
three data attributes (data-speaker, data-time, and data-message) and use the
raw variables directly. This will prevent double-encoding of entities and ensure
the copied plain text payload contains the correct unencoded values.
---
Nitpick comments:
In `@src/utils/sanitize.test.ts`:
- Around line 3-4: The test file currently imports escapeHtml only from the new
location ./domHelpers, which means breakage of the deprecated re-export path
from ./sanitize would go undetected. Add an import statement that also imports
escapeHtml from ./sanitize to verify the deprecated export path continues to
work and won't break during the migration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63672196-3689-4681-88a3-90e805a8d97d
📒 Files selected for processing (6)
src/dashboard.tssrc/offscreenAudioGraph.tssrc/utils/domHelpers.test.tssrc/utils/sanitize.test.tssrc/utils/sanitize.tstests/offscreenAudioGraph.test.ts
|
@shouri123 , I kindly request you to take a look at this pr , under GSSoC . |
Insert a GainNode with channelCount=1, channelCountMode=explicit, and channelInterpretation=speakers between sources and the recorder destination. This instructs the Web Audio API to sum stereo channels to mono, saving ~50% bandwidth/storage while STT quality is unaffected. - connectCaptureSource now routes sources through downmixNode - Analyser and playback bypass the downmix (raw stereo preserved) - connectMicrophoneToOffscreenAudioGraph uses same downmix node
The Semgrep rule javascript.lang.security.audit.unknown-value-with-script-tag was flagging test assertions that check escaped output no longer contains <script> tags. Since these are intentional safety checks on already-escaped data, add nosemgrep comments to suppress the warnings.
Shan7Usmani
force-pushed
the
feat/audio-downmixing-626
branch
from
d408a2c to
a4ec57d
Compare
|
|
@shouri123 this PR (#822 - audio downmixing) is rebased on latest main with all checks passing. Could you please review and merge when you get a chance? Thanks! |
|
This PR has been marked as stale due to inactivity. |
2 participants
Description
Insert a GainNode with channelCount=1, channelCountMode=explicit, and channelInterpretation=speakers between audio sources and the MediaRecorder destination in the offscreen audio graph. This tells the Web Audio API to sum stereo channels to mono, saving ~50% bandwidth and storage while preserving STT quality.
Changes
Type of Change
Checklist
Summary by CodeRabbit
Bug Fixes
Refactor
Tests