Skip RTP padding packets instead of concealing them
Padding/probe packets use a sequence number but carry no audio. They were treated as lost, so each one played an invented 20 ms frame (a glitch) and pushed the buffer a frame deeper for drift correction to remove again. Signature in the first live test: lost ~= drift-drops (74 vs 72 per minute). They are now skipped and counted as 'padding'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
de7ed62d96
commit
f8d0e6fc36
4 files changed
+79
-11
No files matched your search
@@ -40,7 +40,12 @@ Why v2: v1 skipped ahead on an underrun and kept playing from an empty buffer, s
|
|||||||
first hiccup every later wobble was another glitch. It also never corrected for a slow guest
|
first hiccup every later wobble was another glitch. It also never corrected for a slow guest
|
||||||
clock. In simulation that was ~12 glitches/min with a guest clock 30 ppm slow, against ~0
|
clock. In simulation that was ~12 glitches/min with a guest clock 30 ppm slow, against ~0
|
||||||
for v2. The 3 Nov 2026 cohost "jitter" was probably this, but that night was not measured.
|
for v2. The 3 Nov 2026 cohost "jitter" was probably this, but that night was not measured.
|
||||||
The simulation is kept in `test/jb_sim_test.go.txt` (rename to `_test.go` in the package dir to run).
|
The simulation (and a padding regression test, `test/pad_test.go.txt`) is kept in `test/jb_sim_test.go.txt` (rename to `_test.go` in the package dir to run).
|
||||||
|
|
||||||
|
Padding fix (v2b, same day): browsers send RTP padding/probe packets that use a sequence
|
||||||
|
number but carry no audio. v1/v2 treated each one as a lost packet and played an invented
|
||||||
|
frame. That was an audible glitch, plus a frame the drift correction later had to remove
|
||||||
|
(signature in the logs: lost ~= drift-drops). They are now skipped and counted as `padding`.
|
||||||
|
|
||||||
The guest websocket is pinged every 25 s. Without that, the proxy closed it every 90 s
|
The guest websocket is pinged every 25 s. Without that, the proxy closed it every 90 s
|
||||||
(signalling only, audio unaffected, but the guest saw "reconnecting...").
|
(signalling only, audio unaffected, but the guest saw "reconnecting...").
|
||||||
|
|||||||
@@ -266,8 +266,8 @@ func (c *Call) Close() {
|
|||||||
c.jbMu.Unlock()
|
c.jbMu.Unlock()
|
||||||
if jb != nil {
|
if jb != nil {
|
||||||
k, _, t := jb.snapshot()
|
k, _, t := jb.snapshot()
|
||||||
log.Printf("call totals: lost %d (fec %d) concealed %d late %d underruns %d drift-drops %d inserted %d, final target %d ms",
|
log.Printf("call totals: lost %d (fec %d) concealed %d late %d underruns %d drift-drops %d inserted %d padding %d, final target %d ms",
|
||||||
k.Lost, k.FEC, k.PLC, k.Late, k.Underruns, k.Dropped, k.Inserted, t)
|
k.Lost, k.FEC, k.PLC, k.Late, k.Underruns, k.Dropped, k.Inserted, k.Padding, t)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -285,8 +285,8 @@ func (c *Call) logRx(jb *jitterBuffer) {
|
|||||||
case <-t.C:
|
case <-t.C:
|
||||||
}
|
}
|
||||||
k, d, tg := jb.snapshot()
|
k, d, tg := jb.snapshot()
|
||||||
log.Printf("rx 1m: buffer %d/%d ms, lost %d (fec %d), concealed %d, late %d, underruns %d, drift-drops %d, inserted %d",
|
log.Printf("rx 1m: buffer %d/%d ms, lost %d (fec %d), concealed %d, late %d, underruns %d, drift-drops %d, inserted %d, padding %d",
|
||||||
d, tg, k.Lost-prev.Lost, k.FEC-prev.FEC, k.PLC-prev.PLC, k.Late-prev.Late, k.Underruns-prev.Underruns, k.Dropped-prev.Dropped, k.Inserted-prev.Inserted)
|
d, tg, k.Lost-prev.Lost, k.FEC-prev.FEC, k.PLC-prev.PLC, k.Late-prev.Late, k.Underruns-prev.Underruns, k.Dropped-prev.Dropped, k.Inserted-prev.Inserted, k.Padding-prev.Padding)
|
||||||
prev = k
|
prev = k
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -411,7 +411,7 @@ func (c *Call) startPlay(t *webrtc.TrackRemote) {
|
|||||||
log.Printf("decoder: %v", err)
|
log.Printf("decoder: %v", err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
jb := newJitterBuffer(dec)
|
jb := newJitterBuffer(dec, uint8(t.PayloadType()))
|
||||||
c.jbMu.Lock()
|
c.jbMu.Lock()
|
||||||
c.jb = jb
|
c.jb = jb
|
||||||
c.jbMu.Unlock()
|
c.jbMu.Unlock()
|
||||||
@@ -529,6 +529,8 @@ type jitterBuffer struct {
|
|||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
dec *opus.Decoder
|
dec *opus.Decoder
|
||||||
pkts map[uint16][]byte
|
pkts map[uint16][]byte
|
||||||
|
skip map[uint16]bool // sequence numbers that carry no audio (padding/probes)
|
||||||
|
pt uint8 // the negotiated Opus payload type
|
||||||
nextSeq uint16
|
nextSeq uint16
|
||||||
haveSeq bool // nextSeq is meaningful
|
haveSeq bool // nextSeq is meaningful
|
||||||
playing bool // false = filling up to target
|
playing bool // false = filling up to target
|
||||||
@@ -545,6 +547,7 @@ type jitterBuffer struct {
|
|||||||
|
|
||||||
type jbCounters struct {
|
type jbCounters struct {
|
||||||
Lost, FEC, PLC, Dropped, Inserted, Underruns, Late int
|
Lost, FEC, PLC, Dropped, Inserted, Underruns, Late int
|
||||||
|
Padding int // packets with no audio (skipped, not concealed)
|
||||||
}
|
}
|
||||||
|
|
||||||
const (
|
const (
|
||||||
@@ -567,14 +570,15 @@ func (j *jitterBuffer) isQuiet(pcm []int16) bool {
|
|||||||
return r < lim
|
return r < lim
|
||||||
}
|
}
|
||||||
|
|
||||||
func newJitterBuffer(dec *opus.Decoder) *jitterBuffer {
|
func newJitterBuffer(dec *opus.Decoder, pt uint8) *jitterBuffer {
|
||||||
return &jitterBuffer{dec: dec, pkts: map[uint16][]byte{}, target: jbFloor, pcm: make([]int16, 5760*channels), lastBad: time.Now(), floor: quietRMS}
|
return &jitterBuffer{dec: dec, pt: pt, pkts: map[uint16][]byte{}, skip: map[uint16]bool{}, target: jbFloor, pcm: make([]int16, 5760*channels), lastBad: time.Now(), floor: quietRMS}
|
||||||
}
|
}
|
||||||
|
|
||||||
func seqLess(a, b uint16) bool { return int16(a-b) < 0 }
|
func seqLess(a, b uint16) bool { return int16(a-b) < 0 }
|
||||||
|
|
||||||
func (j *jitterBuffer) reset() {
|
func (j *jitterBuffer) reset() {
|
||||||
j.pkts = map[uint16][]byte{}
|
j.pkts = map[uint16][]byte{}
|
||||||
|
j.skip = map[uint16]bool{}
|
||||||
j.haveSeq, j.playing, j.lateRun, j.insertNow = false, false, 0, false
|
j.haveSeq, j.playing, j.lateRun, j.insertNow = false, false, 0, false
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -591,7 +595,17 @@ func (j *jitterBuffer) bump() {
|
|||||||
func (j *jitterBuffer) push(p *rtp.Packet) {
|
func (j *jitterBuffer) push(p *rtp.Packet) {
|
||||||
j.mu.Lock()
|
j.mu.Lock()
|
||||||
defer j.mu.Unlock()
|
defer j.mu.Unlock()
|
||||||
if len(p.Payload) == 0 {
|
if len(p.Payload) == 0 || (j.pt != 0 && p.PayloadType != j.pt) {
|
||||||
|
// Padding / bandwidth probe: it uses a sequence number but carries no audio and no
|
||||||
|
// time. v1/v2a ignored it, which left a hole that was then "concealed" - an
|
||||||
|
// invented 20 ms frame that the drift correction later had to remove again.
|
||||||
|
j.c.Padding++
|
||||||
|
if !j.haveSeq || !seqLess(p.SequenceNumber, j.nextSeq) {
|
||||||
|
if len(j.skip) > 200 {
|
||||||
|
j.skip = map[uint16]bool{}
|
||||||
|
}
|
||||||
|
j.skip[p.SequenceNumber] = true
|
||||||
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if j.haveSeq {
|
if j.haveSeq {
|
||||||
@@ -695,6 +709,11 @@ func (j *jitterBuffer) next() []int16 {
|
|||||||
}
|
}
|
||||||
|
|
||||||
for {
|
for {
|
||||||
|
if j.skip[j.nextSeq] {
|
||||||
|
delete(j.skip, j.nextSeq)
|
||||||
|
j.nextSeq++
|
||||||
|
continue
|
||||||
|
}
|
||||||
if p, ok := j.pkts[j.nextSeq]; ok {
|
if p, ok := j.pkts[j.nextSeq]; ok {
|
||||||
delete(j.pkts, j.nextSeq)
|
delete(j.pkts, j.nextSeq)
|
||||||
j.nextSeq++
|
j.nextSeq++
|
||||||
@@ -744,6 +763,6 @@ func (j *jitterBuffer) stats() map[string]any {
|
|||||||
return map[string]any{
|
return map[string]any{
|
||||||
"depthMs": d, "targetMs": t,
|
"depthMs": d, "targetMs": t,
|
||||||
"lost": c.Lost, "fec": c.FEC, "plc": c.PLC, "dropped": c.Dropped, "inserted": c.Inserted,
|
"lost": c.Lost, "fec": c.FEC, "plc": c.PLC, "dropped": c.Dropped, "inserted": c.Inserted,
|
||||||
"underruns": c.Underruns, "late": c.Late,
|
"underruns": c.Underruns, "late": c.Late, "padding": c.Padding,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -30,7 +30,7 @@ func sim(t *testing.T, name string, noiseFloor float64, ppm float64, jitterMs fu
|
|||||||
for i := 0; i < 50; i++ { talkP = append(talkP, mk(i, true)); quietP = append(quietP, mk(i, false)) }
|
for i := 0; i < 50; i++ { talkP = append(talkP, mk(i, true)); quietP = append(quietP, mk(i, false)) }
|
||||||
payloadFor := func(seq int) []byte { if seq%175 < 125 { return talkP[seq%50] }; return quietP[seq%50] }
|
payloadFor := func(seq int) []byte { if seq%175 < 125 { return talkP[seq%50] }; return quietP[seq%50] }
|
||||||
dec, _ := opus.NewDecoder(rate, channels)
|
dec, _ := opus.NewDecoder(rate, channels)
|
||||||
jb := newJitterBuffer(dec)
|
jb := newJitterBuffer(dec, 111)
|
||||||
r := rand.New(rand.NewSource(1))
|
r := rand.New(rand.NewSource(1))
|
||||||
const dur = 3 * 3600 * 1000.0
|
const dur = 3 * 3600 * 1000.0
|
||||||
type ev struct{ at float64; seq uint16 }
|
type ev struct{ at float64; seq uint16 }
|
||||||
|
|||||||
@@ -0,0 +1,44 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"math"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/pion/rtp"
|
||||||
|
"gopkg.in/hraban/opus.v2"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestPadding(t *testing.T) {
|
||||||
|
for _, name := range []string{"empty payload", "other payload type"} {
|
||||||
|
enc, _ := opus.NewEncoder(rate, channels, opus.AppAudio)
|
||||||
|
dec, _ := opus.NewDecoder(rate, channels)
|
||||||
|
jb := newJitterBuffer(dec, 111)
|
||||||
|
pcm := make([]int16, frameSamps*channels)
|
||||||
|
buf := make([]byte, 4000)
|
||||||
|
seq, ts := uint16(0), 0.0
|
||||||
|
frames := 0
|
||||||
|
// 10 minutes of playout; audio packet every 20 ms, plus a padding packet every 40 seqs.
|
||||||
|
for tms := 0.0; tms < 600000; tms += 20 {
|
||||||
|
for ts <= tms {
|
||||||
|
if seq%40 == 39 {
|
||||||
|
p := &rtp.Packet{Header: rtp.Header{SequenceNumber: seq, PayloadType: 111}}
|
||||||
|
if name != "empty payload" {
|
||||||
|
p.PayloadType, p.Payload = 13, []byte{0x40}
|
||||||
|
}
|
||||||
|
jb.push(p)
|
||||||
|
seq++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
for k := range pcm { pcm[k] = int16(6000 * math.Sin(float64(frames*frameSamps+k/2)*2*math.Pi*220/rate)) }
|
||||||
|
n, _ := enc.Encode(pcm, buf)
|
||||||
|
jb.push(&rtp.Packet{Header: rtp.Header{SequenceNumber: seq, PayloadType: 111}, Payload: append([]byte(nil), buf[:n]...)})
|
||||||
|
seq++; frames++; ts += 20
|
||||||
|
}
|
||||||
|
jb.next()
|
||||||
|
}
|
||||||
|
c, d, tg := jb.snapshot()
|
||||||
|
fmt.Printf("%-20s padding=%d lost=%d fec=%d concealed=%d drift-drops=%d inserted=%d underruns=%d buffer=%d/%dms\n", name, c.Padding, c.Lost, c.FEC, c.PLC, c.Dropped, c.Inserted, c.Underruns, d, tg)
|
||||||
|
if c.Lost+c.PLC+c.Dropped+c.Underruns != 0 { t.Errorf("%s: padding still causes damage", name) }
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user