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

fix: handling range headers correctly when full content requested #69

Merged
merged 3 commits into from
Mar 19, 2025

Conversation

alexmaras
Copy link
Collaborator

This addresses #68

The issue occurred because I didn't implement the full set of Range/Content-Range header requirements.
The setup now handles ranges that have no end byte provided (i.e. Range: bytes=0-) which by the spec return the full length of the content starting from the starting byte.

Just also testing at the moment that Clipious in Dash mode continues to work.

@Fijxu
Copy link
Contributor

Fijxu commented Mar 19, 2025

Works fine. Thanks! I can also seek into unbuffered parts of a video without problems which was something that I couldn't do before because it would throw me "The media playback was aborted due to a corruption problem or because the media used features your browser did not support." if I tried to seek into an unbuffered part of the video.

@alexmaras
Copy link
Collaborator Author

Thanks, good to hear 👍 I'm going to make sure this still works with Clipious before getting @unixfox to review

@alexmaras alexmaras requested a review from unixfox March 19, 2025 00:57
@alexmaras
Copy link
Collaborator Author

OK, seems to work well with Clipious too. Open for review.

@unixfox
Copy link
Member

unixfox commented Mar 19, 2025

Thanks alex. But did you guys check out how invidious videoplayback code works: https://github.com/iv-org/invidious/blob/master/src/invidious/routes/video_playback.cr?

For example, if the range header is not present then it requests a fixed range: https://github.com/iv-org/invidious/blob/master/src/invidious/routes/video_playback.cr#L39-L43

There is also the fact that some videoplayback proxy handle the query string "range" too: https://github.com/TeamPiped/piped-proxy/blob/main/src/main.rs#L310-L336. I'm planning on switching to a range query string for the video.js player: iv-org/invidious#5087

@alexmaras
Copy link
Collaborator Author

I'm happy to look into more range header/query param handling, but I'd prefer to do it in a follow-up PR as this is fixing a bug with the existing player. If I add more checks in, I want to be able to test them out, and it'll be easier to do that in isolation after that's fixed.

If we don't have one already, putting an issue in for those 2 improvements would be great, so it doesn't get forgotten.

@unixfox unixfox merged commit 71341b8 into iv-org:master Mar 19, 2025
1 check passed
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.

3 participants