From 6141567aeff69b338d4c75090e4807fd7240ffc9 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Fri, 3 Dec 2021 15:37:49 +0530 Subject: [PATCH] Unit tests for VP8 pack/unpack in packetMeta. (#228) Deleting unused code. --- pkg/sfu/rtpmunger_test.go | 122 +++++++++++++++++++------------------- pkg/sfu/sequencer.go | 28 ++++----- pkg/sfu/sequencer_test.go | 112 ++++++++++++++++++++++++++++------ pkg/sfu/testutils/data.go | 36 +++++------ 4 files changed, 182 insertions(+), 116 deletions(-) diff --git a/pkg/sfu/rtpmunger_test.go b/pkg/sfu/rtpmunger_test.go index a920108e1..9c0366006 100644 --- a/pkg/sfu/rtpmunger_test.go +++ b/pkg/sfu/rtpmunger_test.go @@ -14,8 +14,8 @@ func TestSetLastSnTs(t *testing.T) { params := &testutils.TestExtPacketParams{ SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, err := testutils.GetTestExtPacket(params) require.NoError(t, err) @@ -30,8 +30,8 @@ func TestSetLastSnTs(t *testing.T) { params = &testutils.TestExtPacketParams{ SequenceNumber: 0, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, err = testutils.GetTestExtPacket(params) require.NoError(t, err) @@ -50,16 +50,16 @@ func TestUpdateSnTsOffsets(t *testing.T) { params := &testutils.TestExtPacketParams{ SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) params = &testutils.TestExtPacketParams{ SequenceNumber: 33333, - Timestamp: 0xabcdef, - SSRC: 0x87654321, + Timestamp: 0xabcdef, + SSRC: 0x87654321, } extPkt, _ = testutils.GetTestExtPacket(params) r.UpdateSnTsOffsets(extPkt, 1, 1) @@ -75,8 +75,8 @@ func TestPacketDropped(t *testing.T) { params := &testutils.TestExtPacketParams{ SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) @@ -89,8 +89,8 @@ func TestPacketDropped(t *testing.T) { // drop a non-head packet, should cause no change in internals params = &testutils.TestExtPacketParams{ SequenceNumber: 33333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ = testutils.GetTestExtPacket(params) r.PacketDropped(extPkt) @@ -100,10 +100,10 @@ func TestPacketDropped(t *testing.T) { // drop a head packet and check offset increases params = &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 44444, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ = testutils.GetTestExtPacket(params) r.PacketDropped(extPkt) @@ -117,8 +117,8 @@ func TestOutOfOrderSequenceNumber(t *testing.T) { params := &testutils.TestExtPacketParams{ SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) @@ -126,15 +126,15 @@ func TestOutOfOrderSequenceNumber(t *testing.T) { // out-of-order sequence number not in the missing sequence number cache params = &testutils.TestExtPacketParams{ SequenceNumber: 23332, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ = testutils.GetTestExtPacket(params) tpExpected := TranslationParamsRTP{ snOrdering: SequenceNumberOrderingOutOfOrder, } - + tp, err := r.UpdateAndGetSnTs(extPkt) require.Error(t, err) require.ErrorIs(t, err, ErrOutOfOrderSequenceNumberCacheMiss) @@ -144,9 +144,9 @@ func TestOutOfOrderSequenceNumber(t *testing.T) { r.missingSNs[23332] = 10 tpExpected = TranslationParamsRTP{ - snOrdering: SequenceNumberOrderingOutOfOrder, + snOrdering: SequenceNumberOrderingOutOfOrder, sequenceNumber: 23322, - timestamp: 0xabcdef, + timestamp: 0xabcdef, } tp, err = r.UpdateAndGetSnTs(extPkt) @@ -158,10 +158,10 @@ func TestDuplicateSequenceNumber(t *testing.T) { r := NewRTPMunger() params := &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) @@ -173,7 +173,7 @@ func TestDuplicateSequenceNumber(t *testing.T) { tpExpected := TranslationParamsRTP{ snOrdering: SequenceNumberOrderingDuplicate, } - + tp, err := r.UpdateAndGetSnTs(extPkt) require.Error(t, err) require.ErrorIs(t, err, ErrDuplicatePacket) @@ -184,10 +184,10 @@ func TestPaddingOnlyPacket(t *testing.T) { r := NewRTPMunger() params := &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) @@ -196,7 +196,7 @@ func TestPaddingOnlyPacket(t *testing.T) { tpExpected := TranslationParamsRTP{ snOrdering: SequenceNumberOrderingContiguous, } - + tp, err := r.UpdateAndGetSnTs(extPkt) require.Error(t, err) require.ErrorIs(t, err, ErrPaddingOnlyPacket) @@ -207,17 +207,17 @@ func TestPaddingOnlyPacket(t *testing.T) { // padding only packet with a gap should not report an error params = &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 23335, - Timestamp: 0xabcdef, - SSRC: 0x12345678, + Timestamp: 0xabcdef, + SSRC: 0x12345678, } extPkt, _ = testutils.GetTestExtPacket(params) tpExpected = TranslationParamsRTP{ - snOrdering: SequenceNumberOrderingGap, + snOrdering: SequenceNumberOrderingGap, sequenceNumber: 23334, - timestamp: 0xabcdef, + timestamp: 0xabcdef, } tp, err = r.UpdateAndGetSnTs(extPkt) @@ -232,11 +232,11 @@ func TestGapInSequenceNumber(t *testing.T) { r := NewRTPMunger() params := &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 65533, - Timestamp: 0xabcdef, - SSRC: 0x12345678, - PayloadSize: 33, + Timestamp: 0xabcdef, + SSRC: 0x12345678, + PayloadSize: 33, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) @@ -246,20 +246,20 @@ func TestGapInSequenceNumber(t *testing.T) { // three lost packets params = &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 1, - Timestamp: 0xabcdef, - SSRC: 0x12345678, - PayloadSize: 33, + Timestamp: 0xabcdef, + SSRC: 0x12345678, + PayloadSize: 33, } extPkt, _ = testutils.GetTestExtPacket(params) tpExpected := TranslationParamsRTP{ - snOrdering: SequenceNumberOrderingGap, + snOrdering: SequenceNumberOrderingGap, sequenceNumber: 1, - timestamp: 0xabcdef, + timestamp: 0xabcdef, } - + tp, err := r.UpdateAndGetSnTs(extPkt) require.NoError(t, err) require.True(t, reflect.DeepEqual(*tp, tpExpected)) @@ -295,11 +295,11 @@ func TestUpdateAndGetPaddingSnTs(t *testing.T) { r := NewRTPMunger() params := &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, - PayloadSize: 20, + Timestamp: 0xabcdef, + SSRC: 0x12345678, + PayloadSize: 20, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) @@ -318,7 +318,7 @@ func TestUpdateAndGetPaddingSnTs(t *testing.T) { for i := 0; i < numPadding; i++ { sntsExpected[i] = SnTs{ sequenceNumber: 23333 + uint16(i) + 1, - timestamp: 0xabcdef + (uint32(i) * clockRate) / frameRate, + timestamp: 0xabcdef + (uint32(i)*clockRate)/frameRate, } } snts, err := r.UpdateAndGetPaddingSnTs(numPadding, clockRate, frameRate, true) @@ -329,7 +329,7 @@ func TestUpdateAndGetPaddingSnTs(t *testing.T) { for i := 0; i < numPadding; i++ { sntsExpected[i] = SnTs{ sequenceNumber: 23343 + uint16(i) + 1, - timestamp: 0xabcdef + (uint32(i + 1) * clockRate) / frameRate, + timestamp: 0xabcdef + (uint32(i+1)*clockRate)/frameRate, } } snts, err = r.UpdateAndGetPaddingSnTs(numPadding, clockRate, frameRate, false) @@ -341,11 +341,11 @@ func TestIsOnFrameBoundary(t *testing.T) { r := NewRTPMunger() params := &testutils.TestExtPacketParams{ - IsHead: true, + IsHead: true, SequenceNumber: 23333, - Timestamp: 0xabcdef, - SSRC: 0x12345678, - PayloadSize: 20, + Timestamp: 0xabcdef, + SSRC: 0x12345678, + PayloadSize: 20, } extPkt, _ := testutils.GetTestExtPacket(params) r.SetLastSnTs(extPkt) @@ -357,12 +357,12 @@ func TestIsOnFrameBoundary(t *testing.T) { // packet with RTP marker params = &testutils.TestExtPacketParams{ - IsHead: true, - SetMarker: true, + IsHead: true, + SetMarker: true, SequenceNumber: 23334, - Timestamp: 0xabcdef, - SSRC: 0x12345678, - PayloadSize: 20, + Timestamp: 0xabcdef, + SSRC: 0x12345678, + PayloadSize: 20, } extPkt, _ = testutils.GetTestExtPacket(params) diff --git a/pkg/sfu/sequencer.go b/pkg/sfu/sequencer.go index 98eafe1c1..976617f27 100644 --- a/pkg/sfu/sequencer.go +++ b/pkg/sfu/sequencer.go @@ -36,26 +36,18 @@ type packetMeta struct { misc uint64 } -func (p *packetMeta) setVP8PayloadMeta(tlz0Idx uint8, picID uint16) { - p.misc = uint64(tlz0Idx)<<16 | uint64(picID) -} - -func (p *packetMeta) getVP8PayloadMeta() (uint8, uint16) { - return uint8(p.misc >> 16), uint16(p.misc) -} - func (p *packetMeta) packVP8(vp8 *buffer.VP8) { p.misc = uint64(vp8.FirstByte)<<56 | - uint64(vp8.PictureIDPresent)<<55 | - uint64(vp8.TL0PICIDXPresent)<<54 | - uint64(vp8.TIDPresent)<<53 | - uint64(vp8.KEYIDXPresent)<<52 | - uint64(vp8.PictureID)<<32 | - uint64(vp8.TL0PICIDX)<<24 | - uint64(vp8.TID)<<22 | - uint64(vp8.Y)<<21 | - uint64(vp8.KEYIDX)<<16 | - uint64(vp8.HeaderSize)<<8 + uint64(vp8.PictureIDPresent&0x1)<<55 | + uint64(vp8.TL0PICIDXPresent&0x1)<<54 | + uint64(vp8.TIDPresent&0x1)<<53 | + uint64(vp8.KEYIDXPresent&0x1)<<52 | + uint64(vp8.PictureID&0xFFFF)<<32 | + uint64(vp8.TL0PICIDX&0xFF)<<24 | + uint64(vp8.TID&0x3)<<22 | + uint64(vp8.Y&0x1)<<21 | + uint64(vp8.KEYIDX&0x1F)<<16 | + uint64(vp8.HeaderSize&0xFF)<<8 } func (p *packetMeta) unpackVP8() *buffer.VP8 { diff --git a/pkg/sfu/sequencer_test.go b/pkg/sfu/sequencer_test.go index ab97d9d3b..7c4194620 100644 --- a/pkg/sfu/sequencer_test.go +++ b/pkg/sfu/sequencer_test.go @@ -5,7 +5,9 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" + "github.com/livekit/livekit-server/pkg/sfu/buffer" + + "github.com/stretchr/testify/require" ) func Test_sequencer(t *testing.T) { @@ -19,35 +21,27 @@ func Test_sequencer(t *testing.T) { time.Sleep(60 * time.Millisecond) req := []uint16{57, 58, 62, 63, 513, 514, 515, 516, 517} res := seq.getSeqNoPairs(req) - assert.Equal(t, len(req), len(res)) + require.Equal(t, len(req), len(res)) for i, val := range res { - assert.Equal(t, val.targetSeqNo, req[i]) - assert.Equal(t, val.sourceSeqNo, req[i]-off) - assert.Equal(t, val.layer, uint8(2)) + require.Equal(t, val.targetSeqNo, req[i]) + require.Equal(t, val.sourceSeqNo, req[i]-off) + require.Equal(t, val.layer, uint8(2)) } res = seq.getSeqNoPairs(req) - assert.Equal(t, 0, len(res)) + require.Equal(t, 0, len(res)) time.Sleep(150 * time.Millisecond) res = seq.getSeqNoPairs(req) - assert.Equal(t, len(req), len(res)) + require.Equal(t, len(req), len(res)) for i, val := range res { - assert.Equal(t, val.targetSeqNo, req[i]) - assert.Equal(t, val.sourceSeqNo, req[i]-off) - assert.Equal(t, val.layer, uint8(2)) + require.Equal(t, val.targetSeqNo, req[i]) + require.Equal(t, val.sourceSeqNo, req[i]-off) + require.Equal(t, val.layer, uint8(2)) } s := seq.push(521, 521+off, 123, 1, true) - var ( - tlzIdx = uint8(15) - picID = uint16(16) - ) - s.setVP8PayloadMeta(tlzIdx, picID) s.sourceSeqNo = 12 m := seq.getSeqNoPairs([]uint16{521 + off}) - assert.Equal(t, 1, len(m)) - tlz0, pID := m[0].getVP8PayloadMeta() - assert.Equal(t, tlzIdx, tlz0) - assert.Equal(t, picID, pID) + require.Equal(t, 1, len(m)) } func Test_sequencer_getNACKSeqNo(t *testing.T) { @@ -97,3 +91,83 @@ func Test_sequencer_getNACKSeqNo(t *testing.T) { }) } } + +func Test_packetMeta_VP8(t *testing.T) { + p := &packetMeta{} + + vp8 := &buffer.VP8{ + FirstByte: 25, + PictureIDPresent: 1, + PictureID: 55467, + MBit: true, + TL0PICIDXPresent: 1, + TL0PICIDX: 233, + TIDPresent: 1, + TID: 13, + Y: 1, + KEYIDXPresent: 1, + KEYIDX: 23, + HeaderSize: 6, + IsKeyFrame: true, + } + + p.packVP8(vp8) + + // booleans are not packed, so they will be `false` in unpacked. + // Also TID is only two bits, so it should be modulo 3. + expectedVP8 := &buffer.VP8{ + FirstByte: 25, + PictureIDPresent: 1, + PictureID: 55467, + MBit: false, + TL0PICIDXPresent: 1, + TL0PICIDX: 233, + TIDPresent: 1, + TID: 13 % 3, + Y: 1, + KEYIDXPresent: 1, + KEYIDX: 23, + HeaderSize: 6, + IsKeyFrame: false, + } + unpackedVP8 := p.unpackVP8() + require.True(t, reflect.DeepEqual(expectedVP8, unpackedVP8)) + + // short picture id and no TL0PICIDX + vp8 = &buffer.VP8{ + FirstByte: 25, + PictureIDPresent: 1, + PictureID: 63, + MBit: false, + TL0PICIDXPresent: 0, + TL0PICIDX: 233, + TIDPresent: 1, + TID: 2, + Y: 1, + KEYIDXPresent: 0, + KEYIDX: 23, + HeaderSize: 23, + IsKeyFrame: true, + } + + p.packVP8(vp8) + + expectedVP8 = &buffer.VP8{ + FirstByte: 25, + PictureIDPresent: 1, + PictureID: 63, + MBit: false, + TL0PICIDXPresent: 0, + TL0PICIDX: 233, + TIDPresent: 1, + TID: 2, + Y: 1, + KEYIDXPresent: 0, + KEYIDX: 23, + HeaderSize: 23, + IsKeyFrame: false, + } + unpackedVP8 = p.unpackVP8() + require.True(t, reflect.DeepEqual(expectedVP8, unpackedVP8)) + +} diff --git a/pkg/sfu/testutils/data.go b/pkg/sfu/testutils/data.go index f4e703b20..2ff520cc8 100644 --- a/pkg/sfu/testutils/data.go +++ b/pkg/sfu/testutils/data.go @@ -9,17 +9,17 @@ import ( //----------------------------------------------------------- type TestExtPacketParams struct { - SetMarker bool - SetPadding bool - IsHead bool - IsKeyFrame bool - PayloadType uint8 + SetMarker bool + SetPadding bool + IsHead bool + IsKeyFrame bool + PayloadType uint8 SequenceNumber uint16 - Timestamp uint32 - SSRC uint32 - PayloadSize int - PaddingSize int - ArrivalTime int64 + Timestamp uint32 + SSRC uint32 + PayloadSize int + PaddingSize int + ArrivalTime int64 } //----------------------------------------------------------- @@ -27,13 +27,13 @@ type TestExtPacketParams struct { func GetTestExtPacket(params *TestExtPacketParams) (*buffer.ExtPacket, error) { packet := rtp.Packet{ Header: rtp.Header{ - Version: 2, - Padding: params.SetPadding, - Marker: params.SetMarker, - PayloadType: params.PayloadType, + Version: 2, + Padding: params.SetPadding, + Marker: params.SetMarker, + PayloadType: params.PayloadType, SequenceNumber: params.SequenceNumber, - Timestamp: params.Timestamp, - SSRC: params.SSRC, + Timestamp: params.Timestamp, + SSRC: params.SSRC, }, Payload: make([]byte, params.PayloadSize), // LK-TODO need a newer version of pion/rtp PaddingSize: params.PaddingSize, @@ -42,13 +42,13 @@ func GetTestExtPacket(params *TestExtPacketParams) (*buffer.ExtPacket, error) { raw, err := packet.Marshal() if err != nil { return nil, err - } + } ep := &buffer.ExtPacket{ Head: params.IsHead, Arrival: params.ArrivalTime, Packet: packet, - KeyFrame: params.IsKeyFrame, + KeyFrame: params.IsKeyFrame, RawPacket: raw, }