Address review comments: SRTP ROC tracking, RTP address selection, protocol hardening by Copilot · Pull Request #24 · codingjoe/VoIP

Copilot · 2026-03-13T19:16:06Z

Addresses all unresolved review comments from the call transcription PR series.

SRTP (`voip/srtp.py`)

Separate send/receive ROC counters — _send_roc/_last_send_seq for encryption, _recv_roc/_last_recv_seq for decryption, per RFC 3711 §3.3.1
Full index estimation on receive — _estimate_recv_index() implements the RFC 3711 §3.3.1 algorithm (plain arithmetic comparisons, not modular)
Constant-time auth tag comparison — replaces != with hmac.compare_digest (stdlib) to prevent timing side-channels

SIP Protocol (`voip/sip/protocol.py`)

Remote RTP address selection — prefers media-level c= → session-level c= → SIP peer IP (per RFC 4566 §5.7); port 0 (inactive media, RFC 4566 §5.14) skips registration and NAT hole-punching
Early INVITE guard — _initialize_task stored on connection_made; _answer() awaits it only when _rtp_protocol is still None, preventing attribute errors without breaking tests
Remove error_received — asyncio.Protocol has no such callback (it belongs to DatagramProtocol); removed method and its tests

Audio (`voip/audio.py`)

WhisperModel injection — WhisperCall.model now accepts a pre-loaded WhisperModel instance in addition to a name string, enabling sharing across concurrent calls:

shared_model = WhisperModel("base")

class MyCall(WhisperCall):
    model = shared_model  # loaded once, reused per call

Docs / minor

STUN docstring — removed stale reference to public_address (attribute no longer exists on STUNProtocol)
RTP docstring — "silently discarded" → "logged at WARNING level and discarded" to match actual behavior
README — removed inaccurate "Unencrypted SIP or RTP is not supported" claim

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…ion, STUN docstring, WhisperModel injection, README cleanup Co-authored-by: codingjoe <[email protected]>

Copilot · 2026-03-13T19:39:53Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

192.0.2.1
- Triggering command: REDACTED, pid is -1 (packet block)
stun.cloudflare.com
- Triggering command: /usr/bin/python python -m pytest tests/test_stun.py tests/test_rtp.py tests/sip/test_protocol.py -x -q --override-ini=addopts= (dns block)
- Triggering command: /usr/bin/python python -m pytest tests/sip/test_protocol.py -x -q --override-ini=addopts= --no-header (dns block)
- Triggering command: /usr/bin/python python -m pytest tests/sip/test_protocol.py::TestAnswer -x -q --override-ini=addopts= --no-header --tags --long --abbrev=40 git --gi�� (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Copilot

Pull request overview

This PR hardens core VoIP call handling by refining SRTP rollover/index tracking, improving SIP call-answer robustness and SDP-derived RTP routing decisions, and aligning docs/tests with actual runtime behavior.

Changes:

SRTP: split send/receive ROC tracking, implement RFC 3711 receive index estimation, and use constant-time auth-tag comparison.
SIP: add early-INVITE initialization guard, improve remote RTP address selection per RFC 4566 (including inactive port 0), and remove the non-existent asyncio.Protocol.error_received.
Audio/docs/tests: allow injecting a pre-loaded WhisperModel, update docstrings/tests/logging text, and adjust README claims.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`voip/stun.py`	Updates STUNProtocol docstring to reflect current callback-based readiness API.
`voip/srtp.py`	Refactors ROC tracking, adds receive index estimation, and hardens auth-tag comparison.
`voip/sip/protocol.py`	Stores initialization task, guards early INVITEs, improves RTP address selection, removes `error_received`.
`voip/rtp.py`	Updates docstring to match WARNING-level logging on SRTP auth failure.
`voip/audio.py`	Allows `WhisperModel` instance injection for shared model reuse across calls.
`tests/test_rtp.py`	Updates SRTP invalid-auth test description to match warning log behavior.
`tests/sip/test_protocol.py`	Removes tests for `error_received` after method removal.
`README.md`	Removes a claim about unencrypted SIP/RTP not being supported.

You can also share your feedback on Copilot code review. Take the survey.

voip/sip/protocol.py

+        # early INVITE must wait for the mux.  Skip if already available.
+        if self._rtp_protocol is None:
+            if self._initialize_task is not None:
+                await self._initialize_task


voip/sip/protocol.py

+        if remote_audio is not None and remote_audio.port != 0:
+            media_conn = remote_audio.connection
+            session_conn = request.body.connection if request.body else None
+            conn = media_conn or session_conn
+            if conn is not None:


voip/srtp.py

+    def _estimate_recv_index(self, seq: int) -> tuple[int, int]:
+        """Estimate the packet index and new ROC for a received sequence number.
+
+        Implements the index estimation algorithm from RFC 3711 §3.3.1.
+


README.md

 All signalling uses **SIP over TLS** (SIPS, RFC 3261 §26) and all media is
 protected with **SRTP** ([RFC 3711](https://tools.ietf.org/html/rfc3711))
 using the `AES_CM_128_HMAC_SHA1_80` cipher suite with SDES key exchange
-([RFC 4568](https://tools.ietf.org/html/rfc4568)). Unencrypted SIP or RTP
-is not supported.
+([RFC 4568](https://tools.ietf.org/html/rfc4568)).


voip/sip/protocol.py

        try:
-            asyncio.get_running_loop().create_task(self._initialize())
+            self._initialize_task = asyncio.get_running_loop().create_task(
+                self._initialize()
+            )
        except RuntimeError:


Initial plan

c79056e

Copilot AI assigned Copilot and codingjoe Mar 13, 2026

Copilot AI mentioned this pull request Mar 13, 2026

Implement call transcription #8

Merged

Copilot started work on behalf of codingjoe March 13, 2026 19:16 View session

Address review comments: SRTP ROC tracking, remote RTP address select…

af545fa

…ion, STUN docstring, WhisperModel injection, README cleanup Co-authored-by: codingjoe <[email protected]>

Copilot AI changed the title ~~[WIP] Implement call transcription~~ Address review comments: SRTP ROC tracking, RTP address selection, protocol hardening Mar 13, 2026

Copilot AI requested a review from codingjoe March 13, 2026 19:40

Copilot finished work on behalf of codingjoe March 13, 2026 19:40

codingjoe marked this pull request as ready for review March 13, 2026 20:11

Copilot AI review requested due to automatic review settings March 13, 2026 20:11

codingjoe merged commit c4d53cf into transcribe-call Mar 13, 2026
1 check failed

codingjoe deleted the copilot/sub-pr-8 branch March 13, 2026 20:11

Copilot started reviewing on behalf of codingjoe March 13, 2026 20:12 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Address review comments: SRTP ROC tracking, RTP address selection, protocol hardening#24

Address review comments: SRTP ROC tracking, RTP address selection, protocol hardening#24
codingjoe merged 2 commits intotranscribe-callfrom
copilot/sub-pr-8

Copilot AI commented Mar 13, 2026 •

edited

Loading

Uh oh!

Copilot AI commented Mar 13, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Copilot AI commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SRTP (voip/srtp.py)

SIP Protocol (voip/sip/protocol.py)

Audio (voip/audio.py)

Docs / minor

Uh oh!

Copilot AI commented Mar 13, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Mar 13, 2026 •

edited

Loading

SRTP (`voip/srtp.py`)

SIP Protocol (`voip/sip/protocol.py`)

Audio (`voip/audio.py`)