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

Minimal async decoder support #46

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

rklaehn
Copy link

@rklaehn rklaehn commented Jan 27, 2023

This is an attempt to split #44 into more manageable parts.

Adds very limited async support. For now just tokio::io::AsyncRead for Decoder and SliceDecoder.
Tries to share the maximum amount possible with DecoderShared.

If the code structure and general approach is OK, it would be easy to also add support for futures::io::AsyncRead.

Seek support is a bit more complex, so probably best done in a subsequent PR.

this is so DecoderShared can be used from async decoders
for now just for the Decoder in inline and outboard mode,
not yet for the SliceDecoder.
src/decode.rs Outdated
}

#[derive(Clone, Debug)]
pub struct AsyncDecoder<T: AsyncRead + Unpin, O: AsyncRead + Unpin> {
Copy link
Author

Choose a reason for hiding this comment

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

It would also be possible to have the Decoder support both async and sync, but the ergonomics of that is really bad. I tried it. So I went with this tiny wrapper instead.

This requires some seek logic, but no actual seeking in the underlying inputs
@rklaehn
Copy link
Author

rklaehn commented Jan 28, 2023

I looked into what it would take to support seek. I think once you do that you can no longer share DecoderShared with the sync version, since you need some state enum... Goddamn async...

just give the state a case Done so we can implement take
we don't have to do this anymore now that we properly track state
@rklaehn rklaehn force-pushed the minimal-async-decoder-support branch from 1639453 to d44f5c7 Compare January 28, 2023 16:51
This can always succeed, because the SliceDecoder will never implement Seek
@rklaehn rklaehn force-pushed the minimal-async-decoder-support branch from 009f74e to 2329a8f Compare February 8, 2023 09:22
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.

1 participant