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

Allow extra keyword arguments in the Transcoding protocol #215

Closed
wants to merge 4 commits into from

Conversation

nhz2
Copy link
Member

@nhz2 nhz2 commented May 15, 2024

This PR adds the ability to extend the Transcoding protocol in a non-breaking way.

This is desirable to support non-streaming codecs better. Ref: JuliaIO/CodecZstd.jl#46

To do this I added two new optional functions to the protocol: process2 and startproc2.
These are equivalent to their previous versions, except they can accept any keyword arguments.

These are optional to maintain compatibility with existing codecs.

This PR also sets the mode to :transcode instead of :write when calling startproc in unsafe_transcode!
This is to allow non-streaming codecs to error if they are used in a streaming context. Ref: JuliaIO/CodecZstd.jl#46

Edit: I'm removing the :transcode mode change because this can be a different PR.

@nhz2 nhz2 marked this pull request as ready for review May 15, 2024 14:03
@nhz2 nhz2 requested a review from mkitti May 15, 2024 14:03
@mkitti
Copy link
Member

mkitti commented May 16, 2024

Can you give me an example of how this would work?

@nhz2 nhz2 marked this pull request as draft May 18, 2024 23:25
@nhz2
Copy link
Member Author

nhz2 commented May 18, 2024

I'm working on a CodecCBlosc1 to see how new keyword arguments could be used.

The goal is to allow a compressor Codec to avoid buffering input if there is no more input in a frame and enough output space is available.

One thing I found is that any input passed to process needs to first get passed to minoutsize, so I'll need a minoutsize2 as well.

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