Skip to content

Commit

Permalink
Fix nack buffer RTX issue
Browse files Browse the repository at this point in the history
  • Loading branch information
kcaffrey committed Dec 16, 2024
1 parent 9251977 commit 645dd64
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 9 deletions.
18 changes: 9 additions & 9 deletions internal/rtpbuffer/rtpbuffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ const (

// RTPBuffer stores RTP packets and allows custom logic around the lifetime of them via the PacketFactory
type RTPBuffer struct {
packets []*RetainablePacket
size uint16
lastAdded uint16
started bool
packets []*RetainablePacket
size uint16
highestAdded uint16
started bool
}

// NewRTPBuffer constructs a new RTPBuffer
Expand Down Expand Up @@ -50,23 +50,24 @@ func (r *RTPBuffer) Add(packet *RetainablePacket) {
seq := packet.sequenceNumber
if !r.started {
r.packets[seq%r.size] = packet
r.lastAdded = seq
r.highestAdded = seq
r.started = true
return
}

diff := seq - r.lastAdded
diff := seq - r.highestAdded
if diff == 0 {
return
} else if diff < Uint16SizeHalf {
for i := r.lastAdded + 1; i != seq; i++ {
for i := r.highestAdded + 1; i != seq; i++ {
idx := i % r.size
prevPacket := r.packets[idx]
if prevPacket != nil {
prevPacket.Release()
}
r.packets[idx] = nil
}
r.highestAdded = seq
}

idx := seq % r.size
Expand All @@ -75,12 +76,11 @@ func (r *RTPBuffer) Add(packet *RetainablePacket) {
prevPacket.Release()
}
r.packets[idx] = packet
r.lastAdded = seq
}

// Get returns the RetainablePacket for the requested sequence number
func (r *RTPBuffer) Get(seq uint16) *RetainablePacket {
diff := r.lastAdded - seq
diff := r.highestAdded - seq
if diff >= Uint16SizeHalf {
return nil
}
Expand Down
4 changes: 4 additions & 0 deletions internal/rtpbuffer/rtpbuffer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ func TestRTPBuffer_WithRTX(t *testing.T) {
assertGet(10)
assertNOTGet(1, 2, 9)

// A late packet coming in (such as due to RTX) shouldn't invalidate other packets.
add(9)
assertGet(3, 4, 5, 6, 7, 8, 9, 10)

add(22)
assertGet(22)
assertNOTGet(3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21)
Expand Down

0 comments on commit 645dd64

Please sign in to comment.