-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix undefined fragData when using parsed manifest object #2108
Conversation
Any issues with this pull request? |
let fragS = parseInt(fragData.s, 10); | ||
let fragT = parseInt(fragData.t, 10); | ||
let startTimeOffset = NaN; | ||
let startTimeOffset = 0; |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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?
There was a problem hiding this 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
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)? |
I take my comment back you are just protecting against null value so I am going to pull in. Thanks for the commit; |
@AkamaiDASH Cool, thanks. Could I please have an ETA on the release of the next version on NPM? |
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. |
Fixes #2088
When you initialize the stream with a parsed manifest object instead of the URL of a MPD,
getStreamStartTime
inPlaybackController
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: