feat: Further implement ls-qpack, especially around stream data in/out #12
Conversation
from ls-qpack upstream docs: "Process next chunk of bytes from the decoder stream"
Feat feed decoder
There was a problem hiding this comment.
Just quick glance. I need to find some spare time to review deeply.
In the meanwhile, I am going to leave a few comments.
I'd suggest to squash some commits to keep the history clean.
Moreover, if it is possible I would improve the code (and this entire library actually) with more unit tests. Very much unsafe code, without deep unit testing it is scaring :D
This is kinda out of scope of your PR in particular, but maybe we can leverage this refinement to ensure the behavior with unit tests.
Thank you for the contribution!
| @@ -37,16 +37,36 @@ use std::pin::Pin; | |||
| /// Error during decoding operations. | |||
| pub struct DecoderError; | |||
|
|
|||
| /// The result of the encoding operation. | |||
| /// | |||
| /// This is the result of [`Encoder::encode_all`] or [`EncodingBlock::encode`]. | |||
There was a problem hiding this comment.
This is called BuffersDecoded but doc mentions encode_all?
| stream: Box<[u8]>, | ||
| } | ||
|
|
||
| impl BuffersDecoded { |
There was a problem hiding this comment.
I would improve this `impl.
Before it was possible to obtain ownership of data. With BuffersDecoded is not possible anymore.
I'd start adding a take method to consume the object and get the memory ownership:
| @@ -339,6 +362,11 @@ mod callbacks { | |||
| this.blocked = blocked; | |||
| } | |||
|
|
|||
| pub(super) fn set_stream_data(self: Pin<&mut Self>, data: &Vec<u8>) { | |||
There was a problem hiding this comment.
Why passing &Vec<u8>? If you need ownership, just pass Vec<u8>
| @@ -353,8 +381,12 @@ mod callbacks { | |||
| self.error | |||
| } | |||
|
|
|||
| pub(super) fn decoded_headers(self) -> Vec<Header> { | |||
| self.decoded_headers | |||
| pub(super) fn decoded_headers(&self) -> Vec<Header> { | |||
There was a problem hiding this comment.
why the change self -> &self?
Hello,
This PR supersede #11
The story remain identical on the goals. The crate you started is really useful, and I would like to contribute
in order to make this further usable in various use cases.
FYI: In this state, the PR introduce backward incompatible changes. I am willing to discuss it further to find a solution that would fit both our interests. Of course.
regards,