Skip to content

Commit

Permalink
fix: Some bad math with the JitterBuffer sequence tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
@thatsnotright
thatsnotright committed May 2, 2024
1 parent 142f17f commit 1a036cf
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 11 deletions.
7 changes: 3 additions & 4 deletions pkg/jitterbuffer/jitter_buffer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package jitterbuffer

import (
"errors"
"math"
"sync"

"github.com/pion/rtp"
Expand Down Expand Up @@ -141,7 +140,7 @@ func (jb *JitterBuffer) SetPlayoutHead(playoutHead uint16) {
func (jb *JitterBuffer) updateStats(lastPktSeqNo uint16) {
// If we have at least one packet, and the next packet being pushed in is not
// at the expected sequence number increment the out of order count
if jb.packets.Length() > 0 && lastPktSeqNo != ((jb.lastSequence+1)%math.MaxUint16) {
if jb.packets.Length() > 0 && lastPktSeqNo != (jb.lastSequence+1) {
jb.stats.outOfOrderCount++
}
jb.lastSequence = lastPktSeqNo
Expand Down Expand Up @@ -215,7 +214,7 @@ func (jb *JitterBuffer) Pop() (*rtp.Packet, error) {
jb.emit(BufferUnderflow)
return nil, err
}
jb.playoutHead = (jb.playoutHead + 1) % math.MaxUint16
jb.playoutHead = (jb.playoutHead + 1)
jb.updateState()
return packet, nil
}
Expand All @@ -233,7 +232,7 @@ func (jb *JitterBuffer) PopAtSequence(sq uint16) (*rtp.Packet, error) {
jb.emit(BufferUnderflow)
return nil, err
}
jb.playoutHead = (jb.playoutHead + 1) % math.MaxUint16
jb.playoutHead = (jb.playoutHead + 1)
jb.updateState()
return packet, nil
}
Expand Down
33 changes: 26 additions & 7 deletions pkg/jitterbuffer/jitter_buffer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,25 @@ func TestJitterBuffer(t *testing.T) {
assert.Equal(jb.packets.Length(), uint16(4))
assert.Equal(jb.lastSequence, uint16(5012))
})
t.Run("Appends packets and wraps", func(*testing.T) {
jb := New(WithMinimumPacketCount(1))
assert.Equal(jb.lastSequence, uint16(0))
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 65535, Timestamp: 500}, Payload: []byte{0x02}})

assert.Equal(jb.lastSequence, uint16(65535))

jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 0, Timestamp: 512}, Payload: []byte{0x02}})

assert.Equal(jb.packets.Length(), uint16(2))
assert.Equal(jb.lastSequence, uint16(0))

head, err := jb.Pop()
assert.Equal(head.SequenceNumber, uint16(65535))
assert.Equal(err, nil)
head, err = jb.Pop()
assert.Equal(head.SequenceNumber, uint16(0))
assert.Equal(err, nil)
})

t.Run("Appends packets and begins playout", func(*testing.T) {
jb := New()
Expand Down Expand Up @@ -64,7 +83,7 @@ func TestJitterBuffer(t *testing.T) {
t.Run("Wraps playout correctly", func(*testing.T) {
jb := New()
for i := 0; i < 100; i++ {
sqnum := uint16((math.MaxUint16 - 32 + i) % math.MaxUint16)
sqnum := uint16(math.MaxUint16 - 32 + i)
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}})
}
assert.Equal(jb.packets.Length(), uint16(100))
Expand All @@ -76,7 +95,7 @@ func TestJitterBuffer(t *testing.T) {
for i := 0; i < 100; i++ {
head, err := jb.Pop()
if i < 99 {
assert.Equal(head.SequenceNumber, uint16((math.MaxUint16-31+i)%math.MaxUint16))
assert.Equal(head.SequenceNumber, uint16((math.MaxUint16 - 31 + i)))
assert.Equal(err, nil)
} else {
assert.Equal(head, (*rtp.Packet)(nil))
Expand All @@ -87,7 +106,7 @@ func TestJitterBuffer(t *testing.T) {
t.Run("Pops at timestamp correctly", func(*testing.T) {
jb := New()
for i := 0; i < 100; i++ {
sqnum := uint16((math.MaxUint16 - 32 + i) % math.MaxUint16)
sqnum := uint16((math.MaxUint16 - 32 + i))
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}})
}
assert.Equal(jb.packets.Length(), uint16(100))
Expand All @@ -113,7 +132,7 @@ func TestJitterBuffer(t *testing.T) {
assert.Equal(pkt.SequenceNumber, uint16(5002))
assert.Equal(err, nil)
for i := 0; i < 100; i++ {
sqnum := uint16((math.MaxUint16 - 32 + i) % math.MaxUint16)
sqnum := uint16((math.MaxUint16 - 32 + i))
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}})
}
pkt, err = jb.Peek(true)
Expand All @@ -124,7 +143,7 @@ func TestJitterBuffer(t *testing.T) {
t.Run("Pops at sequence with an invalid sequence number", func(*testing.T) {
jb := New()
for i := 0; i < 50; i++ {
sqnum := uint16((math.MaxUint16 - 32 + i) % math.MaxUint16)
sqnum := uint16((math.MaxUint16 - 32 + i))
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}})
}
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 1019, Timestamp: uint32(9000)}, Payload: []byte{0x02}})
Expand All @@ -139,7 +158,7 @@ func TestJitterBuffer(t *testing.T) {
t.Run("Pops at timestamp with multiple packets", func(*testing.T) {
jb := New()
for i := 0; i < 50; i++ {
sqnum := uint16((math.MaxUint16 - 32 + i) % math.MaxUint16)
sqnum := uint16((math.MaxUint16 - 32 + i))
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}})
}
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 1019, Timestamp: uint32(9000)}, Payload: []byte{0x02}})
Expand All @@ -161,7 +180,7 @@ func TestJitterBuffer(t *testing.T) {
t.Run("Peeks at timestamp with multiple packets", func(*testing.T) {
jb := New()
for i := 0; i < 50; i++ {
sqnum := uint16((math.MaxUint16 - 32 + i) % math.MaxUint16)
sqnum := uint16((math.MaxUint16 - 32 + i))
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}})
}
jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 1019, Timestamp: uint32(9000)}, Payload: []byte{0x02}})
Expand Down

0 comments on commit 1a036cf

Please sign in to comment.