add flexfec correction tests

2026-08-28 11:44:43 +00:00 · 2026-06-06 10:47:18 -07:00
parent 203d025358
commit ec606596fe
4 changed files with 328 additions and 41 deletions
@@ -25,11 +25,11 @@ import (
 	"github.com/stretchr/testify/require"
 )

-// TestFlexFECRecoveryThroughFactory exercises the full SFU receive path: the source and
-// repair buffers are created through the factory (as the SRTP layer would), associated
-// via SetFECPair, and a lost source packet is recovered from the FlexFEC repair stream
-// and forwarded out of the source buffer.
-func TestFlexFECRecoveryThroughFactory(t *testing.T) {
+// TestPublisherToSFUFlexFECRecoveryThroughFactory exercises the publisher -> SFU receive
+// path: the source and repair buffers are created through the factory (as the SRTP layer
+// would), associated via SetFECPair, and a lost publisher packet is recovered from the
+// FlexFEC repair stream and forwarded out of the source buffer.
+func TestPublisherToSFUFlexFECRecoveryThroughFactory(t *testing.T) {
 	const (
 		mediaSSRC = uint32(0xAABBCCDD)
 		fecSSRC   = uint32(0x11223344)
@@ -38,28 +38,150 @@ func TestFlexFECRecoveryThroughFactory(t *testing.T) {
 		dropIdx   = 6
 	)

-	factory := NewFactoryOfBufferFactory(InitPacketBufferSizeVideo, InitPacketBufferSizeAudio).CreateBufferFactory()
+	mediaBuf, fecBuf := newFECBufferPair(t, mediaSSRC, fecSSRC)
+	defer mediaBuf.Close()
+	defer fecBuf.Close()

-	mediaBuf, ok := factory.GetOrNew(packetio.RTPBufferPacket, mediaSSRC).(*Buffer)
-	require.True(t, ok)
-	mediaBuf.codecType = webrtc.RTPCodecTypeAudio
-	require.NoError(t, mediaBuf.Bind(
-		webrtc.RTPParameters{Codecs: []webrtc.RTPCodecParameters{opusCodec}},
-		opusCodec.RTPCodecCapability,
-		0,
-	))
+	media := makeFECTestMediaPackets(mediaSSRC, baseSeq, numMedia)
+	fecPackets := makeFECTestRepairPackets(t, media, fecSSRC)
+	got := collectExtPackets(mediaBuf)
+
+	writeMediaExcept(t, mediaBuf, media, dropIdx)
+	writeRepairPackets(t, fecBuf, fecPackets)
+
+	requireRecoveredPacket(t, got, media[dropIdx])
+}
+
+// TestPublisherToSFUFlexFECRecoveryReplaysBufferedRepairPackets covers the ordering that
+// can happen on ingress: a FlexFEC repair packet reaches the buffer before the SDP-derived
+// FEC-FR association has been applied. The repair buffer must replay its queued packet
+// into the source decoder once SetFECPair wires the streams together.
+func TestPublisherToSFUFlexFECRecoveryReplaysBufferedRepairPackets(t *testing.T) {
+	const (
+		mediaSSRC = uint32(0x01020304)
+		fecSSRC   = uint32(0x05060708)
+		baseSeq   = uint16(9000)
+		numMedia  = 10
+		dropIdx   = 3
+	)
+
+	factory := NewFactoryOfBufferFactory(InitPacketBufferSizeVideo, InitPacketBufferSizeAudio).CreateBufferFactory()

 	fecBuf, ok := factory.GetOrNew(packetio.RTPBufferPacket, fecSSRC).(*Buffer)
 	require.True(t, ok)
+	defer fecBuf.Close()
+
+	media := makeFECTestMediaPackets(mediaSSRC, baseSeq, numMedia)
+	fecPackets := makeFECTestRepairPackets(t, media, fecSSRC)
+
+	// Repair arrives before the source buffer and FEC-FR association are known. Because
+	// fecBuf is not associated yet, Buffer queues the packet in pPackets.
+	writeRepairPackets(t, fecBuf, fecPackets)
+
+	mediaBuf, ok := factory.GetOrNew(packetio.RTPBufferPacket, mediaSSRC).(*Buffer)
+	require.True(t, ok)
+	defer mediaBuf.Close()
+	bindVideoBuffer(t, mediaBuf)
+
+	got := collectExtPackets(mediaBuf)

-	// associate the repair stream with the source stream (as a=ssrc-group:FEC-FR would)
 	factory.SetFECPair(fecSSRC, mediaSSRC)
 	require.NotNil(t, mediaBuf.fecDecoder)
 	require.Equal(t, mediaBuf, fecBuf.primaryBufferForFEC)

-	// build media packets and the protecting FlexFEC packet
+	writeMediaExcept(t, mediaBuf, media, dropIdx)
+
+	requireRecoveredPacket(t, got, media[dropIdx])
+}
+
+func TestPublisherToSFUFlexFECPairRememberedBeforeBuffers(t *testing.T) {
+	for _, tc := range []struct {
+		name              string
+		createSourceFirst bool
+	}{
+		{name: "source_first", createSourceFirst: true},
+		{name: "repair_first", createSourceFirst: false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			const (
+				mediaSSRC = uint32(0x0A0B0C0D)
+				fecSSRC   = uint32(0x01010101)
+				baseSeq   = uint16(12000)
+				numMedia  = 10
+				dropIdx   = 5
+			)
+
+			factory := NewFactoryOfBufferFactory(InitPacketBufferSizeVideo, InitPacketBufferSizeAudio).CreateBufferFactory()
+			factory.SetFECPair(fecSSRC, mediaSSRC)
+
+			var mediaBuf *Buffer
+			var fecBuf *Buffer
+			if tc.createSourceFirst {
+				mediaBuf = getRTPBuffer(t, factory, mediaSSRC)
+				bindVideoBuffer(t, mediaBuf)
+				fecBuf = getRTPBuffer(t, factory, fecSSRC)
+			} else {
+				fecBuf = getRTPBuffer(t, factory, fecSSRC)
+				mediaBuf = getRTPBuffer(t, factory, mediaSSRC)
+				bindVideoBuffer(t, mediaBuf)
+			}
+			defer mediaBuf.Close()
+			defer fecBuf.Close()
+
+			require.NotNil(t, mediaBuf.fecDecoder)
+			require.Equal(t, mediaBuf, fecBuf.primaryBufferForFEC)
+
+			media := makeFECTestMediaPackets(mediaSSRC, baseSeq, numMedia)
+			fecPackets := makeFECTestRepairPackets(t, media, fecSSRC)
+			got := collectExtPackets(mediaBuf)
+
+			writeMediaExcept(t, mediaBuf, media, dropIdx)
+			writeRepairPackets(t, fecBuf, fecPackets)
+
+			requireRecoveredPacket(t, got, media[dropIdx])
+		})
+	}
+}
+
+func newFECBufferPair(t *testing.T, mediaSSRC, fecSSRC uint32) (*Buffer, *Buffer) {
+	t.Helper()
+
+	factory := NewFactoryOfBufferFactory(InitPacketBufferSizeVideo, InitPacketBufferSizeAudio).CreateBufferFactory()
+
+	mediaBuf := getRTPBuffer(t, factory, mediaSSRC)
+	bindVideoBuffer(t, mediaBuf)
+
+	fecBuf := getRTPBuffer(t, factory, fecSSRC)
+
+	factory.SetFECPair(fecSSRC, mediaSSRC)
+	require.NotNil(t, mediaBuf.fecDecoder)
+	require.Equal(t, mediaBuf, fecBuf.primaryBufferForFEC)
+
+	return mediaBuf, fecBuf
+}
+
+func getRTPBuffer(t *testing.T, factory *Factory, ssrc uint32) *Buffer {
+	t.Helper()
+
+	b, ok := factory.GetOrNew(packetio.RTPBufferPacket, ssrc).(*Buffer)
+	require.True(t, ok)
+	return b
+}
+
+func bindVideoBuffer(t *testing.T, b *Buffer) {
+	t.Helper()
+
+	b.codecType = webrtc.RTPCodecTypeVideo
+	require.NoError(t, b.Bind(
+		webrtc.RTPParameters{Codecs: []webrtc.RTPCodecParameters{vp8Codec}},
+		vp8Codec.RTPCodecCapability,
+		0,
+	))
+}
+
+func makeFECTestMediaPackets(mediaSSRC uint32, baseSeq uint16, numMedia int) []rtp.Packet {
 	media := make([]rtp.Packet, numMedia)
-	for i := 0; i < numMedia; i++ {
+	for i := range media {
 		payload := make([]byte, 16)
 		for j := range payload {
 			payload[j] = byte((i*11 + j*5 + 1) & 0xff)
@@ -67,7 +189,7 @@ func TestFlexFECRecoveryThroughFactory(t *testing.T) {
 		media[i] = rtp.Packet{
 			Header: rtp.Header{
 				Version:        2,
-				PayloadType:    uint8(opusCodec.PayloadType),
+				PayloadType:    uint8(vp8Codec.PayloadType),
 				SequenceNumber: baseSeq + uint16(i),
 				Timestamp:      uint32(8000 + i*960),
 				SSRC:           mediaSSRC,
@@ -75,19 +197,26 @@ func TestFlexFECRecoveryThroughFactory(t *testing.T) {
 			Payload: payload,
 		}
 	}
+	return media
+}
+
+func makeFECTestRepairPackets(t *testing.T, media []rtp.Packet, fecSSRC uint32) []rtp.Packet {
+	t.Helper()

 	encoder := flexfec.NewFlexEncoder03(uint8(49), fecSSRC)
-	mediaForFec := make([]rtp.Packet, numMedia)
+	mediaForFec := make([]rtp.Packet, len(media))
 	copy(mediaForFec, media)
 	fecPackets := encoder.EncodeFec(mediaForFec, 1)
 	require.NotEmpty(t, fecPackets)
+	return fecPackets
+}

-	// collect forwarded packets out of the source buffer
-	got := make(chan *ExtPacket, numMedia*2)
+func collectExtPackets(b *Buffer) <-chan *ExtPacket {
+	got := make(chan *ExtPacket, 32)
 	go func() {
 		var buf [1500]byte
 		for {
-			ep, err := mediaBuf.ReadExtended(buf[:])
+			ep, err := b.ReadExtended(buf[:])
 			if err != nil {
 				return
 			}
@@ -97,44 +226,51 @@ func TestFlexFECRecoveryThroughFactory(t *testing.T) {
 			}
 		}
 	}()
+	return got
+}

-	// deliver all source packets except the dropped one
-	for i := 0; i < numMedia; i++ {
+func writeMediaExcept(t *testing.T, b *Buffer, media []rtp.Packet, dropIdx int) {
+	t.Helper()
+
+	for i := range media {
 		if i == dropIdx {
 			continue
 		}
 		raw, err := media[i].Marshal()
 		require.NoError(t, err)
-		_, err = mediaBuf.Write(raw)
+		_, err = b.Write(raw)
 		require.NoError(t, err)
 	}
+}
+
+func writeRepairPackets(t *testing.T, b *Buffer, fecPackets []rtp.Packet) {
+	t.Helper()

-	// deliver the repair packet to the FEC buffer -> triggers recovery + injection
 	for _, fp := range fecPackets {
 		raw, err := fp.Marshal()
 		require.NoError(t, err)
-		_, err = fecBuf.Write(raw)
+		_, err = b.Write(raw)
 		require.NoError(t, err)
 	}
+}
+
+func requireRecoveredPacket(t *testing.T, got <-chan *ExtPacket, want rtp.Packet) {
+	t.Helper()

-	// the dropped source packet must be recovered and forwarded
-	recoveredSeq := baseSeq + uint16(dropIdx)
 	deadline := time.After(2 * time.Second)
 	seen := map[uint16]*ExtPacket{}
 	for {
 		select {
 		case ep := <-got:
 			seen[ep.Packet.SequenceNumber] = ep
-			if rec, found := seen[recoveredSeq]; found {
-				require.Equal(t, mediaSSRC, rec.Packet.SSRC)
-				require.Equal(t, media[dropIdx].Payload, rec.Packet.Payload)
-				require.Equal(t, media[dropIdx].Timestamp, rec.Packet.Timestamp)
-				_ = mediaBuf.Close()
+			if rec, found := seen[want.SequenceNumber]; found {
+				require.Equal(t, want.SSRC, rec.Packet.SSRC)
+				require.Equal(t, want.Payload, rec.Packet.Payload)
+				require.Equal(t, want.Timestamp, rec.Packet.Timestamp)
 				return
 			}
 		case <-deadline:
-			_ = mediaBuf.Close()
-			t.Fatalf("recovered packet seq %d not forwarded; saw %v", recoveredSeq, seqKeys(seen))
+			t.Fatalf("recovered packet seq %d not forwarded; saw %v", want.SequenceNumber, seqKeys(seen))
 		}
 	}
 }