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

[wip] Enable fseek for files in S3 storage #19073

Closed
wants to merge 1 commit into from
Closed

Conversation

ahti
Copy link
Contributor

@ahti ahti commented Jan 22, 2020

This allows nextcloud to use fseek when using a S3 storage backend, which enables range requests and thus video streaming including fast seeking, and other applications.

#8275 should be fixed by this.

The implementation is a new stream wrapper around the normal http stream. When fseek is used, a new stream is opened with a range header to begin at the right offset.

I'm not really a PHP developer, so there might be bugs, and there is no proper error handling when the storage backend doesn't reply with a content-range header (although I would expect S3 backends to support range requests properly), and probably in some other places, too.

I have not investigated this, but maybe this approach could also work for the other object store backends?

@ahti ahti changed the title [wipEnable fseek for files in S3 storage [wip] Enable fseek for files in S3 storage Jan 22, 2020
@firemdkfighter
Copy link

On version 17.0.1.1 don't work.

@kesselb
Copy link
Contributor

kesselb commented Jan 24, 2020

Thanks 👍

Removed #14617 from the list because the issue is about SMB.

@SimplyCorbett
Copy link

This does not work for me on 18 RC2.

@icewind1991
Copy link
Member

Thanks for the great work @ahti, very nice solution.

I've adjusted the stream logic to be generic over the http source to make it easy to use for other http based storage backends.

Since I can't push to your fork (and thus this PR), I've opened a new PR at #20033 and will close this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants