Skip to content

Conversation

@stschr
Copy link
Contributor

@stschr stschr commented Jun 10, 2025

Preselections:

  • adds support for parsing Preselection elements into MediaInfo
  • only "SRMP"; "MRMP" are considered "not supported"
  • if a Preselection@codecs attribute is present in MPD, this overwrites the codecs string from the main AdaptationSet (per clarification in MPEG-DASH 6th Ed). Setting a "Preselection track" uses this derived codecs string to initialize MSE.

@stschr stschr requested a review from dsilhavy June 10, 2025 11:16
@dsilhavy dsilhavy added this to the 5.1.0 milestone Jun 11, 2025
@dsilhavy dsilhavy moved this to In Progress in dash.js Version 5.1.0 Jun 12, 2025
@dsilhavy
Copy link
Collaborator

Related issue: #4513

Copy link
Collaborator

@dsilhavy dsilhavy left a comment

Choose a reason for hiding this comment

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

Thanks @stschr I added my review. Additional question:

only "SRMP"; "MRMP" are considered "not supported"

What is meant by SRMPand MRMP

Are Preselection Descriptors relevant for this PR?

}

function _createConfiguration(type, rep, codec) {
function _createConfiguration(type, rep, codec, prsl_rep) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use camel case instead of underscore notation

}

function _createVideoConfiguration(rep, codec) {
function _createVideoConfiguration(rep, codec, prsl_rep) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use camel case instead of underscore notation

@dsilhavy
Copy link
Collaborator

@stschr I think you are still working on this PR right?

@stschr
Copy link
Contributor Author

stschr commented Sep 2, 2025

@stschr I think you are still working on this PR right?

No further work is planned for this. Let me rebase...

@stschr stschr force-pushed the dolby/20250610_preselectionSupport branch from c9cb90d to fd3f122 Compare September 3, 2025 16:37
* - Constants.TRACK_SWITCH_MODE_NEVER_REPLACE
* Do not replace existing segments in the buffer
*
* @property {} [includePreselectionsInMediainfo: true]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, that makes it much clearer. I suggest we rename this attribute to includePreselectionsInMediainfoArray. That makes it clearer that the Preselections are an additional entry in the array.

return adaptation && adaptation.Representation && adaptation.Representation.length > 0 ? adaptation.Representation[0].mimeType : null;
}

function getMimeTypeForPreselection(preselection, adaptations) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this function? I don't see any usages of it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above (see getIsTextForPreselection())

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer to remove it at this stage, as it is not (yet) used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, done.

*/
class Preselection {
constructor() {
this.period = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, 6th edition specifies: id: default=1

@stschr stschr force-pushed the dolby/20250610_preselectionSupport branch from b7b5e5f to 9bcb4f3 Compare September 12, 2025 08:59
@stschr
Copy link
Contributor Author

stschr commented Sep 12, 2025

Ok, all comments from today addressed...

@dsilhavy dsilhavy merged commit 837cf98 into Dash-Industry-Forum:development Sep 13, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in dash.js Version 5.1.0 Sep 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants