Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(DASH): Fix playback of some DASH with multiple periods (#7516) #7519

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/dash/dash_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,16 @@ shaka.dash.DashParser = class {
}
}

/**
* Handles deferred releases of old SegmentIndexes
* content type from a previous update.
*/
handleDeferredCloseSegmentIndexes() {
if (this.periodCombiner_) {
this.periodCombiner_.handleDeferredCloseSegmentIndexes_();
}
}

/**
* Clean StreamMap Object to remove reference of deleted Stream Object
* @private
Expand Down
13 changes: 13 additions & 0 deletions lib/media/segment_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,19 @@ shaka.media.SegmentIndex = class {
this.numEvicted_ += diff;
}

/**
* Removes all SegmentReferences that end before the given time.
*
* @param {number} time The time in seconds.
* @export
*/
evictAll() {
if (this.references.length) {
this.numEvicted_ += this.references.length;
this.references = [];
}
}


/**
* Drops references that start after windowEnd, or end before windowStart,
Expand Down
22 changes: 22 additions & 0 deletions lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ shaka.media.StreamingEngine = class {

/** @private {?shaka.media.StreamingEngine.MediaState_} */
this.lastTextMediaStateBeforeUnload_ = null;

/**
* Defered closedSegmentIndex of the periodCombiner.
*
* @private {function}
*/
this.periodCombinerDeferredCloseSegmentIndexesFunction_ = null;
}

/** @override */
Expand Down Expand Up @@ -182,6 +189,7 @@ shaka.media.StreamingEngine = class {
this.playerInterface_ = null;
this.manifest_ = null;
this.config_ = null;
this.periodCombinerDeferredCloseSegmentIndexesFunction_ = null;
}

/**
Expand Down Expand Up @@ -269,6 +277,14 @@ shaka.media.StreamingEngine = class {
}
}

/**
* Provide function for releases of old SegmentIndexes of the PeriodCombiner
* @param {function} extraDeferredCloseSegmentIndexes
*/
setExtraDeferredCloseSegmentIndexesFunction(deferredCloseSegmentIndexesFunction)
{
this.periodCombinerDeferredCloseSegmentIndexesFunction_ = deferredCloseSegmentIndexesFunction;
}

/**
* Applies a playback range. This will only affect non-live content.
Expand Down Expand Up @@ -1255,6 +1271,12 @@ shaka.media.StreamingEngine = class {
}
return;
}

if (this.periodCombinerDeferredCloseSegmentIndexesFunction_)
{
this.periodCombinerDeferredCloseSegmentIndexesFunction_();
}

}

// Update the MediaState.
Expand Down
2 changes: 2 additions & 0 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -2570,6 +2570,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget {

this.streamingEngine_ = this.createStreamingEngine();
this.streamingEngine_.configure(this.config_.streaming);
if (this.parser_.handleDeferredCloseSegmentIndexes)
this.streamingEngine_.setExtraDeferredCloseSegmentIndexesFunction(this.parser_.handleDeferredCloseSegmentIndexes)

// Set the load mode to "loaded with media source" as late as possible so
// that public methods won't try to access internal components until
Expand Down
43 changes: 41 additions & 2 deletions lib/util/periods.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ shaka.util.PeriodCombiner = class {
* @private {!Set.<string>}
*/
this.usedPeriodIds_ = new Set();

/**
* Retains a reference to the function used to close SegmentIndex objects
* for streams which were switched away from during an ongoing update_()
* just after the period removal.
* @private {!Map.<string, !function()>}
*/
this.deferredCloseSegmentIndex_ = new Map();
}

/** @override */
Expand All @@ -69,6 +77,7 @@ shaka.util.PeriodCombiner = class {
stream.segmentIndex.release();
}
}
this.handleDeferredCloseSegmentIndexes_();

this.audioStreams_ = [];
this.videoStreams_ = [];
Expand Down Expand Up @@ -110,6 +119,21 @@ shaka.util.PeriodCombiner = class {
return this.imageStreams_;
}


/**
* Handles deferred releases of old SegmentIndexes
* content type from a previous update.
* @private
*/
handleDeferredCloseSegmentIndexes_() {
for (const [key, value] of this.deferredCloseSegmentIndex_.entries()) {
const streamId = /** @type {string} */ (key);
const closeSegmentIndex = /** @type {!function()} */ (value);
closeSegmentIndex();
this.deferredCloseSegmentIndex_.delete(streamId);
}
}

/**
* Deletes a stream from matchedStreams because it is no longer needed
*
Expand Down Expand Up @@ -154,9 +178,24 @@ shaka.util.PeriodCombiner = class {
});
}
}
if (stream.segmentIndex) {
stream.closeSegmentIndex();

if (stream.segmentIndex && !this.deferredCloseSegmentIndex_.has(stream.id)) {
const originalcloseSegmentIndex = stream.closeSegmentIndex;
stream.closeSegmentIndex = () => {
if(stream.segmentIndex)
{
/**
* as the closure of the segmentIndex is defered
* update the number of evicted references in case or
* than 2 of more period are removed.
*/
stream.segmentIndex.evictAll();
}
originalcloseSegmentIndex();
}
this.deferredCloseSegmentIndex_.set(stream.id, stream.closeSegmentIndex);
}

this.usedPeriodIds_.delete(periodId);
}

Expand Down
188 changes: 188 additions & 0 deletions some-changes.patch
ncocaign marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
diff --git a/lib/dash/dash_parser.js b/lib/dash/dash_parser.js
index ba0ba0772..a92b57062 100644
--- a/lib/dash/dash_parser.js
+++ b/lib/dash/dash_parser.js
@@ -1491,6 +1491,16 @@ shaka.dash.DashParser = class {
}
}

+ /**
+ * Handles deferred releases of old SegmentIndexes
+ * content type from a previous update.
+ */
+ handleDeferredCloseSegmentIndexes() {
+ if (this.periodCombiner_) {
+ this.periodCombiner_.handleDeferredCloseSegmentIndexes_();
+ }
+ }
+
/**
* Clean StreamMap Object to remove reference of deleted Stream Object
* @private
diff --git a/lib/media/segment_index.js b/lib/media/segment_index.js
index 2142aeab2..9cf8d5acd 100644
--- a/lib/media/segment_index.js
+++ b/lib/media/segment_index.js
@@ -308,6 +308,19 @@ shaka.media.SegmentIndex = class {
this.numEvicted_ += diff;
}

+ /**
+ * Removes all SegmentReferences that end before the given time.
+ *
+ * @param {number} time The time in seconds.
+ * @export
+ */
+ evictAll() {
+ if (this.references.length) {
+ this.numEvicted_ += this.references.length;
+ this.references = [];
+ }
+ }
+

/**
* Drops references that start after windowEnd, or end before windowStart,
diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js
index 38056b031..a8e82fe5b 100644
--- a/lib/media/streaming_engine.js
+++ b/lib/media/streaming_engine.js
@@ -148,6 +148,13 @@ shaka.media.StreamingEngine = class {

/** @private {?shaka.media.StreamingEngine.MediaState_} */
this.lastTextMediaStateBeforeUnload_ = null;
+
+ /**
+ * Defered closedSegmentIndex of the periodCombiner.
+ *
+ * @private {function}
+ */
+ this.periodCombinerDeferredCloseSegmentIndexesFunction_ = null;
}

/** @override */
@@ -182,6 +189,7 @@ shaka.media.StreamingEngine = class {
this.playerInterface_ = null;
this.manifest_ = null;
this.config_ = null;
+ this.periodCombinerDeferredCloseSegmentIndexesFunction_ = null;
}

/**
@@ -269,6 +277,14 @@ shaka.media.StreamingEngine = class {
}
}

+ /**
+ * Provide function for releases of old SegmentIndexes of the PeriodCombiner
+ * @param {function} extraDeferredCloseSegmentIndexes
+ */
+ setExtraDeferredCloseSegmentIndexesFunction(deferredCloseSegmentIndexesFunction)
+ {
+ this.periodCombinerDeferredCloseSegmentIndexesFunction_ = deferredCloseSegmentIndexesFunction;
+ }

/**
* Applies a playback range. This will only affect non-live content.
@@ -1255,6 +1271,12 @@ shaka.media.StreamingEngine = class {
}
return;
}
+
+ if (this.periodCombinerDeferredCloseSegmentIndexesFunction_)
+ {
+ this.periodCombinerDeferredCloseSegmentIndexesFunction_();
+ }
+
}

// Update the MediaState.
diff --git a/lib/player.js b/lib/player.js
index 27d8042f3..3b0173a0c 100644
--- a/lib/player.js
+++ b/lib/player.js
@@ -2570,6 +2570,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget {

this.streamingEngine_ = this.createStreamingEngine();
this.streamingEngine_.configure(this.config_.streaming);
+ if (this.parser_.handleDeferredCloseSegmentIndexes)
+ this.streamingEngine_.setExtraDeferredCloseSegmentIndexesFunction(this.parser_.handleDeferredCloseSegmentIndexes)

// Set the load mode to "loaded with media source" as late as possible so
// that public methods won't try to access internal components until
diff --git a/lib/util/periods.js b/lib/util/periods.js
index 2055e944c..24503ac77 100644
--- a/lib/util/periods.js
+++ b/lib/util/periods.js
@@ -56,6 +56,14 @@ shaka.util.PeriodCombiner = class {
* @private {!Set.<string>}
*/
this.usedPeriodIds_ = new Set();
+
+ /**
+ * Retains a reference to the function used to close SegmentIndex objects
+ * for streams which were switched away from during an ongoing update_()
+ * just after the period removal.
+ * @private {!Map.<string, !function()>}
+ */
+ this.deferredCloseSegmentIndex_ = new Map();
}

/** @override */
@@ -69,6 +77,7 @@ shaka.util.PeriodCombiner = class {
stream.segmentIndex.release();
}
}
+ this.handleDeferredCloseSegmentIndexes_();

this.audioStreams_ = [];
this.videoStreams_ = [];
@@ -110,6 +119,21 @@ shaka.util.PeriodCombiner = class {
return this.imageStreams_;
}

+
+ /**
+ * Handles deferred releases of old SegmentIndexes
+ * content type from a previous update.
+ * @private
+ */
+ handleDeferredCloseSegmentIndexes_() {
+ for (const [key, value] of this.deferredCloseSegmentIndex_.entries()) {
+ const streamId = /** @type {string} */ (key);
+ const closeSegmentIndex = /** @type {!function()} */ (value);
+ closeSegmentIndex();
+ this.deferredCloseSegmentIndex_.delete(streamId);
+ }
+ }
+
/**
* Deletes a stream from matchedStreams because it is no longer needed
*
@@ -154,9 +178,24 @@ shaka.util.PeriodCombiner = class {
});
}
}
- if (stream.segmentIndex) {
- stream.closeSegmentIndex();
+
+ if (stream.segmentIndex && !this.deferredCloseSegmentIndex_.has(stream.id)) {
+ const originalcloseSegmentIndex = stream.closeSegmentIndex;
+ stream.closeSegmentIndex = () => {
+ if(stream.segmentIndex)
+ {
+ /**
+ * as the closure of the segmentIndex is defered
+ * update the number of evicted references in case or
+ * than 2 of more period are removed.
+ */
+ stream.segmentIndex.evictAll();
+ }
+ originalcloseSegmentIndex();
+ }
+ this.deferredCloseSegmentIndex_.set(stream.id, stream.closeSegmentIndex);
}
+
this.usedPeriodIds_.delete(periodId);
}

Loading