docs: document 1:1 video calling - #408
Conversation
Reflects the video calling feature added to the VoIP builder API: VideoSource/VideoSink traits, CallHandle::start_video/accept_video/ stop_video, the codec-neutral H.264 Annex-B media plane, and the CLI's --video flag and v/q live controls.
📝 WalkthroughWalkthroughThe documentation expands VoIP coverage from 1:1 voice calls to end-to-end encrypted audio and video calls, including setup, examples, CLI behavior, video I/O APIs, H.264 media handling, encryption, architecture, and roadmap updates. ChangesVoIP calling documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Addresses Greptile review on #408: mic/speaker capture-playout is consumer-owned (was contradicting the Audio I/O section and the new video bullet), the async-channel dependency comment omitted VideoSource/VideoSink, and the mid-call upgrade example was missing the initiator's announce_video_enabled step that completes the UpgradeRequestV2/UpgradeAccept handshake.
|
Thanks for the review — pushed fixes for issues 1–3 (mic-capture ownership contradiction, missing Issue 4 (Opus → MLow in the Generated by Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f98d84384
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses Codex review on #408, verified against wacore source: - VideoPipeline and MediaPipeline derive_e2e_keys() with the same call_key + participant LID, so audio/video share master SRTP keys per participant (only SSRC/sequence/ROC differ) — was documented as independent per-stream keys. - SframeSession lives only in the audio MediaState (engine.rs) and decrypts inbound foreign-Opus payloads; VideoPipeline has no sframe field, so the encryption steps no longer imply a video SFrame layer. - introduction.mdx's "full media plane in Rust" bullet no longer claims video encode/decode run in Rust (only audio/MLow does; H.264 stays consumer-owned per the Video I/O section). - The incoming-call example pointed a later local upgrade at `accept_video` (peer-upgrade-response only); corrected to `start_video`. - CLI table now notes `listen --video` implies `accept` (confirmed by the `listen_video_implies_accept` test in the example). Also splits the mid-call video upgrade snippet into separate initiator/responder/downgrade blocks per Greptile's review.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
Thanks @chatgpt-codex-connector — all 5 findings checked out against
Also split the mid-call video-upgrade snippet into separate initiator/responder/downgrade blocks per Greptile's suggestion. Generated by Claude Code |
|
To use Codex here, create an environment for this repo. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…ideo_enabled Greptile flagged the initiator example as two back-to-back awaits with no indication that announce_video_enabled must wait for the peer's UpgradeAccept event, which arrives asynchronously on handle.events(). Rewrite the snippet as an explicit event loop so it can't be copied into a signaling protocol violation.
Two minor clarity nits from Greptile's review: subscribe to handle.events() before calling start_video (avoids a theoretical race with a fast peer response), and clarify that --video replaces the audio loopback with a separate video-only one rather than running alongside it.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b610adf4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex caught the real defect: src/handlers/call.rs's CallHandler already sends the standalone Enabled stanza automatically when the peer's UpgradeAccept arrives (before dispatching CallEvent to the app), so the previous "call announce_video_enabled from your event loop" example taught a double-send. Rewrite to describe the event as a notification only, and scope announce_video_enabled to consumers driving signaling outside the standard handler. Also: the "peer initiates" flow now covers the legacy VideoState::UpgradeRequest alongside UpgradeRequestV2 (the CLI's own handler matches both), and documents that handle.events() clones are competing consumers sharing one queue, not a broadcast.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
Thanks, this round caught a real bug in the docs. Verified against
On the sentence-case heading nit: every existing heading in this file ( Generated by Claude Code |
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@guides/voip-calls.mdx`:
- Line 16: Update the headings in guides/voip-calls.mdx at lines 16-16 and
281-281 to use sentence case: change “Enabling the Feature” to “Enabling the
feature” and “H.264 Video Plane” to “H.264 video plane”.
- Line 201: Rewrite the paragraph describing peer acceptance and the <call>
stanza handler into concise, single-idea sentences or ordered steps. Separately
state that the handler automatically sends Enabled, that handle.events() emits
VideoState::UpgradeAccept as a notification, that consumers must not call
handle.announce_video_enabled() in response, and that the method is only for
consumers managing signaling outside the standard handler.
In `@introduction.mdx`:
- Line 60: Update the media-plane bullet in introduction.mdx to remove the “Full
media plane in Rust” claim, rename it to accurately reflect Rust-owned
responsibilities, and split capture/playout, audio processing and
encryption/relay, and consumer-owned H.264 encoding/decoding into concise
one-idea sentences.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c9116fc9-cb97-485f-8e28-252ae1013388
📒 Files selected for processing (3)
guides/voip-calls.mdxinstallation.mdxintroduction.mdx
Sentence case for the two new headings per this repo's documented "use sentence case for headings" guideline (previously left as title case to match the file's existing headings, but a house style rule takes precedence over matching now-nonconforming older content). Split the video-upgrade Note and the introduction.mdx media-plane bullet into one-idea-per-sentence prose per CodeRabbit's suggestions.
Confirmed in examples/voip-cli/src/video.rs::run_video_loopback: it's src.recv() -> VideoFrame::new() -> sink.send(), no protect/unprotect call at all, unlike the audio loopback's explicit E2E-SRTP round trip. Greptile flagged that a reader could assume parity with the audio row directly above it.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
The 'audio-only accept can still add video later' note lived inside the if is_video block, reading as a contradiction with the 'required' comment right above it. Moved it after the if so it clearly describes the other branch.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
2 participants
Summary
Updates the VoIP documentation for 1:1 video calling, added in oxidezap/whatsapp-rust#1024.
guides/voip-calls.mdx(main update):.video(source, sink)builder call.VideoSource/VideoSinktraits,VideoFrame, and mid-callstart_video/accept_video/stop_videoonCallHandle.Call Handletable gains the video control methods.--videoflag and thev/qlive-call keys.Encryption/Validationsections extended to cover video's E2E-SRTP path.Roadmapno longer lists video calls as a future item (now shipped).introduction.mdx: "Voice calls" feature section renamed to "Voice & video calls" with new bullets for video capability and the codec-neutral design.installation.mdx:voipfeature-flag table row now mentions the H.264 video plane.Test plan
mint devand verify the newguides/voip-calls.mdxsections render correctly (code fences, tables, callouts)mint broken-links🤖 Generated with Claude Code
Generated by Claude Code
Summary by cubic
Documents 1:1 video calling in
whatsapp-rust, including the codec‑neutral H.264 plane,.video(...), and mid‑call start/accept/stop flows. Clarifies crypto/signaling and I/O ownership, fixes the upgrade handshake and example comments, and notesloopback --videois video‑only and skips the RTP/SRTP plane.New Features
guides/voip-calls.mdx: documents.video(source, sink),VideoSource/VideoSink,VideoFrame(incl. orientation) andrtp_timestamp_stride;CallHandle::{start_video,accept_video,stop_video}with from‑start and upgrade examples;handle.events()notes video state/RTCP; the Call Handle table statesstart_videoreturns immediately.--video,vtoggles video andqhangs up;listen --videoimpliesaccept;loopback --videois a separate video‑only path that skips RTP/SRTP; intro/install pages updated.Bug Fixes
start_videoinitiator flow clarified (subscribe tohandle.events()first; peer acceptance auto‑sendsEnabled;announce_video_enabledonly for custom signaling); responder flow covers legacy and V2;handle.events()clones share one queue; incoming‑call local upgrade usesstart_video.Written for commit 84202a5. Summary will update on new commits.