Skip to content

Fix SRS transmissions being split into fragments - #712

Draft
dharmab wants to merge 1 commit into
mainfrom
fix/srs-transmission-splitting
Draft

Fix SRS transmissions being split into fragments#712
dharmab wants to merge 1 commit into
mainfrom
fix/srs-transmission-splitting

Conversation

@dharmab

@dharmab dharmab commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Context

The SRS protocol has no end-of-transmission marker, so pkg/simpleradio/receive.go infers transmission boundaries from a silence gap. Three bugs in that logic split a single utterance into fragments, which reach whisper as garbled or duplicated requests, or fall under minRxDuration (1s) and get discarded with no log line explaining why.

Design

The channel owns the transmission window, not the caller. A window opens when a packet arrives on an idle channel and closes when no packet has arrived for the grace period. Each receiver's window is evaluated independently, the way a GCI monitoring several frequencies hears each one separately. Within a window the first transmitter wins and anyone stepping on them is dropped.

The three splitting bugs

Was Now
Queue coupling Publishing was gated on len(in) == 0, so traffic on any frequency — including ones no radio is tuned to — could hold another channel's transmission indefinitely The tick drains the queue, then evaluates each deadline independently
Stale-buffer lockout "In progress" meant state has not been reset, not the deadline has not passed, so a finished-but-unpublished transmission locked out the next caller Completed transmissions are published before each packet is buffered
Phantom windows Packet numbers reset to zero on publish, so a delayed packet was mistaken for a new transmission — buffering a stale frame, arming a fresh deadline, blocking the next real caller High-water mark kept per transmitter across transmissions

Verified against a live server

Captured 1180 packets from eu.limakilo.net (receive only, never transmitted):

  • Two transmissions 400.9s apart carried consecutive packet numbers (delta = 1, IDs 3626329→3627508, zero loss). This is the direct confirmation that the reset-to-zero was wrong.
  • Inter-packet gaps: p50 40.0ms, p95 42.2ms, max 52.0ms — the 300ms default has ~6× headroom, so I kept it rather than raising it.
  • Framing 6+31+40+58 = 135 = datagram size, confirming voice.Decode's framing validation (shipped separately in Validate SRS voice packet framing in decoder #721) doesn't reject real traffic.

⚠️ Confirming that assumption also showed relying on it was dangerous. A client that restarted its numbering would fail PacketID <= highWaterMark forever and be silenced permanently — strictly worse than the stale packet the check exists to reject, and something the old code tolerated by accident. A backwards packet number now only counts as stale within 2× the grace period of that transmitter's last packet. Test: TestReceiverAcceptsRenumberedTransmitter.

Also in here

  • --srs-split-transmission-grace-period, default unchanged at 300ms
  • Wall-clock span logged next to audio duration, so "short transmission" and "lossy transmission" are distinguishable
  • Receiver lock taken for every access (waitForClearChannel read a deadline unlocked, publish read the buffer unlocked, coalition filter read clients without clientsLock)

Testing

Regression suite for the receiver: window opening/closing, grace-period boundaries, duplicate/out-of-order/renumbered packets, capture effect, truncation at MaxTransmissionDuration, the queue-coupling fix, and the waitForClearChannel lock fix. Time-dependent tests use testing/synctest, matching pkg/controller.

make test && make lint vet fix format && go mod tidy   # all pass, no further changes
1250 tests, race detector clean

Mutation-checked the regression tests by reverting each fix. One caveat: the break after the first frequency match is now dead defense — the per-transmitter dedup already rejects the repeat, so removing it fails nothing. Kept it to avoid a redundant locked call, but it isn't load-bearing.

Not verified: whether the SRS server filters voice packets by frequency server-side, which is what sets bug 1's real-world severity. The controlled capture on a bogus frequency didn't happen, and the observed packets listed four frequencies I was partly tuned to, so they prove nothing either way. The fix is structural — publication no longer consults queue depth at all — so severity doesn't change the code.

🤖 Generated with Claude Code

The SRS protocol has no end-of-transmission marker, so the receiver infers
transmission boundaries from a silence gap. Three bugs in that logic split a
single utterance into fragments, which reach whisper as garbled or duplicated
requests, or fall under the one second minimum and are discarded with no log
line explaining why.

The channel now owns the transmission window rather than the caller. A window
opens when a packet arrives on an idle channel and closes when no packet has
arrived for the grace period. Each receiver's window is evaluated on its own,
the way a GCI monitoring several frequencies hears each one separately. Within
a window the first transmitter wins and anyone stepping on them is dropped,
mirroring radio capture effect and keeping the buffer single-origin for the
Opus decoder.

Fixed:

- Publishing a finished transmission was gated on the shared inbound queue
  being empty, so traffic on any frequency - including ones no radio is tuned
  to - could hold another channel's transmission indefinitely. The tick now
  drains the queue and then evaluates each deadline independently.

- "In progress" meant "state has not been reset" rather than "the deadline has
  not passed", so a finished but unpublished transmission locked out the next
  caller. Completed transmissions are now published before each packet is
  buffered, so a new caller opens a window the instant the previous one expires.

- Packet numbers were reset to zero on every publish. SRS numbers climb for the
  life of a client, so a packet delayed past the end of its own transmission was
  mistaken for the start of a new one: it buffered a stale frame, armed a fresh
  deadline, and blocked the next real caller. The high-water mark is now kept
  per transmitter across transmissions.

  Confirmed against a live server: two transmissions 400 seconds apart carried
  consecutive packet numbers. Since relying on that could permanently silence a
  client that restarted its numbering, a backwards packet number only counts as
  stale within twice the grace period of that transmitter's last packet.

Also:

- Expose the grace period as --srs-split-transmission-grace-period, default
  unchanged at 300ms. Measured against a live server, packets arrive 40ms apart
  with a worst case of 52ms, so the default has plenty of headroom.

- voice.Decode trusted the length header over the datagram, so a corrupt packet
  could decode into a plausible packet with a garbage origin GUID and hold a
  frequency for a full grace period. It now validates the framing.

- Log the wall clock span alongside the audio duration, so a transmission
  discarded for being short can be told apart from one shortened by packet loss.

- Take the receiver lock for every access. waitForClearChannel read a deadline
  and the publish path read the buffer without it, and the coalition filter read
  the clients map without clientsLock.

- Reduce the inbound voice channel from 64*0xFFFFF to a bounded size. It
  reserved roughly 1.6GB of slice headers and let backlogs grow without limit.

Two unrelated bugs found while working here, called out for review:

- --mute never reached the SRS client, so the flag did nothing.
- RadioFrequency.String() had a transposed format verb and printed hertz, so
  251MHz AM rendered as "251000000.000000.3AM". Nothing calls it today.

The package had almost no tests, so this adds a baseline suite alongside the
regression tests: packet encode/decode round trips and malformed input,
frequency and radio matching, client sync filtering, server settings, message
JSON against a captured wire sample, and the Opus frame round trip. Time
dependent tests use testing/synctest, matching pkg/controller. Enables -race
for the test suite, which the whole tree passes in about 16 seconds.

Co-Authored-By: Claude <noreply@anthropic.com>
@dharmab
dharmab force-pushed the fix/srs-transmission-splitting branch from 09f2956 to ca7ccc9 Compare July 26, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant