-
Notifications
You must be signed in to change notification settings - Fork 73
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 xapi video verbs #17
Open
KirkJohnson
wants to merge
149
commits into
h5p:master
Choose a base branch
from
uhm-coe:add-xapi-video-verbs
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 7 commits
Commits
Show all changes
149 commits
Select commit
Hold shift + click to select a range
fa94009
Added Seeked Functionailty
KirkJohnson 6aea788
Added xAPI extending functionality
KirkJohnson 28c3266
Added xAPI for Youtube
KirkJohnson 6df9a91
xAPI on Youtube, html5, and videojs
KirkJohnson ba72e7d
changes to get Seek working correctly
KirkJohnson 3ffd605
Fixed seeked on Youtube player
KirkJohnson 3a19499
Got Context added to xAPI statements
KirkJohnson 910f714
Adjusted Youtube’s Seek capabilities
KirkJohnson 749f896
Removed unused variables from Youtube
KirkJohnson 73f9072
Ensured Session ID was in every call for each video player
KirkJohnson 9ee8c07
Ensure xAPI played event sent appropriately
KirkJohnson 6cc6009
Moved Competion Prams into a function
KirkJohnson fff638d
Reverted Changes on Video JS
KirkJohnson 68c2482
Modifying for seek
KirkJohnson 3b357b4
Modified Seek Event for YouTube
KirkJohnson 8680f05
Format Float on played segments
KirkJohnson b559407
Fix whitespace
figureone 2cb2dce
Add documentation and reorganize
figureone 920f80e
Made Code DRY
KirkJohnson 3527921
Whitespace
figureone 847bd71
Cleaner logic
figureone 157f527
Whitespace (indentation)
figureone 60015e7
Refactor xAPI object function names
figureone 871fb29
Refactor xAPI function names
figureone e52a8dd
Fix for misnamed variable
figureone 449df2d
Refactor var names (use camelCase, not snake case)
figureone 4924b66
Set default value of getArgsXAPIInitialized param
figureone 8c66406
Added Seeked Functionailty
KirkJohnson 8ef5ba9
Added xAPI extending functionality
KirkJohnson e08d9c5
Added xAPI for Youtube
KirkJohnson 1c102c7
xAPI on Youtube, html5, and videojs
KirkJohnson 2b5ed92
changes to get Seek working correctly
KirkJohnson 9e8aa9b
Fixed seeked on Youtube player
KirkJohnson 9f22a59
Got Context added to xAPI statements
KirkJohnson 828611e
Adjusted Youtube’s Seek capabilities
KirkJohnson c16cc62
Removed unused variables from Youtube
KirkJohnson 1988a2d
Ensured Session ID was in every call for each video player
KirkJohnson 6b57fc4
Ensure xAPI played event sent appropriately
KirkJohnson 7245458
Moved Competion Prams into a function
KirkJohnson 25ede9a
Reverted Changes on Video JS
KirkJohnson 778330f
Modifying for seek
KirkJohnson 7561c34
Modified Seek Event for YouTube
KirkJohnson a640440
Format Float on played segments
KirkJohnson dd3cefc
Fix whitespace
figureone 18be43b
Add documentation and reorganize
figureone c4e2861
Made Code DRY
KirkJohnson 3c6e0a9
Whitespace
figureone 1434112
Cleaner logic
figureone 129d56e
Whitespace (indentation)
figureone ebadcc9
Refactor xAPI object function names
figureone c16d0fd
Refactor xAPI function names
figureone a95ae2c
Fix for misnamed variable
figureone b85a5ec
Refactor var names (use camelCase, not snake case)
figureone 7100eb6
Set default value of getArgsXAPIInitialized param
figureone 6c8dffa
Merge branch 'add-xapi-video-verbs' of https://github.com/uhm-coe/h5p…
figureone 954f51f
Change Completed to Finished
KirkJohnson f82c2cb
Got finished working
KirkJohnson a8e7b3f
Fix whitespace
figureone 215da89
Whitespace
figureone 52256cc
Mark verbs not currently working
figureone 15473c2
Fix html5 player firing paused before finished
figureone 15a58c8
Moved statement creation to separate file
KirkJohnson b1e9a27
Move of xapi statement to new file
KirkJohnson 28b9654
Rename class VideoXapi to VideoXAPI to match capitalization rules
figureone 5eb7492
Bugfix in seekedTo time calculation
figureone 9fe5f95
Don’t include extensions for non set variables
KirkJohnson 58d5638
Changed Finished to Completed
KirkJohnson 02607f0
Adjustments to xapi statements
KirkJohnson a3d024c
Convert duration in completed to ISO8601
KirkJohnson c516b82
Refactor VideoXAPI to a helper object
figureone a35b66c
Fix for video description in xAPI statement
figureone 56d8f0d
Add comments
KirkJohnson 35daa0a
Fix bug with small video durations
figureone 57f9e75
Whitespace
figureone d292a6f
Remove irrelevant hasOwnProperty() check
figureone 4086a79
guid() is unneeded because of H5P.createUUID()
figureone 66a7928
Remove redundant logic
figureone 735bf09
Fix typo
figureone 745ab2a
Remove unecessary class variable seekStart
figureone 44a36df
Replace obsolete HTML feature Document.fullscreen
figureone c9a68cf
Whitespace
figureone 5ab4d28
Update completed verb to match change in spec
figureone cb1b90e
Annotate spec format for progress extension
figureone 0ef32d9
Standardize on 3-decimal precision for floats
figureone f210eba
Remove unecessary local variables
figureone 467ee4c
Whitespace
figureone 4cf8602
Replace obsolete HTML feature Document.fullscreen
figureone 2012f5b
Remove unused local variable
figureone ca9a196
Adjusting getProgress()
KirkJohnson 48f51e1
Merge branch 'add-xapi-video-verbs' of https://github.com/uhm-coe/h5p…
KirkJohnson ca6cd81
Remove unused variable playedSegmentsSegmentEnd
figureone 080f2ce
Rename local variable playedSegmentsSegmentStart to playingSegmentStart
figureone 09583ab
Docs update and whitespace
figureone a00b8b5
Refactor endPlayedSegment(), getProgress(), and playedSegments
figureone a4e7dd6
Mark optional params in function headers
figureone ff3ffb8
Formatting (match H5P code guidelines)
figureone af8e985
Set quality param to height if not provided
figureone 5558d8f
Lint (redeclaration)
figureone ff9f1c0
Lint (whitespace)
figureone ec75b45
Lint (semicolons)
figureone 719add4
Fix syntax in getWidthOrHeight() function
figureone ef6aef4
Lint (remove ES6 syntax)
figureone 4995735
Fix for played segments
figureone 2eb31e9
Lowered threshold of time for endPlayedSegment
KirkJohnson 7fc4d18
Revert to previous version to accomodate pull
KirkJohnson 31ce7e6
Merge branch 'add-xapi-video-verbs' of https://github.com/uhm-coe/h5p…
KirkJohnson c8940b2
Re-Add EOF line deleted in last commit
KirkJohnson a88c17b
HFP-1779 Refactor xAPI statement building
otacke f3581ac
HFP-1779 Allow false for full-screen extension on init
otacke 0585bbb
HFP-1779 Build xAPIObject with base information only once
otacke 94aa8c2
HFP-1779 Move xAPI trigger to video.js
otacke 40db1c3
HFP-1779 Move video state variables to video object
otacke 4346b92
Fix for uncaught error when editing a new video
figureone 6c0dbf9
Cache result of getXAPIObject (optimization)
figureone 2224edd
Whitespace
figureone d0bc373
HFP-1779 Allow different verb definition origins
otacke 83c472a
Merge remote-tracking branch 'upstream/add-xapi-video-verbs' into add…
otacke f132369
Merge pull request #1 from otacke/add-xapi-video-verbs
figureone f3ed616
Implement volumechange event in youtube.js
figureone d6e4f24
Whitespace
figureone 91936f3
Add note about fullscreen event not implemented
figureone 166c8d3
Trigger completed statement at 95% or more watched
figureone 5c1f995
HFP-1779 Move videoXAPI variable to video.js
otacke 1f18bdb
HFP-1779 Prevent triggering paused before seeked
otacke f977a64
HFP-1779 Clean up code
otacke 283f2e3
HFP-1779 Clean code
otacke 6dd68ac
HFP-1779 Stop setting segment start time on play event
otacke 8223c20
HFP-1779 Move finished threshold to variable
otacke 12912a8
Add Completion Threshold to initializes extensions
KirkJohnson cbef273
Merge pull request #2 from otacke/add-xapi-video-verbs
figureone 4f70c16
Add video length to initialized statement
KirkJohnson 94ff7b3
Merge branch 'add-xapi-video-verbs' of https://github.com/uhm-coe/h5p…
KirkJohnson 035d23a
Use Youtube API to pass correct duration
3707be2
Don’t report video-playback-size if not defined
figureone d243bb1
Fix logic error in determining isFullscreen
figureone 5501579
Whitespace
figureone 7ab090b
Don’t report video length if not defined
figureone e1af0c1
Replay 5c1f995 due to incorrect merge conflict resolution in 94ff7b3
figureone 9d612b7
Replay 1f18bdb due to incorrect merge conflict resolution in 94ff7b3
figureone 3e6a584
Replay f977a64 due to incorrect merge conflict resolution in 94ff7b3
figureone 85b0b86
Replay 8223c20 due to incorrect merge conflict resolution in 94ff7b3
figureone ef6d166
Add existence check to completion-threshold extension
figureone 4f595f7
Comments (update function doc header)
figureone 26b8f94
Revert 6dd68ac
figureone 0ea01a7
Merge remote-tracking branch 'upstream/master'
figureone 17052be
Merge remote-tracking branch 'upstream/master' into add-xapi-video-verbs
figureone d9d2202
Merge remote-tracking branch 'upstream/master'
figureone 706855d
Merge remote-tracking branch 'origin/master' into add-xapi-video-verbs
figureone 9bc078a
Merge remote-tracking branch 'upstream/master' into add-xapi-video-verbs
figureone File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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.
discussion
This works nicely. In some of my test runs, I however got a progress like
0.999
andfinished
wasn't triggered. I also noticed that sometime (but not always), when you pause a video for the first time, the segment says it started at 0.01 instead of 0. Could be related.I have not yet looked in to this, and this may well be a problem in H5P, but maybe you have an idea since you worked with the video library lately?
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 was getting this randomly in my testing, too (progress getting close to 1.0, but not quite reaching it). I noticed when repeatedly pausing/playing the video, the recorded end time when pausing and the recorded start time when playing would be a few milliseconds apart.
Example
playedSegments
where I paused an 8-second video twice, once around the 3 second mark, and once around the 5 second mark:0.018[.]3.116[,]3.124[.]5.056[,]5.074[.]8.021
We've talked at length about what "completing" a video means, and I'm pretty sure it's not "watched every last millisecond of the video." But it's hard to pin down (what if a video rolls credits for a couple minutes at the end, can you skip that? What if a student skips a few seconds of a repetitive part in the middle of the video?).
For our purposes, I would be ok with allowing some delta away from 100% that would still be considered "complete"; we could say
if (progress >= 0.95)
, or maybe define a class constant for some delta value, and then sayif (progress >= 1 - delta)
. 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.
I can expand on this a bit. The video profile CoP agrees that there are use cases (technical or otherwise) that would require complete to trigger on a progress value < 1.0.
We have a profile solution for this that we are finalizing. It requires adding an extension in the initialized statement that defines the minimum progress value used by the Activity Provider (H5P in this case) to trigger the completed statement. As long as this value exists then analysis can still be implement across data sets with different minimum values.
The issue is.. where is this value set for H5P? I think this is what @figureone is pointing out at the end. Short term solution is hard code
if (progress >= 0.95)
, but we would ideally prefer a UI element to set the delta. That being said, we are in a rush to get this done so that may be a change for down the road.Whether
0.999
issue is H5P or not.. I am not sure. @jhaag75 & @liveaspankaj did most of the development on the communities videojs version. Perhaps they can say if they ran into this issue on there as well.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 had faced the issue of fractional time gap between paused/play even if they happen at the same point in the player.
For that what I did in VideoJS prototype was:
When a play happens, I checked the difference between time of play and last paused. And when the time difference was less than 1 seconds. I used the paused time instead of actual reported play time.
e.g. paused at 12.31 second. then play at 12.78 second. I use 12.31 second as the play time.
Probably, 1-2 second adjustment for margin of error could be done for progress calculation too?
Regarding adjusting progress value. I guess, it is a tough decision, and could be left to the user or decided by the authoring tool (H5P)