Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

Fix excessive segment loads on seeks #925

Merged
merged 4 commits into from
Nov 29, 2016
Merged

Conversation

mjneil
Copy link
Contributor

@mjneil mjneil commented Nov 29, 2016

Description

Sometimes when seeking, buffer time can be really long and the player will load all segments between the the seek points. There are a few cases that cause this behavior.

First being the synchronous call to fillBuffer_() in abort() to avoid waiting for the check buffer timer. This makes aborting and reseting the loader order dependent. When fillBuffer_ is called before the reset happens, it miscalculates the segment it should fetch. The eventual goal is to make any synchronous fillBuffer_ calls to become asynchronous with a very short delay so state modifiers such as abort and reset are not order dependent. For now this PR just uses a quick fix of swapping the order of these two calls.

The other case is that abort() only aborts the segment when the loader is in the WAITING state. However, both the DECRYPTING and APPENDING states happen asynchronously. Since handleUpdateEnd_() changes the loader's mediaIndex and fillAtBuffer_ states, if the timing is right and a seek happens in one of these two states happens, the loader will reset and resync, but won't cancel the segment since the state is not WAITING. This means that when the asynchronous process of decrypting or appending finishes and we reach handleUpdateEnd_, mediaIndex and fillAtBuffer_ get updated with an outdated segment, causing confusion for checkBuffer_. If we clear pendingSegment_ on an abort while not in the WAITING state, and check for null anywhere we access pendingSegment_ we can avoid using outdated segment information.

Specific Changes proposed

  • Change the order of abort() and resetEverything() so loader state is correct before fetching more segments.
  • Null pendingSegment_ on an abort if it exists and the loader is not in the WAITING state.

Requirements Checklist

  • Feature implemented / Bug fixed
  • Reviewed by Two Core Contributors

@mjneil mjneil changed the title Reset abort Fix excessive segment loads on seeks Nov 29, 2016
@forbesjo forbesjo self-assigned this Nov 29, 2016
@imbcmdth
Copy link
Member

LGTM. Awaiting QA approval before merging.

@forbesjo
Copy link
Contributor

LGTM

@forbesjo forbesjo removed their assignment Nov 29, 2016
@imbcmdth imbcmdth merged commit aa33081 into videojs:master Nov 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants