Repository navigation
avformat/shared: add an offset option - #87
Merged
Merged
Conversation
This additionally fixes a prior silent corruption when a parent `offset` option was implicitly passed down to the inner protocol, resulting in a stream that did not start at 0, yet the inner protocol offset tracking did not account for this. By just always handling this option ourselves, we avoid this class of problem. Fixes: #80 See-Also: nxtedition/casparcg#103 Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <git@haasn.dev>
There was a problem hiding this comment.
🟡 Changes recommended
end_offset must be consumed safely or otherwise prevented from truncating the shared cache during range requests.
1 open issue
What changed in this PR
Adds an offset option to the shared protocol, initializing the logical read position in the cache layer rather than forwarding it to the underlying protocol.
Changes:
- Adds and applies
SharedContext.offset. - Documents the new option and its non-forwarding behavior.
- Leaves
end_offsetforwarded to the inner protocol, which can truncate shared caches for HLS/DASH range requests.
| File | Summary |
|---|---|
libavformat/shared.c |
Implements the shared protocol offset, but leaves a critical unresolved issue where end_offset may be forwarded and permanently truncate cached resources. |
doc/protocols.texi |
Documents the new offset option and its interaction with the underlying protocol. |
Review effort: Lite
| { "retry_errors", "Re-request blocks even if they previously failed", OFFSET(retry_errors), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, .flags = D }, | ||
| { "retry_corrupt", "Re-request blocks that fail the CRC check", OFFSET(retry_corrupt), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, .flags = D }, | ||
| { "cache_size_max", "Limit the maximum amount of data cached", OFFSET(cache_size_max), AV_OPT_TYPE_INT64, {.i64 = 0}, 0, INT64_MAX, .flags = D }, | ||
| { "offset", "Initial byte offset", OFFSET(offset), AV_OPT_TYPE_INT64, {.i64 = 0}, 0, INT64_MAX, .flags = D }, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This additionally fixes a prior silent corruption when a parent
offsetoption was implicitly passed down to the inner protocol, resulting in a stream that did not start at 0, yet the inner protocol offset tracking did not account for this.By just always handling this option ourselves, we avoid this class of problem.
Fixes: #80
See-Also: nxtedition/casparcg#103
Sponsored-by: nxtedition AB