From 1d7e761aad19f4c7d0ef9f5e9df57a134baf84de Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Tue, 6 Oct 2026 09:57:40 +0530 Subject: [PATCH] fix: switch VP8 temporal layer up only at a layer sync frame (#4947) --- pkg/sfu/forwarder_test.go | 42 +++++++++++++++++++ .../temporallayerselector/vp8.go | 11 ++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/pkg/sfu/forwarder_test.go b/pkg/sfu/forwarder_test.go index 931c79e7e..146662802 100644 --- a/pkg/sfu/forwarder_test.go +++ b/pkg/sfu/forwarder_test.go @@ -1933,6 +1933,48 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) { require.Equal(t, f.lastSSRC, params.SSRC) } +func TestForwarderVP8TemporalUpSwitchAtLayerSync(t *testing.T) { + f := newForwarder(testutils.TestVP8Codec, webrtc.RTPCodecTypeVideo) + f.vls.SetCurrent(buffer.VideoLayer{Spatial: 0, Temporal: 0}) + f.vls.SetTarget(buffer.VideoLayer{Spatial: 0, Temporal: 2}) + + // a frame without Y may reference a dropped frame of its layer + steps := []struct { + name string + tid uint8 + y bool + expectedThis int32 + expectedCurrent int32 + }{ + {name: "TL2 without Y", tid: 2, expectedThis: 0, expectedCurrent: 0}, + {name: "TL1 without Y", tid: 1, expectedThis: 0, expectedCurrent: 0}, + {name: "TL0", tid: 0, expectedThis: 0, expectedCurrent: 0}, + {name: "TL2 with Y before TL1 sync", tid: 2, y: true, expectedThis: 0, expectedCurrent: 0}, + {name: "TL1 with Y", tid: 1, y: true, expectedThis: 1, expectedCurrent: 1}, + {name: "TL2 without Y after TL1 switch", tid: 2, expectedThis: 1, expectedCurrent: 1}, + {name: "TL2 with Y", tid: 2, y: true, expectedThis: 2, expectedCurrent: 2}, + } + for i, step := range steps { + extPkt, err := testutils.GetTestExtPacketVP8( + &testutils.TestExtPacketParams{SequenceNumber: uint16(i), PayloadSize: 20}, + &codec.VP8{S: true, I: true, PictureID: uint16(i), T: true, TID: step.tid, Y: step.y}, + ) + require.NoError(t, err) + require.Equal(t, step.expectedThis, f.vls.SelectTemporal(extPkt), step.name) + require.Equal(t, step.expectedCurrent, f.vls.GetCurrent().Temporal, step.name) + } + + // key frame refreshes all buffers, up-switch at it even without Y + f.vls.SetCurrent(buffer.VideoLayer{Spatial: 0, Temporal: 0}) + extPkt, err := testutils.GetTestExtPacketVP8( + &testutils.TestExtPacketParams{SequenceNumber: 10, PayloadSize: 20}, + &codec.VP8{S: true, I: true, PictureID: 10, T: true, TID: 0, IsKeyFrame: true}, + ) + require.NoError(t, err) + require.Equal(t, int32(2), f.vls.SelectTemporal(extPkt)) + require.Equal(t, int32(2), f.vls.GetCurrent().Temporal) +} + func TestForwarderGetSnTsForPadding(t *testing.T) { f := newForwarder(testutils.TestVP8Codec, webrtc.RTPCodecTypeVideo) diff --git a/pkg/sfu/videolayerselector/temporallayerselector/vp8.go b/pkg/sfu/videolayerselector/temporallayerselector/vp8.go index 39480c439..bc5679c58 100644 --- a/pkg/sfu/videolayerselector/temporallayerselector/vp8.go +++ b/pkg/sfu/videolayerselector/temporallayerselector/vp8.go @@ -44,7 +44,16 @@ func (v *VP8) Select(extPkt *buffer.ExtPacket, current int32, target int32) (thi tid := extPkt.Temporal if current < target { - if tid > current && tid <= target && vp8.S { + // Up-switch only where nothing dropped can be referenced: a layer sync + // (Y) frame depends only on TL0 (RFC 7741), a key frame refreshes all + // reference buffers. Key frames also cover encoders that never set Y. + // A sync frame raises one layer at a time, the layers in between would + // otherwise forward frames that reference their dropped frames. + switch { + case vp8.IsKeyFrame && tid <= target: + this = target + next = target + case tid == current+1 && tid <= target && vp8.S && vp8.Y: this = tid next = tid }