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

feat(server): Allow .mkv, .wmv, .flv, .mpg videos to be uploaded #2045

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

samip5
Copy link
Contributor

@samip5 samip5 commented Mar 22, 2023

No description provided.

@vercel
Copy link

vercel bot commented Mar 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
immich ⬜️ Ignored (Inspect) Mar 22, 2023 at 4:21PM (UTC)

@samip5 samip5 changed the title feat(server): Allow .mkv, .wmv, .flv, .mpg videos to be uploaded. feat(server): Allow .mkv, .wmv, .flv, .mpg videos to be uploaded Mar 22, 2023
Copy link
Contributor

@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.

Looks good, assuming you have verified the the thumbnail extraction process works correctly.

@alextran1502
Copy link
Contributor

Have you had a chance to test all of the new file format?

@samip5
Copy link
Contributor Author

samip5 commented Mar 22, 2023

Have you had a chance to test all of the new file format?

Yes, I have and all of them work fine. :)

@michelheusschen
Copy link
Contributor

michelheusschen commented Mar 22, 2023

All file types get uploaded via web and a thumbnail is correctly generated. However, video/x-matroska and video/x-flv files are not transcoded while having limited or no support by browsers for direct playback

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 22, 2023

I think it only transcodes them if the video codec isn't h264.

@michelheusschen
Copy link
Contributor

Yep tested it with the h264 codec only, a h265 video in .mkv did get transcoded. Tested on Android and all files can be played, so we can either accept those file types are unplayable in some browsers or implement remuxing to mp4.

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 22, 2023

I think you might be right that some browsers can't play them, even if the video codec itself is playable. Basically codec + container need to be a valid combination. I think we might be able to leave this as-is for now. I know @zackpollard had mentioned wanting to potentially update the container in addition to the codec for situations like this. That would be a separate PR though.

@zackpollard
Copy link
Contributor

Happy for this change to go in, will add this to the required transcoding changes, need to be smarter with the way transcoding is done anyway.

@alextran1502 alextran1502 merged commit 0616a66 into immich-app:main Mar 22, 2023
@samip5 samip5 deleted the feat/more-formats branch March 23, 2023 05:54
@Johan1976
Copy link

Have you had a chance to test all of the new file format?

Yes, I have and all of them work fine. :)

For me the thumbnail images of 16:9 mpg/Mpeg2 videos are in wrong aspect ratio (4:3). The video also plays in 4:3 in Android app. On web or on Ipad they play in 16:9. I have played both the mpg files as well as the transcoded versions in external videoplayer directly from my nas and they both play fine in 16:9. Something seems wrong on server side (and not just Android app) since thumbnails are wrong. What do you need from me to check/verify? Video info say video resolution are 576x720.

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

Successfully merging this pull request may close these issues.

6 participants