Choose max available layer when current becomes unavailable. (#1631)

When current became unavailable, it was possible for
target to be set to opportunistic. Because of that,
the downgrade did not happen and PLI layer lock was
requested continuously.

pkg/sfu/forwarder.go

@@ -579,26 +579,51 @@ func (f *Forwarder) AllocateOptimal(availableLayers []int32, brs Bitrates, allow
 		alloc.RequestLayerSpatial = alloc.TargetLayer.Spatial
 
 	default:
+		// lots of different events could end up here
+		//   1. Publisher side layer resuming/stopping
+		//   2. Bitrate becoming available
+		//   3. New max published spatial layer or max temporal layer seen
+		//   4. Subscriber layer changes
+		//
+		// to handle all of the above
+		//   1. Find highest that can be requested - takes into account available layers and overshoot.
+		//      This should catch scenarios like layers resuming/stopping.
+		//   2. If current is a valid layer, check against currently available layers and continue at current
+		//      if possible. Else, choose the highest available layer as the next target.
+		//   3. If current is not valid, set next target to be opportunistic.
+		maxLayerSpatialLimit := int32(math.Min(float64(maxLayer.Spatial), float64(maxSeenLayer.Spatial)))
+		highestAvailableLayer := buffer.InvalidLayerSpatial
 		requestLayerSpatial := buffer.InvalidLayerSpatial
 		for _, al := range availableLayers {
-			if al > requestLayerSpatial {
+			if al > requestLayerSpatial && al <= maxLayerSpatialLimit {
 				requestLayerSpatial = al
 			}
+			if al > highestAvailableLayer {
+				highestAvailableLayer = al
+			}
 		}
-		maxLayerSpatialLimit := int32(math.Min(float64(maxLayer.Spatial), float64(maxSeenLayer.Spatial)))
-		if requestLayerSpatial > maxLayerSpatialLimit {
-			requestLayerSpatial = maxLayerSpatialLimit
+		if requestLayerSpatial == buffer.InvalidLayerSpatial && highestAvailableLayer != buffer.InvalidLayerSpatial && allowOvershoot && f.vls.IsOvershootOkay() {
+			requestLayerSpatial = highestAvailableLayer
 		}
-		if currentLayer.IsValid() && ((requestLayerSpatial == requestSpatial && currentLayer.Spatial == requestSpatial) || requestLayerSpatial == buffer.InvalidLayerSpatial) {
-			// 1. current is locked to desired, stay there
-			// OR
-			// 2. feed may be dry, let it continue at current layer if valid.
-			// covers the cases of
-			//   1. mis-detection of layer stop - can continue streaming
-			//   2. current layer resuming - can latch on when it starts
-			alloc.TargetLayer = buffer.VideoLayer{
-				Spatial:  currentLayer.Spatial,
-				Temporal: getMaxTemporal(),
+
+		if currentLayer.IsValid() {
+			if (requestLayerSpatial == requestSpatial && currentLayer.Spatial == requestSpatial) || requestLayerSpatial == buffer.InvalidLayerSpatial {
+				// 1. current is locked to desired, stay there
+				// OR
+				// 2. feed may be dry, let it continue at current layer if valid.
+				// covers the cases of
+				//   1. mis-detection of layer stop - can continue streaming
+				//   2. current layer resuming - can latch on when it starts
+				alloc.TargetLayer = buffer.VideoLayer{
+					Spatial:  currentLayer.Spatial,
+					Temporal: getMaxTemporal(),
+				}
+			} else {
+				// current layer has stopped, switch to highest available
+				alloc.TargetLayer = buffer.VideoLayer{
+					Spatial:  requestLayerSpatial,
+					Temporal: getMaxTemporal(),
+				}
 			}
 			alloc.RequestLayerSpatial = alloc.TargetLayer.Spatial
 		} else {

pkg/sfu/forwarder_test.go

@@ -302,6 +302,23 @@ func TestForwarderAllocateOptimal(t *testing.T) {
 	require.Equal(t, expectedResult, f.lastAllocation)
 	require.Equal(t, buffer.DefaultMaxLayer, f.TargetLayer())
 
+	// opportunistic target if feed is dry and current is not valid, i. e. not forwarding
+	expectedResult = VideoAllocation{
+		PauseReason:         VideoPauseReasonNone,
+		BandwidthRequested:  bitrates[2][1],
+		BandwidthDelta:      0,
+		BandwidthNeeded:     bitrates[2][1],
+		Bitrates:            bitrates,
+		TargetLayer:         buffer.DefaultMaxLayer,
+		RequestLayerSpatial: 2,
+		MaxLayer:            buffer.DefaultMaxLayer,
+		DistanceToDesired:   -0.5,
+	}
+	result = f.AllocateOptimal(nil, bitrates, true)
+	require.Equal(t, expectedResult, result)
+	require.Equal(t, expectedResult, f.lastAllocation)
+	require.Equal(t, buffer.DefaultMaxLayer, f.TargetLayer())
+
 	// if feed is not dry and current is not locked, should be opportunistic (with and without overshoot)
 	f.vls.SetTarget(buffer.InvalidLayer)
 	expectedResult = VideoAllocation{
@@ -341,7 +358,7 @@ func TestForwarderAllocateOptimal(t *testing.T) {
 	// switches request layer to highest available if feed is not dry and current is valid and current is not available
 	f.vls.SetCurrent(buffer.VideoLayer{Spatial: 0, Temporal: 1})
 	expectedTargetLayer = buffer.VideoLayer{
-		Spatial:  2,
+		Spatial:  1,
 		Temporal: buffer.DefaultMaxLayerTemporal,
 	}
 	expectedResult = VideoAllocation{
@@ -353,7 +370,7 @@ func TestForwarderAllocateOptimal(t *testing.T) {
 		TargetLayer:         expectedTargetLayer,
 		RequestLayerSpatial: 1,
 		MaxLayer:            buffer.DefaultMaxLayer,
-		DistanceToDesired:   -0.5,
+		DistanceToDesired:   0.5,
 	}
 	result = f.AllocateOptimal([]int32{1}, bitrates, true)
 	require.Equal(t, expectedResult, result)
@@ -380,28 +397,6 @@ func TestForwarderAllocateOptimal(t *testing.T) {
 	result = f.AllocateOptimal([]int32{0}, emptyBitrates, true)
 	require.Equal(t, expectedResult, result)
 	require.Equal(t, expectedResult, f.lastAllocation)
-
-	// opportunistic if feed is not dry and current is valid, but request layer has changed
-	f.vls.SetMax(buffer.VideoLayer{Spatial: 2, Temporal: 1})
-	f.vls.SetCurrent(buffer.VideoLayer{Spatial: 0, Temporal: 1})
-	f.vls.SetRequestSpatial(0)
-	expectedTargetLayer = buffer.VideoLayer{
-		Spatial:  2,
-		Temporal: 1,
-	}
-	expectedResult = VideoAllocation{
-		PauseReason:         VideoPauseReasonFeedDry,
-		BandwidthRequested:  0,
-		BandwidthDelta:      0,
-		Bitrates:            emptyBitrates,
-		TargetLayer:         expectedTargetLayer,
-		RequestLayerSpatial: 1,
-		MaxLayer:            f.vls.GetMax(),
-		DistanceToDesired:   -1,
-	}
-	result = f.AllocateOptimal([]int32{0, 1}, emptyBitrates, true)
-	require.Equal(t, expectedResult, result)
-	require.Equal(t, expectedResult, f.lastAllocation)
 }
 
 func TestForwarderProvisionalAllocate(t *testing.T) {