Notify max layer taking into account overshoot. (#1117)

* Notify max layer taking into account overshoot.

An attempt to handle case of FF stopping layer 0, but not layer 1.
When max subscribed layer is layer 0, server allows overshoot to layer 1
to ensure continued streaming when the channel is not congested.

But, dynacast could have reported maximum subscribed layer as layer 0.

This is a very simple attempt to address that by taking overshoot
into account. Needs testing if this works well or not.

NOTE: When subsriber/down track is unmuted, it will report
the max subscribed layer as the max required layer. In those cases,
if the client does not start layer 0, there will still be an issue.
IOW, server is not keeping track of client behaviour that the client
has stopped layer 0 and publishing only layer 1. Server is just
accommodating overshoot with this change.

* Fix a couple of tests and change reflect.DeepEqual -> require.Equal as
much as possible
This commit is contained in:
Raja Subramanian
2022-10-23 22:46:01 +05:30
committed by GitHub
parent 794c74360b
commit 222ce5d1a8
4 changed files with 32 additions and 32 deletions
+1 -1
View File
@@ -1462,7 +1462,7 @@ func (f *Forwarder) getTranslationParamsVideo(extPkt *buffer.ExtPacket, layer in
// if f.ddLayerSelector != nil {
// f.ddLayerSelector.SelectLayer(f.currentLayers)
// }
if f.currentLayers.Spatial == f.maxLayers.Spatial {
if f.currentLayers.Spatial >= f.maxLayers.Spatial {
tp.isSwitchingToMaxLayer = true
}
}
+23 -22
View File
@@ -1,7 +1,6 @@
package sfu
import (
"reflect"
"testing"
"github.com/pion/webrtc/v3"
@@ -1209,7 +1208,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err := f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
require.True(t, f.started)
require.Equal(t, f.lastSSRC, params.SSRC)
@@ -1219,7 +1218,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// out-of-order packet not in cache should be dropped
params = &testutils.TestExtPacketParams{
@@ -1236,7 +1235,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// padding only packet in order should be dropped
params = &testutils.TestExtPacketParams{
@@ -1251,7 +1250,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// in order packet should be forwarded
params = &testutils.TestExtPacketParams{
@@ -1271,7 +1270,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// padding only packet after a gap should be forwarded
params = &testutils.TestExtPacketParams{
@@ -1290,7 +1289,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// out-of-order should be forwarded using cache
params = &testutils.TestExtPacketParams{
@@ -1310,7 +1309,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// switching source should lock onto the new source, but sequence number should be contiguous
params = &testutils.TestExtPacketParams{
@@ -1330,7 +1329,7 @@ func TestForwarderGetTranslationParamsAudio(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
require.Equal(t, f.lastSSRC, params.SSRC)
}
@@ -1368,7 +1367,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
require.NoError(t, err)
require.Equal(t, expectedTP, *actualTP)
// although target layer matches, not a key frame, so should drop and ask to send PLI
// although target layer matches, not a key frame, so should drop
f.targetLayers = VideoLayers{
Spatial: 0,
Temporal: 1,
@@ -1398,6 +1397,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
extPkt, _ = testutils.GetTestExtPacketVP8(params, vp8)
expectedTP = TranslationParams{
isSwitchingToMaxLayer: true,
rtp: &TranslationParamsRTP{
snOrdering: SequenceNumberOrderingContiguous,
sequenceNumber: 23333,
@@ -1423,7 +1423,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
require.True(t, f.started)
require.Equal(t, f.lastSSRC, params.SSRC)
@@ -1433,7 +1433,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// out-of-order packet not in cache should be dropped
params = &testutils.TestExtPacketParams{
@@ -1449,7 +1449,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// padding only packet in order should be dropped
params = &testutils.TestExtPacketParams{
@@ -1463,7 +1463,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// in order packet should be forwarded
params = &testutils.TestExtPacketParams{
@@ -1499,7 +1499,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// temporal layer higher than target, should be dropped
params = &testutils.TestExtPacketParams{
@@ -1529,7 +1529,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// RTP sequence number and VP8 picture id should be contiguous after dropping higher temporal layer picture
params = &testutils.TestExtPacketParams{
@@ -1580,7 +1580,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// padding only packet after a gap should be forwarded
params = &testutils.TestExtPacketParams{
@@ -1599,7 +1599,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// out-of-order should be forwarded using cache, even if it is padding only
params = &testutils.TestExtPacketParams{
@@ -1618,7 +1618,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 0)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
// switching SSRC (happens for new layer or new track source)
// should lock onto the new source, but sequence number should be contiguous
@@ -1651,6 +1651,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
extPkt, _ = testutils.GetTestExtPacketVP8(params, vp8)
expectedTP = TranslationParams{
isSwitchingToMaxLayer: true,
rtp: &TranslationParamsRTP{
snOrdering: SequenceNumberOrderingContiguous,
sequenceNumber: 23338,
@@ -1676,7 +1677,7 @@ func TestForwarderGetTranslationParamsVideo(t *testing.T) {
}
actualTP, err = f.GetTranslationParams(extPkt, 1)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(expectedTP, *actualTP))
require.Equal(t, expectedTP, *actualTP)
require.Equal(t, f.lastSSRC, params.SSRC)
}
@@ -1869,7 +1870,7 @@ func TestForwardGetPaddingVP8(t *testing.T) {
IsKeyFrame: true,
}
blankVP8 := f.GetPaddingVP8(true)
require.True(t, reflect.DeepEqual(expectedVP8, *blankVP8))
require.Equal(t, expectedVP8, *blankVP8)
// getting padding with no frame end needed, should get next picture id
expectedVP8 = buffer.VP8{
@@ -1888,5 +1889,5 @@ func TestForwardGetPaddingVP8(t *testing.T) {
IsKeyFrame: true,
}
blankVP8 = f.GetPaddingVP8(false)
require.True(t, reflect.DeepEqual(expectedVP8, *blankVP8))
require.Equal(t, expectedVP8, *blankVP8)
}
+2 -3
View File
@@ -144,7 +144,7 @@ func Test_packetMeta_VP8(t *testing.T) {
IsKeyFrame: true,
}
unpackedVP8 := p.unpackVP8()
require.True(t, reflect.DeepEqual(expectedVP8, unpackedVP8))
require.Equal(t, expectedVP8, unpackedVP8)
// short picture id and no TL0PICIDX
vp8 = &buffer.VP8{
@@ -181,6 +181,5 @@ func Test_packetMeta_VP8(t *testing.T) {
IsKeyFrame: true,
}
unpackedVP8 = p.unpackVP8()
require.True(t, reflect.DeepEqual(expectedVP8, unpackedVP8))
require.Equal(t, expectedVP8, unpackedVP8)
}
+6 -6
View File
@@ -216,7 +216,7 @@ func TestOutOfOrderPictureId(t *testing.T) {
tp, err = v.UpdateAndGet(extPkt, SequenceNumberOrderingGap, 2)
require.NoError(t, err)
require.NotNil(t, tp)
require.True(t, reflect.DeepEqual(tpExpected, *tp))
require.Equal(t, tpExpected, *tp)
// all three, the last, the current and the in-between should have been added to missing picture id cache
value, ok := v.PictureIdOffset(13467)
@@ -255,7 +255,7 @@ func TestOutOfOrderPictureId(t *testing.T) {
tp, err = v.UpdateAndGet(extPkt, SequenceNumberOrderingOutOfOrder, 2)
require.NoError(t, err)
require.NotNil(t, tp)
require.True(t, reflect.DeepEqual(tpExpected, *tp))
require.Equal(t, tpExpected, *tp)
}
func TestTemporalLayerFiltering(t *testing.T) {
@@ -363,7 +363,7 @@ func TestGapInSequenceNumberSamePicture(t *testing.T) {
}
tp, err := v.UpdateAndGet(extPkt, SequenceNumberOrderingContiguous, 2)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(*tp, tpExpected))
require.Equal(t, tpExpected, *tp)
// telling there is a gap in sequence number will add pictures to missing picture cache
tpExpected = TranslationParamsVP8{
@@ -385,7 +385,7 @@ func TestGapInSequenceNumberSamePicture(t *testing.T) {
}
tp, err = v.UpdateAndGet(extPkt, SequenceNumberOrderingGap, 2)
require.NoError(t, err)
require.True(t, reflect.DeepEqual(*tp, tpExpected))
require.Equal(t, tpExpected, *tp)
value, ok := v.PictureIdOffset(13467)
require.True(t, ok)
@@ -437,7 +437,7 @@ func TestUpdateAndGetPadding(t *testing.T) {
HeaderSize: 6,
IsKeyFrame: true,
}
require.True(t, reflect.DeepEqual(expectedVP8, *blankVP8))
require.Equal(t, expectedVP8, *blankVP8)
// getting padding with new picture
blankVP8 = v.UpdateAndGetPadding(true)
@@ -456,7 +456,7 @@ func TestUpdateAndGetPadding(t *testing.T) {
HeaderSize: 6,
IsKeyFrame: true,
}
require.True(t, reflect.DeepEqual(expectedVP8, *blankVP8))
require.Equal(t, expectedVP8, *blankVP8)
}
func TestVP8PictureIdWrapHandler(t *testing.T) {