fix: retry appends on QUOTA_EXCEEDED_ERR #1093
Conversation
gesinger
changed the title
| return; | ||
| } | ||
|
|
||
| this.error(`${type} append of ${bytes.length}b failed for segment ` + |
There was a problem hiding this comment.
should we log the error itself too? Maybe the message or callstack in it could have more info for us?
There was a problem hiding this comment.
Yeah, probably a good idea to add that in.
| const audioBufferStart = audioBuffered.length ? audioBuffered.start(0) : 0; | ||
| const audioBufferEnd = audioBuffered.length ? | ||
| audioBuffered.end(audioBuffered.length - 1) : 0; | ||
| const videoBufferStart = videoBuffered.length ? videoBuffered.start(0) : 0; | ||
| const videoBufferEnd = videoBuffered.length ? | ||
| videoBuffered.end(videoBuffered.length - 1) : 0; |
There was a problem hiding this comment.
any issues with just clearing from 0 to now-MIN_BACK_BUFFER??
There was a problem hiding this comment.
Ah, I see. This is to see if the segment is too large and removing the back buffer won't help.
| // wait the length of time alotted in the back buffer to prevent wasted | ||
| // attempts (since we can't clear less than the minimum) |
There was a problem hiding this comment.
shouldn't we be waiting based on the forward buffer? Though, waiting 1s each time makes sense. Just not sure it makes say to make it based on back buffer.
There was a problem hiding this comment.
Also, can't we wait for updateend to know that stuff have been removed?
There was a problem hiding this comment.
The forward buffer may be changing in the case of demuxed content.
And updateend won't always fire for the removes (particularly if the buffer doesn't exist).
| const quotaExceededError = new Error(); | ||
|
|
||
| quotaExceededError.code = QUOTA_EXCEEDED_ERR; | ||
|
|
||
| throw quotaExceededError; |
There was a problem hiding this comment.
not that we should change this, but the alternative would've been throw {code: QUOTA_EXCEEDED_ERR};
There was a problem hiding this comment.
The vjsstandard linter yelled at me for doing that at first (which I was surprised by, but changing to an actual error helped).
|
This PR makes Boat 4K play, fully, in Chrome without any errors in the console. In firefox, it now fails with |
|
Overall, definitely an improvement. We definitely should revisit this to try doing one or both of these:
|
| `Appended byte length: ${bytes.byteLength}, ` + | ||
| `audio buffer: ${timeRangesToArray(audioBuffered).join(', ')}, ` + | ||
| `video buffer: ${timeRangesToArray(videoBuffered).join(', ')}, `); | ||
| this.error({ |
There was a problem hiding this comment.
I wonder if we should try splitting the single append into two appends and halving the segment.
There was a problem hiding this comment.
Yeah, I added that as a comment above. I think it would be good to try as a TODO, but I'm hesitant to do it just yet as I'm not sure how different browsers will behave if we append bytes that aren't split up on proper box boundaries.
Requirements Checklist