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
Refactor streams playback start #3308
Refactor streams playback start #3308
Conversation
Thanks to this PR, use cases as described in issue #3146 are correctly handled |
let startTime = streamSeekTime; | ||
if (isNaN(startTime)) { | ||
startTime = isDynamic ? e.liveStartTime : e.streamInfo.start; | ||
startTime = Math.max(startTime, getStartTimeFromUri()); |
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.
Following up the discussion we had on Slack and using #t during startup: Using #t for live will not work with dynamic manifests as the live edge will always have a higher value than the #t. I think it should be Math.min but we need to consider the DVR window at this point. If #t is out of the DVR window we can either ignore it or adjust its value.
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.
Addition: For Math.min for live, Math.max for VoD
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.
getStartTimeFromUri() should return NaN instead of -1
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.
No, getStartTimeFromUri() should not return NaN
Math.max and Math.min are expecting each parameter to ba a valid number
Math.max(x, NaN) and Math.min(x, NaN) return NaN
But I do modify acording your remarks and take advantage of this PR to make code conformed to MPD anchors spec
- always use seeking process from PlaybackController to seek controllers - adjust seek time in BufferController according to effective appended ranges - ScheduleController: do not stop scheduler when updating data (at manifest updates)
a29a380
to
6689278
Compare
As discussed on Slack, tested: VoD
Live
|
it('should start static stream at period start if #t is beyond period end', function (done) { | ||
doneFn = done; | ||
|
||
let uriStartTime = 700; |
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.
Suggest to define this as staticStreamInfo.duration + 100 in case we ever change the value in staticStreamInfo.duration
* Refactor stream playback starting process: - always use seeking process from PlaybackController to seek controllers - adjust seek time in BufferController according to effective appended ranges - ScheduleController: do not stop scheduler when updating data (at manifest updates) * revert gap/discontinuity limit for buffer range tolerance (to do in another PR) * update code comments * BufferController: optimise effective range seeking if seek target * avoid seeking at NaN * remove commented code * remove commented code * Add processing of MPD time anchor (#t) according to Annex C of MPEG DASH spec * StreamProcessor: update DVR info metrics also if SEGMENTS_UPDATE_FAILED_ERROR_CODE * rename getStartTimeFromUriParameters() parameter to 'rangeStart' * Start at DVR window start if #t is before DVR window range * Ignore #t=posix: notation for static streams * #t=<time> shall be relative to period start for dynamic streams * update unit tests
This PR intends to refactor and simplify streams playback start process:
The changes are illustrated in following sequence diagram: https://sketchboard.me/nCcqJmksDzlb#/