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

Possible to expose H5P.Video instance? #6

Open
sdcloudt opened this issue Aug 25, 2018 · 2 comments
Open

Possible to expose H5P.Video instance? #6

sdcloudt opened this issue Aug 25, 2018 · 2 comments

Comments

@sdcloudt
Copy link
Contributor

Hi,
I am working on a private H5P library based on H5P.Question. For this custom question I would like to add some interactivity to it, for example play a certain part of the video on a wrong answer, and another part for a correct answer. For my custom question this could be extended to specific feedback on specific mistakes. To do this I need to control the video instance which is unfortenately private in sections.video.instance. Only the play and pause functions are available to me.

I see two possible solutions:

  1. We add a getter function to access the media instance. This could be a function getVideoInstance and getImageInstance, or with a little bit complexity a getMediaInstance function. This way child libraries will be able to control the media instances. A possible danger might be that child libraries have access to the media instance and might mess things up.

  2. Now the functions play and pause exists. Likewise we could add functions seek and getCurrentTime. Below is the list of possible functions to add:

  • seek
  • getCurrentTime
  • getDuration
  • getBuffered
  • mute
  • unmute
  • isMuted
  • getVolume
  • setVolume
  • getPlaybackRates
  • getPlaybackRate
  • setPlaybackRate
  • setCaptionsTrack
  • getCaptionsTrack
    For my requirements seek, getCurrentTime and getDuration would suffice. The remaining functions might not be meaningful to add to a question.

I would like feedback on my feature request. I am willing to provide code for both modifications in a pull request if you like the idea.

@fnoks
Copy link
Contributor

fnoks commented Aug 30, 2018

There is a PR for adding Audio as well (#4), we should also keep that in mind. For me, adding a getMediaInstance seems like a good solution. But even if you create a pull request for this, I know it will take quite some time before it is released, so I propose you just override the H5P.Question.setVideo() function for now. Another option would be to temporary override H5P.newRunnable() right before setVideo() is invoked from your implementation (but I can't recommend that since it might come with some unwanted side effects, and it feels even more hacky)

@sdcloudt
Copy link
Contributor Author

I will work on a PR, taking the existence of Audio into account. I came across the following:
getMediaInstance might not be the best way, because the Question library in itself does not limit you in using multiple media elements. Therefore I think getVideoInstance and getAudioInstance might be better. With getImageInstance I have another problem because image is not implemented using H5P.Image. So basically if I return the image element this is not consistent behaviour with getVideoInstance and getAudioInstance. I don't know if there is any particular reason for the fact H5P.Image is not used. If there is none I am willing to replace setImage by a version that uses H5P.Image.

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

No branches or pull requests

2 participants