From a35eecd03dda5653457b47a40c5d59d1e447227a Mon Sep 17 00:00:00 2001
From: Raja Subramanian <raja.gobi@tutanota.com>
Date: Wed, 1 Mar 2023 11:12:54 +0530
Subject: [PATCH] Fix a case of changing video quality not succeeding. (#1483)

In the following order, got the wrong layer
- Max layer is 0, max published is 0, request layer is 0
- Current locks to 0.
- Max changes to 1. Nothing changes as 1 is not published yet.
- Max published changes to 1.
- As curernt layer is valid, available and locked to request layer, it
  was kept. But, it should have checked if the request layer changed
  and updated accordingly.
---
 pkg/sfu/forwarder.go      |  9 +++------
 pkg/sfu/forwarder_test.go | 25 ++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/pkg/sfu/forwarder.go b/pkg/sfu/forwarder.go
index 88557093b..bc6c6cd65 100644
--- a/pkg/sfu/forwarder.go
+++ b/pkg/sfu/forwarder.go
@@ -544,10 +544,6 @@ func (f *Forwarder) AllocateOptimal(availableLayers []int32, brs Bitrates, allow
 		alloc.targetLayers = f.parkedLayers
 		alloc.requestLayerSpatial = alloc.targetLayers.Spatial
 
-	case f.maxLayers != f.lastAllocation.maxLayers:
-		opportunisticAlloc()
-		alloc.requestLayerSpatial = int32(math.Min(float64(f.maxLayers.Spatial), float64(f.maxPublishedLayer)))
-
 	case len(availableLayers) == 0:
 		// feed may be dry
 		if f.currentLayers.IsValid() {
@@ -585,14 +581,15 @@ func (f *Forwarder) AllocateOptimal(availableLayers []int32, brs Bitrates, allow
 
 			alloc.requestLayerSpatial = alloc.targetLayers.Spatial
 		} else {
-			if f.currentLayers.IsValid() && f.currentLayers.Spatial == f.requestLayerSpatial {
+			requestLayerSpatial := int32(math.Min(float64(f.maxLayers.Spatial), float64(f.maxPublishedLayer)))
+			if f.currentLayers.IsValid() && requestLayerSpatial == f.requestLayerSpatial && f.currentLayers.Spatial == f.requestLayerSpatial {
 				// current is locked to desired, stay there
 				alloc.targetLayers = f.currentLayers
 				alloc.requestLayerSpatial = f.requestLayerSpatial
 			} else {
 				// opportunistically latch on to anything
 				opportunisticAlloc()
-				alloc.requestLayerSpatial = int32(math.Min(float64(f.maxLayers.Spatial), float64(f.maxPublishedLayer)))
+				alloc.requestLayerSpatial = requestLayerSpatial
 			}
 		}
 	}
diff --git a/pkg/sfu/forwarder_test.go b/pkg/sfu/forwarder_test.go
index a7e54c686..82c4a4b08 100644
--- a/pkg/sfu/forwarder_test.go
+++ b/pkg/sfu/forwarder_test.go
@@ -354,6 +354,7 @@ func TestForwarderAllocateOptimal(t *testing.T) {
 	require.Equal(t, expectedResult, f.lastAllocation)
 
 	// stays the same if feed is not dry and current is valid, available and locked
+	f.maxLayers = VideoLayers{Spatial: 0, Temporal: 1}
 	f.currentLayers = VideoLayers{Spatial: 0, Temporal: 1}
 	f.requestLayerSpatial = 0
 	expectedTargetLayers = VideoLayers{
@@ -367,7 +368,29 @@ func TestForwarderAllocateOptimal(t *testing.T) {
 		bitrates:            emptyBitrates,
 		targetLayers:        expectedTargetLayers,
 		requestLayerSpatial: 0,
-		maxLayers:           DefaultMaxLayers,
+		maxLayers:           f.maxLayers,
+		distanceToDesired:   0,
+	}
+	result = f.AllocateOptimal([]int32{0, 1}, 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.maxLayers = VideoLayers{Spatial: 2, Temporal: 1}
+	f.currentLayers = VideoLayers{Spatial: 0, Temporal: 1}
+	f.requestLayerSpatial = 0
+	expectedTargetLayers = VideoLayers{
+		Spatial:  2,
+		Temporal: 3,
+	}
+	expectedResult = VideoAllocation{
+		pauseReason:         VideoPauseReasonFeedDry,
+		bandwidthRequested:  0,
+		bandwidthDelta:      0,
+		bitrates:            emptyBitrates,
+		targetLayers:        expectedTargetLayers,
+		requestLayerSpatial: 2,
+		maxLayers:           f.maxLayers,
 		distanceToDesired:   0,
 	}
 	result = f.AllocateOptimal([]int32{0, 1}, emptyBitrates, true)