Fix svc freeze caused by key frame id wrap around (#2302)

* Fix svc freeze caused by key frame id wrap around

* rename

pkg/sfu/videolayerselector/dependencydescriptor.go

@@ -39,12 +39,14 @@ type DependencyDescriptor struct {
 
 	decodeTargetsLock sync.RWMutex
 	decodeTargets     []*DecodeTarget
+	fnWrapper         FrameNumberWrapper
 }
 
 func NewDependencyDescriptor(logger logger.Logger) *DependencyDescriptor {
 	return &DependencyDescriptor{
 		Base:      NewBase(logger),
 		decisions: NewSelectorDecisionCache(256, 80),
+		fnWrapper: FrameNumberWrapper{logger: logger},
 	}
 }
 
@@ -52,6 +54,7 @@ func NewDependencyDescriptorFromNull(vls VideoLayerSelector) *DependencyDescript
 	return &DependencyDescriptor{
 		Base:      vls.(*Null).Base,
 		decisions: NewSelectorDecisionCache(256, 80),
+		fnWrapper: FrameNumberWrapper{logger: vls.(*Null).logger},
 	}
 }
 
@@ -291,13 +294,26 @@ func (d *DependencyDescriptor) Select(extPkt *buffer.ExtPacket, _layer int32) (r
 		Descriptor: dd,
 		Structure:  d.structure,
 	}
+
+	unWrapFn := uint16(d.fnWrapper.UpdateAndGet(extFrameNum, ddwdt.StructureUpdated))
+	var ddClone *dede.DependencyDescriptor
+	if unWrapFn != dd.FrameNumber {
+		clone := *dd
+		ddClone = &clone
+		ddClone.FrameNumber = unWrapFn
+		ddExtension.Descriptor = ddClone
+	}
+
 	if dd.AttachedStructure == nil {
 		if d.activeDecodeTargetsBitmask != nil {
-			// clone and override activebitmask
-			// DD-TODO: if the packet that contains the bitmask is acknowledged by RR, then we don't need it until it changed.
-			ddClone := *ddExtension.Descriptor
+			if ddClone == nil {
+				// clone and override activebitmask
+				// DD-TODO: if the packet that contains the bitmask is acknowledged by RR, then we don't need it until it changed.
+				clone := *dd
+				ddClone = &clone
+				ddExtension.Descriptor = ddClone
+			}
 			ddClone.ActiveDecodeTargetsBitmask = d.activeDecodeTargetsBitmask
-			ddExtension.Descriptor = &ddClone
 			// d.logger.Debugw("set active decode targets bitmask", "activeDecodeTargetsBitmask", d.activeDecodeTargetsBitmask)
 		}
 	}

pkg/sfu/videolayerselector/framenumberwrapper.go

@@ -0,0 +1,41 @@
+package videolayerselector
+
+import "github.com/livekit/protocol/logger"
+
+type FrameNumberWrapper struct {
+	offset uint64
+	last   uint64
+	inited bool
+	logger logger.Logger
+}
+
+// UpdateAndGet returns the wrapped frame number from the given frame number, and updates the offset to
+// make sure the returned frame number is always inorder. Should only updateOffset if the new frame is a keyframe
+// because frame dependencies uses on the frame number diff so frames inside a GOP should have the same offset.
+func (f *FrameNumberWrapper) UpdateAndGet(new uint64, updateOffset bool) uint64 {
+	if !f.inited {
+		f.last = new
+		f.inited = true
+		return new
+	}
+
+	if new <= f.last {
+		return new + f.offset
+	}
+
+	if updateOffset {
+		new16 := uint16(new + f.offset)
+		last16 := uint16(f.last + f.offset)
+		// if new frame number wraps around and is considered as earlier by client, increase offset to make it later
+		if diff := new16 - last16; diff > 0x8000 || (diff == 0x8000 && new16 <= last16) {
+			// increase offset by 6000, nearly 10 seconds for 30fps video with 3 spatial layers
+			prevOffset := f.offset
+			f.offset += uint64(65535 - diff + 6000)
+
+			// TODO: remove this
+			f.logger.Infow("wrap around frame number seen, update offset", "new", new, "last", f.last, "offset", f.offset, "prevOffset", prevOffset, "lastWrapFn", last16, "newWrapFn", new16)
+		}
+	}
+	f.last = new
+	return new + f.offset
+}

pkg/sfu/videolayerselector/framenumberwrapper_test.go

@@ -0,0 +1,82 @@
+package videolayerselector
+
+import (
+	"testing"
+
+	"math/rand"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/livekit/livekit-server/pkg/sfu/utils"
+	"github.com/livekit/protocol/logger"
+)
+
+func TestFrameNumberWrapper(t *testing.T) {
+
+	logger.InitFromConfig(&logger.Config{Level: "debug"}, t.Name())
+
+	fnWrap := &FrameNumberWrapper{logger: logger.GetLogger()}
+
+	fnWrapAround := utils.NewWrapAround[uint16, uint64](utils.WrapAroundParams{IsRestartAllowed: false})
+
+	firstF := uint16(1000)
+
+	testFrameOrder := func(frame uint16, isKeyFrame bool, frame2 uint16, isKeyFrame2, expectInorder bool) {
+		frameUnwrap := fnWrapAround.Update(frame).ExtendedVal
+		wrappedFrame := uint16(fnWrap.UpdateAndGet(frameUnwrap, isKeyFrame))
+
+		// make sure wrap around always get in order frame number
+		fnWrapAround.Update(frame + (frame2-frame)/2)
+
+		frame2Unwrap := fnWrapAround.Update(frame2).ExtendedVal
+		wrappedFrame2 := uint16(fnWrap.UpdateAndGet(frame2Unwrap, isKeyFrame2))
+		// keeps order
+		require.Equal(t, expectInorder, inOrder(wrappedFrame2, wrappedFrame), "frame %d, frame2 %d, wrappedFrame %d, wrapped Frame2 %d, frameUnwrap %d, frame2Unwrap %d", frame, frame2, wrappedFrame, wrappedFrame2, frameUnwrap, frame2Unwrap)
+		// frame number diff should be the same if frame2 is not a key frame
+		if !isKeyFrame2 {
+			require.Equal(t, frame2-frame, wrappedFrame2-wrappedFrame)
+		}
+	}
+
+	secondF := getFrame(firstF, true)
+	testFrameOrder(firstF, true, secondF, false, true)
+
+	// non key frame keeps diff and order
+	for i := 0; i < 100; i++ {
+		// frame in order
+		firstF = secondF
+		secondF = getFrame(firstF, true)
+		testFrameOrder(firstF, false, secondF, false, true)
+
+		// frame out of order
+		firstF = secondF
+		secondF = getFrame(firstF, false)
+		testFrameOrder(firstF, false, secondF, false, false)
+
+		// key frame in order
+		firstF = secondF
+		secondF = getFrame(firstF, true)
+		testFrameOrder(firstF, false, secondF, true, true)
+
+		// frame in order
+		firstF = secondF
+		secondF = getFrame(firstF, true)
+		testFrameOrder(firstF, false, secondF, false, true)
+
+		// key frame out of order but should be in order after wrap around
+		firstF = secondF
+		secondF = getFrame(firstF, false)
+		testFrameOrder(firstF, false, secondF, true, true)
+	}
+}
+
+func inOrder(a, b uint16) bool {
+	return a-b < 0x8000 || (a-b == 0x8000 && a > b)
+}
+
+func getFrame(base uint16, inorder bool) uint16 {
+	if inorder {
+		return base + uint16(rand.Intn(0x8000))
+	}
+	return base + uint16(rand.Intn(0x8000)) + 0x8000
+}