Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Paused canplay #139

Merged
merged 3 commits into from
Feb 25, 2015
Merged

Paused canplay #139

merged 3 commits into from
Feb 25, 2015

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Feb 12, 2015

Better handling of ye old netstream.

This is already done in play.
Trigger canplay immediately after loadedmetadata.
@@ -261,7 +261,6 @@ package com.videojs.providers{
public function load():void{
_pauseOnStart = true;
_isPlaying = false;
_isPaused = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paused is supposed to be true by default. Is this going to change that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the default
But we might need to make sure it's paused at the end of a video.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's paused at the end of the video and before the video has started playing.

@heff
Copy link
Member

heff commented Feb 12, 2015

Good stuff. Was this meant to fix something specific, or just cleanup?

Should definitely get other eyes on this as I'm not super familiar with ye old netstream. We should also do some testing around different play/pause/autoplay/preload/source change combinations to be sure this doesn't change the behavior.

@gkatsev
Copy link
Member Author

gkatsev commented Feb 13, 2015

We were testing this with ads in a single player, with and without autoplay. So, it should work pretty well.
@seniorflexdeveloper can you review this PR?

@tomjohnson916
Copy link
Contributor

This LGTM. Definitely needed improvement. I would just question whether or not the other providers should exhibit the same functionality for normalization.

@gkatsev
Copy link
Member Author

gkatsev commented Feb 25, 2015

Merging it in since we have two LGTM. We should get this released ASAP.

gkatsev added a commit that referenced this pull request Feb 25, 2015
@gkatsev gkatsev merged commit db55e92 into master Feb 25, 2015
heff added a commit to heff/video-js-swf that referenced this pull request Mar 17, 2015
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