This repository has been archived by the owner on Dec 10, 2020. It is now read-only.
Moved 'loadstarted' setting to fix networkState #106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ns the correct value Also updated to the latest version of video.js in the devDependencies.
heff
added a commit
to heff/video.js
that referenced
this pull request
Jul 11, 2014
…rmine if loadstart should be manually fired. Previously this was only done in the HTML5 tech, but Flash needs it as well. This relies on videojs/video-js-swf#106. Also removed the 'not implemented' warnings from the media tech functions. I think there's a better way we can help tech authors here, and it was blocking the ability to check if a function was implemented for real. Fixes videojs#1300, fixes videojs#1341
|
This looks correct to me but I do not trust my own judgement when it comes to everything that's going on in HTTPVideoProvider. I'm preaching to the choir here, but that file needs to be rewritten. |
|
Looks good to me also. I would also concur w @dmlap comments on not being a fan of this class in its current state. |
|
Thanks guys, going to pull this in. We should also probably take some time to review long term options for the swf. It's definitely not very maintainable in it's current state. |
paullryan
pushed a commit
to paullryan/video.js
that referenced
this pull request
Jul 17, 2014
…rmine if loadstart should be manually fired. Previously this was only done in the HTML5 tech, but Flash needs it as well. This relies on videojs/video-js-swf#106. Also removed the 'not implemented' warnings from the media tech functions. I think there's a better way we can help tech authors here, and it was blocking the ability to check if a function was implemented for real. Fixes videojs#1300, fixes videojs#1341
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1300 and #1341 -- controls don't show when auto playing with Flash
In a previous change we moved the
loadstartevent to be fired early to match html5 more closely (it's fired as soon as there is a source basically, even if preload=none). This made it so the event might fire before the object is registered with the player, making it so loadstart is never fired, vjs-has-started is never added as a class, and controls don't show.This is similar to html5 element in that we might miss the loadstart event, and we address that by checking the networkState value. If it's not zero, loadstart was already fired.
The Flash swf has a networkState value, but was returning zero even after loadstart was fired, so that's what this fixes. It moves the loadstarted=true to right before the loadstart event is fired, so that networkState returns greater than zero after that.
@seniorflexdeveloper, @dmlap, @gkatsev -- I'd appreciate a second set of eyes on this (rather urgent) patch. There's other places where loadstarted is checked, and I want to make sure I'm not introducing new errors by moving it to earlier. For instance, it looks like there's a few places where loadstarted is checked to determine if a netstream function should be called, and after this fix we're setting loadstarted to true before netstream is initialized. I can't tell if that's going to be a problem.