feat: add RFC 3264 call hold / resume - #39
Closed
wavekat-eason wants to merge 4 commits into
Closed
Conversation
Add a first-class signaled hold primitive so consumers don't hand-roll SDP re-INVITEs or fake hold with a local audio gate the far end never learns about. - sdp: MediaDirection (sendrecv/sendonly/recvonly/inactive) with attr/ parse/responding; build_sdp_with_direction; RemoteMedia::direction parsed from the answer (default sendrecv). - hold: reoffer_media() sends a directional re-INVITE over any SessionDialogOps dialog and returns the parsed answer; AcceptedCall/ AcceptedDial::set_hold() convenience wrappers. What plays on RTP while held (silence, music-on-hold) and answering a peer-initiated hold stay with the consumer. Unit-tested against a scripted dialog mock; docs/08 + RFC-COVERAGE updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsYLx8QaSspoApH5A8s9KL
Hold/resume re-INVITEs reused a hardcoded `o=wavekat 0 0` origin line, so every re-offer carried session version 0. RFC 3264 §8 requires the version to increment on each re-offer that changes the session. Carrier SBCs accept the first re-offer (hold) but reject the second (resume) with 500 Server Internal Error, then BYE the call. Thread a session version into build_sdp_with_direction and give HoldHandle a call-scoped Arc<AtomicU64> counter (seeded to 0 to match the initial offer/answer) so successive holds/resumes advance one monotonic series — hold=1, resume=2, … The Arc lives on AcceptedCall/AcceptedDial and is cloned into every handle, since hold_handle() mints a fresh handle per request and a per-handle counter would reset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKGMQeftv6ztjoq1zXCL7g
A refused hold/resume re-INVITE logged only the bare status code
("rejected: 500 ServerInternalError"), which says nothing about why the
server refused. Pull the Warning (RFC 3261 §20.43) and Reason (RFC 3326)
headers out of the non-2xx response and append them, so a rejection logs
something a person can act on.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GKGMQeftv6ztjoq1zXCL7g
local_rtp_addr was the RTP socket's bind address (0.0.0.0:port), and the hold/resume re-offer built its SDP connection line from it — so every re-INVITE carried c=IN IP4 0.0.0.0. That is the legacy RFC 2543 hold signal: the peer (2talk) could not send media to 0.0.0.0 and answered recvonly even to a sendrecv resume, leaving the call with broken/one-way audio after resume. The initial answer/offer already used the real local IP; re-offers now match it. Store local_rtp_addr with the real local IP (the address actually advertised), not the wildcard bind. Adds a regression assertion in caller_dial that the advertised RTP address is the endpoint's local IP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKGMQeftv6ztjoq1zXCL7g
Contributor
Author
|
Superseded by #43 — hold/resume is reimplemented on the in-house engine there ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a first-class signaled call-hold primitive so consumers can put a confirmed call on hold (and resume it) via an RFC 3264 re-INVITE, instead of hand-rolling SDP re-INVITEs or faking hold with a local audio gate the far end never learns about. Per the crate's placement rules, SIP signaling belongs here.
What's added
sdp::MediaDirection—SendRecv/SendOnly/RecvOnly/Inactive, withattr()(wire token),parse(), andresponding()(the direction to answer a peer's offer with). Defaults toSendRecv.sdp::build_sdp_with_direction()— the existingbuild_sdpbody with an explicit direction line and ano=session version;build_sdpdelegates to it (SendRecv, version0), so its output is unchanged.sdp::RemoteMedia::direction— the parsed direction of a remote SDP (defaultSendRecvwhen absent), so a consumer can also detect a peer-initiated hold.hold::reoffer_media()— sends the directional re-INVITE over anySessionDialogOpsdialog (reusing the session-refresh seam) and returns the re-parsed answer.hold::HoldHandle— a cheap, cloneable handle (both inner dialog types areClone) so a consumer can snapshot it, release whatever lock guards its call table, and only then await the re-INVITE round-trip. It carries a call-scopedArc<AtomicU64>o=version counter so successive hold/resume re-offers advance one monotonic series.AcceptedCall::set_hold/hold_handle,AcceptedDial::set_hold/hold_handle— convenience wrappers. They own the version counter (seeded to0) and clone it into every handle.The re-offer keeps the same connection address, port, and codecs — only the
a=direction moves — so the dialog and any session timer survive the transition.RFC 3264 §8
o=version (hold→resume interop)The
o=origin line keeps a fixed session id but its version must increment on every re-offer that changes the session. An early revision hardcodedo=wavekat 0 0, so hold and resume both carried version0— and a carrier SBC (2talk) accepted the hold but rejected the resume with500 Server Internal Error, then BYE'd the call. The version is now threaded throughbuild_sdp_with_directionand sourced from the per-call counter: initial offer/answer0, first re-offer1, next2, … Becausehold_handle()mints a fresh handle per request, the counter lives onAcceptedCall/AcceptedDialand is cloned in — a per-handle counter would reset and reintroduce the bug.Out of scope (consumer's job)
What plays on the RTP stream while held (silence, music-on-hold) is audio, not signaling. Answering a peer-initiated hold re-INVITE is left to the consumer too — this crate supplies
RemoteMedia::direction+MediaDirection::responding()but does not auto-answer.Tests
sdp.rscovers the direction round-trip (all four directions), the absent-attribute default,responding(), and that theo=version tracks the fedsession_version.hold.rsunit-testsreoffer_mediaagainst a scriptedSessionDialogOpsmock: asserts the offer's direction/port ando=version, parses the answer, maps a bare 2xx toOk(None), errors on a non-2xx / unconfirmed dialog, and checks the version counter yields a strictly increasing1, 2, 3, …series. No live SIP server needed.cargo fmt/clippy/test/docall clean.Design note:
docs/08-call-hold.md; coverage updated indocs/RFC-COVERAGE.md.🤖 Generated with Claude Code