-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: initial implementation for WebM player for recordings #832
feat: initial implementation for WebM player for recordings #832
Conversation
webapp/player/index.html
Outdated
<head> | ||
<meta name="viewport" content="width=device-width"> | ||
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/xterm-player@latest/dist/css/xterm-player.min.css" /> | ||
<script src="https://cdn.jsdelivr.net/npm/xterm@4.4.0/lib/xterm.min.js"></script> |
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.
@nicolesabourin1 I'm not sure how we currently handle the dependencies for the rest of our web frontend, but we'd need to build and include external dependencies alongside the rest of the frontend such that it can work without Internet access. No external CDN delivery can be used for this, as this causes problems for GDPR compliance in addition to breaking things when used without Internet access
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.
My understanding is that we should use the @xterm/xterm
npm package: https://www.npmjs.com/package/@xterm/xterm
It’s then possible to use ES6 module syntax:
import { Terminal } from '@xterm/xterm';
Can you confirm? @kristahouse
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.
Yes, you can use ES6 module syntax to import modules in a plain JavaScript setup within an HTML page.
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.
how quickly can this be fixed? we're looking into making a new release soon, and it would be great to have a V1 of the web player included in it
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.
The problem here is we need a way to host the static web files inside the gateway web server for the xterm js and css files, just like we did for the webapp static files, but in this case the webapp may not be enabled, @CBenoit will need to make an endpoint for the static files.
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.
@pauldumais Just like for the webapp, the GET /jet/jrec/play/*
endpoint is serving all the files found in the player/
folder. Is it not enough in this case? It’s working really the same as the GET /jet/webapp/client/*
endpoint.
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.
@CBenoit awesome, I just added the xterm player files locally to the folder
req.send(null); | ||
} | ||
|
||
function convertTRPtoCast(fileArray) { |
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.
we have our answer, Paul baked in a live TRP to .cast converter directly in the Javascript of the player, so it'll be able to play the older TRP file format as well :)
Adds a video and xterm player at the
GET /jet/jrec/play
endpoint which supports multiple videos and builds the page dynamically based on the type of recording.Issue: DGW-110