Move manual events into tech controller #1415
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,16 @@ vjs.MediaTechController = vjs.Component.extend({ | |
| options.reportTouchActivity = false; | ||
| vjs.Component.call(this, player, options, ready); | ||
|
|
||
| // Manually track progress in cases where the browser/flash player doesn't report it. | ||
| if (!this.features['progressEvents']) { | ||
| this.manualProgressOn(); | ||
| } | ||
|
|
||
| // Manually track timeudpates in cases where the browser/flash player doesn't report it. | ||
| if (!this.features['timeupdateEvents']) { | ||
| this.manualTimeUpdatesOn(); | ||
| } | ||
|
|
||
| this.initControlsListeners(); | ||
| } | ||
| }); | ||
|
|
@@ -153,6 +163,109 @@ vjs.MediaTechController.prototype.onTap = function(){ | |
| this.player().userActive(!this.player().userActive()); | ||
| }; | ||
|
|
||
| /* Fallbacks for unsupported event types | ||
| ================================================================================ */ | ||
| // Manually trigger progress events based on changes to the buffered amount | ||
| // Many flash players and older HTML5 browsers don't send progress or progress-like events | ||
| vjs.MediaTechController.prototype.manualProgressOn = function(){ | ||
| this.manualProgress = true; | ||
|
|
||
| // Trigger progress watching when a source begins loading | ||
| this.trackProgress(); | ||
|
|
||
| // Watch for a native progress event call on the tech element | ||
| // In HTML5, some older versions don't support the progress event | ||
| // So we're assuming they don't, and turning off manual progress if they do. | ||
| // As opposed to doing user agent detection | ||
| this.one('progress', function(){ | ||
|
|
||
| // Update known progress support for this playback technology | ||
| this.features['progressEvents'] = true; | ||
|
|
||
| // Turn off manual progress tracking | ||
| this.manualProgressOff(); | ||
| }); | ||
| }; | ||
|
|
||
| vjs.MediaTechController.prototype.manualProgressOff = function(){ | ||
| this.manualProgress = false; | ||
| this.stopTrackingProgress(); | ||
| }; | ||
|
|
||
| vjs.MediaTechController.prototype.trackProgress = function(){ | ||
|
|
||
| this.progressInterval = setInterval(vjs.bind(this, function(){ | ||
| // Don't trigger unless buffered amount is greater than last time | ||
|
|
||
| var bufferedPercent = this.player().bufferedPercent(); | ||
|
|
||
| if (this.bufferedPercent_ != bufferedPercent) { | ||
| this.player().trigger('progress'); | ||
| } | ||
|
|
||
| this.bufferedPercent_ = bufferedPercent; | ||
|
|
||
| if (bufferedPercent === 1) { | ||
| this.stopTrackingProgress(); | ||
| } | ||
| }), 500); | ||
| }; | ||
| vjs.MediaTechController.prototype.stopTrackingProgress = function(){ clearInterval(this.progressInterval); }; | ||
|
|
||
| /*! Time Tracking -------------------------------------------------------------- */ | ||
| vjs.MediaTechController.prototype.manualTimeUpdatesOn = function(){ | ||
| this.manualTimeUpdates = true; | ||
|
|
||
| this.player().on('play', vjs.bind(this, this.trackCurrentTime)); | ||
| this.player().on('pause', vjs.bind(this, this.stopTrackingCurrentTime)); | ||
| // timeupdate is also called by .currentTime whenever current time is set | ||
|
|
||
| // Watch for native timeupdate event | ||
| this.one('timeupdate', function(){ | ||
| // Update known progress support for this playback technology | ||
| this.features['timeupdateEvents'] = true; | ||
| // Turn off manual progress tracking | ||
| this.manualTimeUpdatesOff(); | ||
| }); | ||
| }; | ||
|
|
||
| vjs.MediaTechController.prototype.manualTimeUpdatesOff = function(){ | ||
| this.manualTimeUpdates = false; | ||
| this.stopTrackingCurrentTime(); | ||
| this.off('play', this.trackCurrentTime); | ||
| this.off('pause', this.stopTrackingCurrentTime); | ||
| }; | ||
|
|
||
| vjs.MediaTechController.prototype.trackCurrentTime = function(){ | ||
| if (this.currentTimeInterval) { this.stopTrackingCurrentTime(); } | ||
| this.currentTimeInterval = setInterval(vjs.bind(this, function(){ | ||
| this.player().trigger('timeupdate'); | ||
| }), 250); // 42 = 24 fps // 250 is what Webkit uses // FF uses 15 | ||
| }; | ||
|
|
||
| // Turn off play progress tracking (when paused or dragging) | ||
| vjs.MediaTechController.prototype.stopTrackingCurrentTime = function(){ | ||
| clearInterval(this.currentTimeInterval); | ||
|
|
||
| // #1002 - if the video ends right before the next timeupdate would happen, | ||
| // the progress bar won't make it all the way to the end | ||
| this.player().trigger('timeupdate'); | ||
| }; | ||
|
|
||
| vjs.MediaTechController.prototype.dispose = function() { | ||
| // Turn off any manual progress or timeupdate tracking | ||
| if (this.manualProgress) { this.manualProgressOff(); } | ||
|
|
||
| if (this.manualTimeUpdates) { this.manualTimeUpdatesOff(); } | ||
|
|
||
| vjs.Component.prototype.dispose.call(this); | ||
| }; | ||
|
|
||
| vjs.MediaTechController.prototype.setCurrentTime = function() { | ||
|
There was a problem hiding this comment. What if we added a listener to the seeking event instead of having this in the setCurrentTime function? That would avoid needing this include in the sub-techs. There was a problem hiding this comment. I'm not seeing There was a problem hiding this comment. Seeking should be in there. But I believe we're currently trigger it directly on the player in the case of flash, in case you were listening for it on the tech or the object itself. There was a problem hiding this comment. In practice, doesn't seem to be working. Filed videojs/video-js-swf#114 to track it. |
||
| // improve the accuracy of manual timeupdates | ||
| if (this.manualTimeUpdates) { this.player().trigger('timeupdate'); } | ||
| }; | ||
|
|
||
| /** | ||
| * Provide a default setPoster method for techs | ||
| * | ||
|
|
||
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
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
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
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
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
Oops, something went wrong.
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.
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.
Unless you think otherwise, let's kill this native event listener. It was needed when certain browsers didn't support these events, e.g. firefox 3.5 or Safari 3. As far as I know even devices are consistent with these events now.
If we do that we also need to set those features to true on the html5 video tech, so that these are never turned on.
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 delete all the wonderful tests you wrote around that feature. :(
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 will shed a tear for my beautiful tests but I agree it's better to get rid of that outdated browser support if it's feasible. I'll double check on the worst Android phone I can find just to be sure.
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 like these events are still being relied on by the Flash tech to display the buffer state in the progress bar. We could move these into the Flash tech but I think that might be better handled in a separate patch. Your thoughts?
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.
Are we talking about the same thing? I mean remove the
this.one('progress', function(){that listens for the real events incase they do exist, not removing the manual events all together. Flash should never have the real events.