Skip to content

Conversation

@midzelis
Copy link
Collaborator

Description

Use the vidstack.io player for the large asset-viewer. The existing native video player for web works ok on desktop, but isn't great on mobile-web, particularly for iOS. The native controls overlap the navigation elements, and the swipe left/right behavior doesn't work.

The vidstack.io is a continuation of the popular https://plyr.io/ player.

This is more of a POC, and to provoke some discussion.

The vidstack API is largely the same as the native video element (it wraps it). The initial commit to this PR just maintains existing functionality. The base function does add google cast support (if detected) as well as airplay (if detected) and picture-in-picture controls. Additionally, it is fully keyboard accessible, and also supports changing playback speed. This is all there out of the box.

With additional work, thumbnails could be generated for the video scrub bar, so you can see a preview of the video while scrubbing. And of course, you can totally customize all the icons/colors/etc if you wanted to.

Will add the preview label to this - play around on both desktop/mobile and see what you think.

@github-actions
Copy link
Contributor

Deploying preview environment to https://pr-17567.preview.internal.immich.cloud/

@zackpollard
Copy link
Member

This looks great! Only comment is that it appears that the buffering bar doesn't move after the initial load, it is just stuck in a random position. You can see it here in this screenshot for this short motion photo, but it happens for longer videos too.

image

@midzelis
Copy link
Collaborator Author

This looks great! Only comment is that it appears that the buffering bar doesn't move after the initial load, it is just stuck in a random position. You can see it here in this screenshot for this short motion photo, but it happens for longer videos too.

I believe this happens because the browser only loads enough video as it thinks it needs to be able to play without stutter. That bar indicates how much of the file was actually loaded. Meaning, you can seek to any point without delay in that area.

@midzelis midzelis requested review from jrasm91 and zackpollard April 29, 2025 01:18
@midzelis midzelis changed the title feat: vidstack player feat(web): vidstack player Apr 29, 2025
Copy link
Member

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

can you revert this change?

@midzelis midzelis requested a review from jrasm91 April 30, 2025 03:58
@alextran1502
Copy link
Member

Hey Min, great work! Some strange UI issues I notice are

  • I don't see the progress indicator unless I hover over the progress bar.
  • It looks like the progress bar is divided into two segment independently and the buffer looks strange
image

@YarosMallorca
Copy link
Collaborator

I noticed the same issue
Screenshot 2025-05-03 at 16 39 44

@alextran1502
Copy link
Member

Hi Min! Looks good to me. Last comment, is it possible to always show the slider position instead of only showing when you are hovering on the progress bar?

Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

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

Code looks good to me; haven't actually tested the PR

"svelte.enable-ts-plugin": true,
"typescript.preferences.importModuleSpecifier": "non-relative"
"typescript.preferences.importModuleSpecifier": "non-relative",
"html.customData": [".web/node_modules/vidstack/vscode.html-data.json"]
Copy link
Member

Choose a reason for hiding this comment

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

Wth does this do?

Copy link
Member

Choose a reason for hiding this comment

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

Oh it's a vite plugin and you need this to hint it at the correct type or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Vidstack is a web component, and i think this adds some of the attributes/tags that vidstack ships to the vscode HTML LSP: https://github.com/microsoft/vscode-html-languageservice/blob/main/docs/customData.md

Comment on lines +55 to +60
if (event.detail.direction === 'left') {
onNextAsset();
}
if (event.detail.direction === 'right') {
onPreviousAsset();
}
Copy link
Member

Choose a reason for hiding this comment

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

Could argue that an else if (or probably even nicer a switch) would look better here because it'd emphasize that those are always mutually exclusive. Minor thing though given this part is trivial

midzelis and others added 3 commits May 12, 2025 21:39
Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com>
Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com>
@midzelis midzelis requested a review from danieldietzler May 13, 2025 01:52
@bo0tzz bo0tzz removed the preview label May 23, 2025
@midzelis midzelis marked this pull request as draft June 5, 2025 02:24
@zackpollard
Copy link
Member

Hey Min, going to close this for now as it's gone stale. If you do continue to work on it, feel free to open the PR again!

@jrasm91 jrasm91 deleted the vidstack branch July 17, 2025 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants