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

Add optional pledgeinsize function to transcoding protocol #239

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

nhz2
Copy link
Member

@nhz2 nhz2 commented Sep 6, 2024

The pledgeinsize(codec::C, insize::Int64, error::Error)::Symbol method is used
when transcode is called to tell the codec the total input size.
This is called after startproc and before process. Some
compressors can add this total input size to a header, making expectedsize
accurate during later decompression. By default this just returns :ok.
If there is an error, the return code must be :error and the error argument
must be set to an exception object. Setting an inaccurate insize may cause the
codec to error later on while processing data. A negative insize means unknown
content size.

Ref: JuliaIO/CodecZstd.jl#58

I think this is a nicer alternative to #215

@nhz2 nhz2 requested a review from mkitti September 6, 2024 02:52
@mkitti
Copy link
Member

mkitti commented Sep 6, 2024

I wonder if it would be appropriate to call this sizehint!.

@nhz2 nhz2 marked this pull request as ready for review September 7, 2024 02:25
@nhz2
Copy link
Member Author

nhz2 commented Sep 7, 2024

I chose the name to match the style of the other functions in the transcoding protocol. This is also slightly stronger than a hint since with Zstd setting the wrong size will cause an error.

@mkitti
Copy link
Member

mkitti commented Sep 8, 2024

I guess the convention here is not to use !?

Could we make it clearer the order in which these functions will be called?

Also do we have a mechanism to pledge the size even in streaming mode?

Copy link
Member

@mkitti mkitti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document call order and consider potential uses outside of transcode. For example, what if I have a large file of known size that I plan to stream.

@nhz2
Copy link
Member Author

nhz2 commented Sep 8, 2024

I added some more details to the order of the function calls. The transcoding protocol can be used outside of the transcode function since it is part of the stable API, though care has to be taken because these functions are potentially memory-unsafe.

mkitti
mkitti previously approved these changes Sep 12, 2024
@nhz2
Copy link
Member Author

nhz2 commented Sep 13, 2024

@mkitti I added some examples of how pledgeinsize could work in one of the test codecs.

@nhz2 nhz2 merged commit d3d4197 into master Sep 13, 2024
24 of 26 checks passed
@nhz2 nhz2 deleted the nz/pledgeinsize branch September 13, 2024 15:50
nhz2 added a commit that referenced this pull request Oct 6, 2024
This release adds the optional `pledgeinsize` function to the transcoding protocol (PR #239)
@nhz2 nhz2 mentioned this pull request Oct 6, 2024
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.

2 participants