Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a very basic played() method #2384

Closed
wants to merge 2 commits into from
Closed

Conversation

dmlap
Copy link
Member

@dmlap dmlap commented Jul 20, 2015

Expose the played attribute on the HTML5 tech. Add a default implementation of played() to the base tech that tracks whether the video has played at all but doesn't provide valid ranges.

For #2323

@pam
Copy link

pam commented Jul 20, 2015

Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: f839478
Build details: https://travis-ci.org/pam/video.js/builds/71789788

(Please note that this is a fully automated comment.)

@pam
Copy link

pam commented Jul 20, 2015

Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: aa77c10
Build details: https://travis-ci.org/pam/video.js/builds/71795620

(Please note that this is a fully automated comment.)

Expose the played attribute on the HTML5 tech. Add a default implementation of played() to the base tech that tracks whether the video has played at all but doesn't provide valid ranges.
The SWF doesn't have an implementation of played so allow Flash to fallback to the base tech implementation.
@pam
Copy link

pam commented Jul 21, 2015

Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: 64b785b
Build details: https://travis-ci.org/pam/video.js/builds/71967853

(Please note that this is a fully automated comment.)

@dmlap
Copy link
Member Author

dmlap commented Jul 21, 2015

@pam retry

@pam
Copy link

pam commented Jul 21, 2015

Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: 64b785b
Build details: https://travis-ci.org/pam/video.js/builds/71989157

(Please note that this is a fully automated comment.)

@heff
Copy link
Member

heff commented Jul 21, 2015

Why are we not trying to provide a complete fallback implementation? Time constraint or something else?

@dmlap
Copy link
Member Author

dmlap commented Jul 21, 2015

The complete fallback seems like it would be pretty complicated. If viewers start seeking, we have to keep track of all the time ranges that have been viewed and then figure out whether new timeupdate events should be "merged" with an existing range or create a new one. This implementation allows you to find out if the video has been played at all (providing similar functionality to vjs-has-started at the tech level) which is really useful.

@heff
Copy link
Member

heff commented Jul 21, 2015

Sounds like a good project for a flight. :) I'm fine with merging this in the mean time.

We could probably build it without trying to manage seeking events. Just have a threshold of closeness, e.g. 2s. If a timeupdate happens within 2s of an existing timerange, extend it, otherwise start a new one. Then have a followup task that does any merging needed.

@dmlap dmlap closed this in 1102452 Jul 21, 2015
@dmlap dmlap deleted the mvp-played branch July 21, 2015 20:56
@mmcc mmcc mentioned this pull request Jul 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants