Support captions for audio and video manifests with ranges #95
Conversation
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 pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
DanOlson
commented
Jul 25, 2024
| @@ -95,7 +105,6 @@ export class CanvasInstance extends BaseComponent { | |||
| this._$element = $(this.options.target); | |||
| this._data = this.options.data; | |||
| this.$playerElement = $('<div class="player player--loading"></div>'); | |||
| this.$playerElement.hide(); | |||
There was a problem hiding this comment.
I had to remove this in order to see the controls container on the demo application. Doing so didn't seem to have a negative effect when building the library into UV either, so I'm not sure if this is needed.
This provides support for rendering captions when the manifest's annotation items have a text type rendering of format 'text/vtt'. The implementation is modeled after that of MediaElement.js
|
I imagine that this PR is going to be discussed as part of this week's UV accessibility sprint. However, I have never worked with the AV component, so I don't feel particularly well-equipped to review. Wondering if @stephenwf might be willing to take a look.... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This provides support for rendering captions when the manifest's annotation items have a text type rendering of format 'text/vtt'.
I'm hoping to gather input on direction, and use this as a starting point for moving this issue forward. In UV, manifests without ranges support captions via MediaElement.js for video content, but there is not yet caption support audio content or for manifests with ranges. Since this component is used when rendering ranged manifests, I've taken a stab at an implementation modeled after that of MediaElement.js.
It's mostly working well, though I'm seeing intermittent issues when jumping from one range to another, where captions will occasionally not display without "help" (pause then play, seek 20s forward or backward, etc). It seems to work when playing straight through, however, so it may be something to do with the jump to the exact boundary of the range. I suspect a synchronization issue between the CanvasInstance's_canvasClockTimeand the start/end values of the current/newly active content annotation after the jump to the next range. I'm hoping one of you maintainers might have some advice in that area, otherwise, I'll continue to look into it.EDIT: I believe the above issue is resolved
The below manifest URLs should work with the demo player (the one shown when running
npm start)Example manifest for video
Example manifest for audio
Please let me know if you have any questions, need additional manifest examples, or anything else. I'm very excited to get this issue over the line.
Addresses UniversalViewer/universalviewer#747