-
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: detect demuxed video underflow gaps #948
Conversation
let gap; | ||
|
||
if (videoBuffered.length && audioBuffered.length) { | ||
const lastVideoRange = Ranges.findRange(videoBuffered, currentTime - 3); |
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.
why the -3
?
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.
gapFromVideoUnderflow
implies that audio will keep playing for ~3 seconds during a video underflow and that is what I have seen as well. I will add a comment.
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.
Should we add a fudge to it?
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'm thinking that we shouldn't as 3 is already a "fudge" here.
// Even if there is no available next range, there is still a possibility we are | ||
// stuck in a gap due to video underflow. | ||
const gap = this.gapFromVideoUnderflow_(buffered, currentTime); |
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.
and this generic buffered gap check isn't necessary because we still check for this gap above which may not be a video underflow? Or is this equivalent to checking against videoBuffered?
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.
The generic buffered
only worked when the it was referring to muxed content because we will go off of the "video buffer", which is really the muxed content, in that case. Now that we have a case that checks for demuxed content we can just use videoBuffered for the original muxed check and both buffers for the demuxed check.
acc += val; | ||
|
||
if (val.stack) { | ||
acc += '\n' + val.stack; |
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.
actually print out log error stack traces when we assert.
let gap; | ||
|
||
if (videoBuffered.length && audioBuffered.length) { | ||
const lastVideoRange = Ranges.findRange(videoBuffered, currentTime - 3); |
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.
Should we add a fudge to it?
Description
We are seeing a video underflow for demuxed fmp4 content. I think the main issue is timing related, but I have yet to narrow down the cause. While going through our code I noticed that we already detect video underflows for muxed content. I added code to detect demuxed video underflows so that we can fix those as well.