- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 4.6k
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
base: master
Are you sure you want to change the base?
Conversation
2adc819    to
    e9cdee4      
    Compare
  
    | Updated with a test to see if encryption is enabled on the file, and if so, do not attempt to connect direct. | 
e9cdee4    to
    14b91cd      
    Compare
  
    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.
Looks good codewise, but needs tests.
Not sure why there is the str_starts_with test for http?
| 
 The test is needed because  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. | 
14b91cd    to
    ebefeb7      
    Compare
  
    | Fixed my commit message and force pushed. | 
| 
 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  edit: Added. | 
ebefeb7    to
    a581ed6      
    Compare
  
    | Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) | 
1f8ffd3    to
    42ca705      
    Compare
  
    | Updated to ensure the stream is seekable as supposedly not all AWS S3 streams are, So even though I have PR #53952 in the works using a sparse file generation method, I think having this one put in place also would be worthwhile, as it allows  | 
7a18ea3    to
    5658824      
    Compare
  
    | Squashed commits and cs fixed. | 
5658824    to
    7fede77      
    Compare
  
    | Pushed again, cs should be good now. | 
8701212    to
    1381ee4      
    Compare
  
    | Rebased to 33 and reworked the code into the new  | 
f58eefe    to
    73a42d6      
    Compare
  
    73a42d6    to
    a3ec763      
    Compare
  
    Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com> Signed-off-by: invario <67800603+invario@users.noreply.github.com>
a3ec763    to
    ca45b70      
    Compare
  
    | Not sure if this will ever get merged but rebased and pushed just in case. | 
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 10 minute which should be more than enough for this purpose.
TODO
Checklist