Skip to content

feat(previews): allow ffmpeg to connect direct for AWS S3 buckets #53634

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

invario
Copy link
Contributor

@invario invario commented Jun 21, 2025

Summary

Allow preview generation on AWS S3 buckets by creating a presigned URL for the file and then passing it directly to ffmpeg which is able to handle http/https already. It can locate the moov atom even if it's at the end of the video file and it does not need to retrieve the whole file to do it.

The presigned URL expires after 1 minute which should be more than enough for this purpose.

TODO

  • Need more testing.
  • Open to suggestions, critiques, criticisms.

Checklist

@invario invario requested a review from a team as a code owner June 21, 2025 05:34
@invario invario requested review from nfebe, skjnldsv and leftybournes and removed request for a team June 21, 2025 05:34
@skjnldsv skjnldsv requested review from juliusknorr and come-nc and removed request for skjnldsv June 21, 2025 07:56
@invario invario force-pushed the preview-direct-download branch from 2adc819 to e9cdee4 Compare June 22, 2025 14:25
@invario
Copy link
Contributor Author

invario commented Jun 22, 2025

Updated with a test to see if encryption is enabled on the file, and if so, do not attempt to connect direct.

@invario invario force-pushed the preview-direct-download branch from e9cdee4 to 14b91cd Compare June 22, 2025 14:30
Copy link
Contributor

@come-nc come-nc 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 codewise, but needs tests.
Not sure why there is the str_starts_with test for http?

@invario
Copy link
Contributor Author

invario commented Jun 23, 2025

Looks good codewise, but needs tests. Not sure why there is the str_starts_with test for http?

The test is needed because getDirectDownload varies by the external mount provider. SMB, for example, doesn't currently return anything but in the future it might return something like smb://user@example.domain/foo.bar

ffmpeg doesn't support smb out of the box and I'm not even sure it would work without passing credentials directly on the command line. Same thing for ftp and potentially others.

@invario invario force-pushed the preview-direct-download branch from 14b91cd to ebefeb7 Compare June 24, 2025 01:38
@invario
Copy link
Contributor Author

invario commented Jun 24, 2025

Fixed my commit message and force pushed.

@invario invario changed the title enhancement(previews): allow ffmpeg to connect direct for AWS S3 buckets feat(previews): allow ffmpeg to connect direct for AWS S3 buckets Jun 24, 2025
@come-nc
Copy link
Contributor

come-nc commented Jun 24, 2025

Looks good codewise, but needs tests. Not sure why there is the str_starts_with test for http?

The test is needed because getDirectDownload varies by the external mount provider. SMB, for example, doesn't currently return anything but in the future it might return something like smb://user@example.domain/foo.bar

ffmpeg doesn't support smb out of the box and I'm not even sure it would work without passing credentials directly on the command line. Same thing for ftp and potentially others.

Hmm, yeah but a provider may also give an http link that does not work without credentials then? It feels a bit arbitrary to me that http* is fine and all else is not.

@invario
Copy link
Contributor Author

invario commented Jun 24, 2025

Looks good codewise, but needs tests. Not sure why there is the str_starts_with test for http?

The test is needed because getDirectDownload varies by the external mount provider. SMB, for example, doesn't currently return anything but in the future it might return something like smb://user@example.domain/foo.bar
ffmpeg doesn't support smb out of the box and I'm not even sure it would work without passing credentials directly on the command line. Same thing for ftp and potentially others.

Hmm, yeah but a provider may also give an http link that does not work without credentials then? It feels a bit arbitrary to me that http* is fine and all else is not.

Very true. I can add a parameter in addition to the url one. Perhaps a presigned one and check for its value?

edit: Added.

Signed-off-by: invario <67800603+invario@users.noreply.github.com>
@invario invario force-pushed the preview-direct-download branch from ebefeb7 to a581ed6 Compare June 24, 2025 14:23
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.

2 participants