Conversation
pallasathena92
requested review from
CatherineSue,
key4ng and
slin1237
as code owners
github-actions
bot
added
dependencies
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a foundational WebRTC relay feature, enabling the system to act as an intermediary for real-time communication. It establishes and manages two distinct WebRTC peer connections—one facing the client and another facing an upstream service like OpenAI. The primary function is to transparently relay both audio streams and data channel messages, ensuring full visibility and control over the real-time traffic flow. This enhancement is crucial for supporting advanced real-time API capabilities. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughAdds a new WebRtcBridge module to the OpenAI realtime router that establishes a dual peer-connection relay (client ↔ upstream) with SDP exchange, ICE candidate handling (host/srflx), UDP socket relay, data-channel buffering, and RTP forwarding; also updates workspace dependencies for model_gateway. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Bridge as WebRtcBridge
participant Upstream as UpstreamServer
participant Registry as RealtimeRegistry
Client->>Bridge: setup(client SDP offer)
Bridge->>Bridge: bind dual UDP sockets
Bridge->>Bridge: optional STUN gather (srflx)
Bridge->>Upstream: POST SDP offer / multipart
Upstream-->>Bridge: SDP answer
Bridge->>Client: return sanitized answer SDP
Client->>Bridge: run(registry)
loop Relay loop
Client->>Bridge: UDP packet (media/data)
Bridge->>Bridge: feed RTC input / handle events
Bridge->>Upstream: forward packets / channel data
Upstream->>Bridge: UDP response / channel data
Bridge->>Client: forward packets / channel data
Bridge->>Registry: update connection state
end
Client->>Bridge: cancellation
Bridge-->>Registry: final state update
Bridge-->>Client: exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a WebRTC relay bridge, a significant new feature. The implementation in webrtc_bridge.rs is comprehensive and well-structured. I've identified a couple of areas for improvement. A high-severity issue is the use of blocking I/O in an async context, which could lead to performance problems. I've also suggested a medium-severity efficiency improvement for the main event loop's timeout handling. Overall, this is a solid implementation of a complex feature.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model_gateway/src/routers/openai/realtime/webrtc_bridge.rs`:
- Around line 467-475: The relay loop currently swallows UDP send errors when
handling Output::Transmit, so DTLS/SRTP packets are dropped silently; modify the
block that calls self.socket(side).send_to in the Output::Transmit arm to check
the Result, log the error with context (call_id, side via
Self::side_label(side), destination, and length) and then immediately tear down
or close the affected side (use the existing teardown/close/shutdown routine for
the bridge or ICE on this side) instead of ignoring the error; ensure the change
touches the Output::Transmit match arm and uses the same identifiers
(self.socket(side), t.destination, t.contents.len(), Self::side_label, call_id)
so failures are visible and the side is taken down promptly.
- Line 293: The code currently treats upstream_channel (upstream_channel_id) as
usable immediately and caches it after Event::ChannelClose, which causes
client→upstream events to be dropped when self.upstream_rtc.channel(ch_id) is
None; add explicit upstream channel readiness tracking and a small pending
queue: introduce a field (e.g., upstream_channel_state or upstream_channel_ready
plus a VecDeque pending_client_events) and when setting upstream_channel:
Some(upstream_channel_id) do not mark it usable until you receive
Event::ChannelOpen for that id; on client events, if
self.upstream_rtc.channel(ch_id) is None or the readiness flag is false, push
the event into pending_client_events; on Event::ChannelOpen for that id, set
readiness true and flush the queued events to the upstream channel; on
Event::ChannelClose for that id, clear the cached upstream_channel id, set
readiness false, and drop/clear the pending queue so nothing is mistakenly
retained; update all related branches (the ones around upstream_channel usage at
the shown diff and the other ranges noted) to consult the readiness flag and
queue instead of silently dropping events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9f4fb1c2-fe69-427d-9015-a5cd435f1884
📒 Files selected for processing (4)
Cargo.tomlmodel_gateway/Cargo.tomlmodel_gateway/src/routers/openai/realtime/mod.rsmodel_gateway/src/routers/openai/realtime/webrtc_bridge.rs
Signed-off-by: yifeliu <yifengliu9@gmail.com>
pallasathena92
force-pushed
the
yifeliu/realtime-webrtc-pr3-bridge
branch
from
e6a6e46 to
3ea1ba1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model_gateway/src/routers/openai/realtime/webrtc_bridge.rs`:
- Line 546: The write calls currently drop errors with "let _ =
ch.write(data.binary, &data.data)" which hides failures; change each occurrence
(the ch.write calls that pass data.binary and &data.data) to check the Result
and log failures at trace level (e.g., if let Err(e) = ch.write(data.binary,
&data.data) { trace!(error = ?e, binary = data.binary, "DataChannel write
failed"); }) so write errors are recorded without noisy logs; apply this pattern
to all similar sites (the ch.write calls around the same code paths).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 934d8ddf-74e0-40bf-9a53-5c174f840e7d
📒 Files selected for processing (4)
Cargo.tomlmodel_gateway/Cargo.tomlmodel_gateway/src/routers/openai/realtime/mod.rsmodel_gateway/src/routers/openai/realtime/webrtc_bridge.rs
1 participant
Description
Problem
The WebRTC relay feature needs a core engine that manages dual peer connections (client-facing and upstream-facing), relays RTP audio and data channel messages bidirectionally, and handles ICE/STUN candidate gathering.
Solution
Add WebRtcBridge — a self-contained module that creates two str0m Rtc peers, performs SDP offer/answer exchange with the upstream (OpenAI), and runs a bidirectional relay loop using tokio::select! over two UDP sockets. The
client-facing peer uses ICE-lite; the upstream peer optionally gathers a server-reflexive candidate via a minimal STUN Binding Request (RFC 5389).
Client (browser)
|
[UDP socket 1]
|
client_rtc (ICE-lite, answerer)
|
┌──────────┴──────────┐
│ WebRtcBridge │
│ - data channel relay│
│ - RTP audio relay │
│ - STUN gathering │
└──────────┬──────────┘
|
upstream_rtc (offerer)
|
[UDP socket 2]
|
OpenAI upstream
Changes
Cargo.toml(workspace) — Add str0m = { version = "0.16", default-features = false, features = ["openssl"] }.model_gateway/Cargo.toml— Add str0m, multer workspace deps. Add "multipart" to reqwest features.model_gateway/src/routers/openai/realtime/webrtc_bridge.rs— New file — WebRtcBridge with setup(), run(), SDP exchange, RTP forwarding, data channel relay, STUN gathering, buffered pending-to-client queue.model_gateway/src/routers/openai/realtime/mod.rs— Add pub mod webrtc_bridge;. Update module doc.Test Plan
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit