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 undefined fragData when using parsed manifest object #2108

Merged

Conversation

AlecAnthonyGarcia
Copy link
Contributor

Fixes #2088

When you initialize the stream with a parsed manifest object instead of the URL of a MPD, getStreamStartTime in PlaybackController tries to get the parsed media fragments but it's undefined since the parsing is never done for a parsed manifest object, it's only parsed from the URL of an MPD.

Example manifest that is passed in as a string to the parser: https://gist.github.com/CaliAlec/4ae753844344aab7da98e2418b22a624

In order to parse the manifest string, I'm using internal classes:

const manifest = MANIFEST_STRING;
const parser = DashParser().create();
const xlink = XLinkController().create({});
const mpd = parser.parse(manifest, xlink);

@nicosang nicosang requested a review from LloydW93 August 3, 2017 13:22
@AlecAnthonyGarcia
Copy link
Contributor Author

Any issues with this pull request?

let fragS = parseInt(fragData.s, 10);
let fragT = parseInt(fragData.t, 10);
let startTimeOffset = NaN;
let startTimeOffset = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a load of isNaN(startTimeOffset) checks below here. Why has the initial value changed to 0 - surely, at least if ignoreStartTimeOffset is true, it should still be NaN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my case, when loading a parsed manifest object, if startTimeOffset is NaN, I will get the following error: Uncaught TypeError: Failed to set the 'currentTime' property on 'HTMLMediaElement': The provided double value is non-finite.

It needs to be 0 so that the stream starts from the beginning. I guess a better approach would be to leave the initial value as NaN, and if fragData is null, set startTimeOffset to 0?

Copy link
Contributor Author

@AlecAnthonyGarcia AlecAnthonyGarcia left a comment

Choose a reason for hiding this comment

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

I reverted the initial value and only set it to 0 if fragData is null

@dsparacio
Copy link
Contributor

It is not pulled in yet because there is concern for regression here. There are so many use cases around that code for Multiperiod, live starttime, anchor points etc.

What streams have you tested with? Can you ensure that it works with all streams in the first three sub folders of this player (VOD, Live and Subtitles)?
http://reference.dashif.org/dash.js/nightly/samples/dash-if-reference-player/index.html

@dsparacio
Copy link
Contributor

I take my comment back you are just protecting against null value so I am going to pull in. Thanks for the commit;

@dsparacio dsparacio merged commit 66d6492 into Dash-Industry-Forum:development Aug 18, 2017
@AlecAnthonyGarcia
Copy link
Contributor Author

@AkamaiDASH Cool, thanks. Could I please have an ETA on the release of the next version on NPM?

@dsparacio
Copy link
Contributor

We are targeting to release NPM and tagged Github release no later than Sept 1st pending any major issue found in next two weeks of testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants