Skip to content

fix(rtp): keep the voice activity bit out of the audio level's sign - #1032

Merged
algesten merged 2 commits into
algesten:mainfrom
AlexanderSerbul:fix-audio-level-voice-activity-bit
Aug 28, 2026
Merged

fix(rtp): keep the voice activity bit out of the audio level's sign#1032
algesten merged 2 commits into
algesten:mainfrom
AlexanderSerbul:fix-audio-level-voice-activity-bit

Conversation

@AlexanderSerbul

Copy link
Copy Markdown
Contributor

What

When serializing the RFC 6464 ssrc-audio-level extension, the voice activity
(V) bit is forced to 1 for every non-zero level. A header written with
voice_activity: Some(false) is parsed back as Some(true).

The audio level itself is always written correctly — only the flag is lost.

Why

buf[0] = if v2 { 0x80 } else { 0 } | (-(0x7f & v1) as u8);

audio_level is negative, so masking it before negating leaves the sign in
place. For v1 = -37:

step value
v1 as bits 0xDB
0x7f & v1 0x5B = 128 − 37
-(0x5B) as u8 0xA5 = 128 + 37

Bit 7 is already set before the flag term is OR-ed in, and OR cannot clear it.
The low seven bits come out as the magnitude, which is why the level survived
and only the flag was lost — and why this went unnoticed: with
voice_activity: Some(true) everything matches.

Fix

buf[0] = if v2 { 0x80 } else { 0 } | (v1.unsigned_abs() & 0x7f);

Checked exhaustively over the scale: of the 256 (level, voice_activity) pairs,
127 used to round-trip with the wrong flag — all with the flag false and a
non-zero level — and all 256 round-trip correctly now. The added test
audio_level_round_trip walks the whole scale and fails on main
(left: Some(true), right: Some(false)).

Test changes

The two golden-byte tests in rtp::header encoded the old behaviour: they build
headers with voice_activity: Some(false) and expect 170 (0xAA) where the
correct byte is 42 (0x2A). Updated with a note explaining why, so the change
does not read as a loosened expectation.

The parsing tests are deliberately untouched: 0xAA on the wire really does
mean "voice, level 42", and the parse side needs no change.

cargo test --lib is green (707 passed), cargo fmt --check clean.

How it was found

Building an SFU on str0m. Our own test asserted that a declared "not speech"
flag survives forwarding, and it came back as speech every time. For an SFU this
matters: the point of RFC 6464 is to make forwarding decisions without
decrypting or decoding, and speaker detection that trusts the V bit cannot tell
speech from steady background noise.

Alexander Serbul and others added 2 commits August 28, 2026 21:12
When serializing the RFC 6464 `ssrc-audio-level` extension, the V bit was
forced to 1 for every non-zero level, so `voice_activity: Some(false)` was
written — and read back — as `Some(true)`.

`audio_level` is negative, so masking before negating keeps the sign in place:
for -37, `0x7f & v1` is 0x5B (= 128 - 37) and `-(0x5B) as u8` is 0xA5
(= 128 + 37), whose bit 7 is already set. OR-ing the flag in afterwards can
never clear it. The low seven bits happen to come out right, which is why the
level always survived and only the flag was lost.

Taking the magnitude first fixes it. Checked exhaustively: of the 256
(level, voice_activity) pairs on the scale, 127 used to round-trip with the
wrong flag — all of them with the flag false and a non-zero level — and all 256
round-trip correctly now.

The two golden-byte tests in `rtp::header` encoded the old behaviour: they build
a header with `voice_activity: Some(false)` and expected 170 (0xAA) where the
correct byte is 42 (0x2A). Updated, with a note saying why. The parsing tests
are untouched — 0xAA on the wire really does mean "voice, level 42".

Found while building an SFU on str0m: our own test asserted that a declared
"not speech" flag survives forwarding, and it came back as speech every time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@algesten algesten left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent find! Thanks!

@algesten
algesten merged commit b616675 into algesten:main Aug 28, 2026
68 checks passed
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.

2 participants