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

stream.hls: refactor segment byterange calculation #4302

Merged
merged 1 commit into from
Jan 22, 2022

Conversation

bastimeyer
Copy link
Member

  • Move "Range" header value calculation to separate class:
    Don't cache offsets by segment URL, and instead refer to the previous
    sequence. Initialization maps always require an existing offset value.
  • Refactor HLSStreamWriter.fetch and related methods
  • Add byterange tests

rel #4300, #4301

EXT-X-BYTERANGE
https://datatracker.ietf.org/doc/html/rfc8216#section-4.3.2.2
EXT-X-MAP
https://datatracker.ietf.org/doc/html/rfc8216#section-4.3.2.5

The spec does not say anything about mandatory offset values for initialization map byteranges, so I'm not sure if what I've implemented is correct. It currently raises a StreamError when the offset is missing on a map's byterange, and the download will fail. It could also default to zero in this case, but I'm not sure if this is implied by the specification, because maps can't refer to the previous segment like regular byterange tags do. Am I missing something?

- Move "Range" header value calculation to separate class:
  Don't cache offsets by segment URL, and instead refer to the previous
  sequence. Initialization maps always require an existing offset value.
- Refactor HLSStreamWriter.fetch and related methods
- Add byterange tests
@gravyboat
Copy link
Member

I don't think you're missing anything. It seems unclear in the spec to me regarding what that offset value should be.

@back-to back-to merged commit 8153a6b into streamlink:master Jan 22, 2022
@bastimeyer bastimeyer deleted the stream/hls/refactor-byterange branch January 22, 2022 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants