Add missing reason to SwitchRequest in ABRRulesCollection.js (Dash-Industry-Forum#3487)

* Add missing reason to SwitchRequest in ABRRulesCollection.js

* Add additional unit test

Author: dsilhavy

src/streaming/rules/abr/ABRRulesCollection.js

@@ -135,46 +135,57 @@ function ABRRulesCollection(config) {
         return srArray.filter(sr => sr.quality > SwitchRequest.NO_CHANGE);
     }
 
+    /**
+     *
+     * @param {array} srArray
+     * @return {object} SwitchRequest
+     */
     function getMinSwitchRequest(srArray) {
         const values = {};
+        let newSwitchReq = null;
         let i,
             len,
             req,
-            newQuality,
-            quality;
+            quality,
+            reason;
 
         if (srArray.length === 0) {
             return;
         }
 
-        values[SwitchRequest.PRIORITY.STRONG] = SwitchRequest.NO_CHANGE;
-        values[SwitchRequest.PRIORITY.WEAK] = SwitchRequest.NO_CHANGE;
-        values[SwitchRequest.PRIORITY.DEFAULT] = SwitchRequest.NO_CHANGE;
+        values[SwitchRequest.PRIORITY.STRONG] = { quality: SwitchRequest.NO_CHANGE, reason: null };
+        values[SwitchRequest.PRIORITY.WEAK] = { quality: SwitchRequest.NO_CHANGE, reason: null };
+        values[SwitchRequest.PRIORITY.DEFAULT] = { quality: SwitchRequest.NO_CHANGE, reason: null };
 
         for (i = 0, len = srArray.length; i < len; i += 1) {
             req = srArray[i];
             if (req.quality !== SwitchRequest.NO_CHANGE) {
-                values[req.priority] = values[req.priority] > SwitchRequest.NO_CHANGE ? Math.min(values[req.priority], req.quality) : req.quality;
+                // We only use the new quality in case it is lower than the already saved one or if no new quality has been selected for the respective priority
+                if (values[req.priority].quality === SwitchRequest.NO_CHANGE || values[req.priority].quality > req.quality) {
+                    values[req.priority].quality = req.quality;
+                    values[req.priority].reason = req.reason || null;
+                }
             }
         }
 
-        if (values[SwitchRequest.PRIORITY.WEAK] !== SwitchRequest.NO_CHANGE) {
-            newQuality = values[SwitchRequest.PRIORITY.WEAK];
+        if (values[SwitchRequest.PRIORITY.WEAK].quality !== SwitchRequest.NO_CHANGE) {
+            newSwitchReq = values[SwitchRequest.PRIORITY.WEAK];
         }
 
-        if (values[SwitchRequest.PRIORITY.DEFAULT] !== SwitchRequest.NO_CHANGE) {
-            newQuality = values[SwitchRequest.PRIORITY.DEFAULT];
+        if (values[SwitchRequest.PRIORITY.DEFAULT].quality !== SwitchRequest.NO_CHANGE) {
+            newSwitchReq = values[SwitchRequest.PRIORITY.DEFAULT];
         }
 
-        if (values[SwitchRequest.PRIORITY.STRONG] !== SwitchRequest.NO_CHANGE) {
-            newQuality = values[SwitchRequest.PRIORITY.STRONG];
+        if (values[SwitchRequest.PRIORITY.STRONG].quality !== SwitchRequest.NO_CHANGE) {
+            newSwitchReq = values[SwitchRequest.PRIORITY.STRONG];
         }
 
-        if (newQuality !== SwitchRequest.NO_CHANGE) {
-            quality = newQuality;
+        if (newSwitchReq) {
+            quality = newSwitchReq.quality;
+            reason = newSwitchReq.reason;
         }
 
-        return SwitchRequest(context).create(quality);
+        return SwitchRequest(context).create(quality, reason);
     }
 
     function getMaxQuality(rulesContext) {
@@ -211,6 +222,7 @@ function ABRRulesCollection(config) {
         initialize,
         reset,
         getMaxQuality,
+        getMinSwitchRequest,
         shouldAbandonFragment,
         getQualitySwitchRules
     };

test/unit/streaming.rules.abr.ABRRulesCollection.js

@@ -81,5 +81,132 @@ describe('ABRRulesCollection', function () {
             expect(shouldAbandonFragment.quality).to.be.equal(SwitchRequest.NO_CHANGE);  // jshint ignore:line
         });
 
+        it('should return correct switch request in getMinSwitchRequest for a single item', () => {
+            const srArray = [{
+                quality: 5,
+                priority: SwitchRequest.PRIORITY.WEAK,
+                reason: {
+                    throughput: 1000
+                }
+            }];
+
+            const sr = abrRulesCollection.getMinSwitchRequest(srArray);
+
+            expect(sr.quality).to.be.equal(5);
+            expect(sr.reason.throughput).to.be.equal(1000);
+        });
+
+        it('should return correct switch request in getMinSwitchRequest for multiple items with similar priorities', () => {
+            const srArray = [
+                {
+                    quality: 6,
+                    priority: SwitchRequest.PRIORITY.WEAK,
+                    reason: {
+                        throughput: 60
+                    }
+                },
+                {
+                    quality: 4,
+                    priority: SwitchRequest.PRIORITY.WEAK,
+                    reason: {
+                        throughput: 40
+                    }
+                },
+                {
+                    quality: 5,
+                    priority: SwitchRequest.PRIORITY.WEAK,
+                    reason: {
+                        throughput: 50
+                    }
+                }
+            ];
+
+            const sr = abrRulesCollection.getMinSwitchRequest(srArray);
+
+            expect(sr.quality).to.be.equal(4);
+            expect(sr.reason.throughput).to.be.equal(40);
+        });
+
+        it('should return correct switch request in getMinSwitchRequest for multiple items with different priorities', () => {
+            const srArray = [
+                {
+                    quality: 6,
+                    priority: SwitchRequest.PRIORITY.DEFAULT,
+                    reason: {
+                        throughput: 60
+                    }
+                },
+                {
+                    quality: 5,
+                    priority: SwitchRequest.PRIORITY.STRONG,
+                    reason: {
+                        throughput: 50
+                    }
+                },
+                {
+                    quality: 7,
+                    priority: SwitchRequest.PRIORITY.WEAK,
+                    reason: {
+                        throughput: 70
+                    }
+                }
+            ];
+
+            const sr = abrRulesCollection.getMinSwitchRequest(srArray);
+
+            expect(sr.quality).to.be.equal(5);
+            expect(sr.reason.throughput).to.be.equal(50);
+        });
+
+        it('should return correct switch request in getMinSwitchRequest for multiple items with different and similar priorities', () => {
+            const srArray = [
+                {
+                    quality: 6,
+                    priority: SwitchRequest.PRIORITY.DEFAULT,
+                    reason: {
+                        throughput: 60
+                    }
+                },
+                {
+                    quality: 5,
+                    priority: SwitchRequest.PRIORITY.STRONG,
+                    reason: {
+                        throughput: 50
+                    }
+                },
+                {
+                    quality: 4,
+                    priority: SwitchRequest.PRIORITY.STRONG,
+                    reason: {
+                        throughput: 40
+                    }
+                },
+                {
+                    quality: 7,
+                    priority: SwitchRequest.PRIORITY.WEAK,
+                    reason: {
+                        throughput: 70
+                    }
+                }
+            ];
+
+            const sr = abrRulesCollection.getMinSwitchRequest(srArray);
+
+            expect(sr.quality).to.be.equal(4);
+            expect(sr.reason.throughput).to.be.equal(40);
+        });
+
+        it('should return correct switch request in getMinSwitchRequest for a single item without reason', () => {
+            const srArray = [{
+                quality: 5,
+                priority: SwitchRequest.PRIORITY.WEAK
+            }];
+
+            const sr = abrRulesCollection.getMinSwitchRequest(srArray);
+
+            expect(sr.quality).to.be.equal(5);
+            expect(sr.reason).to.be.null; // jshint ignore:line
+        });
+
     });
 });