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.

Sign up for GitHub

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

feat(DASH): Allow PeriodCombiner for using streams once #6097

Merged
merged 3 commits into from
Jan 15, 2024
Merged

feat(DASH): Allow PeriodCombiner for using streams once #6097

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4357c88
feat(DASH): Allow PeriodCombiner for using streams once
tykus160 Jan 12, 2024
2e64df7
revert formatting
tykus160 Jan 12, 2024
ddd26d4
export PeriodCombiner config methods & bugfix
tykus160 Jan 12, 2024
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
2 changes: 2 additions & 0 deletions demo/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ shakaDemo.Config = class {
/* canBeUnset= */ true)
.addBoolInput_('Enable DASH sequence mode',
'manifest.dash.sequenceMode')
.addBoolInput_('Use stream once in period flattening',
'manifest.dash.useStreamOnceInPeriodFlattening')
.addBoolInput_('Disable Audio', 'manifest.disableAudio')
.addBoolInput_('Disable Video', 'manifest.disableVideo')
.addBoolInput_('Disable Text', 'manifest.disableText')
Expand Down
10 changes: 8 additions & 2 deletions externs/shaka/player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,8 @@ shaka.extern.InitDataTransform;
* manifestPreprocessor: function(!Element),
* sequenceMode: boolean,
* enableAudioGroups: boolean,
* multiTypeVariantsAllowed: boolean
* multiTypeVariantsAllowed: boolean,
* useStreamOnceInPeriodFlattening: boolean
* }}
*
* @property {string} clockSyncUri
Expand Down Expand Up @@ -928,6 +929,12 @@ shaka.extern.InitDataTransform;
* Might result in undesirable behavior if mediaSource.codecSwitchingStrategy
* is not set to SMOOTH.
* Defaults to true if SMOOTH codec switching is supported, RELOAD overwise.
* @property {boolean} useStreamOnceInPeriodFlattening
* If period combiner is used, this option ensures every stream is used
* only once in period flattening. It speeds up underlying algorithm
* but may raise issues if manifest does not have stream consistency
* between periods.
* Defaults to <code>false</code>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me this should default to true, @littlespex , what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be more than happy to set it to true by default, but potentially it will increase number of PERIOD_FLATTENING_FAILED errors on not well structured manifests. We'd have to add mention of this setting in error description or in FAQ (or both).

Copy link
Contributor

@littlespex littlespex Jan 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaulting to true would technically be a breaking change for us. You would have call out in the release notes that if you must opt-out if you want consistent behavior with previous versions after upgrading.

* @exportDoc
*/
shaka.extern.DashManifestConfiguration;
Expand Down Expand Up @@ -1095,7 +1102,6 @@ shaka.extern.MssManifestConfiguration;
* @property {boolean} raiseFatalErrorOnManifestUpdateRequestFailure
* If true, manifest update request failures will cause a fatal error.
* Defaults to <code>false</code> if not provided.
*
* @exportDoc
*/
shaka.extern.ManifestConfiguration;
Expand Down
2 changes: 2 additions & 0 deletions lib/dash/dash_parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ shaka.dash.DashParser = class {
this.periodCombiner_.setAllowMultiTypeVariants(
this.config_.dash.multiTypeVariantsAllowed &&
shaka.media.Capabilities.isChangeTypeSupported());
this.periodCombiner_.setUseStreamOnce(
this.config_.dash.useStreamOnceInPeriodFlattening);
}
}

Expand Down
17 changes: 17 additions & 0 deletions lib/util/periods.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ shaka.util.PeriodCombiner = class {
/** @private {boolean} */
this.multiTypeVariantsAllowed_ = false;

/** @private {boolean} */
this.useStreamOnce_ = false;

/**
* The IDs of the periods we have already used to generate streams.
* This helps us identify the periods which have been added when a live
Expand Down Expand Up @@ -1037,6 +1040,12 @@ shaka.util.PeriodCombiner = class {
}
}

// Remove just found stream if configured to, so possible future linear
// searches can be faster.
if (this.useStreamOnce_ && !shaka.util.PeriodCombiner.isDummy_(best)) {
streams.delete(best);
}

return best;
}

Expand Down Expand Up @@ -1076,6 +1085,14 @@ shaka.util.PeriodCombiner = class {
this.multiTypeVariantsAllowed_ = allowed;
}

/**
* @param {boolean} useOnce if true, stream will be used only once in period
* flattening algoritnm.
*/
setUseStreamOnce(useOnce) {
this.useStreamOnce_ = useOnce;
}

/**
* @param {T} outputStream An audio or video output stream
* @param {T} candidate A candidate stream to be combined with the output
Expand Down
1 change: 1 addition & 0 deletions lib/util/player_configuration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ shaka.util.PlayerConfiguration = class {
sequenceMode: false,
enableAudioGroups: false,
multiTypeVariantsAllowed,
useStreamOnceInPeriodFlattening: false,
},
hls: {
ignoreTextStreamFailures: false,
Expand Down
Loading