Fix a bug that lead to starting the the schedule timer twice. When th…

…e quality is changed as part of the scheduling logic the ScheduleController.js should not call _getNextFragment (#3654)
Dash-Industry-Forum · Jun 1, 2021 · c3906de · c3906de
1 parent 0379181
commit c3906de
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 51 deletions.
diff --git a/src/streaming/StreamProcessor.js b/src/streaming/StreamProcessor.js
@@ -652,6 +652,7 @@ function StreamProcessor(config) {
         }
 
         if (e.error && e.request.serviceLocation) {
+            logger.info(`Fragment loading completed with an error`);
             setExplicitBufferingTime(e.request.startTime + (e.request.duration / 2));
             scheduleController.startScheduleTimer(0);
         }

diff --git a/src/streaming/controllers/AbrController.js b/src/streaming/controllers/AbrController.js
@@ -577,55 +577,57 @@ function AbrController() {
      * @param {string} streamId
      */
     function checkPlaybackQuality(type, streamId) {
-        if (!type || !streamProcessorDict || !streamProcessorDict[streamId] || !streamProcessorDict[streamId][type]) {
-            return;
-        }
-
-        if (droppedFramesHistory) {
-            const playbackQuality = videoModel.getPlaybackQuality();
-            if (playbackQuality) {
-                droppedFramesHistory.push(streamId, playbackIndex, playbackQuality);
+        try {
+            if (!type || !streamProcessorDict || !streamProcessorDict[streamId] || !streamProcessorDict[streamId][type]) {
+                return false;
             }
-        }
 
-        // ABR is turned off, do nothing
-        if (!settings.get().streaming.abr.autoSwitchBitrate[type]) {
-            return;
-        }
+            if (droppedFramesHistory) {
+                const playbackQuality = videoModel.getPlaybackQuality();
+                if (playbackQuality) {
+                    droppedFramesHistory.push(streamId, playbackIndex, playbackQuality);
+                }
+            }
 
-        const oldQuality = getQualityFor(type, streamId);
-        const rulesContext = RulesContext(context).create({
-            abrController: instance,
-            switchHistory: switchHistoryDict[streamId][type],
-            droppedFramesHistory: droppedFramesHistory,
-            streamProcessor: streamProcessorDict[streamId][type],
-            currentValue: oldQuality,
-            useBufferOccupancyABR: isUsingBufferOccupancyAbrDict[type],
-            useL2AABR: isUsingL2AAbrDict[type],
-            useLoLPABR: isUsingLoLPAbrDict[type],
-            videoModel
-        });
-        const minIdx = getMinAllowedIndexFor(type, streamId);
-        const maxIdx = getMaxAllowedIndexFor(type, streamId);
-        const switchRequest = abrRulesCollection.getMaxQuality(rulesContext);
-        let newQuality = switchRequest.quality;
+            // ABR is turned off, do nothing
+            if (!settings.get().streaming.abr.autoSwitchBitrate[type]) {
+                return false;
+            }
 
-        if (minIdx !== undefined && ((newQuality > SwitchRequest.NO_CHANGE) ? newQuality : oldQuality) < minIdx) {
-            newQuality = minIdx;
-        }
-        if (newQuality > maxIdx) {
-            newQuality = maxIdx;
-        }
+            const oldQuality = getQualityFor(type, streamId);
+            const rulesContext = RulesContext(context).create({
+                abrController: instance,
+                switchHistory: switchHistoryDict[streamId][type],
+                droppedFramesHistory: droppedFramesHistory,
+                streamProcessor: streamProcessorDict[streamId][type],
+                currentValue: oldQuality,
+                useBufferOccupancyABR: isUsingBufferOccupancyAbrDict[type],
+                useL2AABR: isUsingL2AAbrDict[type],
+                useLoLPABR: isUsingLoLPAbrDict[type],
+                videoModel
+            });
+            const minIdx = getMinAllowedIndexFor(type, streamId);
+            const maxIdx = getMaxAllowedIndexFor(type, streamId);
+            const switchRequest = abrRulesCollection.getMaxQuality(rulesContext);
+            let newQuality = switchRequest.quality;
+
+            if (minIdx !== undefined && ((newQuality > SwitchRequest.NO_CHANGE) ? newQuality : oldQuality) < minIdx) {
+                newQuality = minIdx;
+            }
+            if (newQuality > maxIdx) {
+                newQuality = maxIdx;
+            }
 
-        switchHistoryDict[streamId][type].push({ oldValue: oldQuality, newValue: newQuality });
+            switchHistoryDict[streamId][type].push({ oldValue: oldQuality, newValue: newQuality });
 
-        if (newQuality > SwitchRequest.NO_CHANGE && newQuality !== oldQuality) {
-            if (abandonmentStateDict[streamId][type].state === MetricsConstants.ALLOW_LOAD || newQuality > oldQuality) {
+            if (newQuality > SwitchRequest.NO_CHANGE && newQuality !== oldQuality && (abandonmentStateDict[streamId][type].state === MetricsConstants.ALLOW_LOAD || newQuality > oldQuality)) {
                 _changeQuality(type, oldQuality, newQuality, maxIdx, switchRequest.reason, streamId);
+                return true;
             }
-        } else if (settings.get().debug.logLevel === Debug.LOG_LEVEL_DEBUG) {
-            const bufferLevel = dashMetrics.getCurrentBufferLevel(type, true);
-            logger.debug('[' + type + '] stay on ' + oldQuality + '/' + maxIdx + ' (buffer: ' + bufferLevel + ')');
+
+            return false;
+        } catch (e) {
+            return false;
         }
 
     }

diff --git a/src/streaming/controllers/BufferController.js b/src/streaming/controllers/BufferController.js
@@ -942,8 +942,7 @@ function BufferController(config) {
             }
 
             return totalBufferedTime;
-        }
-        catch(e) {
+        } catch (e) {
             return 0;
         }
     }

diff --git a/src/streaming/controllers/ScheduleController.js b/src/streaming/controllers/ScheduleController.js
@@ -134,11 +134,15 @@ function ScheduleController(config) {
             }
 
             if (_shouldScheduleNextRequest()) {
+                let qualityChange = false;
                 if (checkPlaybackQuality) {
-                    // in case the playback quality is supposed to be changed, the corresponding StreamProcessor will update the currentRepresentation
-                    abrController.checkPlaybackQuality(type, streamInfo.id);
+                    // in case the playback quality is supposed to be changed, the corresponding StreamProcessor will update the currentRepresentation.
+                    // The StreamProcessor will also start the schedule timer again once the quality switch has beeen prepared. Consequently, we only call _getNextFragment if the quality is not changed.
+                    qualityChange = abrController.checkPlaybackQuality(type, streamInfo.id);
+                }
+                if (!qualityChange) {
+                    _getNextFragment();
                 }
-                _getNextFragment();
 
             } else {
                 startScheduleTimer(settings.get().streaming.lowLatencyEnabled ? settings.get().streaming.scheduling.lowLatencyTimeout : settings.get().streaming.scheduling.defaultTimeout);
@@ -298,12 +302,11 @@ function ScheduleController(config) {
             const streamInfo = currentRepresentationInfo.mediaInfo.streamInfo;
             if (abrController.isPlayingAtTopQuality(streamInfo)) {
                 const isLongFormContent = streamInfo.manifestInfo.duration >= settings.get().streaming.buffer.longFormContentDurationThreshold;
-                 return isLongFormContent ? settings.get().streaming.buffer.bufferTimeAtTopQualityLongForm : settings.get().streaming.buffer.bufferTimeAtTopQuality;
+                return isLongFormContent ? settings.get().streaming.buffer.bufferTimeAtTopQualityLongForm : settings.get().streaming.buffer.bufferTimeAtTopQuality;
             } else {
-                 return mediaPlayerModel.getStableBufferTime();
+                return mediaPlayerModel.getStableBufferTime();
             }
-        }
-        catch(e) {
+        } catch (e) {
             return mediaPlayerModel.getStableBufferTime();
         }
     }
@@ -356,7 +359,7 @@ function ScheduleController(config) {
     }
 
     function _onBytesAppended(e) {
-        logger.debug(`Appended bytes for ${e.mediaType}`);
+        logger.debug(`Appended bytes for ${e.mediaType} and stream id ${streamInfo.id}`);
 
         // we save the last initialized quality. That way we make sure that the media fragments we are about to append match the init segment
         if (isNaN(e.index)) {