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 video playback #74

Merged
merged 1 commit into from Oct 21, 2020
Merged

Add video playback #74

merged 1 commit into from Oct 21, 2020

Conversation

heyhippari
Copy link
Contributor

@heyhippari heyhippari commented Sep 11, 2020

  • Adds an initial (very rough) player based on Shaka Player
  • Adds ability to play movies (Transcoding, Remuxing or Direct Play)

Next steps:

  • Add a playback manager (Vuex store, probably)
  • Add playlist support
  • Allow playback of TV shows
  • Custom UI

Note: Typings are suppressed for Mux.js and Shaka Player. Ideally, we should submit Mux.js typings to DefinitelyTyped. Shaka Player has typings coming in the next version (probably).

@heyhippari heyhippari force-pushed the feat/playback branch 2 times, most recently from 381e78d to c121c3d Compare September 19, 2020 00:35
@sonarcloud
Copy link

sonarcloud bot commented Sep 19, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Oct 19, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@heyhippari heyhippari marked this pull request as ready for review October 19, 2020 21:58
Copy link

@Xantios Xantios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what it's worth just had a look through the code because i was curious how you implemented the player.

import { stringify } from 'qs';
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
import shaka from 'shaka-player/dist/shaka-player.ui';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered adding the shaka-player types? ( first hit: https://github.com/niklaskorz/shaka-player/tree/feature/generate-typescript-typedefs/test/typescript )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have, but unfortunately that repository is severely out of date. We also prefer to stick with the upstream versions of packages.

The Shaka team does intend to ship typings very soon, however, so this will be fixed when the next version of Shaka Player is released (but that's a guess based on recent activity on their repository)

video {
width: 100vw;
height: 100vh;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you have tested this, but just to be sure... have you considered overflowing in some browsers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't, but we probably need to account for safe areas, yeah.

@ferferga
Copy link
Member

ferferga commented Oct 21, 2020

Are you backporting the strings from Jellyfin Web or using Google Translate? If they're from JF-web we shouldn't include them, as context of the strings might be different. Imo, we should rely on new translations for those imo.

Feature parity with Jellyfin Web automation moved this from In progress to Reviewer approved Oct 21, 2020
@heyhippari
Copy link
Contributor Author

Are you backporting the strings from Jellyfin Web or using Google Translate? If they're from JF-web we shouldn't include them, as context of the strings might be different. Imo, we should rely on new translations for those imo.

We're usually writing entirely new strings. I don't Translate stuff (aside from French once in a while), I leave that to the folks over on Weblate.

@heyhippari heyhippari merged commit 2b019d6 into master Oct 21, 2020
Feature parity with Jellyfin Web automation moved this from Reviewer approved to Done Oct 21, 2020
@heyhippari heyhippari deleted the feat/playback branch October 21, 2020 11:58
@Nickbert7
Copy link

Is there a way to exit the playback?
I cannot find any "Back" or "Stop" button🤔

@camc314
Copy link
Contributor

camc314 commented Oct 21, 2020

Is there a way to exit the playback?

I cannot find any "Back" or "Stop" button🤔

This is currently using the default Shaka playback UI. There is no way to exit the playback and go back to the item details page as current

@Nickbert7
Copy link

OK, good to know.
BTW nice work so far👍

@heyhippari
Copy link
Contributor Author

You can use the back button on the browser, but that's about it until we add a custom UI.

@heyhippari heyhippari added this to the Preview Release 1 milestone Dec 6, 2020
@heyhippari heyhippari added this to In progress in Video playback via automation Dec 6, 2020
Feature parity with Jellyfin Web automation moved this from Done to Reviewer approved Dec 7, 2020
Feature parity with Jellyfin Web automation moved this from Reviewer approved to Done Dec 7, 2020
@barronpm barronpm moved this from In progress to Done in Video playback Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants