-
Notifications
You must be signed in to change notification settings - Fork 424
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(live): cue end time not finite #1411
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1411 +/- ##
=======================================
Coverage 85.56% 85.56%
=======================================
Files 41 41
Lines 10146 10147 +1
Branches 2352 2353 +1
=======================================
+ Hits 8681 8682 +1
Misses 1465 1465
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Looks good!
@@ -234,7 +234,8 @@ export const addMetadata = ({ | |||
// Map each cue group's endTime to the next group's startTime | |||
sortedStartTimes.forEach((startTime, idx) => { | |||
const cueGroup = cuesGroupedByStartTime[startTime]; | |||
const nextTime = Number(sortedStartTimes[idx + 1]) || videoDuration; |
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.
Sorry for commenting on merged PR, but maybe we should consider endTime
= startTime
instead of 0 in this case?
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.
Good catch. Just saw a link to this from another issue. Will put up a quick PR for that fix, I agree start time is more appropriate.
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.
Sry to comment this, but when the endTime is set to the starttime or "0".... the cue doesn't get active anymore.
Description
When creating metadata cues, the last cue uses the videoDuration as the end time, in live playback this is
Infinity
and throws an error.Specific Changes proposed
Add an
isFinite
check to the duration, if it's not finite we use 0 instead.Requirements Checklist