Repository: pion/interceptor (go)

Issue:
Jitter Buffer error
<!--
  Come join our community on Slack https://pion.ly/slack for quicker support and more resources

  FAQ of common issues and questions https://github.com/pion/webrtc/wiki/FAQ

  Free book that covers general WebRTC https://webrtcforthecurious.com/

  Please answer these questions before submitting your issue. Thanks!
-->

### Your environment.
  - Version: *v0.1.29 and main branch*

### What did you do?
I push and pop packets to the Jitter Buffer.

### What did you expect?
If I added N packets then I can get N packets.

### What happened?
If I pushed packet with a SequenceNumber equal MaxUint16, then I won't be able to pop it. Packets with this SequenceNumber are possible.

I think the problem is in the following [code](https://github.com/pion/interceptor/blob/142f17f90b527434b62ce2fd92676e6e262cd807/pkg/jitterbuffer/jitter_buffer.go#L218) and similar. 

@thatsnotright Have a look, please.

Pull request:
fix: Some bad math with the JitterBuffer sequence tracking
#### Description
I'm not sure what I was thinking, uint16 should wrap itself correctly.

#### Reference issue
Fixes #253 


Patch:
diff --git a/pkg/jitterbuffer/jitter_buffer.go b/pkg/jitterbuffer/jitter_buffer.go
index 976ea76..863a0a8 100644
--- a/pkg/jitterbuffer/jitter_buffer.go
+++ b/pkg/jitterbuffer/jitter_buffer.go
@@ -7,7 +7,6 @@ package jitterbuffer
 
 import (
 	"errors"
-	"math"
 	"sync"
 
 	"github.com/pion/rtp"
@@ -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
@@ -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
 }
@@ -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
 }


Test patch:
diff --git a/pkg/jitterbuffer/jitter_buffer_test.go b/pkg/jitterbuffer/jitter_buffer_test.go
index 205e610..f163a8f 100644
--- a/pkg/jitterbuffer/jitter_buffer_test.go
+++ b/pkg/jitterbuffer/jitter_buffer_test.go
@@ -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()
@@ -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))
@@ -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))
@@ -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))
@@ -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)
@@ -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}})
@@ -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}})
@@ -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}})